netdev - Re: [PATCH bpf-next 1/2] bpf: Add flag BPF_F_NO_TUNNEL_KEY to bpf_skb_set_tunnel

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]

Date:   Mon, 19 Dec 2022 10:41:36 -0800
From:   sdf@...gle.com
To:     Christian Ehrig <cehrig@...udflare.com>
Cc:     bpf@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>,
        Jiri Olsa <jolsa@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
        Joanne Koong <joannelkoong@...il.com>,
        Kui-Feng Lee <kuifeng@...com>,
        Maxim Mikityanskiy <maximmi@...dia.com>,
        Kaixi Fan <fankaixi.li@...edance.com>,
        Shmulik Ladkani <shmulik@...anetworks.com>,
        Paul Chaignon <paul@...valent.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next 1/2] bpf: Add flag BPF_F_NO_TUNNEL_KEY to bpf_skb_set_tunnel_key()

On 12/18, Christian Ehrig wrote:
> This patch allows to remove TUNNEL_KEY from the tunnel flags bitmap
> when using bpf_skb_set_tunnel_key by providing a BPF_F_NO_TUNNEL_KEY
> flag. On egress, the resulting tunnel header will not contain a tunnel
> key if the protocol and implementation supports it.

> At the moment bpf_tunnel_key wants a user to specify a numeric tunnel
> key. This will wrap the inner packet into a tunnel header with the key
> bit and value set accordingly. This is problematic when using a tunnel
> protocol that supports optional tunnel keys and a receiving tunnel
> device that is not expecting packets with the key bit set. The receiver
> won't decapsulate and drop the packet.

> RFC 2890 and RFC 2784 GRE tunnels are examples where this flag is
> useful. It allows for generating packets, that can be decapsulated by
> a GRE tunnel device not operating in collect metadata mode or not
> expecting the key bit set.

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

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

> ---
>   include/uapi/linux/bpf.h       | 4 ++++
>   net/core/filter.c              | 5 ++++-
>   tools/include/uapi/linux/bpf.h | 4 ++++
>   3 files changed, 12 insertions(+), 1 deletion(-)

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 464ca3f01fe7..bc1a3d232ae4 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2001,6 +2001,9 @@ union bpf_attr {
>    * 			sending the packet. This flag was added for GRE
>    * 			encapsulation, but might be used with other protocols
>    * 			as well in the future.
> + * 		**BPF_F_NO_TUNNEL_KEY**
> + * 			Add a flag to tunnel metadata indicating that no tunnel
> + * 			key should be set in the resulting tunnel header.
>    *
>    * 		Here is a typical usage on the transmit path:
>    *
> @@ -5764,6 +5767,7 @@ enum {
>   	BPF_F_ZERO_CSUM_TX		= (1ULL << 1),
>   	BPF_F_DONT_FRAGMENT		= (1ULL << 2),
>   	BPF_F_SEQ_NUMBER		= (1ULL << 3),
> +	BPF_F_NO_TUNNEL_KEY		= (1ULL << 4),
>   };

>   /* BPF_FUNC_skb_get_tunnel_key flags. */
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 929358677183..c746e4d77214 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4615,7 +4615,8 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff  
> *, skb,
>   	struct ip_tunnel_info *info;

>   	if (unlikely(flags & ~(BPF_F_TUNINFO_IPV6 | BPF_F_ZERO_CSUM_TX |
> -			       BPF_F_DONT_FRAGMENT | BPF_F_SEQ_NUMBER)))
> +			       BPF_F_DONT_FRAGMENT | BPF_F_SEQ_NUMBER |
> +			       BPF_F_NO_TUNNEL_KEY)))
>   		return -EINVAL;
>   	if (unlikely(size != sizeof(struct bpf_tunnel_key))) {
>   		switch (size) {
> @@ -4653,6 +4654,8 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff  
> *, skb,
>   		info->key.tun_flags &= ~TUNNEL_CSUM;
>   	if (flags & BPF_F_SEQ_NUMBER)
>   		info->key.tun_flags |= TUNNEL_SEQ;
> +	if (flags & BPF_F_NO_TUNNEL_KEY)
> +		info->key.tun_flags &= ~TUNNEL_KEY;

>   	info->key.tun_id = cpu_to_be64(from->tunnel_id);
>   	info->key.tos = from->tunnel_tos;
> diff --git a/tools/include/uapi/linux/bpf.h  
> b/tools/include/uapi/linux/bpf.h
> index 464ca3f01fe7..bc1a3d232ae4 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -2001,6 +2001,9 @@ union bpf_attr {
>    * 			sending the packet. This flag was added for GRE
>    * 			encapsulation, but might be used with other protocols
>    * 			as well in the future.
> + * 		**BPF_F_NO_TUNNEL_KEY**
> + * 			Add a flag to tunnel metadata indicating that no tunnel
> + * 			key should be set in the resulting tunnel header.
>    *
>    * 		Here is a typical usage on the transmit path:
>    *
> @@ -5764,6 +5767,7 @@ enum {
>   	BPF_F_ZERO_CSUM_TX		= (1ULL << 1),
>   	BPF_F_DONT_FRAGMENT		= (1ULL << 2),
>   	BPF_F_SEQ_NUMBER		= (1ULL << 3),
> +	BPF_F_NO_TUNNEL_KEY		= (1ULL << 4),
>   };

>   /* BPF_FUNC_skb_get_tunnel_key flags. */
> --
> 2.37.4