| Message ID | 20230318141010.513424-7-netdev@kapio-technology.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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 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 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 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 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 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
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(+)