netdev - Re: [PATCH net-next RFC v1 2/4] veth: use generic-XDP functions when dealing with SKBs

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: <87msyg91gl.fsf@toke.dk>
Date: Thu, 24 Aug 2023 12:30:50 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Jesper Dangaard Brouer <hawk@...nel.org>, netdev@...r.kernel.org,
 edumazet@...gle.com
Cc: Jesper Dangaard Brouer <hawk@...nel.org>, pabeni@...hat.com,
 kuba@...nel.org, davem@...emloft.net, lorenzo@...nel.org, Ilias Apalodimas
 <ilias.apalodimas@...aro.org>, mtahhan@...hat.com,
 huangjie.albert@...edance.com, Yunsheng Lin <linyunsheng@...wei.com>,
 Liang Chen <liangchen.linux@...il.com>
Subject: Re: [PATCH net-next RFC v1 2/4] veth: use generic-XDP functions
 when dealing with SKBs

Jesper Dangaard Brouer <hawk@...nel.org> writes:

> The root-cause the realloc issue is that veth_xdp_rcv_skb() code path (that
> handles SKBs like generic-XDP) is calling a native-XDP function
> xdp_do_redirect(), instead of simply using xdp_do_generic_redirect() that can
> handle SKBs.
>
> The existing code tries to steal the packet-data from the SKB (and frees the SKB
> itself). This cause issues as SKBs can have different memory models that are
> incompatible with native-XDP call xdp_do_redirect(). For this reason the checks
> in veth_convert_skb_to_xdp_buff() becomes more strict. This in turn makes this a
> bad approach. Simply leveraging generic-XDP helpers e.g. generic_xdp_tx() and
> xdp_do_generic_redirect() as this resolves the issue given netstack can handle
> these different SKB memory models.

While this does solve the memory issue, it's also a subtle change of
semantics. For one thing, generic_xdp_tx() has this comment above it:

/* When doing generic XDP we have to bypass the qdisc layer and the
 * network taps in order to match in-driver-XDP behavior. This also means
 * that XDP packets are able to starve other packets going through a qdisc,
 * and DDOS attacks will be more effective. In-driver-XDP use dedicated TX
 * queues, so they do not have this starvation issue.
 */

Also, more generally, this means that if you have a setup with
XDP_REDIRECT-based forwarding in on a host with a mix of physical and
veth devices, all the traffic originating from the veth devices will go
on different TXQs than that originating from a physical NIC. Or if a
veth device has a mix of xdp_frame-backed packets and skb-backed
packets, those will also go on different queues, potentially leading to
reordering.

I'm not sure exactly how much of an issue this is in practice, but at
least from a conceptual PoV it's a change in behaviour that I don't
think we should be making lightly. WDYT?

-Toke