Re: [PATCH bpf] veth: convert frag_list skbs before running XDP

On Mon, Jul 20, 2026 at 11:24:47AM +0100, Matt Fleming wrote:

On Fri, Jul 17, 2026 at 11:56:41AM +0200, Toke Høiland-Jørgensen wrote:

quoted

Matt Fleming [off-list ref] writes:

quoted

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 1c5142149175..efb24aae1f26 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c

@@ -756,7 +756,7 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 	u32 frame_sz;
 
 	if (skb_shared(skb) || skb_head_is_locked(skb) ||
-	    skb_shinfo(skb)->nr_frags ||
+	    skb_shinfo(skb)->nr_frags || skb_has_frag_list(skb) ||

Isn't 'skb_shinfo(skb)->nr_frags || skb_has_frag_list(skb)' basically
the same as 'skb_is_nonlinear(skb)'? Which, incidentally, is what
generic XDP uses in the check that guards calling into the
skb_pp_cow_data() path.

 
Yeah, you're right. I tested that expression and it still fixes the
bug. I'll update v2 to use skb_is_nonlinear().

quoted

Looking at those two places, generic XDP checks for 'skb_cloned(skb)',
while veth checks 'skb_shared(skb) || skb_head_is_locked(skb)'. AFAICT,
the latter is stricter; should we update the generic XDP check?

 
Possibly, but the surrounding code isn't set up to deal with
skb_shared() SKBs so that'd be a larger change. I can take a look at
that too but I'm going to need more time to get my head around making
that change correctly given that there's different fallback rules than
veth.

Hi all,

I had an RFC that made veth to reuse generic XDP code path [0]. Jakub
commented we could have a common pp cow check on both sides. My plan is to
revive this work as this still causes page_pool issues when AF_XDP is used
on veth.

[0]: https://lore.kernel.org/bpf/20260509084858.773921-1-maciej.fijalkowski@intel.com/ (local)

Thanks,
Maciej

quoted

quoted

 	    skb_headroom(skb) < XDP_PACKET_HEADROOM) {
 		if (skb_pp_cow_data(rq->page_pool, pskb, XDP_PACKET_HEADROOM))
 			goto drop;

@@ -771,7 +771,7 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 	xdp_prepare_buff(xdp, skb->head, skb_headroom(skb),
 			 skb_headlen(skb), true);
 
-	if (skb_is_nonlinear(skb)) {
+	if (skb_shinfo(skb)->nr_frags) {
 		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
 		xdp_buff_set_frags_flag(xdp);
 	} else {

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 18dabb4e9cfa..1e837d01a908 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c

@@ -936,12 +936,11 @@ int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
 	int err, i, head_off;
 	void *data;
 
-	/* XDP does not support fraglist so we need to linearize
-	 * the skb.
+	/*
+	 * skb_copy_bits() handles both frags[] and frag_list input. If the
+	 * copied skb remains non-linear, it uses frags[], which is the
+	 * representation used by XDP multi-buffer.
 	 */

This comment sorta reads like a function documentation comment, but it
ends up sitting weirdly in the middle of the function body. The comment
you're replacing was tied to the statement below, but this one isn't,
really. Should we turn it into an actual function doc comment instead?

Good point. I'll make this a function doc comment.

Thanks,
Matt