Thread (16 messages) 16 messages, 7 authors, 2024-04-11

Re: [PATCH net-next v5] virtio_net: Support RX hash XDP hint

From: Paolo Abeni <pabeni@redhat.com>
Date: 2024-02-07 14:27:33
Also in: bpf, lkml, virtualization

On Wed, 2024-02-07 at 10:54 +0800, Liang Chen wrote:
On Tue, Feb 6, 2024 at 6:44 PM Paolo Abeni [off-list ref] wrote:
quoted
On Sat, 2024-02-03 at 10:56 +0800, Liang Chen wrote:
quoted
On Sat, Feb 3, 2024 at 12:20 AM Jesper Dangaard Brouer [off-list ref] wrote:
quoted
On 02/02/2024 13.11, Liang Chen wrote:
[...]
quoted
quoted
quoted
@@ -1033,6 +1039,16 @@ static void put_xdp_frags(struct xdp_buff *xdp)
      }
  }

+static void virtnet_xdp_save_rx_hash(struct virtnet_xdp_buff *virtnet_xdp,
+                                  struct net_device *dev,
+                                  struct virtio_net_hdr_v1_hash *hdr_hash)
+{
+     if (dev->features & NETIF_F_RXHASH) {
+             virtnet_xdp->hash_value = hdr_hash->hash_value;
+             virtnet_xdp->hash_report = hdr_hash->hash_report;
+     }
+}
+
Would it be possible to store a pointer to hdr_hash in virtnet_xdp_buff,
with the purpose of delaying extracting this, until and only if XDP
bpf_prog calls the kfunc?
That seems to be the way v1 works,
https://lore.kernel.org/all/20240122102256.261374-1-liangchen.linux@gmail.com/ (local)
. But it was pointed out that the inline header may be overwritten by
the xdp prog, so the hash is copied out to maintain its integrity.
Why? isn't XDP supposed to get write access only to the pkt
contents/buffer?
Normally, an XDP program accesses only the packet data. However,
there's also an XDP RX Metadata area, referenced by the data_meta
pointer. This pointer can be adjusted with bpf_xdp_adjust_meta to
point somewhere ahead of the data buffer, thereby granting the XDP
program access to the virtio header located immediately before the
AFAICS bpf_xdp_adjust_meta() does not allow moving the meta_data before
xdp->data_hard_start:

https://elixir.bootlin.com/linux/latest/source/net/core/filter.c#L4210

and virtio net set such field after the virtio_net_hdr:

https://elixir.bootlin.com/linux/latest/source/drivers/net/virtio_net.c#L1218
https://elixir.bootlin.com/linux/latest/source/drivers/net/virtio_net.c#L1420

I don't see how the virtio hdr could be touched? Possibly even more
important: if such thing is possible, I think is should be somewhat
denied (for the same reason an H/W nic should prevent XDP from
modifying its own buffer descriptor).

Cheers,

Paolo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help