| Message ID | 20240815000707.2006121-9-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e29b82ef27616777e21c07dc263a8769cbdaa358 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | VLAN fixes for Ocelot driver | expand |
On Thu, Aug 15, 2024 at 03:07:01AM +0300, Vladimir Oltean wrote: > The bridge VLAN implementation w.r.t. VLAN protocol is described in > merge commit 1a0b20b25732 ("Merge branch 'bridge-next'"). We are only > sensitive to those VLAN tags whose TPID is equal to the bridge's > vlan_protocol. Thus, an 802.1ad VLAN should be treated as 802.1Q-untagged. > > Add 3 tests which validate that: > - 802.1ad-tagged traffic is learned into the PVID of an 802.1Q-aware > bridge > - Double-tagged traffic is forwarded when just the PVID of the port is > present in the VLAN group of the ports > - Double-tagged traffic is not forwarded when the PVID of the port is > absent from the VLAN group of the ports > > The test passes with both veth and ocelot. Thanks for the test. Passes with mlxsw as well. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Tested-by: Ido Schimmel <idosch@nvidia.com> One question below > --- > .../net/forwarding/bridge_vlan_aware.sh | 54 ++++++++++++++++++- > 1 file changed, 53 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh > index 64bd00fe9a4f..90f8a244ea90 100755 > --- a/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh > +++ b/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh > @@ -1,7 +1,7 @@ > #!/bin/bash > # SPDX-License-Identifier: GPL-2.0 > > -ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding vlan_deletion extern_learn" > +ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding vlan_deletion extern_learn other_tpid" > NUM_NETIFS=4 > CHECK_TC="yes" > source lib.sh > @@ -142,6 +142,58 @@ extern_learn() > bridge fdb del de:ad:be:ef:13:37 dev $swp1 master vlan 1 &> /dev/null > } > > +other_tpid() > +{ > + local mac=de:ad:be:ef:13:37 > + > + # Test that packets with TPID 802.1ad VID 3 + TPID 802.1Q VID 5 are > + # classified as untagged by a bridge with vlan_protocol 802.1Q, and > + # are processed in the PVID of the ingress port (here 1). Not VID 3, > + # and not VID 5. > + RET=0 > + > + tc qdisc add dev $h2 clsact > + tc filter add dev $h2 ingress protocol all pref 1 handle 101 \ > + flower dst_mac $mac action drop > + ip link set $h2 promisc on > + ethtool -K $h2 rx-vlan-filter off rx-vlan-stag-filter off Any reason not to undo it at the end of the test like other settings? > + > + $MZ -q $h1 -c 1 -b $mac -a own "88:a8 00:03 81:00 00:05 08:00 aa-aa-aa-aa-aa-aa-aa-aa-aa" > + sleep 1 > + > + # Match on 'self' addresses as well, for those drivers which > + # do not push their learned addresses to the bridge software > + # database > + bridge -j fdb show $swp1 | \ > + jq -e ".[] | select(.mac == \"$(mac_get $h1)\") | select(.vlan == 1)" &> /dev/null > + check_err $? "FDB entry was not learned when it should" > + > + log_test "FDB entry in PVID for VLAN-tagged with other TPID" > + > + RET=0 > + tc -j -s filter show dev $h2 ingress \ > + | jq -e ".[] | select(.options.handle == 101) \ > + | select(.options.actions[0].stats.packets == 1)" &> /dev/null > + check_err $? "Packet was not forwarded when it should" > + log_test "Reception of VLAN with other TPID as untagged" > + > + bridge vlan del dev $swp1 vid 1 > + > + $MZ -q $h1 -c 1 -b $mac -a own "88:a8 00:03 81:00 00:05 08:00 aa-aa-aa-aa-aa-aa-aa-aa-aa" > + sleep 1 > + > + RET=0 > + tc -j -s filter show dev $h2 ingress \ > + | jq -e ".[] | select(.options.handle == 101) \ > + | select(.options.actions[0].stats.packets == 1)" &> /dev/null > + check_err $? "Packet was forwarded when should not" > + log_test "Reception of VLAN with other TPID as untagged (no PVID)" > + > + bridge vlan add dev $swp1 vid 1 pvid untagged > + ip link set $h2 promisc off > + tc qdisc del dev $h2 clsact > +} > + > trap cleanup EXIT > > setup_prepare > -- > 2.34.1 >
On Thu, Aug 15, 2024 at 12:11:07PM +0300, Ido Schimmel wrote: > Thanks for the test. Passes with mlxsw as well. Thanks for testing. > > + ethtool -K $h2 rx-vlan-filter off rx-vlan-stag-filter off > > Any reason not to undo it at the end of the test like other settings? A combination of laziness to add even more logic to save/restore the ethtool features (should probably live in lib.sh), plus a persistent question of "who cares anyway". The default values are driver defined anyway, so it's likely that anyone who cares will control these features prior to starting their application. Plus, turning off RX VLAN filtering offload should not technically leave anything broken behind.
diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh index 64bd00fe9a4f..90f8a244ea90 100755 --- a/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh +++ b/tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh @@ -1,7 +1,7 @@ #!/bin/bash # SPDX-License-Identifier: GPL-2.0 -ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding vlan_deletion extern_learn" +ALL_TESTS="ping_ipv4 ping_ipv6 learning flooding vlan_deletion extern_learn other_tpid" NUM_NETIFS=4 CHECK_TC="yes" source lib.sh @@ -142,6 +142,58 @@ extern_learn() bridge fdb del de:ad:be:ef:13:37 dev $swp1 master vlan 1 &> /dev/null } +other_tpid() +{ + local mac=de:ad:be:ef:13:37 + + # Test that packets with TPID 802.1ad VID 3 + TPID 802.1Q VID 5 are + # classified as untagged by a bridge with vlan_protocol 802.1Q, and + # are processed in the PVID of the ingress port (here 1). Not VID 3, + # and not VID 5. + RET=0 + + tc qdisc add dev $h2 clsact + tc filter add dev $h2 ingress protocol all pref 1 handle 101 \ + flower dst_mac $mac action drop + ip link set $h2 promisc on + ethtool -K $h2 rx-vlan-filter off rx-vlan-stag-filter off + + $MZ -q $h1 -c 1 -b $mac -a own "88:a8 00:03 81:00 00:05 08:00 aa-aa-aa-aa-aa-aa-aa-aa-aa" + sleep 1 + + # Match on 'self' addresses as well, for those drivers which + # do not push their learned addresses to the bridge software + # database + bridge -j fdb show $swp1 | \ + jq -e ".[] | select(.mac == \"$(mac_get $h1)\") | select(.vlan == 1)" &> /dev/null + check_err $? "FDB entry was not learned when it should" + + log_test "FDB entry in PVID for VLAN-tagged with other TPID" + + RET=0 + tc -j -s filter show dev $h2 ingress \ + | jq -e ".[] | select(.options.handle == 101) \ + | select(.options.actions[0].stats.packets == 1)" &> /dev/null + check_err $? "Packet was not forwarded when it should" + log_test "Reception of VLAN with other TPID as untagged" + + bridge vlan del dev $swp1 vid 1 + + $MZ -q $h1 -c 1 -b $mac -a own "88:a8 00:03 81:00 00:05 08:00 aa-aa-aa-aa-aa-aa-aa-aa-aa" + sleep 1 + + RET=0 + tc -j -s filter show dev $h2 ingress \ + | jq -e ".[] | select(.options.handle == 101) \ + | select(.options.actions[0].stats.packets == 1)" &> /dev/null + check_err $? "Packet was forwarded when should not" + log_test "Reception of VLAN with other TPID as untagged (no PVID)" + + bridge vlan add dev $swp1 vid 1 pvid untagged + ip link set $h2 promisc off + tc qdisc del dev $h2 clsact +} + trap cleanup EXIT setup_prepare
The bridge VLAN implementation w.r.t. VLAN protocol is described in merge commit 1a0b20b25732 ("Merge branch 'bridge-next'"). We are only sensitive to those VLAN tags whose TPID is equal to the bridge's vlan_protocol. Thus, an 802.1ad VLAN should be treated as 802.1Q-untagged. Add 3 tests which validate that: - 802.1ad-tagged traffic is learned into the PVID of an 802.1Q-aware bridge - Double-tagged traffic is forwarded when just the PVID of the port is present in the VLAN group of the ports - Double-tagged traffic is not forwarded when the PVID of the port is absent from the VLAN group of the ports The test passes with both veth and ocelot. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- .../net/forwarding/bridge_vlan_aware.sh | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-)