Re: [PATCH bpf-next 1/2] bpf: Add flag BPF_F_NO_TUNNEL_KEY to bpf_skb_set_tunnel_key()

[sdf@google.com]

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@cloudflare.com>

Acked-by: Stanislav Fomichev <sdf@google.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