netdev - Re: [PATCH bpf-next v2 0/7] Add support for 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]

Date:   Mon, 17 Dec 2018 14:39:57 +0100
From:   Björn Töpel <bjorn.topel@...el.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>,
        Björn Töpel <bjorn.topel@...il.com>
Cc:     magnus.karlsson@...el.com, magnus.karlsson@...il.com,
        ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        u9012063@...il.com, qi.z.zhang@...el.com,
        jakub.kicinski@...ronome.com, andrew@...n.ch
Subject: Re: [PATCH bpf-next v2 0/7] Add support for XDP_ATTACH



On 2018-12-17 13:50, Jesper Dangaard Brouer wrote:
> On Mon, 17 Dec 2018 11:24:54 +0100
> Björn Töpel <bjorn.topel@...il.com> wrote:
> 
>> XDP_ATTACH associates an XDP socket to a specific netdev Rx queue. To
>> redirect a packet to an attached socket from XDP, the bpf_xsk_redirect
>> helper is used. The bpf_xsk_redirect helper is also introduced in this
>> series.
>>
>> Many XDP socket users just need a simple way of creating/binding a
>> socket and receiving frames right away without a complicated XDP
>> program. "Attached" XDP sockets removes the need for the XSKMAP, and
>> allows for a trivial XDP program, e.g.:
>>
>>    SEC("xdp")
>>    int xdp_prog(struct xdp_md *ctx)
>>    {
>>          return bpf_xsk_redirect(ctx);
>>    }
>>
>> An attached XDP socket also has better performance than the XSKMAP
>> based sockets (performance numbers below).
> 
> I still have a general problem with this approach.
>

And I appreciate that you have comments on the design/code! :-) Thank you!


> The AF_XDP socket is build around (and gets its performance) from
> being tied to a specific RX-queue.  That design begs to have an XDP
> program per RX-queue.
>
> Patchset-v1 moved towards this goal.  But in this patchset-v2 you
> steer away from this again, and work-around the issue with the current
> limitations of 1-XDP program per netdev.  (Which result in; if a
> single AF_XDP socket is used in the system, which can ONLY be for a
> single RX-queue by design, then ALL other XDP_PASS traffic also have
> to take the overhead of indirect BPF call).
>

I agree that a per-queue-program would be a good fit, but I think it
still makes sense to add XDP_ATTACH and the helper, to make it easier
for the XDP program authors to use AF_XDP. Would you prefer that this
functionality was help back, until per-queue programs are introduced?

OTOH the implementation would probably look different if there was a
per-q program, because this would open up for more optimizations. One
could even imagine that the socket fd was part of the program (part of
relocation process) when loading the program. That could get rid of yet
another if-statement that check for socket existence. :-)

> IMHO with this use-case, now is the time to introduce XDP programs per
> RX-queue.  Yes, it will be more work, but I will offer to helpout.
> This should be generalized as XDP programs per RX-queue can be used by
> other use-cases too:
>    In general terms: We can setup a NIC hardware filter to deliver
> frame matching some criteria, then we can avoid rechecking these
> criterias in on the (RX) CPU when/if we can attach an XDP prog to this
> specific RX-queue directly.  This *IS* exactly what AF_XDP does, but it
> is in general useful for others, like CPUMAP redirect.
>

Fair enough, and thank you for offering to help out. And the fact that
*other than AF_XDP* can benefit from that is good. Before we dive down
this hole, what are the opinions of the BPF maintainers? Maybe it's
better to hack an RFC, and then take the discussion?


Björn