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] [day] [month] [year] [list]

Date:   Sat, 11 Jan 2020 09:53:26 +0900
From:   Prashant Bhole <bholeprashant.oss@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        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 1/7/2020 8:35 PM, Toke Høiland-Jørgensen wrote:
> Prashant Bhole <prashantbhole.linux@...il.com> writes:
> 
>> 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?
> 
> I agree that the *PROG_TX stuff is confusing.

Ok. It seems s/TX/EGRESS is good for now.

> 
> Why not just keep the same XDP attach command, and just make this a new
> attach mode? I.e., today you can do
> 
> bpf_set_link_xdp_fd(ifindex, prog_fd, XDP_FLAGS_DRV_MODE);
> 
> so for this, just add support for:
> 
> bpf_set_link_xdp_fd(ifindex, prog_fd, XDP_FLAGS_EGRESS_MODE);
> 
> No need for a new command/netlink attribute. We already support multiple
> attach modes (HW+DRV), so this should be a straight-forward extension,
> no?

Initially we had implemented it the same way. I am ok with this way too.
- new attachment flag BPF_XDP_EGRESS for verifier purpose
- new xdp flag XDP_FLAGS_EGRESS for libbpf


Thanks