netdev - Re: [PATCH net-next 4/6] xsk: Extend channels to support combined XSK/non-XSK traffic

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: <20190426121158.47c14fb8@cakuba.netronome.com>
Date:   Fri, 26 Apr 2019 12:11:58 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Maxim Mikityanskiy <maximmi@...lanox.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Björn Töpel 
        <bjorn.topel@...el.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Jonathan Lemon <bsd@...com>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [PATCH net-next 4/6] xsk: Extend channels to support combined
 XSK/non-XSK traffic

On Fri, 26 Apr 2019 11:42:37 +0000, Maxim Mikityanskiy wrote:
> Currently, the drivers that implement AF_XDP zero-copy support (e.g.,
> i40e) switch the channel into a different mode when an XSK is opened. It
> causes some issues that have to be taken into account. For example, RSS
> needs to be reconfigured to skip the XSK-enabled channels, or the XDP
> program should filter out traffic not intended for that socket and
> XDP_PASS it with an additional copy. As nothing validates or forces the
> proper configuration, it's easy to have packets drops, when they get
> into an XSK by mistake, and, in fact, it's the default configuration.
> There has to be some tool to have RSS reconfigured on each socket open
> and close event, but such a tool is problematic to implement, because no
> one reports these events, and it's race-prone.
> 
> This commit extends XSK to support both kinds of traffic (XSK and
> non-XSK) in the same channel. It implies having two RX queues in
> XSK-enabled channels: one for the regular traffic, and the other for
> XSK. It solves the problem with RSS: the default configuration just
> works without the need to manually reconfigure RSS or to perform some
> possibly complicated filtering in the XDP layer. It makes it easy to run
> both AF_XDP and regular sockets on the same machine. In the XDP program,
> the QID's most significant bit will serve as a flag to indicate whether
> it's the XSK queue or not. The extension is compatible with the legacy
> configuration, so if one wants to run the legacy mode, they can
> reconfigure RSS and ignore the flag in the XDP program (implemented in
> the reference XDP program in libbpf). mlx5e will support this extension.
> 
> A single XDP program can run both with drivers supporting or not
> supporting this extension. The xdpsock sample and libbpf are updated
> accordingly.
> 
> Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com>

It's been a while but this patch makes very little sense to me.

User installs a UMEM on a queue, all packets are delivered to that
UMEM, if XDP program doesn't redirect the packet to a XSK socket the
driver should copy it to a normal kernel page and proceed.  No drops
should happen.

Having two queues which pretend to share an ID but really one of them
has a magic encoding does not help IMO.

I definitely agree with you that the way queues are taken out of the
stack's ID space and the fact they aren't automatically excluded from
RSS etc is quite suboptimal..