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 <liuhang...@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 via email to