Message ID | 1528977666-26477-1-git-send-email-u9012063@gmail.com |
---|---|
State | Accepted, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf-net] selftests/bpf: delete xfrm tunnel when test exits. | expand |
On Thu, Jun 14, 2018 at 05:01:06AM -0700, William Tu wrote: > Make the printting of bpf xfrm tunnel better and > cleanup xfrm state and policy when xfrm test finishes. LGTM. The subject tag actually meant s/bpf-net/bpf-next/? It makes sense to be in bpf-next but I think bpf-next is still closed. Please repost later. > > Signed-off-by: William Tu <u9012063@gmail.com> > --- > tools/testing/selftests/bpf/test_tunnel.sh | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh > index aeb2901f21f4..7b1946b340be 100755 > --- a/tools/testing/selftests/bpf/test_tunnel.sh > +++ b/tools/testing/selftests/bpf/test_tunnel.sh > @@ -608,28 +608,26 @@ setup_xfrm_tunnel() > test_xfrm_tunnel() > { > config_device > - #tcpdump -nei veth1 ip & > - output=$(mktemp) > - cat /sys/kernel/debug/tracing/trace_pipe | tee $output & > - setup_xfrm_tunnel > + > /sys/kernel/debug/tracing/trace > + setup_xfrm_tunnel > tc qdisc add dev veth1 clsact > tc filter add dev veth1 proto ip ingress bpf da obj test_tunnel_kern.o \ > sec xfrm_get_state > ip netns exec at_ns0 ping $PING_ARG 10.1.1.200 > sleep 1 > - grep "reqid 1" $output > + grep "reqid 1" /sys/kernel/debug/tracing/trace > check_err $? > - grep "spi 0x1" $output > + grep "spi 0x1" /sys/kernel/debug/tracing/trace > check_err $? > - grep "remote ip 0xac100164" $output > + grep "remote ip 0xac100164" /sys/kernel/debug/tracing/trace > check_err $? > cleanup > > if [ $ret -ne 0 ]; then > - echo -e ${RED}"FAIL: xfrm tunnel"${NC} > - return 1 > - fi > - echo -e ${GREEN}"PASS: xfrm tunnel"${NC} > + echo -e ${RED}"FAIL: xfrm tunnel"${NC} > + return 1 > + fi > + echo -e ${GREEN}"PASS: xfrm tunnel"${NC} > } > > attach_bpf() > @@ -657,6 +655,10 @@ cleanup() > ip link del ip6geneve11 2> /dev/null > ip link del erspan11 2> /dev/null > ip link del ip6erspan11 2> /dev/null > + ip xfrm policy delete dir out src 10.1.1.200/32 dst 10.1.1.100/32 2> /dev/null > + ip xfrm policy delete dir in src 10.1.1.100/32 dst 10.1.1.200/32 2> /dev/null > + ip xfrm state delete src 172.16.1.100 dst 172.16.1.200 proto esp spi 0x1 2> /dev/null > + ip xfrm state delete src 172.16.1.200 dst 172.16.1.100 proto esp spi 0x2 2> /dev/null > } > > cleanup_exit() > -- > 2.7.4 >
On 06/15/2018 12:30 AM, Martin KaFai Lau wrote: > On Thu, Jun 14, 2018 at 05:01:06AM -0700, William Tu wrote: >> Make the printting of bpf xfrm tunnel better and >> cleanup xfrm state and policy when xfrm test finishes. > LGTM. The subject tag actually meant s/bpf-net/bpf-next/? > > It makes sense to be in bpf-next but I think bpf-next is still closed. > Please repost later. But given this fixes up missing cleanup of xfrm policy/state that was added earlier as part of the test, I think bpf would be fine. (Subject is a bit confusing indeed either bpf resp. net tree or bpf-next was meant.) Thanks, Daniel
On 06/15/2018 02:05 AM, Daniel Borkmann wrote: > On 06/15/2018 12:30 AM, Martin KaFai Lau wrote: >> On Thu, Jun 14, 2018 at 05:01:06AM -0700, William Tu wrote: >>> Make the printting of bpf xfrm tunnel better and >>> cleanup xfrm state and policy when xfrm test finishes. >> LGTM. The subject tag actually meant s/bpf-net/bpf-next/? >> >> It makes sense to be in bpf-next but I think bpf-next is still closed. >> Please repost later. > > But given this fixes up missing cleanup of xfrm policy/state that was > added earlier as part of the test, I think bpf would be fine. (Subject > is a bit confusing indeed either bpf resp. net tree or bpf-next was > meant.) Ok, took it in, thanks William!
> On 14 Jun 2018, at 15:01, William Tu <u9012063@gmail.com> wrote: > > Make the printting of bpf xfrm tunnel better and > cleanup xfrm state and policy when xfrm test finishes. Yeah the ‘tee’ was useful when developing the test - I could see what’s going on :) Now that it’s in ‘selftests’ it’s definitely better without it. Thanks for the cleanup! Eyal.
On Thu, Jun 14, 2018 at 10:24 PM, Eyal Birger <eyal.birger@gmail.com> wrote: > > >> On 14 Jun 2018, at 15:01, William Tu <u9012063@gmail.com> wrote: >> >> Make the printting of bpf xfrm tunnel better and >> cleanup xfrm state and policy when xfrm test finishes. > > Yeah the ‘tee’ was useful when developing the test - I could see what’s going on :) > > Now that it’s in ‘selftests’ it’s definitely better without it. > > Thanks for the cleanup! > Eyal. Hi Eyal, Thanks for double check! Hi Daniel and Martin, Sorry for the confusing "bpf-net". It should be "net" Thanks William
On 06/15/2018 02:43 PM, William Tu wrote: > On Thu, Jun 14, 2018 at 10:24 PM, Eyal Birger <eyal.birger@gmail.com> wrote: >> >> >>> On 14 Jun 2018, at 15:01, William Tu <u9012063@gmail.com> wrote: >>> >>> Make the printting of bpf xfrm tunnel better and >>> cleanup xfrm state and policy when xfrm test finishes. >> >> Yeah the ‘tee’ was useful when developing the test - I could see what’s going on :) >> >> Now that it’s in ‘selftests’ it’s definitely better without it. >> >> Thanks for the cleanup! >> Eyal. > > Hi Eyal, > Thanks for double check! > > Hi Daniel and Martin, > Sorry for the confusing "bpf-net". It should be "net" Yep, took it into bpf, PR to David will come later today. Thanks, Daniel
diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh index aeb2901f21f4..7b1946b340be 100755 --- a/tools/testing/selftests/bpf/test_tunnel.sh +++ b/tools/testing/selftests/bpf/test_tunnel.sh @@ -608,28 +608,26 @@ setup_xfrm_tunnel() test_xfrm_tunnel() { config_device - #tcpdump -nei veth1 ip & - output=$(mktemp) - cat /sys/kernel/debug/tracing/trace_pipe | tee $output & - setup_xfrm_tunnel + > /sys/kernel/debug/tracing/trace + setup_xfrm_tunnel tc qdisc add dev veth1 clsact tc filter add dev veth1 proto ip ingress bpf da obj test_tunnel_kern.o \ sec xfrm_get_state ip netns exec at_ns0 ping $PING_ARG 10.1.1.200 sleep 1 - grep "reqid 1" $output + grep "reqid 1" /sys/kernel/debug/tracing/trace check_err $? - grep "spi 0x1" $output + grep "spi 0x1" /sys/kernel/debug/tracing/trace check_err $? - grep "remote ip 0xac100164" $output + grep "remote ip 0xac100164" /sys/kernel/debug/tracing/trace check_err $? cleanup if [ $ret -ne 0 ]; then - echo -e ${RED}"FAIL: xfrm tunnel"${NC} - return 1 - fi - echo -e ${GREEN}"PASS: xfrm tunnel"${NC} + echo -e ${RED}"FAIL: xfrm tunnel"${NC} + return 1 + fi + echo -e ${GREEN}"PASS: xfrm tunnel"${NC} } attach_bpf() @@ -657,6 +655,10 @@ cleanup() ip link del ip6geneve11 2> /dev/null ip link del erspan11 2> /dev/null ip link del ip6erspan11 2> /dev/null + ip xfrm policy delete dir out src 10.1.1.200/32 dst 10.1.1.100/32 2> /dev/null + ip xfrm policy delete dir in src 10.1.1.100/32 dst 10.1.1.200/32 2> /dev/null + ip xfrm state delete src 172.16.1.100 dst 172.16.1.200 proto esp spi 0x1 2> /dev/null + ip xfrm state delete src 172.16.1.200 dst 172.16.1.100 proto esp spi 0x2 2> /dev/null } cleanup_exit()
Make the printting of bpf xfrm tunnel better and cleanup xfrm state and policy when xfrm test finishes. Signed-off-by: William Tu <u9012063@gmail.com> --- tools/testing/selftests/bpf/test_tunnel.sh | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)