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 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:   Sat, 28 Dec 2019 09:15:54 +0900
From:   Prashant Bhole <prashantbhole.linux@...il.com>
To:     Jesper Dangaard Brouer <jbrouer@...hat.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 12/27/2019 11:27 PM, Jesper Dangaard Brouer wrote:
> 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.

It's been named like that because it is likely that a new program
type tx path will be introduced later. It can re-use IFLA_XDP_TX
XDP_SETUP_PROG_TX, XDP_QUERY_PROG_TX. Do think that it should not
be shared by two different type of programs?

> 
> BTW, should the XDP_EGRESS program also inspect XDP_TX packets?

Yes, makes sense. But I missed to handle this case in tun driver
changes.

Thanks

> 
> 
>> 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
>>   };
> 
> 
>