netdev - Re: [PATCH bpf-next 2/2] selftest/bpf: remove unused bpf tunnel testing code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201106075536.GT2531@dhcp-12-153.nay.redhat.com>
Date:   Fri, 6 Nov 2020 15:55:36 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     netdev@...r.kernel.org, William Tu <u9012063@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next 2/2] selftest/bpf: remove unused bpf tunnel
 testing code

On Thu, Nov 05, 2020 at 11:30:35PM -0800, Martin KaFai Lau wrote:
> [ ... ]
> 
> > @@ -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;
> v4 hdr here.

Ah, right, I didn't notice this. I will fix it, maybe by checking
skb->family and use different IPv4,v6 hdr.

> > -SEC("ip6ip6_set_tunnel")
> > -int _ip6ip6_set_tunnel(struct __sk_buff *skb)
> > -{
> > -	struct bpf_tunnel_key key = {};
> > -	void *data = (void *)(long)skb->data;
> > -	struct ipv6hdr *iph = data;
> IIUC, the patch is to replace _ip6ip6_set_tunnel with _ipip6_set_tunnel.
> 
> Are they testing the same thing?  At least, _ip6ip6_set_tunnel()
> is expecting a v6 hdr here.

Yes, the v4/v6 hdr here is just to check the data length.

> 
> > -	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) {
> > -		ERROR(1);
> > -		return TC_ACT_SHOT;
> > -	}

^^ here

> > -
> > -	key.remote_ipv6[0] = bpf_htonl(0x2401db00);
> > -	key.tunnel_ttl = 64;

The code logic is same. It set tunnel remote addr to dst IPv6 address, as
they are both testing IP(v4 or v6) over IPv6 tunnel.

Thanks
Hangbin