Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 19 Dec 2022 10:41:51 -0800
In-Reply-To: <20221218051734.31411-2-cehrig@cloudflare.com>
Mime-Version: 1.0
References: <20221218051734.31411-1-cehrig@cloudflare.com> <20221218051734.31411-2-cehrig@cloudflare.com>
Message-ID: <Y6Cwb875k9sJyBfx@google.com>
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: Add BPF_F_NO_TUNNEL_KEY test
From:   sdf@google.com
To:     Christian Ehrig <cehrig@cloudflare.com>
Cc:     bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Hao Luo <haoluo@google.com>,
        Jiri Olsa <jolsa@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Mykola Lysenko <mykolal@fb.com>, Shuah Khan <shuah@kernel.org>,
        Joanne Koong <joannelkoong@gmail.com>,
        Kui-Feng Lee <kuifeng@fb.com>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        Maxim Mikityanskiy <maximmi@nvidia.com>,
        Kaixi Fan <fankaixi.li@bytedance.com>,
        Paul Chaignon <paul@isovalent.com>,
        Shmulik Ladkani <shmulik@metanetworks.com>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        linux-kselftest@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"; format=flowed; delsp=yes
Precedence: bulk

On 12/18, Christian Ehrig wrote:
> This patch adds a selftest simulating a GRE sender and receiver using
> tunnel headers without tunnel keys. It validates if packets encapsulated
> using BPF_F_NO_TUNNEL_KEY are decapsulated by a GRE receiver not
> configured with tunnel keys.

> Signed-off-by: Christian Ehrig <cehrig@cloudflare.com>

Acked-by: Stanislav Fomichev <sdf@google.com>

> ---
>   .../selftests/bpf/progs/test_tunnel_kern.c    | 21 ++++++++++
>   tools/testing/selftests/bpf/test_tunnel.sh    | 40 +++++++++++++++++--
>   2 files changed, 58 insertions(+), 3 deletions(-)

> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c  
> b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index 98af55f0bcd3..508da4a23c4f 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -81,6 +81,27 @@ int gre_set_tunnel(struct __sk_buff *skb)
>   	return TC_ACT_OK;
>   }

> +SEC("tc")
> +int gre_set_tunnel_no_key(struct __sk_buff *skb)
> +{
> +	int ret;
> +	struct bpf_tunnel_key key;
> +
> +	__builtin_memset(&key, 0x0, sizeof(key));
> +	key.remote_ipv4 = 0xac100164; /* 172.16.1.100 */
> +	key.tunnel_ttl = 64;
> +
> +	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
> +				     BPF_F_ZERO_CSUM_TX | BPF_F_SEQ_NUMBER |
> +				     BPF_F_NO_TUNNEL_KEY);
> +	if (ret < 0) {
> +		log_err(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
>   SEC("tc")
>   int gre_get_tunnel(struct __sk_buff *skb)
>   {
> diff --git a/tools/testing/selftests/bpf/test_tunnel.sh  
> b/tools/testing/selftests/bpf/test_tunnel.sh
> index 2eaedc1d9ed3..06857b689c11 100755
> --- a/tools/testing/selftests/bpf/test_tunnel.sh
> +++ b/tools/testing/selftests/bpf/test_tunnel.sh
> @@ -66,15 +66,20 @@ config_device()

>   add_gre_tunnel()
>   {
> +	tun_key=
> +	if [ -n "$1" ]; then
> +		tun_key="key $1"
> +	fi
> +
>   	# at_ns0 namespace
>   	ip netns exec at_ns0 \
> -        ip link add dev $DEV_NS type $TYPE seq key 2 \
> +        ip link add dev $DEV_NS type $TYPE seq $tun_key \
>   		local 172.16.1.100 remote 172.16.1.200
>   	ip netns exec at_ns0 ip link set dev $DEV_NS up
>   	ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24

>   	# root namespace
> -	ip link add dev $DEV type $TYPE key 2 external
> +	ip link add dev $DEV type $TYPE $tun_key external
>   	ip link set dev $DEV up
>   	ip addr add dev $DEV 10.1.1.200/24
>   }
> @@ -238,7 +243,7 @@ test_gre()

>   	check $TYPE
>   	config_device
> -	add_gre_tunnel
> +	add_gre_tunnel 2
>   	attach_bpf $DEV gre_set_tunnel gre_get_tunnel
>   	ping $PING_ARG 10.1.1.100
>   	check_err $?
> @@ -253,6 +258,30 @@ test_gre()
>           echo -e ${GREEN}"PASS: $TYPE"${NC}
>   }

> +test_gre_no_tunnel_key()
> +{
> +	TYPE=gre
> +	DEV_NS=gre00
> +	DEV=gre11
> +	ret=0
> +
> +	check $TYPE
> +	config_device
> +	add_gre_tunnel
> +	attach_bpf $DEV gre_set_tunnel_no_key gre_get_tunnel
> +	ping $PING_ARG 10.1.1.100
> +	check_err $?
> +	ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> +	check_err $?
> +	cleanup
> +
> +        if [ $ret -ne 0 ]; then
> +                echo -e ${RED}"FAIL: $TYPE"${NC}
> +                return 1
> +        fi
> +        echo -e ${GREEN}"PASS: $TYPE"${NC}
> +}
> +
>   test_ip6gre()
>   {
>   	TYPE=ip6gre
> @@ -589,6 +618,7 @@ cleanup()
>   	ip link del ipip6tnl11 2> /dev/null
>   	ip link del ip6ip6tnl11 2> /dev/null
>   	ip link del gretap11 2> /dev/null
> +	ip link del gre11 2> /dev/null
>   	ip link del ip6gre11 2> /dev/null
>   	ip link del ip6gretap11 2> /dev/null
>   	ip link del geneve11 2> /dev/null
> @@ -641,6 +671,10 @@ bpf_tunnel_test()
>   	test_gre
>   	errors=$(( $errors + $? ))

> +	echo "Testing GRE tunnel (without tunnel keys)..."
> +	test_gre_no_tunnel_key
> +	errors=$(( $errors + $? ))
> +
>   	echo "Testing IP6GRE tunnel..."
>   	test_ip6gre
>   	errors=$(( $errors + $? ))
> --
> 2.37.4