Am 31.01.25 um 00:16 schrieb Stanislav Fomichev:
On 01/30, Marcus Wichelmann wrote:Enable the support for bpf_xdp_adjust_meta for XDP buffers initialized by the tun driver. This is useful to pass metadata from an XDP program that's attached to a tap device to following XDP/TC programs. When used together with vhost_net, the batched XDP buffers were already initialized with metadata support by the vhost_net driver, but the metadata was not yet passed to the skb on XDP_PASS. So this also adds the required skb_metadata_set calls.Can you expand more on what kind of metadata is present with vhost_net and who fills it in? Is it virtio header stuff? I wonder how you want to consume it..
Hm, my commit message may have been a bit misleading.
I'm talking about regular support for the bpf_xdp_adjust_meta helper
function. This allows to reserve a metadata area that is useful to pass
any information from one XDP program to another one, for example when
using tail-calls.
Whether it can be used in an XDP program depends on how the xdp_buff
was initialized. Most net drivers initialize the xdp_buff in a way, that
allows bpf_xdp_adjust_meta to be used. In case of the tun driver, this is
not always the case.
There are two code paths in the tun driver that lead to a bpf_prog_run_xdp:
1. tun_build_skb, which is called by tun_get_user and is used when writing
packets from userspace into the tap device. In this case, the xdp_buff
created in tun_build_skb has no support for bpf_xdp_adjust_meta and calls
of that helper function result in ENOTSUPP.
2. tun_xdp_one, which is called by tun_sendmsg which again is called by other
drivers (e.g. vhost_net). When the TUN_MSG_PTR mode is used, another driver
may pass a batch of xdp_buffs to the tun driver. In this case, that other
driver is the one initializing the xdp_buff already. And in the case of
vhost_net, it already initializes the buffer with metadata support (see
xdp_prepare_buff call).
See 043d222f93ab ("tuntap: accept an array of XDP buffs through sendmsg()")
for details.
This patch enables metadata support for the first code path.
It also adds the missing skb_metadata_set calls for both code paths. This is
important when the attached XDP program returns with XDP_PASS, because then
the code continues with creating an skb and that skb should be aware of the
metadata area's size. At a later stage, a TC program, for example, can then
access the metadata again using __sk_buff->data_meta.
So I'm doing not much new here but am rather enabling a feature that is
supported by other drivers already.
Can you also add a selftest to use this new functionality?
Of course, I'll see what I can do.
Signed-off-by: Marcus Wichelmann <marcus.wichelmann@xxxxxxxxxxxxxxxx> --- drivers/net/tun.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index e816aaba8..d3cfea40a 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1600,7 +1600,8 @@ static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,static struct sk_buff *__tun_build_skb(struct tun_file *tfile,struct page_frag *alloc_frag, char *buf, - int buflen, int len, int pad) + int buflen, int len, int pad, + int metasize) { struct sk_buff *skb = build_skb(buf, buflen);@@ -1609,6 +1610,8 @@ static struct sk_buff *__tun_build_skb(struct tun_file *tfile, skb_reserve(skb, pad);skb_put(skb, len); + if (metasize) + skb_metadata_set(skb, metasize); skb_set_owner_w(skb, tfile->socket.sk);get_page(alloc_frag->page);@@ -1668,6 +1671,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, char *buf; size_t copied; int pad = TUN_RX_PAD; + int metasize = 0; int err = 0;rcu_read_lock();@@ -1695,7 +1699,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, if (hdr->gso_type || !xdp_prog) { *skb_xdp = 1; return __tun_build_skb(tfile, alloc_frag, buf, buflen, len, - pad); + pad, metasize); }*skb_xdp = 0;@@ -1709,7 +1713,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, u32 act;xdp_init_buff(&xdp, buflen, &tfile->xdp_rxq);- xdp_prepare_buff(&xdp, buf, pad, len, false); + xdp_prepare_buff(&xdp, buf, pad, len, true);act = bpf_prog_run_xdp(xdp_prog, &xdp);if (act == XDP_REDIRECT || act == XDP_TX) { @@ -1730,12 +1734,16 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,pad = xdp.data - xdp.data_hard_start;len = xdp.data_end - xdp.data; + + metasize = xdp.data - xdp.data_meta;[..]+ metasize = metasize > 0 ? metasize : 0;Why is this part needed?
When an xdp_buff was initialized withouth metadata support (meta_valid argument of xdp_prepare_buff is false), then data_meta == data + 1. So this check makes sure that metadata was supported for the given xdp_buff and metasize is not -1 (data - data_meta). But you have a good point here: Because we have control over the initialization of xdp_buff in the tun_build_skb function (first code path), we know, that metadata is always supported for that buffer and metasize is never < 0. So this check is redundant and I'll remove it. But in the tun_xdp_one function (second code path), I'd prefer to keep that check, as the xdp_buff is externally passed to tun_sendmsg and the tun driver should probably not make assumptions about the metadata support of buffers created by other drivers (e.g. vhost_net). Thank you for taking a look, I hope things are more clear now. Marcus
