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

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Prashant Bhole <prashantbhole.linux@gmail.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@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.
>
> 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