On Fri, Feb 18, 2022 at 04:51:48PM +0100, Hans Schultz wrote: > These tests check that the basic locked port feature works, so that no 'host' > can communicate (ping) through a locked port unless the MAC address of the > 'host' interface is in the forwarding database of the bridge.
Thanks for adding the test. I assume this was tested with both mv88e6xxx and veth? > > Signed-off-by: Hans Schultz <[email protected]> > --- > .../testing/selftests/net/forwarding/Makefile | 1 + > .../net/forwarding/bridge_locked_port.sh | 174 ++++++++++++++++++ > tools/testing/selftests/net/forwarding/lib.sh | 16 ++ > 3 files changed, 191 insertions(+) > create mode 100755 > tools/testing/selftests/net/forwarding/bridge_locked_port.sh > > diff --git a/tools/testing/selftests/net/forwarding/Makefile > b/tools/testing/selftests/net/forwarding/Makefile > index 72ee644d47bf..8fa97ae9af9e 100644 > --- a/tools/testing/selftests/net/forwarding/Makefile > +++ b/tools/testing/selftests/net/forwarding/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0+ OR MIT > > TEST_PROGS = bridge_igmp.sh \ > + bridge_locked_port.sh \ > bridge_port_isolation.sh \ > bridge_sticky_fdb.sh \ > bridge_vlan_aware.sh \ > diff --git a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh > b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh > new file mode 100755 > index 000000000000..d2805441b325 > --- /dev/null > +++ b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh > @@ -0,0 +1,174 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > + > +ALL_TESTS="locked_port_ipv4 locked_port_ipv6 locked_port_vlan" > +NUM_NETIFS=4 > +CHECK_TC="no" > +source lib.sh > + > +h1_create() > +{ > + simple_if_init $h1 192.0.2.1/24 2001:db8:1::1/64 > + vrf_create "vrf-vlan-h1" > + ip link set dev vrf-vlan-h1 up > + vlan_create $h1 100 vrf-vlan-h1 192.0.3.1/24 2001:db8:3::1/64 In the tests we try to use only addresses specified in RFC 5737. Instead of 192.0.3.0/24 I suggest 198.51.100.0/24 > +} > + > +h1_destroy() > +{ > + vlan_destroy $h1 100 > + simple_if_fini $h1 192.0.2.1/24 2001:db8:1::1/64 > +} > + > +h2_create() > +{ > + simple_if_init $h2 192.0.2.2/24 2001:db8:1::2/64 > + vrf_create "vrf-vlan-h2" > + ip link set dev vrf-vlan-h2 up > + vlan_create $h2 100 vrf-vlan-h2 192.0.3.2/24 2001:db8:3::2/64 > +} > + > +h2_destroy() > +{ > + vlan_destroy $h2 100 > + simple_if_fini $h2 192.0.2.2/24 2001:db8:1::2/64 > +} > + > +switch_create() > +{ > + ip link add dev br0 type bridge vlan_filtering 1 > + > + ip link set dev $swp1 master br0 > + ip link set dev $swp2 master br0 > + > + ip link set dev br0 up > + ip link set dev $swp1 up > + ip link set dev $swp2 up > + > + bridge link set dev $swp1 learning off > +} > + > +switch_destroy() > +{ > + ip link set dev $swp2 down > + ip link set dev $swp1 down > + > + ip link del dev br0 > +} > + > +setup_prepare() > +{ > + h1=${NETIFS[p1]} > + swp1=${NETIFS[p2]} > + > + swp2=${NETIFS[p3]} > + h2=${NETIFS[p4]} > + > + vrf_prepare > + > + h1_create > + h2_create > + > + switch_create > +} > + > +cleanup() > +{ > + pre_cleanup > + > + switch_destroy > + > + h2_destroy > + h1_destroy > + > + vrf_cleanup > +} > + > +ifaddr() We already have mac_get() > +{ > + ip -br link show dev "$1" | awk '{ print($3); }' > +} > + > +locked_port_ipv4() > +{ > + RET=0 > + > + check_locked_port_support || return 0 > + > + ping_do $h1 192.0.2.2 > + check_err $? "Ping didn't work when it should have" Better to use unique error messages that pinpoint the problem: "Ping did not work before locking port" > + > + bridge link set dev $swp1 locked on > + > + ping_do $h1 192.0.2.2 > + check_fail $? "Ping worked when it should not have" "Ping worked after locking port, but before adding a FDB entry" > + > + bridge fdb add `ifaddr $h1` dev $swp1 master static bridge fdb add $(mac_get $h1) dev $swp1 master static > + > + ping_do $h1 192.0.2.2 > + check_err $? "Ping didn't work when it should have" "Ping did not work after locking port and adding a FDB entry" > + > + bridge link set dev $swp1 locked off > + bridge fdb del `ifaddr $h1` dev $swp1 master static I suggest to add another test case here to see that ping works after unlocking the port and removing the FDB entry Same comments on the other test cases > + log_test "Locked port ipv4" > +} > + > +locked_port_vlan() > +{ > + RET=0 > + > + check_locked_port_support || return 0 > + check_vlan_filtering_support || return 0 Why this check is needed? The bridge was already created with "vlan_filtering 1" > + > + bridge vlan add vid 100 dev $swp1 tagged Not familiar with "tagged" keyword. I believe iproute2 ignores it. Please drop it > + bridge vlan add vid 100 dev $swp2 tagged > + > + ping_do $h1.100 192.0.3.2 > + check_err $? "Ping didn't work when it should have" > + > + bridge link set dev $swp1 locked on > + ping_do $h1.100 192.0.3.2 > + check_fail $? "Ping worked when it should not have" > + > + bridge fdb add `ifaddr $h1` dev $swp1 vlan 100 master static > + > + ping_do $h1.100 192.0.3.2 > + check_err $? "Ping didn't work when it should have" > + > + bridge link set dev $swp1 locked off > + bridge vlan del vid 100 dev $swp1 > + bridge vlan del vid 100 dev $swp2 > + bridge fdb del `ifaddr $h1` dev $swp1 vlan 100 master static > + log_test "Locked port vlan" > +} > + > +locked_port_ipv6() > +{ > + RET=0 > + check_locked_port_support || return 0 > + > + ping6_do $h1 2001:db8:1::2 > + check_err $? "Ping6 didn't work when it should have" > + > + bridge link set dev $swp1 locked on > + > + ping6_do $h1 2001:db8:1::2 > + check_fail $? "Ping worked when it should not have" > + > + bridge fdb add `ifaddr $h1` dev $swp1 master static > + ping6_do $h1 2001:db8:1::2 > + check_err $? "Ping didn't work when it should have" > + > + bridge link set dev $swp1 locked off > + bridge fdb del `ifaddr $h1` dev $swp1 master static > + log_test "Locked port ipv6" > +} > + > +trap cleanup EXIT > + > +setup_prepare > +setup_wait > + > +tests_run > + > +exit $EXIT_STATUS > diff --git a/tools/testing/selftests/net/forwarding/lib.sh > b/tools/testing/selftests/net/forwarding/lib.sh > index 7da783d6f453..9ded90f17ead 100644 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -125,6 +125,22 @@ check_ethtool_lanes_support() > fi > } > > +check_locked_port_support() > +{ > + if ! bridge -d link show | grep -q " locked"; then > + echo "SKIP: iproute2 too old; Locked port feature not > supported." > + return $ksft_skip > + fi > +} > + > +check_vlan_filtering_support() > +{ > + if ! bridge -d vlan show | grep -q "state forwarding"; then > + echo "SKIP: vlan filtering not supported." > + return $ksft_skip > + fi > +} > + > if [[ "$(id -u)" -ne 0 ]]; then > echo "SKIP: need root privileges" > exit $ksft_skip > -- > 2.30.2 >
