Subject: Re: [PATCH v2 net-next 6/6] selftests: forwarding: add dynamic FDB test

From: Ido Schimmel <[email-masked]>
Date: Mon, 20 Mar 2023 10:44:50 +0200

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
>