| Message ID | 20220125081717.1260849-2-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | selftests/bpf: use temp netns for testing | expand |
On Tue, Jan 25, 2022 at 12:17 AM Hangbin Liu <liuhangbin@gmail.com> wrote: > > Use temp netns instead of hard code name for testing in case the netns > already exists. > > Remove the hard code interface index when creating the veth interfaces. > Because when the system loads some virtual interface modules, e.g. tunnels. > the ifindex of 2 will be used and the cmd will fail. > > As the netns has not created if checking environment failed. Trap the > clean up function after checking env. > > Fixes: 8955c1a32987 ("selftests/bpf/xdp_redirect_multi: Limit the tests in netns") > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- LGTM Acked-by: William Tu <u9012063@gmail.com>
Hangbin Liu wrote: > Use temp netns instead of hard code name for testing in case the netns > already exists. > > Remove the hard code interface index when creating the veth interfaces. > Because when the system loads some virtual interface modules, e.g. tunnels. > the ifindex of 2 will be used and the cmd will fail. > > As the netns has not created if checking environment failed. Trap the > clean up function after checking env. > > Fixes: 8955c1a32987 ("selftests/bpf/xdp_redirect_multi: Limit the tests in netns") > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > .../selftests/bpf/test_xdp_redirect_multi.sh | 60 ++++++++++--------- > 1 file changed, 31 insertions(+), 29 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > index 05f872740999..cc57cb87e65f 100755 > --- a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > +++ b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > @@ -32,6 +32,11 @@ DRV_MODE="xdpgeneric xdpdrv xdpegress" > PASS=0 > FAIL=0 > LOG_DIR=$(mktemp -d) > +declare -a NS > +NS[0]="ns0-$(mktemp -u XXXXXX)" > +NS[1]="ns1-$(mktemp -u XXXXXX)" > +NS[2]="ns2-$(mktemp -u XXXXXX)" > +NS[3]="ns3-$(mktemp -u XXXXXX)" > > test_pass() > { > @@ -47,11 +52,9 @@ test_fail() > > clean_up() > { > - for i in $(seq $NUM); do > - ip link del veth$i 2> /dev/null > - ip netns del ns$i 2> /dev/null > + for i in $(seq 0 $NUM); do > + ip netns del ${NS[$i]} 2> /dev/null You dropped the `ip link del veth$i` why is this ok? > done > - ip netns del ns0 2> /dev/null > }
On Wed, Jan 26, 2022 at 09:34:52PM -0800, John Fastabend wrote: > Hangbin Liu wrote: > > Use temp netns instead of hard code name for testing in case the netns > > already exists. > > > > Remove the hard code interface index when creating the veth interfaces. > > Because when the system loads some virtual interface modules, e.g. tunnels. > > the ifindex of 2 will be used and the cmd will fail. > > > > As the netns has not created if checking environment failed. Trap the > > clean up function after checking env. > > > > Fixes: 8955c1a32987 ("selftests/bpf/xdp_redirect_multi: Limit the tests in netns") > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > > --- > > .../selftests/bpf/test_xdp_redirect_multi.sh | 60 ++++++++++--------- > > 1 file changed, 31 insertions(+), 29 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > > index 05f872740999..cc57cb87e65f 100755 > > --- a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > > +++ b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > > @@ -32,6 +32,11 @@ DRV_MODE="xdpgeneric xdpdrv xdpegress" > > PASS=0 > > FAIL=0 > > LOG_DIR=$(mktemp -d) > > +declare -a NS > > +NS[0]="ns0-$(mktemp -u XXXXXX)" > > +NS[1]="ns1-$(mktemp -u XXXXXX)" > > +NS[2]="ns2-$(mktemp -u XXXXXX)" > > +NS[3]="ns3-$(mktemp -u XXXXXX)" > > > > test_pass() > > { > > @@ -47,11 +52,9 @@ test_fail() > > > > clean_up() > > { > > - for i in $(seq $NUM); do > > - ip link del veth$i 2> /dev/null > > - ip netns del ns$i 2> /dev/null > > + for i in $(seq 0 $NUM); do > > + ip netns del ${NS[$i]} 2> /dev/null > > You dropped the `ip link del veth$i` why is this ok? All the veth interfaces are created and attached in the netns. So remove the netns should also remove related veth interfaces. [...] > ip -n ${NS[$i]} link add veth0 type veth peer name veth$i netns ${NS[0]} [...] Thanks Hangbin
Hangbin Liu wrote: > On Wed, Jan 26, 2022 at 09:34:52PM -0800, John Fastabend wrote: > > Hangbin Liu wrote: > > > Use temp netns instead of hard code name for testing in case the netns > > > already exists. > > > > > > Remove the hard code interface index when creating the veth interfaces. > > > Because when the system loads some virtual interface modules, e.g. tunnels. > > > the ifindex of 2 will be used and the cmd will fail. > > > > > > As the netns has not created if checking environment failed. Trap the > > > clean up function after checking env. > > > > > > Fixes: 8955c1a32987 ("selftests/bpf/xdp_redirect_multi: Limit the tests in netns") > > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > > > --- > > > .../selftests/bpf/test_xdp_redirect_multi.sh | 60 ++++++++++--------- > > > 1 file changed, 31 insertions(+), 29 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > > > index 05f872740999..cc57cb87e65f 100755 > > > --- a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > > > +++ b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh > > > @@ -32,6 +32,11 @@ DRV_MODE="xdpgeneric xdpdrv xdpegress" > > > PASS=0 > > > FAIL=0 > > > LOG_DIR=$(mktemp -d) > > > +declare -a NS > > > +NS[0]="ns0-$(mktemp -u XXXXXX)" > > > +NS[1]="ns1-$(mktemp -u XXXXXX)" > > > +NS[2]="ns2-$(mktemp -u XXXXXX)" > > > +NS[3]="ns3-$(mktemp -u XXXXXX)" > > > > > > test_pass() > > > { > > > @@ -47,11 +52,9 @@ test_fail() > > > > > > clean_up() > > > { > > > - for i in $(seq $NUM); do > > > - ip link del veth$i 2> /dev/null > > > - ip netns del ns$i 2> /dev/null > > > + for i in $(seq 0 $NUM); do > > > + ip netns del ${NS[$i]} 2> /dev/null > > > > You dropped the `ip link del veth$i` why is this ok? > > All the veth interfaces are created and attached in the netns. So remove > the netns should also remove related veth interfaces. Assumed so and just checked. Would have been nice to mention in the commit so it couldn't be mistaken for a typo. Anyways Ack from me.
diff --git a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh index 05f872740999..cc57cb87e65f 100755 --- a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh +++ b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh @@ -32,6 +32,11 @@ DRV_MODE="xdpgeneric xdpdrv xdpegress" PASS=0 FAIL=0 LOG_DIR=$(mktemp -d) +declare -a NS +NS[0]="ns0-$(mktemp -u XXXXXX)" +NS[1]="ns1-$(mktemp -u XXXXXX)" +NS[2]="ns2-$(mktemp -u XXXXXX)" +NS[3]="ns3-$(mktemp -u XXXXXX)" test_pass() { @@ -47,11 +52,9 @@ test_fail() clean_up() { - for i in $(seq $NUM); do - ip link del veth$i 2> /dev/null - ip netns del ns$i 2> /dev/null + for i in $(seq 0 $NUM); do + ip netns del ${NS[$i]} 2> /dev/null done - ip netns del ns0 2> /dev/null } # Kselftest framework requirement - SKIP code is 4. @@ -79,23 +82,22 @@ setup_ns() mode="xdpdrv" fi - ip netns add ns0 + ip netns add ${NS[0]} for i in $(seq $NUM); do - ip netns add ns$i - ip -n ns$i link add veth0 index 2 type veth \ - peer name veth$i netns ns0 index $((1 + $i)) - ip -n ns0 link set veth$i up - ip -n ns$i link set veth0 up - - ip -n ns$i addr add 192.0.2.$i/24 dev veth0 - ip -n ns$i addr add 2001:db8::$i/64 dev veth0 + ip netns add ${NS[$i]} + ip -n ${NS[$i]} link add veth0 type veth peer name veth$i netns ${NS[0]} + ip -n ${NS[$i]} link set veth0 up + ip -n ${NS[0]} link set veth$i up + + ip -n ${NS[$i]} addr add 192.0.2.$i/24 dev veth0 + ip -n ${NS[$i]} addr add 2001:db8::$i/64 dev veth0 # Add a neigh entry for IPv4 ping test - ip -n ns$i neigh add 192.0.2.253 lladdr 00:00:00:00:00:01 dev veth0 - ip -n ns$i link set veth0 $mode obj \ + ip -n ${NS[$i]} neigh add 192.0.2.253 lladdr 00:00:00:00:00:01 dev veth0 + ip -n ${NS[$i]} link set veth0 $mode obj \ xdp_dummy.o sec xdp &> /dev/null || \ { test_fail "Unable to load dummy xdp" && exit 1; } IFACES="$IFACES veth$i" - veth_mac[$i]=$(ip -n ns0 link show veth$i | awk '/link\/ether/ {print $2}') + veth_mac[$i]=$(ip -n ${NS[0]} link show veth$i | awk '/link\/ether/ {print $2}') done } @@ -104,10 +106,10 @@ do_egress_tests() local mode=$1 # mac test - ip netns exec ns2 tcpdump -e -i veth0 -nn -l -e &> ${LOG_DIR}/mac_ns1-2_${mode}.log & - ip netns exec ns3 tcpdump -e -i veth0 -nn -l -e &> ${LOG_DIR}/mac_ns1-3_${mode}.log & + ip netns exec ${NS[2]} tcpdump -e -i veth0 -nn -l -e &> ${LOG_DIR}/mac_ns1-2_${mode}.log & + ip netns exec ${NS[3]} tcpdump -e -i veth0 -nn -l -e &> ${LOG_DIR}/mac_ns1-3_${mode}.log & sleep 0.5 - ip netns exec ns1 ping 192.0.2.254 -i 0.1 -c 4 &> /dev/null + ip netns exec ${NS[1]} ping 192.0.2.254 -i 0.1 -c 4 &> /dev/null sleep 0.5 pkill tcpdump @@ -123,18 +125,18 @@ do_ping_tests() local mode=$1 # ping6 test: echo request should be redirect back to itself, not others - ip netns exec ns1 ip neigh add 2001:db8::2 dev veth0 lladdr 00:00:00:00:00:02 + ip netns exec ${NS[1]} ip neigh add 2001:db8::2 dev veth0 lladdr 00:00:00:00:00:02 - ip netns exec ns1 tcpdump -i veth0 -nn -l -e &> ${LOG_DIR}/ns1-1_${mode}.log & - ip netns exec ns2 tcpdump -i veth0 -nn -l -e &> ${LOG_DIR}/ns1-2_${mode}.log & - ip netns exec ns3 tcpdump -i veth0 -nn -l -e &> ${LOG_DIR}/ns1-3_${mode}.log & + ip netns exec ${NS[1]} tcpdump -i veth0 -nn -l -e &> ${LOG_DIR}/ns1-1_${mode}.log & + ip netns exec ${NS[2]} tcpdump -i veth0 -nn -l -e &> ${LOG_DIR}/ns1-2_${mode}.log & + ip netns exec ${NS[3]} tcpdump -i veth0 -nn -l -e &> ${LOG_DIR}/ns1-3_${mode}.log & sleep 0.5 # ARP test - ip netns exec ns1 arping -q -c 2 -I veth0 192.0.2.254 + ip netns exec ${NS[1]} arping -q -c 2 -I veth0 192.0.2.254 # IPv4 test - ip netns exec ns1 ping 192.0.2.253 -i 0.1 -c 4 &> /dev/null + ip netns exec ${NS[1]} ping 192.0.2.253 -i 0.1 -c 4 &> /dev/null # IPv6 test - ip netns exec ns1 ping6 2001:db8::2 -i 0.1 -c 2 &> /dev/null + ip netns exec ${NS[1]} ping6 2001:db8::2 -i 0.1 -c 2 &> /dev/null sleep 0.5 pkill tcpdump @@ -180,7 +182,7 @@ do_tests() xdpgeneric) drv_p="-S";; esac - ip netns exec ns0 ./xdp_redirect_multi $drv_p $IFACES &> ${LOG_DIR}/xdp_redirect_${mode}.log & + ip netns exec ${NS[0]} ./xdp_redirect_multi $drv_p $IFACES &> ${LOG_DIR}/xdp_redirect_${mode}.log & xdp_pid=$! sleep 1 if ! ps -p $xdp_pid > /dev/null; then @@ -197,10 +199,10 @@ do_tests() kill $xdp_pid } -trap clean_up EXIT - check_env +trap clean_up EXIT + for mode in ${DRV_MODE}; do setup_ns $mode do_tests $mode
Use temp netns instead of hard code name for testing in case the netns already exists. Remove the hard code interface index when creating the veth interfaces. Because when the system loads some virtual interface modules, e.g. tunnels. the ifindex of 2 will be used and the cmd will fail. As the netns has not created if checking environment failed. Trap the clean up function after checking env. Fixes: 8955c1a32987 ("selftests/bpf/xdp_redirect_multi: Limit the tests in netns") Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- .../selftests/bpf/test_xdp_redirect_multi.sh | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-)