| 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
| ||
|
Message-ID: <CAH3MdRVm1PJyGdpKOUw0RVQGX-48m05u8tS0+bwBHio=1up+ew@mail.gmail.com>
Date: Mon, 22 Apr 2019 16:40:21 -0700
From: Y Song <ys114321@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev <netdev@...r.kernel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
alan.maguire@...cle.com, Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: expand test_tc_tunnel with
SIT encap
On Mon, Apr 22, 2019 at 7:58 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> From: Willem de Bruijn <willemb@...gle.com>
>
> So far, all BPF tc tunnel testcases encapsulate in the same network
> protocol. Add an encap testcase that requires updating skb->protocol.
>
> The 6in4 tunnel encapsulates an IPv6 packet inside an IPv4 tunnel.
> Verify that bpf_skb_net_grow correctly updates skb->protocol to
> select the right protocol handler in __netif_receive_skb_core.
>
> The BPF program should also manually update the link layer header to
> encode the right network protocol.
>
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> ---
> tools/testing/selftests/bpf/config | 1 +
> .../selftests/bpf/progs/test_tc_tunnel.c | 55 +++++++++++++++++--
> tools/testing/selftests/bpf/test_tc_tunnel.sh | 20 ++++++-
> 3 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 8c976476f6fdc..f7a0744db31e1 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -33,3 +33,4 @@ CONFIG_MPLS=y
> CONFIG_NET_MPLS_GSO=m
> CONFIG_MPLS_ROUTING=m
> CONFIG_MPLS_IPTUNNEL=m
> +CONFIG_IPV6_SIT=m
> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> index ab56a6a72b7a5..94ae1caab2bfc 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> @@ -77,17 +77,43 @@ static __always_inline int encap_ipv4(struct __sk_buff *skb, __u8 encap_proto,
> struct v4hdr h_outer;
> struct tcphdr tcph;
> int olen, l2_len;
> + int tcp_off;
> __u64 flags;
>
> - if (bpf_skb_load_bytes(skb, ETH_HLEN, &iph_inner,
> - sizeof(iph_inner)) < 0)
> - return TC_ACT_OK;
> + if (encap_proto == IPPROTO_IPV6) {
> + const __u32 saddr = (192 << 24) | (168 << 16) | (1 << 8) | 1;
> + const __u32 daddr = (192 << 24) | (168 << 16) | (1 << 8) | 2;
> + struct ipv6hdr iph6_inner;
> +
> + if (bpf_skb_load_bytes(skb, ETH_HLEN, &iph6_inner,
> + sizeof(iph6_inner)) < 0)
> + return TC_ACT_OK;
> +
> + /* convert to viable ipv4 header */
> + memset(&iph_inner, 0, sizeof(iph_inner));
> + iph_inner.version = 4;
> + iph_inner.ihl = 5;
> + iph_inner.tot_len = bpf_htons(sizeof(iph6_inner) +
> + bpf_ntohs(iph6_inner.payload_len));
> + iph_inner.ttl = iph6_inner.hop_limit - 1;
> + iph_inner.protocol = iph6_inner.nexthdr;
> + iph_inner.saddr = __bpf_constant_htonl(saddr);
> + iph_inner.daddr = __bpf_constant_htonl(daddr);
The code seems correctly. But maybe some variable renaming or
comments can help improve readability.
For example, here iph_inner (ipv4) intends to represent the
inner ipv6 and iph_inner.protocol is assigned to iph6_inner.nexthdr
although it is correctly handled later with h_outer.ip logic.
> +
> + tcp_off = sizeof(iph6_inner);
> + } else {
> + if (bpf_skb_load_bytes(skb, ETH_HLEN, &iph_inner,
> + sizeof(iph_inner)) < 0)
> + return TC_ACT_OK;
> +
> + tcp_off = sizeof(iph_inner);
> + }
>
> /* filter only packets we want */
> if (iph_inner.ihl != 5 || iph_inner.protocol != IPPROTO_TCP)
> return TC_ACT_OK;
>
> - if (bpf_skb_load_bytes(skb, ETH_HLEN + sizeof(iph_inner),
> + if (bpf_skb_load_bytes(skb, ETH_HLEN + tcp_off,
> &tcph, sizeof(tcph)) < 0)
> return TC_ACT_OK;
>
> @@ -129,6 +155,7 @@ static __always_inline int encap_ipv4(struct __sk_buff *skb, __u8 encap_proto,
> l2_len);
> break;
> case IPPROTO_IPIP:
> + case IPPROTO_IPV6:
> break;
> default:
> return TC_ACT_OK;
> @@ -164,6 +191,17 @@ static __always_inline int encap_ipv4(struct __sk_buff *skb, __u8 encap_proto,
> BPF_F_INVALIDATE_HASH) < 0)
> return TC_ACT_SHOT;
>
> + /* if changing outer proto type, update eth->h_proto */
> + if (encap_proto == IPPROTO_IPV6) {
> + struct ethhdr eth;
> +
> + if (bpf_skb_load_bytes(skb, 0, ð, sizeof(eth)) < 0)
> + return TC_ACT_SHOT;
> + eth.h_proto = bpf_htons(ETH_P_IP);
> + if (bpf_skb_store_bytes(skb, 0, ð, sizeof(eth), 0) < 0)
> + return TC_ACT_SHOT;
> + }
> +
> return TC_ACT_OK;
> }
>
> @@ -325,6 +363,15 @@ int __encap_udp_eth(struct __sk_buff *skb)
> return TC_ACT_OK;
> }
>
> +SEC("encap_sit_none")
> +int __encap_sit_none(struct __sk_buff *skb)
> +{
> + if (skb->protocol == __bpf_constant_htons(ETH_P_IPV6))
> + return encap_ipv4(skb, IPPROTO_IPV6, ETH_P_IP);
> + else
> + return TC_ACT_OK;
> +}
> +
> SEC("encap_ip6tnl_none")
> int __encap_ip6tnl_none(struct __sk_buff *skb)
> {
[...]
> --
> 2.21.0.593.g511ec345e18-goog
>
Powered by blists - more mailing lists