Linux Ethernet Bridging - Sep 2022 - [Bridge] [PATCH v5 net-next 6/6] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests

On Tue, Sep 20, 2022 at 11:29:12PM +0200, netdev at kapio-technology.com
wrote:
> I have made a blackhole selftest, which looks like this:
> 
> test_blackhole_fdb()
> {
>         RET=0
> 
>         check_blackhole_fdb_support || return 0
> 
>         tcpdump_start $h2
>         $MZ $h1 -q -t udp -a $h1 -b $h2
I don't think you can give an interface name to '-a' and
'-b'?
>         tcpdump_stop
>         tcpdump_show | grep -q udp
>         check_err $? "test_blackhole_fdb: No packet seen on
initial"
>         tcpdump_cleanup
> 
>         bridge fdb add `mac_get $h2` dev br0 blackhole
>         bridge fdb show dev br0 | grep -q "blackhole"
Make this grep more specific so that we are sure it is the entry user
space installed. Something like this:

bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
>         check_err $? "test_blackhole_fdb: No blackhole FDB entry
found"
> 
>         tcpdump_start $h2
>         $MZ $h1 -q -t udp -a $h1 -b $h2
>         tcpdump_stop
>         tcpdump_show | grep -q udp
>         check_fail $? "test_blackhole_fdb: packet seen with blackhole
fdb
> entry"
>         tcpdump_cleanup
The tcpdump filter is not specific enough. It can catch other UDP
packets (e.g., multicast) being received by $h2. Anyway, to be sure the
feature works as expected we need to make sure that the packets are not
even egressing $swp2. Checking that they are not received by $h2 is not
enough. See this (untested) suggestion [1] that uses a tc filter on the
egress of $swp2.
> 
>         bridge fdb del `mac_get $h2` dev br0 blackhole
>         bridge fdb show dev br0 | grep -q "blackhole"
>         check_fail $? "test_blackhole_fdb: Blackhole FDB entry not
deleted"
> 
>         tcpdump_start $h2
>         $MZ $h1 -q -t udp -a $h1 -b $h2
>         tcpdump_stop
>         tcpdump_show | grep -q udp
>         check_err $? "test_blackhole_fdb: No packet seen after
removing
> blackhole FDB entry"
>         tcpdump_cleanup
> 
>         log_test "Blackhole FDB entry test"
> }
> 
> the setup is simple and is the same as in bridge_sticky_fdb.sh.
> 
> Does the test look sound or is there obvious mistakes?
[1]
blackhole_fdb()
{
	RET=0

	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 $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \
		-a own -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 before adding blackhole
entry"

	bridge fdb add `mac_get $h2` dev br0 blackhole
	bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
	check_err $? "Blackhole entry not found"

	$MZ $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \
		-a own -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 seen on egress after adding blackhole entry"

	# Check blackhole entries can be replaced.
	bridge fdb replace `mac_get $h2` dev $swp2 master static
	bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
	check_fail $? "Blackhole entry found after replacement"

	$MZ $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \
		-a own -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q

	tc_check_packets "dev $swp2 egress" 1 2
	check_err $? "Packet not seen on egress after replacing blackhole
entry"

	bridge fdb del `mac_get $h2` dev $swp2 master static
	tc filter del dev $swp2 egress protocol ip pref 1 handle 1 flower

	log_test "Blackhole FDB entry"
}

On 2022-09-21 09:15, Ido Schimmel wrote:
> On Tue, Sep 20, 2022 at 11:29:12PM +0200, netdev at kapio-technology.com 
> wrote:
>> I have made a blackhole selftest, which looks like this:
>> 
>> test_blackhole_fdb()
>> {
>>         RET=0
>> 
>>         check_blackhole_fdb_support || return 0
>> 
>>         tcpdump_start $h2
>>         $MZ $h1 -q -t udp -a $h1 -b $h2
> 
> I don't think you can give an interface name to '-a' and
'-b'?
> 
>>         tcpdump_stop
>>         tcpdump_show | grep -q udp
>>         check_err $? "test_blackhole_fdb: No packet seen on
initial"
>>         tcpdump_cleanup
>> 
>>         bridge fdb add `mac_get $h2` dev br0 blackhole
>>         bridge fdb show dev br0 | grep -q "blackhole"
> 
> Make this grep more specific so that we are sure it is the entry user
> space installed. Something like this:
> 
> bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
> 
>>         check_err $? "test_blackhole_fdb: No blackhole FDB entry 
>> found"
>> 
>>         tcpdump_start $h2
>>         $MZ $h1 -q -t udp -a $h1 -b $h2
>>         tcpdump_stop
>>         tcpdump_show | grep -q udp
>>         check_fail $? "test_blackhole_fdb: packet seen with
blackhole
>> fdb
>> entry"
>>         tcpdump_cleanup
> 
> The tcpdump filter is not specific enough. It can catch other UDP
> packets (e.g., multicast) being received by $h2. Anyway, to be sure the
> feature works as expected we need to make sure that the packets are not
> even egressing $swp2. Checking that they are not received by $h2 is not
> enough. See this (untested) suggestion [1] that uses a tc filter on the
> egress of $swp2.
> 
>> 
>>         bridge fdb del `mac_get $h2` dev br0 blackhole
>>         bridge fdb show dev br0 | grep -q "blackhole"
>>         check_fail $? "test_blackhole_fdb: Blackhole FDB entry not
>> deleted"
>> 
>>         tcpdump_start $h2
>>         $MZ $h1 -q -t udp -a $h1 -b $h2
>>         tcpdump_stop
>>         tcpdump_show | grep -q udp
>>         check_err $? "test_blackhole_fdb: No packet seen after 
>> removing
>> blackhole FDB entry"
>>         tcpdump_cleanup
>> 
>>         log_test "Blackhole FDB entry test"
>> }
>> 
>> the setup is simple and is the same as in bridge_sticky_fdb.sh.
>> 
>> Does the test look sound or is there obvious mistakes?
> 
> [1]
> blackhole_fdb()
> {
> 	RET=0
> 
> 	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 $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \
> 		-a own -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 before adding blackhole
entry"
> 
> 	bridge fdb add `mac_get $h2` dev br0 blackhole
> 	bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
> 	check_err $? "Blackhole entry not found"
> 
> 	$MZ $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \
> 		-a own -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 seen on egress after adding blackhole
entry"
> 
> 	# Check blackhole entries can be replaced.
> 	bridge fdb replace `mac_get $h2` dev $swp2 master static
> 	bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
> 	check_fail $? "Blackhole entry found after replacement"
> 
> 	$MZ $h1 -c 1 -p 128 -t udp "sp=54321,dp=12345" \
> 		-a own -b `mac_get $h2` -A 192.0.2.1 -B 192.0.2.2 -q
> 
> 	tc_check_packets "dev $swp2 egress" 1 2
> 	check_err $? "Packet not seen on egress after replacing blackhole 
> entry"
> 
> 	bridge fdb del `mac_get $h2` dev $swp2 master static
> 	tc filter del dev $swp2 egress protocol ip pref 1 handle 1 flower
> 
> 	log_test "Blackhole FDB entry"
> }
Thx, looks good.
I have tried to run the test as far as I can manually, but I don't seem 
to have 'busywait' in the system, which tc_check_packets() depends on, 
and I couldn't find any 'busywait' in Buildroot.

On 2022-09-21 09:15, Ido Schimmel wrote:
> 	# Check blackhole entries can be replaced.
> 	bridge fdb replace `mac_get $h2` dev $swp2 master static
> 	bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
> 	check_fail $? "Blackhole entry found after replacement"
There seems to be a problem with replacing blackhole fdb entries as 
fdb_find_rcu() does not find the associated fdb entry (addr, vid) and I 
don't know why that is the case?

On 2022-09-21 09:15, Ido Schimmel wrote:
> 	# Check blackhole entries can be replaced.
> 	bridge fdb replace `mac_get $h2` dev $swp2 master static
> 	bridge fdb get `mac_get $h2` br br0 | grep -q blackhole
> 	check_fail $? "Blackhole entry found after replacement"
> 
I am quite in doubt if the driver will be able to overwrite a blackhole 
entry added by userspace as the replace action must be to delete and 
then add the replacement afaics, but a NEWNEIGH event using 
port_fdb_add() will not succeed with that using the ops I use now. 
Otherwise it has be all with port_fb_add() with new lists keeping the 
userspace added blackhole fdb entries.

On 2022-09-21 09:15, Ido Schimmel wrote:
> 	bridge fdb add `mac_get $h2` dev br0 blackhole
To make this work, I think we need to change the concept, so that 
blackhole FDB entries are added to ports connected to the bridge, thus
      bridge fdb add MAC dev $swpX master blackhole

This makes sense as the driver adds them based on the port where the 
SMAC is seen, even though the effect of the blackhole FDB entry is 
switch wide.

Adding them to the bridge (e.g. f.ex. br0) will not work in the SW 
bridge as the entries then are not found. We could deny this possibility 
or just document the use?


For offloaded I can change the add, so that it does a delete (even if 
none are present) and a add, thus facilitating the replace.

How does this sound?