All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <kafai@fb.com>
To: Hangbin Liu <liuhangbin@gmail.com>
Cc: <netdev@vger.kernel.org>, William Tu <u9012063@gmail.com>,
	Daniel Borkmann <daniel@iogearbox.net>, <bpf@vger.kernel.org>
Subject: Re: [PATCHv2 net 1/2] selftest/bpf: add missed ip6ip6 test back
Date: Fri, 6 Nov 2020 18:15:44 -0800	[thread overview]
Message-ID: <20201107021544.tajvaxcxnc3pmppe@kafai-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <20201106090117.3755588-2-liuhangbin@gmail.com>

On Fri, Nov 06, 2020 at 05:01:16PM +0800, Hangbin Liu wrote:
> In comment 173ca26e9b51 ("samples/bpf: add comprehensive ipip, ipip6,
> ip6ip6 test") we added ip6ip6 test for bpf tunnel testing. But in commit
> 933a741e3b82 ("selftests/bpf: bpf tunnel test.") when we moved it to
> the current folder, we didn't add it.
> 
> This patch add the ip6ip6 test back to bpf tunnel test. Update the ipip6's
> topology for both IPv4 and IPv6 testing. Since iperf test is removed as
> currect framework simplified it in purpose, I also removed unused tcp
> checkings in test_tunnel_kern.c.
> 
> Fixes: 933a741e3b82 ("selftests/bpf: bpf tunnel test.")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> 
> v2:
> update add_ipip6tnl_tunnel() to add_ip6tnl_tunnel()
> keep the _ip6ip6_set_tunnel() section.
> ---
>  .../selftests/bpf/progs/test_tunnel_kern.c    | 44 +++----------------
>  tools/testing/selftests/bpf/test_tunnel.sh    | 43 ++++++++++++++++--
>  2 files changed, 44 insertions(+), 43 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index f48dbfe24ddc..7fd95befef56 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -15,7 +15,6 @@
>  #include <linux/ip.h>
>  #include <linux/ipv6.h>
>  #include <linux/types.h>
> -#include <linux/tcp.h>
>  #include <linux/socket.h>
>  #include <linux/pkt_cls.h>
>  #include <linux/erspan.h>
> @@ -528,30 +527,17 @@ int _ipip_set_tunnel(struct __sk_buff *skb)
>  	struct bpf_tunnel_key key = {};
>  	void *data = (void *)(long)skb->data;
>  	struct iphdr *iph = data;
> -	struct tcphdr *tcp = data + sizeof(*iph);
>  	void *data_end = (void *)(long)skb->data_end;
>  	int ret;
>  
>  	/* single length check */
> -	if (data + sizeof(*iph) + sizeof(*tcp) > data_end) {
> +	if (data + sizeof(*iph) > data_end) {
>  		ERROR(1);
>  		return TC_ACT_SHOT;
>  	}
>  
> +	key.remote_ipv4 = 0xac100164; /* 172.16.1.100 */
>  	key.tunnel_ttl = 64;
> -	if (iph->protocol == IPPROTO_ICMP) {
> -		key.remote_ipv4 = 0xac100164; /* 172.16.1.100 */
I think it is still good to check IPPROTO_ICMP
even ping is the only test.

> -	} else {
> -		if (iph->protocol != IPPROTO_TCP || iph->ihl != 5)
> -			return TC_ACT_SHOT;
> -
> -		if (tcp->dest == bpf_htons(5200))
> -			key.remote_ipv4 = 0xac100164; /* 172.16.1.100 */
> -		else if (tcp->dest == bpf_htons(5201))
> -			key.remote_ipv4 = 0xac100165; /* 172.16.1.101 */
> -		else
> -			return TC_ACT_SHOT;
> -	}
>  
>  	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key), 0);
>  	if (ret < 0) {
> @@ -585,12 +571,11 @@ int _ipip6_set_tunnel(struct __sk_buff *skb)
>  	struct bpf_tunnel_key key = {};
>  	void *data = (void *)(long)skb->data;
>  	struct iphdr *iph = data;
> -	struct tcphdr *tcp = data + sizeof(*iph);
>  	void *data_end = (void *)(long)skb->data_end;
>  	int ret;
>  
>  	/* single length check */
> -	if (data + sizeof(*iph) + sizeof(*tcp) > data_end) {
> +	if (data + sizeof(*iph) > data_end) {
>  		ERROR(1);
>  		return TC_ACT_SHOT;
>  	}
> @@ -634,37 +619,18 @@ int _ip6ip6_set_tunnel(struct __sk_buff *skb)
>  	struct bpf_tunnel_key key = {};
>  	void *data = (void *)(long)skb->data;
>  	struct ipv6hdr *iph = data;
> -	struct tcphdr *tcp = data + sizeof(*iph);
>  	void *data_end = (void *)(long)skb->data_end;
>  	int ret;
>  
>  	/* single length check */
> -	if (data + sizeof(*iph) + sizeof(*tcp) > data_end) {
> +	if (data + sizeof(*iph) > data_end) {
>  		ERROR(1);
>  		return TC_ACT_SHOT;
>  	}
>  
> -	key.remote_ipv6[0] = bpf_htonl(0x2401db00);
> +	key.remote_ipv6[3] = bpf_htonl(0x11); /* ::11 */
>  	key.tunnel_ttl = 64;
>  
> -	if (iph->nexthdr == 58 /* NEXTHDR_ICMP */) {
Same here. Can this check be kept?

Others LGTM.

> -		key.remote_ipv6[3] = bpf_htonl(1);
> -	} else {
> -		if (iph->nexthdr != 6 /* NEXTHDR_TCP */) {
> -			ERROR(iph->nexthdr);
> -			return TC_ACT_SHOT;
> -		}
> -
> -		if (tcp->dest == bpf_htons(5200)) {
> -			key.remote_ipv6[3] = bpf_htonl(1);
> -		} else if (tcp->dest == bpf_htons(5201)) {
> -			key.remote_ipv6[3] = bpf_htonl(2);
> -		} else {
> -			ERROR(tcp->dest);
> -			return TC_ACT_SHOT;
> -		}
> -	}
> -
>  	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
>  				     BPF_F_TUNINFO_IPV6);
>  	if (ret < 0) {

  reply	other threads:[~2020-11-07  2:16 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-03  4:29 [PATCH bpf-next 0/2] selftest/bpf: improve bpf tunnel test Hangbin Liu
2020-11-03  4:29 ` [PATCH bpf-next 1/2] selftest/bpf: add missed ip6ip6 test back Hangbin Liu
2020-11-04 18:40   ` Martin KaFai Lau
2020-11-05  3:20     ` Hangbin Liu
2020-11-03  4:29 ` [PATCH bpf-next 2/2] selftest/bpf: remove unused bpf tunnel testing code Hangbin Liu
2020-11-06  7:30   ` Martin KaFai Lau
2020-11-06  7:55     ` Hangbin Liu
2020-11-06  8:04       ` Hangbin Liu
2020-11-06  9:01 ` [PATCHv2 net 0/2] Remove unused test_ipip.sh test and add missed ip6ip6 test Hangbin Liu
2020-11-06  9:01   ` [PATCHv2 net 1/2] selftest/bpf: add missed ip6ip6 test back Hangbin Liu
2020-11-07  2:15     ` Martin KaFai Lau [this message]
2020-11-09  3:00       ` Hangbin Liu
2020-11-09 22:25         ` Martin KaFai Lau
2020-11-06  9:01   ` [PATCHv2 net 2/2] samples/bpf: remove unused test_ipip.sh Hangbin Liu
2020-11-07  2:16     ` Martin KaFai Lau
2020-11-06 18:56   ` [PATCHv2 net 0/2] Remove unused test_ipip.sh test and add missed ip6ip6 test Jakub Kicinski
2020-11-06 18:58     ` Alexei Starovoitov
2020-11-07  1:20     ` Hangbin Liu
2020-11-10  1:50   ` [PATCHv3 bpf " Hangbin Liu
2020-11-10  1:50     ` [PATCHv3 bpf 1/2] selftest/bpf: add missed ip6ip6 test back Hangbin Liu
2020-11-10  1:50     ` [PATCHv3 bpf 2/2] samples/bpf: remove unused test_ipip.sh Hangbin Liu
2020-11-10 17:35     ` [PATCHv3 bpf 0/2] Remove unused test_ipip.sh test and add missed ip6ip6 test Martin KaFai Lau
2020-11-11  0:56       ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201107021544.tajvaxcxnc3pmppe@kafai-mbp.dhcp.thefacebook.com \
    --to=kafai@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=u9012063@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.