Re: [PATCH net-next v3 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr

[sdf@google.com]

On 06/01, Tanner Love wrote:
> From: Tanner Love <tannerlove@google.com>

> Amend the bpf flow dissector program type to accept virtio_net_hdr
> members. Do this to enable bpf flow dissector programs to perform
> virtio-net header validation. The next patch in this series will add
> a flow dissection hook in virtio_net_hdr_to_skb and make use of this
> extended functionality. That commit message has more background on the
> use case.

> Signed-off-by: Tanner Love <tannerlove@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> Reviewed-by: Petar Penkov <ppenkov@google.com>
> ---
>   drivers/net/bonding/bond_main.c |  2 +-
>   include/linux/skbuff.h          | 26 ++++++++++++----
>   include/net/flow_dissector.h    |  6 ++++
>   include/uapi/linux/bpf.h        |  6 ++++
>   net/core/filter.c               | 55 +++++++++++++++++++++++++++++++++
>   net/core/flow_dissector.c       | 24 ++++++++++++--
>   tools/include/uapi/linux/bpf.h  |  6 ++++
>   7 files changed, 116 insertions(+), 9 deletions(-)

> diff --git a/drivers/net/bonding/bond_main.c  
> b/drivers/net/bonding/bond_main.c
> index 7e469c203ca5..5d2d7d5c5704 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3554,7 +3554,7 @@ static bool bond_flow_dissect(struct bonding *bond,  
> struct sk_buff *skb,
>   	case BOND_XMIT_POLICY_ENCAP34:
>   		memset(fk, 0, sizeof(*fk));
>   		return __skb_flow_dissect(NULL, skb, &flow_keys_bonding,
> -					  fk, NULL, 0, 0, 0, 0);
> +					  fk, NULL, 0, 0, 0, 0, NULL);
>   	default:
>   		break;
>   	}
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index dbf820a50a39..fef8f4b5db6e 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1312,18 +1312,20 @@ struct bpf_flow_dissector;
>   bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector  
> *ctx,
>   		      __be16 proto, int nhoff, int hlen, unsigned int flags);

> +struct virtio_net_hdr;
>   bool __skb_flow_dissect(const struct net *net,
>   			const struct sk_buff *skb,
>   			struct flow_dissector *flow_dissector,
>   			void *target_container, const void *data,
> -			__be16 proto, int nhoff, int hlen, unsigned int flags);
> +			__be16 proto, int nhoff, int hlen, unsigned int flags,
> +			const struct virtio_net_hdr *vhdr);

>   static inline bool skb_flow_dissect(const struct sk_buff *skb,
>   				    struct flow_dissector *flow_dissector,
>   				    void *target_container, unsigned int flags)
>   {
>   	return __skb_flow_dissect(NULL, skb, flow_dissector,
> -				  target_container, NULL, 0, 0, 0, flags);
> +				  target_container, NULL, 0, 0, 0, flags, NULL);
>   }

>   static inline bool skb_flow_dissect_flow_keys(const struct sk_buff *skb,
> @@ -1332,7 +1334,20 @@ static inline bool  
> skb_flow_dissect_flow_keys(const struct sk_buff *skb,
>   {
>   	memset(flow, 0, sizeof(*flow));
>   	return __skb_flow_dissect(NULL, skb, &flow_keys_dissector,
> -				  flow, NULL, 0, 0, 0, flags);
> +				  flow, NULL, 0, 0, 0, flags, NULL);
> +}
> +
> +static inline bool
> +__skb_flow_dissect_flow_keys_basic(const struct net *net,
> +				   const struct sk_buff *skb,
> +				   struct flow_keys_basic *flow,
> +				   const void *data, __be16 proto,
> +				   int nhoff, int hlen, unsigned int flags,
> +				   const struct virtio_net_hdr *vhdr)
> +{
> +	memset(flow, 0, sizeof(*flow));
> +	return __skb_flow_dissect(net, skb, &flow_keys_basic_dissector, flow,
> +				  data, proto, nhoff, hlen, flags, vhdr);
>   }

>   static inline bool
> @@ -1342,9 +1357,8 @@ skb_flow_dissect_flow_keys_basic(const struct net  
> *net,
>   				 const void *data, __be16 proto,
>   				 int nhoff, int hlen, unsigned int flags)
>   {
> -	memset(flow, 0, sizeof(*flow));
> -	return __skb_flow_dissect(net, skb, &flow_keys_basic_dissector, flow,
> -				  data, proto, nhoff, hlen, flags);
> +	return __skb_flow_dissect_flow_keys_basic(net, skb, flow, data, proto,
> +						  nhoff, hlen, flags, NULL);
>   }

>   void skb_flow_dissect_meta(const struct sk_buff *skb,
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index ffd386ea0dbb..0796ad745e69 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -370,6 +370,12 @@ struct bpf_flow_dissector {
>   	const struct sk_buff	*skb;
>   	const void		*data;
>   	const void		*data_end;
> +	__u8			vhdr_flags;
> +	__u8			vhdr_gso_type;
> +	__u16			vhdr_hdr_len;
> +	__u16			vhdr_gso_size;
> +	__u16			vhdr_csum_start;
> +	__u16			vhdr_csum_offset;
>   };

>   static inline void
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 418b9b813d65..de525defd462 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5155,6 +5155,12 @@ struct __sk_buff {
>   	__u32 gso_segs;
>   	__bpf_md_ptr(struct bpf_sock *, sk);
>   	__u32 gso_size;

[..]

> +	__u8  vhdr_flags;
> +	__u8  vhdr_gso_type;
> +	__u16 vhdr_hdr_len;
> +	__u16 vhdr_gso_size;
> +	__u16 vhdr_csum_start;
> +	__u16 vhdr_csum_offset;

These are flow dissector specific, any reason not to add them to
struct bpf_flow_keys instead?