Re: [RFC v2 net-next 01/12] net: introduce BPF_XDP_EGRESS attach type for XDP

[Jesper Dangaard Brouer <jbrouer@redhat.com>]

On Thu, 26 Dec 2019 11:31:49 +0900
Prashant Bhole <prashantbhole.linux@gmail.com> wrote:

> This patch introduces a new bpf attach type BPF_XDP_EGRESS. Programs
> having this attach type will be allowed to run in the tx path. It is
> because we need to prevent the programs from accessing rxq info when
> they are running in tx path. Verifier can reject the programs those
> have this attach type and trying to access rxq info.
> 
> Patch also introduces a new netlink attribute IFLA_XDP_TX which can
> be used for setting XDP program in tx path and to get information of
> such programs.
> 
> Drivers those want to support tx path XDP needs to handle
> XDP_SETUP_PROG_TX and XDP_QUERY_PROG_TX cases in their ndo_bpf.

Why do you keep the "TX" names, when you introduce the "EGRESS"
attachment type?

Netlink attribute IFLA_XDP_TX is particularly confusing.

I personally like that this is called "*_XDP_EGRESS" to avoid confusing
with XDP_TX action.

BTW, should the XDP_EGRESS program also inspect XDP_TX packets?


> Signed-off-by: David Ahern <dahern@digitalocean.com>
> Co-developed-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> ---
>  include/linux/netdevice.h      |   4 +-
>  include/uapi/linux/bpf.h       |   1 +
>  include/uapi/linux/if_link.h   |   1 +
>  net/core/dev.c                 |  34 +++++++---
>  net/core/filter.c              |   8 +++
>  net/core/rtnetlink.c           | 112 ++++++++++++++++++++++++++++++++-
>  tools/include/uapi/linux/bpf.h |   1 +
>  7 files changed, 150 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 469a297b58c0..ac3e88d86581 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -865,8 +865,10 @@ enum bpf_netdev_command {
>  	 */
>  	XDP_SETUP_PROG,
>  	XDP_SETUP_PROG_HW,
> +	XDP_SETUP_PROG_TX,
>  	XDP_QUERY_PROG,
>  	XDP_QUERY_PROG_HW,
> +	XDP_QUERY_PROG_TX,
>  	/* BPF program for offload callbacks, invoked at program load time. */
>  	BPF_OFFLOAD_MAP_ALLOC,
>  	BPF_OFFLOAD_MAP_FREE,
> @@ -3725,7 +3727,7 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>  
>  typedef int (*bpf_op_t)(struct net_device *dev, struct netdev_bpf *bpf);
>  int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
> -		      int fd, u32 flags);
> +		      int fd, u32 flags, bool tx);
>  u32 __dev_xdp_query(struct net_device *dev, bpf_op_t xdp_op,
>  		    enum bpf_netdev_command cmd);
>  int xdp_umem_query(struct net_device *dev, u16 queue_id);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index dbbcf0b02970..23c1841c8086 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -203,6 +203,7 @@ enum bpf_attach_type {
>  	BPF_TRACE_RAW_TP,
>  	BPF_TRACE_FENTRY,
>  	BPF_TRACE_FEXIT,
> +	BPF_XDP_EGRESS,
>  	__MAX_BPF_ATTACH_TYPE
>  };
>  
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 1d69f637c5d6..be97c9787140 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -170,6 +170,7 @@ enum {
>  	IFLA_PROP_LIST,
>  	IFLA_ALT_IFNAME, /* Alternative ifname */
>  	IFLA_PERM_ADDRESS,
> +	IFLA_XDP_TX,
>  	__IFLA_MAX
>  };



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer