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

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Thu, 24 Aug 2023 12:30:50 +0200

Jesper Dangaard Brouer <[email-masked]> 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