diff mbox series

[bpf-net] selftests/bpf: delete xfrm tunnel when test exits.

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

Commit Message

William Tu June 14, 2018, 12:01 p.m. UTC
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(-)

Comments

Martin KaFai Lau June 14, 2018, 10:30 p.m. UTC | #1
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
>
Daniel Borkmann June 15, 2018, 12:05 a.m. UTC | #2
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
Daniel Borkmann June 15, 2018, 1:33 a.m. UTC | #3
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!
Eyal Birger June 15, 2018, 5:24 a.m. UTC | #4
> 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.
William Tu June 15, 2018, 12:43 p.m. UTC | #5
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
Daniel Borkmann June 15, 2018, 12:52 p.m. UTC | #6
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 mbox series

Patch

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()