> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 6ed3ee81153f..0afa0661ada1 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -775,6 +775,11 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq
> *rq,
> if (skb_shinfo(skb)->nr_frags) {
> skb_shinfo(skb)->xdp_frags_size = skb->data_len;
> xdp_buff_set_frags_flag(xdp);
> + /* A nonlinear skb was cow'd into rq->page_pool above, so the
> + * frags must be freed to that pool, not via the rxq's
> + * MEM_TYPE_PAGE_SHARED.
> + */
> + xdp_buff_set_frag_pp(xdp);
> } else {
> xdp_buff_clear_frags_flag(xdp);
> }
[ ... ]
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 38336858c168..be36020484b6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5532,6 +5532,11 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb,
> struct xdp_buff *xdp,
> if (skb_is_nonlinear(skb)) {
> skb_shinfo(skb)->xdp_frags_size = skb->data_len;
> xdp_buff_set_frags_flag(xdp);
> + /* A nonlinear skb was cow'd into page_pool memory by
> + * skb_cow_data_for_xdp() before we got here, so the frags must
> + * be freed to that pool, not via the rxq's
> MEM_TYPE_PAGE_SHARED.
> + */
> + xdp_buff_set_frag_pp(xdp);
> } else {
> xdp_buff_clear_frags_flag(xdp);
> }
This isn't a bug, but is the comment completely accurate?
The comment states the skb was cow'd "before we got here" by
skb_cow_data_for_xdp(). This is correct for the netif_receive_generic_xdp()
caller, where netif_skb_check_for_xdp() calls skb_cow_data_for_xdp() before
bpf_prog_run_generic_xdp().
However, bpf_prog_run_generic_xdp() has three other callers:
cpu_map_bpf_prog_run_skb():
kernel/bpf/cpumap.c calls bpf_prog_run_generic_xdp() directly
dev_map_bpf_prog_run_skb():
kernel/bpf/devmap.c calls bpf_prog_run_generic_xdp() directly
For these callers, skb_cow_data_for_xdp() is not called immediately before
this function. Instead, the page_pool invariant is inherited from an earlier
ingress generic-XDP pass where the skb went through
netif_receive_generic_xdp() -> netif_skb_check_for_xdp().
Should the comment acknowledge that the cow may have happened earlier in the
packet's path for some callers, rather than stating it always happens
"before we got here"?
[ ... ]
> So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the
> two
> skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
This isn't a bug, but is "the two skb-cow sites" the right description?
The commit message states "the two skb-cow sites set it", referring to
drivers/net/veth.c and net/core/dev.c where xdp_buff_set_frag_pp() is
called.
Looking at the code:
drivers/net/veth.c:782 is inside veth_convert_skb_to_xdp_buff(), which does
call skb_pp_cow_data() a few lines earlier to cow the nonlinear skb into
rq->page_pool. This is a cow site.
net/core/dev.c:5539 is inside bpf_prog_run_generic_xdp(), which does not
itself call skb_cow_data_for_xdp(). The cow happens one level up in
netif_receive_generic_xdp() -> netif_skb_check_for_xdp().
Would it be more precise to say "one cow site and one post-cow site" or "the
two sites where skb frags are known to be page_pool memory", since
bpf_prog_run_generic_xdp() is not itself performing the cow?
Also, bpf_prog_run_generic_xdp() has three callers that never cow at all:
cpu_map_bpf_prog_run_skb() and dev_map_bpf_prog_run_skb(). Their skbs
originate from xdp_do_generic_redirect_map() after going through
netif_receive_generic_xdp() where the cow already happened. So the flag is
set even though no cow occurs in the current call chain, which makes "two
skb-cow sites" a bit ambiguous.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32686145995
