netdev - Re: [RFC v2 net-next 01/12] net: introduce BPF_XDP

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]

Message-ID: <20191227152752.6b04c562@carbon>
Date:   Fri, 27 Dec 2019 15:27:52 +0100
From:   Jesper Dangaard Brouer <jbrouer@...hat.com>
To:     Prashant Bhole <prashantbhole.linux@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        David Ahern <dahern@...italocean.com>,
        Jason Wang <jasowang@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        John Fastabend <john.fastabend@...il.com>,
        Toshiaki Makita <toshiaki.makita1@...il.com>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>, netdev@...r.kernel.org
Subject: Re: [RFC v2 net-next 01/12] net: introduce BPF_XDP_EGRESS attach
 type for XDP

On Thu, 26 Dec 2019 11:31:49 +0900
Prashant Bhole <prashantbhole.linux@...il.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@...italocean.com>
> Co-developed-by: Prashant Bhole <prashantbhole.linux@...il.com>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@...il.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