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: <87woa3ijo7.fsf@toke.dk>
Date:   Tue, 07 Jan 2020 12:35:04 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Prashant Bhole <prashantbhole.linux@...il.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

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.

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?

-Toke