Message ID | 20230318141010.513424-7-netdev@kapio-technology.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:604a:0:0:0:0:0 with SMTP id j10csp313427wrt; Sat, 18 Mar 2023 07:26:30 -0700 (PDT) X-Google-Smtp-Source: AK7set8ZQuh6YoCANIafyuMCcHHoBd5+wegipx+kRuGSI8em7xv8mTdWdGsh7igWk8b+RoMtj1og X-Received: by 2002:a05:6a20:7d94:b0:cd:c79:514b with SMTP id v20-20020a056a207d9400b000cd0c79514bmr14684471pzj.2.1679149590536; Sat, 18 Mar 2023 07:26:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679149590; cv=none; d=google.com; s=arc-20160816; b=PrtvIxZv5ht7QXr5DMsqNuDGOmWtndbBF5SGRFDMhl9YZTmIzhp3zUbPc2fz2S3i1z UE0Gl18yo4xOQ68o61LborSCl6Gsb9YsJkYuusMw/vdVOhVqA+sU3yGZGwJMrssX7gsi LGdoRMSCyrb80Ex7/I39YUCZKJ8XpfByQYEqxS+MjXYhNKLfZrrbkQA8XMCohU/qP+Wd Lt4XCSOQlKpgNqWRpN5808ZENvRjOt79CELX78R0dTvFgqwJgAgjND7oiaDMMf9FtGQr EJ3QkN8AzHpnFrnD56dFLdK9UMpYvpKnW9OCGjGkAaSHWSCtebtrKi6TJxYnTh84m2Om k2Vg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:organization :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from; bh=Jn7MTuccQB8AO3nxo6ArCG9xE/ImFLp3unk+n10M6yo=; b=LSvGI3EdPpIa04CdXlJc+rygnq+GFwMJMXRJMgOfJs1DU0OG9S1fh+JivaI8rQMufL enVf28OrYtPWXFPCB0YlcG9WqHEvWQilw1rtfbgMEJGMVC+xt/CLHsNGMXA8agGYypdA hjgHkAX08H2gKXZzb1iGLOpGml1K5je5i5xE7Y31WTH+riQgO2WuyuZjKq1h4PYp4MNR NLKX00C5O+gBJ981qTH7/REVqL1aTXOGMdX2ovMPRBajhdaTOj/p/dxsk94GjrBbmwpe JC7wbz9ijt8JlujCjZf+dBil6shAtVu8gZ+H6g/oiRogukryS4ZCOUv+HBaXF1WmHg5G tsCw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id u190-20020a6385c7000000b0050c1054814esi4444374pgd.558.2023.03.18.07.26.17; Sat, 18 Mar 2023 07:26:30 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229945AbjCROYj (ORCPT <rfc822;pusanteemu@gmail.com> + 99 others); Sat, 18 Mar 2023 10:24:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56016 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229478AbjCROYd (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 18 Mar 2023 10:24:33 -0400 Received: from mailout-taastrup.gigahost.dk (mailout-taastrup.gigahost.dk [46.183.139.199]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C608F34F74; Sat, 18 Mar 2023 07:24:29 -0700 (PDT) Received: from mailout.gigahost.dk (mailout.gigahost.dk [89.186.169.112]) by mailout-taastrup.gigahost.dk (Postfix) with ESMTP id E41D018839C7; Sat, 18 Mar 2023 14:12:46 +0000 (UTC) Received: from smtp.gigahost.dk (smtp.gigahost.dk [89.186.169.109]) by mailout.gigahost.dk (Postfix) with ESMTP id DB0C325002BC; Sat, 18 Mar 2023 14:12:46 +0000 (UTC) Received: by smtp.gigahost.dk (Postfix, from userid 1000) id D13B49B403E2; Sat, 18 Mar 2023 14:12:46 +0000 (UTC) X-Screener-Id: 413d8c6ce5bf6eab4824d0abaab02863e8e3f662 Received: from fujitsu.vestervang (2-104-116-184-cable.dk.customer.tdc.net [2.104.116.184]) by smtp.gigahost.dk (Postfix) with ESMTPSA id 1B3E49B403E1; Sat, 18 Mar 2023 14:12:46 +0000 (UTC) From: "Hans J. Schultz" <netdev@kapio-technology.com> To: davem@davemloft.net, kuba@kernel.org Cc: netdev@vger.kernel.org, "Hans J. Schultz" <netdev@kapio-technology.com>, Florian Fainelli <f.fainelli@gmail.com>, Andrew Lunn <andrew@lunn.ch>, Vladimir Oltean <olteanv@gmail.com>, Eric Dumazet <edumazet@google.com>, Paolo Abeni <pabeni@redhat.com>, Kurt Kanzenbach <kurt@linutronix.de>, Hauke Mehrtens <hauke@hauke-m.de>, Woojung Huh <woojung.huh@microchip.com>, UNGLinuxDriver@microchip.com (maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER), Sean Wang <sean.wang@mediatek.com>, Landen Chao <Landen.Chao@mediatek.com>, DENG Qingfang <dqfext@gmail.com>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Claudiu Manoil <claudiu.manoil@nxp.com>, Alexandre Belloni <alexandre.belloni@bootlin.com>, =?utf-8?b?Q2zDqW1lbnQg?= =?utf-8?b?TMOpZ2Vy?= <clement.leger@bootlin.com>, Jiri Pirko <jiri@resnulli.us>, Ivan Vecera <ivecera@redhat.com>, Roopa Prabhu <roopa@nvidia.com>, Nikolay Aleksandrov <razor@blackwall.org>, Shuah Khan <shuah@kernel.org>, Christian Marangi <ansuelsmth@gmail.com>, Ido Schimmel <idosch@nvidia.com>, linux-kernel@vger.kernel.org (open list), linux-arm-kernel@lists.infradead.org (moderated list:ARM/Mediatek SoC support), linux-mediatek@lists.infradead.org (moderated list:ARM/Mediatek SoC support), linux-renesas-soc@vger.kernel.org (open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER), bridge@lists.linux-foundation.org (moderated list:ETHERNET BRIDGE), linux-kselftest@vger.kernel.org (open list:KERNEL SELFTEST FRAMEWORK) Subject: [PATCH v2 net-next 6/6] selftests: forwarding: add dynamic FDB test Date: Sat, 18 Mar 2023 15:10:10 +0100 Message-Id: <20230318141010.513424-7-netdev@kapio-technology.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318141010.513424-1-netdev@kapio-technology.com> References: <20230318141010.513424-1-netdev@kapio-technology.com> MIME-Version: 1.0 Organization: Westermo Network Technologies AB Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW, SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1760715960662245665?= X-GMAIL-MSGID: =?utf-8?q?1760715960662245665?= |
Series |
ATU and FDB synchronization on locked ports
|
|
Commit Message
Hans Schultz
March 18, 2023, 2:10 p.m. UTC
Test FDB ageing of user entry created by
bridge fdb replace ADDR dev <DEV> master dynamic
Use LOW_AGEING_TIME variable in forwarding.config to set a low ageing time.
Beware, DSA might not accept the ageing time you want. Check the
age_time_coeff value for your driver.
Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
---
.../net/forwarding/bridge_locked_port.sh | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
Comments
On Sat, Mar 18, 2023 at 03:10:10PM +0100, Hans J. Schultz wrote: > +# Test of dynamic FDB entries. > +locked_port_dyn_fdb() > +{ > + local mac=00:01:02:03:04:05 > + local ageing_time > + > + RET=0 > + ageing_time=$(bridge_ageing_time_get br0) > + tc qdisc add dev $swp2 clsact > + ip link set dev br0 type bridge ageing_time $LOW_AGEING_TIME > + bridge link set dev $swp1 learning on locked on > + > + bridge fdb replace $mac dev $swp1 master dynamic > + tc filter add dev $swp2 egress protocol ip pref 1 handle 1 flower \ > + dst_ip 192.0.2.2 ip_proto udp dst_port 12345 action pass > + > + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ > + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q Packets should be injected via $h1, not $swp1. See other test cases in this file. > + tc_check_packets "dev $swp2 egress" 1 1 > + check_err $? "Packet not seen on egress after adding dynamic FDB" Does this actually work? The packet is transmitted via $swp1, I don't understand how it can arrive at the egress of $swp2. > + > + sleep $((LOW_AGEING_TIME / 100 + 10)) > + > + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ > + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q > + tc_check_packets "dev $swp2 egress" 1 1 > + check_fail $? "Dynamic FDB entry did not age out" Shouldn't this be check_err()? After the FDB entry was aged you want to make sure that packets received via $swp1 with SMAC being $mac are no longer forwarded by the bridge. Also, I suggest executing 'bridge fdb get' to make sure the entry is no longer present in the bridge FDB. > + > + ip link set dev br0 type bridge ageing_time $ageing_time > + bridge link set dev $swp1 learning off locked off > + tc qdisc del dev $swp2 clsact > + > + log_test "Locked port dyn FDB" > +} > + > trap cleanup EXIT > > setup_prepare > -- > 2.34.1 >
On Mon, Mar 20, 2023 at 10:44, Ido Schimmel <idosch@nvidia.com> wrote: >> + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ >> + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q >> + tc_check_packets "dev $swp2 egress" 1 1 >> + check_fail $? "Dynamic FDB entry did not age out" > > Shouldn't this be check_err()? After the FDB entry was aged you want to > make sure that packets received via $swp1 with SMAC being $mac are no > longer forwarded by the bridge. I was thinking that check_fail() will pass when tc_check_packets() does not see any packets, thus the test passing here when no packets are forwarded? > > Also, I suggest executing 'bridge fdb get' to make sure the entry is no > longer present in the bridge FDB. >
On Sun, Mar 26, 2023 at 05:41:06PM +0200, Hans Schultz wrote: > On Mon, Mar 20, 2023 at 10:44, Ido Schimmel <idosch@nvidia.com> wrote: > >> + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ > >> + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q > >> + tc_check_packets "dev $swp2 egress" 1 1 > >> + check_fail $? "Dynamic FDB entry did not age out" > > > > Shouldn't this be check_err()? After the FDB entry was aged you want to > > make sure that packets received via $swp1 with SMAC being $mac are no > > longer forwarded by the bridge. > > I was thinking that check_fail() will pass when tc_check_packets() does > not see any packets, thus the test passing here when no packets are forwarded? What do you mean by "I was *thinking*"? How is it possible that you are submitting a selftest that you didn't bother running?! I see you trimmed my earlier question: "Does this actually work?" I tried it and it passed: # ./bridge_locked_port.sh TEST: Locked port ipv4 [ OK ] TEST: Locked port ipv6 [ OK ] TEST: Locked port vlan [ OK ] TEST: Locked port MAB [ OK ] TEST: Locked port MAB roam [ OK ] TEST: Locked port MAB configuration [ OK ] TEST: Locked port MAB FDB flush [ OK ] And I couldn't understand how that's even possible. Then I realized that the entire test is dead code because the patch is missing this fundamental hunk: ``` diff --git a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh index dbc7017fd45d..5bf6b2aa1098 100755 --- a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh +++ b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh @@ -9,6 +9,7 @@ ALL_TESTS=" locked_port_mab_roam locked_port_mab_config locked_port_mab_flush + locked_port_dyn_fdb " NUM_NETIFS=4 ``` Which tells me that you didn't even try running it once. Now the test failed as I expected: # ./bridge_locked_port.sh TEST: Locked port ipv4 [ OK ] TEST: Locked port ipv6 [ OK ] TEST: Locked port vlan [ OK ] TEST: Locked port MAB [ OK ] TEST: Locked port MAB roam [ OK ] TEST: Locked port MAB configuration [ OK ] TEST: Locked port MAB FDB flush [ OK ] TEST: Locked port dyn FDB [FAIL] Packet not seen on egress after adding dynamic FDB Fixed by: ``` @@ -336,7 +337,7 @@ locked_port_dyn_fdb() tc filter add dev $swp2 egress protocol ip pref 1 handle 1 flower \ dst_ip 192.0.2.2 ip_proto udp dst_port 12345 action pass - $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ + $MZ $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q tc_check_packets "dev $swp2 egress" 1 1 check_err $? "Packet not seen on egress after adding dynamic FDB" ``` Ran it again and it failed because of the second issue I pointed out: # ./bridge_locked_port.sh TEST: Locked port ipv4 [ OK ] TEST: Locked port ipv6 [ OK ] TEST: Locked port vlan [ OK ] TEST: Locked port MAB [ OK ] TEST: Locked port MAB roam [ OK ] TEST: Locked port MAB configuration [ OK ] TEST: Locked port MAB FDB flush [ OK ] TEST: Locked port dyn FDB [FAIL] Dynamic FDB entry did not age out Fixed by: ``` @@ -346,7 +347,7 @@ locked_port_dyn_fdb() $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q tc_check_packets "dev $swp2 egress" 1 1 - check_fail $? "Dynamic FDB entry did not age out" + check_err $? "Dynamic FDB entry did not age out" ip link set dev br0 type bridge ageing_time $ageing_time bridge link set dev $swp1 learning off locked off ``` # ./bridge_locked_port.sh TEST: Locked port ipv4 [ OK ] TEST: Locked port ipv6 [ OK ] TEST: Locked port vlan [ OK ] TEST: Locked port MAB [ OK ] TEST: Locked port MAB roam [ OK ] TEST: Locked port MAB configuration [ OK ] TEST: Locked port MAB FDB flush [ OK ] TEST: Locked port dyn FDB [ OK ] Sigh
On Tue, Mar 28, 2023 at 19:40, Ido Schimmel <idosch@nvidia.com> wrote: > On Sun, Mar 26, 2023 at 05:41:06PM +0200, Hans Schultz wrote: >> On Mon, Mar 20, 2023 at 10:44, Ido Schimmel <idosch@nvidia.com> wrote: >> >> + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ >> >> + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q >> >> + tc_check_packets "dev $swp2 egress" 1 1 >> >> + check_fail $? "Dynamic FDB entry did not age out" >> > >> > Shouldn't this be check_err()? After the FDB entry was aged you want to >> > make sure that packets received via $swp1 with SMAC being $mac are no >> > longer forwarded by the bridge. >> >> I was thinking that check_fail() will pass when tc_check_packets() does >> not see any packets, thus the test passing here when no packets are forwarded? > > What do you mean by "I was *thinking*"? How is it possible that you are > submitting a selftest that you didn't bother running?! > Sorry, but I have sent you several emails telling you about the problems I have with running the selftests due to changes in the phy etc. Maybe you have just not received all those emails? Have you checked spamfilters? With the kernels now, I cannot even test with the software bridge and selftests as the compile fails - probably due to changes in uapi headers compared to what the packages my system uses expects.
On Tue, Mar 28, 2023 at 09:30:08PM +0200, Hans Schultz wrote: > On Tue, Mar 28, 2023 at 19:40, Ido Schimmel <idosch@nvidia.com> wrote: > > On Sun, Mar 26, 2023 at 05:41:06PM +0200, Hans Schultz wrote: > >> On Mon, Mar 20, 2023 at 10:44, Ido Schimmel <idosch@nvidia.com> wrote: > >> >> + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ > >> >> + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q > >> >> + tc_check_packets "dev $swp2 egress" 1 1 > >> >> + check_fail $? "Dynamic FDB entry did not age out" > >> > > >> > Shouldn't this be check_err()? After the FDB entry was aged you want to > >> > make sure that packets received via $swp1 with SMAC being $mac are no > >> > longer forwarded by the bridge. > >> > >> I was thinking that check_fail() will pass when tc_check_packets() does > >> not see any packets, thus the test passing here when no packets are forwarded? > > > > What do you mean by "I was *thinking*"? How is it possible that you are > > submitting a selftest that you didn't bother running?! > > > > Sorry, but I have sent you several emails telling you about the problems > I have with running the selftests due to changes in the phy etc. Maybe > you have just not received all those emails? > > Have you checked spamfilters? > > With the kernels now, I cannot even test with the software bridge and > selftests as the compile fails - probably due to changes in uapi headers > compared to what the packages my system uses expects. My spam filters are fine. I saw your emails where you basically said that you are too lazy to setup a VM to test your patches and that your time is more valuable than mine, which is why I should be testing them. Stop making your problems our problems. It's hardly the first time. If you are unable to test your patches, then invest the time in fixing your setup instead of submitting completely broken patches and making it our problem to test and fix them. I refuse to invest time in reviewing / testing / reworking your submissions as long as you insist on doing less than the bare minimum. Good luck
On Thu, Mar 30, 2023 at 09:37, Ido Schimmel <idosch@nvidia.com> wrote: > On Tue, Mar 28, 2023 at 09:30:08PM +0200, Hans Schultz wrote: >> >> Sorry, but I have sent you several emails telling you about the problems >> I have with running the selftests due to changes in the phy etc. Maybe >> you have just not received all those emails? >> >> Have you checked spamfilters? >> >> With the kernels now, I cannot even test with the software bridge and >> selftests as the compile fails - probably due to changes in uapi headers >> compared to what the packages my system uses expects. > > My spam filters are fine. I saw your emails where you basically said > that you are too lazy to setup a VM to test your patches and that your > time is more valuable than mine, which is why I should be testing them. > Stop making your problems our problems. It's hardly the first time. If > you are unable to test your patches, then invest the time in fixing your > setup instead of submitting completely broken patches and making it our > problem to test and fix them. I refuse to invest time in reviewing / > testing / reworking your submissions as long as you insist on doing less > than the bare minimum. > > Good luck I never said or indicated that my time is more valuable than yours. I have a VM to run these things that some have spent countless hours to develop with the right tools etc installed and set up. Fixing that system will take quite many hours for me, so I am asking for some simple assistance from someone who already has a system running supporting the newest kernel. Alternatively if there is an open sourced system available that would be great. As this patch-set is for the community and some companies that would like to use it and not for myself, I am asking for some help from the community with a task that when someone has the system in place should not take more than 15-20 minutes, maybe even less.
On 30/03/2023 13:29, Hans Schultz wrote: > On Thu, Mar 30, 2023 at 09:37, Ido Schimmel <idosch@nvidia.com> wrote: >> On Tue, Mar 28, 2023 at 09:30:08PM +0200, Hans Schultz wrote: >>> >>> Sorry, but I have sent you several emails telling you about the problems >>> I have with running the selftests due to changes in the phy etc. Maybe >>> you have just not received all those emails? >>> >>> Have you checked spamfilters? >>> >>> With the kernels now, I cannot even test with the software bridge and >>> selftests as the compile fails - probably due to changes in uapi headers >>> compared to what the packages my system uses expects. >> >> My spam filters are fine. I saw your emails where you basically said >> that you are too lazy to setup a VM to test your patches and that your >> time is more valuable than mine, which is why I should be testing them. >> Stop making your problems our problems. It's hardly the first time. If >> you are unable to test your patches, then invest the time in fixing your >> setup instead of submitting completely broken patches and making it our >> problem to test and fix them. I refuse to invest time in reviewing / >> testing / reworking your submissions as long as you insist on doing less >> than the bare minimum. >> >> Good luck > > I never said or indicated that my time is more valuable than yours. I > have a VM to run these things that some have spent countless hours to > develop with the right tools etc installed and set up. Fixing that > system will take quite many hours for me, so I am asking for some simple > assistance from someone who already has a system running supporting the > newest kernel. > > Alternatively if there is an open sourced system available that would be > great. > > As this patch-set is for the community and some companies that would > like to use it and not for myself, I am asking for some help from the > community with a task that when someone has the system in place should > not take more than 15-20 minutes, maybe even less. I'm sorry but this is absolutely the wrong way to go about this. Your setup's problems are yours to figure out and fix, if you are going to send *any* future patches make absolutely sure they build, run and work as intended. Please do not send any future patches without them being fully tested and, as Ido mentioned, cover at least the bare minimum for a submission. Thanks, Nik
On Tue, Mar 28, 2023 at 19:40, Ido Schimmel <idosch@nvidia.com> wrote: > On Sun, Mar 26, 2023 at 05:41:06PM +0200, Hans Schultz wrote: >> On Mon, Mar 20, 2023 at 10:44, Ido Schimmel <idosch@nvidia.com> wrote: >> >> + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ >> >> + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q >> >> + tc_check_packets "dev $swp2 egress" 1 1 >> >> + check_fail $? "Dynamic FDB entry did not age out" >> > >> > Shouldn't this be check_err()? After the FDB entry was aged you want to >> > make sure that packets received via $swp1 with SMAC being $mac are no >> > longer forwarded by the bridge. >> >> I was thinking that check_fail() will pass when tc_check_packets() does >> not see any packets, thus the test passing here when no packets are forwarded? > > What do you mean by "I was *thinking*"? How is it possible that you are > submitting a selftest that you didn't bother running?! > > I see you trimmed my earlier question: "Does this actually work?" > > I tried it and it passed: > > # ./bridge_locked_port.sh > TEST: Locked port ipv4 [ OK ] > TEST: Locked port ipv6 [ OK ] > TEST: Locked port vlan [ OK ] > TEST: Locked port MAB [ OK ] > TEST: Locked port MAB roam [ OK ] > TEST: Locked port MAB configuration [ OK ] > TEST: Locked port MAB FDB flush [ OK ] > > And I couldn't understand how that's even possible. Then I realized that > the entire test is dead code because the patch is missing this > fundamental hunk: > > ``` > diff --git a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh > index dbc7017fd45d..5bf6b2aa1098 100755 > --- a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh > +++ b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh > @@ -9,6 +9,7 @@ ALL_TESTS=" > locked_port_mab_roam > locked_port_mab_config > locked_port_mab_flush > + locked_port_dyn_fdb > " > > NUM_NETIFS=4 > ``` > > Which tells me that you didn't even try running it once. Not true, it reveals that I forgot to put it in the patch, that's all. As I cannot run several of these tests because of memory constraints I link the file to a copy in a rw area where I modify the list and just run one of the subtests at a time. If I try to run the whole it always fails after a couple of sub-tests with an error. It seems to me that these scripts are quite memory consuming as they accumulate memory consuption in relation to what is loaded along the way. A major problem with my system.
On Thu, Mar 30, 2023 at 09:07:53PM +0200, Hans Schultz wrote: > Not true, it reveals that I forgot to put it in the patch, that's all. As > I cannot run several of these tests because of memory constraints I link > the file to a copy in a rw area where I modify the list and just run one > of the subtests at a time. If I try to run the whole it always fails > after a couple of sub-tests with an error. > > It seems to me that these scripts are quite memory consuming as they > accumulate memory consuption in relation to what is loaded along the > way. A major problem with my system. I'm sorry for perhaps asking something entirely obvious, but have you tried: kernel-dir $ rsync -avr tools/testing/selftests/ root@$board:selftests/ board $ cd selftests/drivers/net/dsa/ board $ ./bridge_locked_port.sh lan0 lan1 lan2 lan3 ? This is how I always run them, and it worked fine with both Debian (where it's easy to add missing packages to the rootfs) or with a more embedded-oriented Buildroot.
On Thu, Mar 30, 2023 at 22:27, Vladimir Oltean <olteanv@gmail.com> wrote: > On Thu, Mar 30, 2023 at 09:07:53PM +0200, Hans Schultz wrote: >> Not true, it reveals that I forgot to put it in the patch, that's all. As >> I cannot run several of these tests because of memory constraints I link >> the file to a copy in a rw area where I modify the list and just run one >> of the subtests at a time. If I try to run the whole it always fails >> after a couple of sub-tests with an error. >> >> It seems to me that these scripts are quite memory consuming as they >> accumulate memory consuption in relation to what is loaded along the >> way. A major problem with my system. > > I'm sorry for perhaps asking something entirely obvious, but have you tried: > > kernel-dir $ rsync -avr tools/testing/selftests/ root@$board:selftests/ > board $ cd selftests/drivers/net/dsa/ > board $ ./bridge_locked_port.sh lan0 lan1 lan2 lan3 > > ? > > This is how I always run them, and it worked fine with both Debian > (where it's easy to add missing packages to the rootfs) or with a more > embedded-oriented Buildroot. I am not entirely clear of your idea. You need somehow to boot into a system with the patched net-next kernel or you have a virtual machine boot into a virtual OS. I guess it is the last option you refer to using Debian?
On Thu, Mar 30, 2023 at 22:27, Vladimir Oltean <olteanv@gmail.com> wrote: > On Thu, Mar 30, 2023 at 09:07:53PM +0200, Hans Schultz wrote: >> Not true, it reveals that I forgot to put it in the patch, that's all. As >> I cannot run several of these tests because of memory constraints I link >> the file to a copy in a rw area where I modify the list and just run one >> of the subtests at a time. If I try to run the whole it always fails >> after a couple of sub-tests with an error. >> >> It seems to me that these scripts are quite memory consuming as they >> accumulate memory consuption in relation to what is loaded along the >> way. A major problem with my system. > > I'm sorry for perhaps asking something entirely obvious, but have you tried: > > kernel-dir $ rsync -avr tools/testing/selftests/ root@$board:selftests/ > board $ cd selftests/drivers/net/dsa/ > board $ ./bridge_locked_port.sh lan0 lan1 lan2 lan3 > > ? > > This is how I always run them, and it worked fine with both Debian > (where it's easy to add missing packages to the rootfs) or with a more > embedded-oriented Buildroot. The memory problems are of course on the embedded target. In that case I think it would be a very good idea to do something to design the system better, so that it frees memory between the subtests. If all tests are always run on the bridge only, I think they don't make much sense as these patchsets are directed towards switchcores.
On Fri, Mar 31, 2023 at 09:43:34AM +0200, Hans Schultz wrote: > On Thu, Mar 30, 2023 at 22:27, Vladimir Oltean <olteanv@gmail.com> wrote: > > This is how I always run them, and it worked fine with both Debian > > (where it's easy to add missing packages to the rootfs) or with a more > > embedded-oriented Buildroot. > > I am not entirely clear of your idea. You need somehow to boot into a > system with the patched net-next kernel You have to do that anyway for any kind of kernel work, no? > or you have a virtual machine boot into a virtual OS. I guess it is > the last option you refer to using Debian? You could do that too, but you don't have to. Debian, like many other Linux distributions, supports a wide variety of CPU architectures; it can be run on embedded systems just as well as on desktop PCs or VMs. I didn't say you have to use Debian, though, I just said I ran the selftests on a Debian-based rootfs and that it was easy to prepare the environment there. The Debian rootfs and the selftests were deployed to the target board with the DSA switch on it, in case that wasn't clear.
On Fri, Mar 31, 2023 at 10:06:34AM +0200, Hans Schultz wrote: > The memory problems are of course on the embedded target. In that case I > think it would be a very good idea to do something to design the system > better, so that it frees memory between the subtests. People like Martin Blumenstingl have managed to deploy and run the networking kselftests on OpenWRT, which typically runs on very resource-constrained embedded devices. https://lore.kernel.org/netdev/CAFBinCDX5XRyMyOd-+c_Zkn6dawtBpQ9DaPkA4FDC5agL-t8CA@mail.gmail.com/ https://lore.kernel.org/netdev/20220707135532.1783925-1-martin.blumenstingl@googlemail.com/ Considering that, you'll have to come with a much more concrete description of why the system should be "designed better" and "free memory between subtests" (what memory?!) before you could run it on your target system. Either that, or at least take into serious consideration the fact that you may be hung up on doing something which isn't necessary for the end goal. I simply have no clue what you're talking about. It's as if we're talking about completely different things. > If all tests are always run on the bridge only, I think they don't make > much sense as these patchsets are directed towards switchcores. Is this supposed to mean something, or is it just a random thought you had, that you believed it would be good to share with us? The tools/testing/selftests/net/forwarding/lib.sh central framework has the NETIF_TYPE and NETIF_CREATE variables, which indicate that by default, veth interfaces are created. When running a bridge selftest with veth as bridge ports, indeed software bridging should take place, and those selftests should work fine. In Linux, the software behavior represents a model for the offload behavior, since offloads are 100% transparent to the user most of the time. Below in lib.sh, there is a line which sources "$relative_path/forwarding.config", a file which can contain customizations of the default variables used by the framework. Even though it isn't strictly necessary to put the customized bash variables in a forwarding.config file, it is more convenient to do this than to specify them as environment variables. If you "cd tools/testing/selftests/drivers/net/dsa/", you will find precisely such a forwarding.config file there, which contains the line "NETIF_CREATE=no", which means that when you run the symlinked sub-group of forwarding tests relevant to DSA from this folder, the expectation is that the bridge ports are not veth interfaces created for the test, but rather, physical ports. So, by running the command I posted in the earlier email, you actually run it on the physical DSA user port interfaces, and it should pass there too. This is based on the equivalency principle between the software and the hardware data paths that I was talking about. If you're actively and repeatedly making an effort to work with your eyes closed, and then build strawmen around the fact that you don't see, then you're not going to get very friendly reactions from people, me included, who explain things to you that pertain to your due diligence. This is because these people know the things that they're explaining to you out of their own due diligence, and, as a result, are not easily fooled by your childish excuses.
On Fri, Mar 31, 2023 at 12:37, Vladimir Oltean <olteanv@gmail.com> wrote: > > So, by running the command I posted in the earlier email, you actually > run it on the physical DSA user port interfaces, and it should pass > there too. Okay, that sounds like a good idea which I have not done before. I am seeing how I can install Debian in an Qemu or VMWare setup to be able to test that way. > This is based on the equivalency principle between the > software and the hardware data paths that I was talking about. > > If you're actively and repeatedly making an effort to work with your eyes > closed, and then build strawmen around the fact that you don't see, then > you're not going to get very friendly reactions from people, me included, > who explain things to you that pertain to your due diligence. This is > because these people know the things that they're explaining to you out > of their own due diligence, and, as a result, are not easily fooled by > your childish excuses. I am not coming with excuses here, and certainly not childish ones at that either. I am just pointing out that on my device the tests don't run well because of memory shortage and my reasoning why I think it is so. I will as long as the system is as it is with these selftests, just run single subtests at a time on target, but if I have new phy problems like the one you have seen I have had before, then testing on target becomes off limits.
On Fri, Mar 31, 2023 at 02:43:11PM +0200, Hans Schultz wrote: > I will as long as the system is as it is with these selftests, just run > single subtests at a time on target, but if I have new phy problems like > the one you have seen I have had before, then testing on target becomes > off limits. Please open a dedicated communication channel (separate email thread on netdev@vger.kernel.org) with the appropriate maintainers for the PHY code that is failing for you in To:, and you will get the help that you need to resolve that and to be able to test on the target board.
On Thu, Apr 06, 2023 at 18:24, Vladimir Oltean <olteanv@gmail.com> wrote: > On Fri, Mar 31, 2023 at 02:43:11PM +0200, Hans Schultz wrote: >> I will as long as the system is as it is with these selftests, just run >> single subtests at a time on target, but if I have new phy problems like >> the one you have seen I have had before, then testing on target becomes >> off limits. > > Please open a dedicated communication channel (separate email thread on > netdev@vger.kernel.org) with the appropriate maintainers for the PHY > code that is failing for you in To:, and you will get the help that you > need to resolve that and to be able to test on the target board. The errors from the phy I saw in February. Maybe something was fixed in the meantime as I did not see the same warning and exception last I tried to run the newest kernel on target a little over a week ago.
diff --git a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh index dc92d32464f6..dbc7017fd45d 100755 --- a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh +++ b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh @@ -14,6 +14,7 @@ ALL_TESTS=" NUM_NETIFS=4 CHECK_TC="no" source lib.sh +source tc_common.sh h1_create() { @@ -319,6 +320,41 @@ locked_port_mab_flush() log_test "Locked port MAB FDB flush" } +# Test of dynamic FDB entries. +locked_port_dyn_fdb() +{ + local mac=00:01:02:03:04:05 + local ageing_time + + RET=0 + ageing_time=$(bridge_ageing_time_get br0) + tc qdisc add dev $swp2 clsact + ip link set dev br0 type bridge ageing_time $LOW_AGEING_TIME + bridge link set dev $swp1 learning on locked on + + bridge fdb replace $mac dev $swp1 master dynamic + tc filter add dev $swp2 egress protocol ip pref 1 handle 1 flower \ + dst_ip 192.0.2.2 ip_proto udp dst_port 12345 action pass + + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q + tc_check_packets "dev $swp2 egress" 1 1 + check_err $? "Packet not seen on egress after adding dynamic FDB" + + sleep $((LOW_AGEING_TIME / 100 + 10)) + + $MZ $swp1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \ + -a $mac -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q + tc_check_packets "dev $swp2 egress" 1 1 + check_fail $? "Dynamic FDB entry did not age out" + + ip link set dev br0 type bridge ageing_time $ageing_time + bridge link set dev $swp1 learning off locked off + tc qdisc del dev $swp2 clsact + + log_test "Locked port dyn FDB" +} + trap cleanup EXIT setup_prepare