All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>,
	Jesper Dangaard Brouer <hawk@kernel.org>, <bpf@vger.kernel.org>,
	<ast@kernel.org>, <daniel@iogearbox.net>,
	<ilias.apalodimas@linaro.org>, <toke@redhat.com>,
	<lorenzo@kernel.org>, <netdev@vger.kernel.org>,
	<magnus.karlsson@intel.com>, <andrii@kernel.org>,
	<stfomichev@gmail.com>,
	<syzbot+ff145014d6b0ce64a173@syzkaller.appspotmail.com>,
	Ihor Solodrai <ihor.solodrai@linux.dev>,
	Octavian Purdila <tavip@google.com>
Subject: Re: [PATCH v2 bpf 1/2] xdp: update xdp_rxq_info's mem type in XDP generic hook
Date: Wed, 22 Oct 2025 07:04:01 -0700	[thread overview]
Message-ID: <20251022070401.0a02452d@kernel.org> (raw)
In-Reply-To: <aPi+cfwuGq2r+8RZ@boxer>

On Wed, 22 Oct 2025 13:22:25 +0200 Maciej Fijalkowski wrote:
> > > veth's pp works on order-0 pages well, however I agree it would be better
> > > to use virt_to_head_page() here.  
> > 
> > In this case the mem.type update is for consuming frags only, right?
> > We can't free the head itself since the skb is attached to it.
> > So running the predicates on xdp->data is probably wrong.  
> 
> See the veth patch where we bump refcount of related pages in order to
> keep the data as skb is consumed.

That one is wrong too. In veth's case since we care about refs on
individual frags you _should_ actually use skb->pp_recycle for the
condition. And you need to test whether each individual fragment
is from the PP. Basically call skb_pp_frag_ref() like
skb_try_coalesce() does?

BTW it is _not_ legal to call get_page() on slab pages any more
so all of this code should probably fail when head_frag=0.
But let's leave that problem to someone else.

> > Is it possible to get to bpf_prog_run_generic_xdp() (with frags)
> > and without going thru netif_skb_check_for_xdp() ? If no then
> > frags must have come from skb_pp_cow(). 
> > And the type is always MEM_TYPE_PAGE_POOL ?  
> 
> We have a fallback path netif_skb_check_for_xdp() that linearizes skb, in
> case COW code failed. But bare in mind that bpf_prog_run_generic_xdp() has
> other callsites, besides generic XDP itself (cpumap and devmap). That is
> why I wouldn't like to base this helper on assumptions such as frags
> presence -> mem_type is pp.

I assumed cpumap and devmap must be hit from a generic XDP handler.
Ergo we also much have gone thru the COW.


  reply	other threads:[~2025-10-22 14:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-17 14:31 [PATCH v2 bpf 0/2] xdp: fix page_pool leaks Maciej Fijalkowski
2025-10-17 14:31 ` [PATCH v2 bpf 1/2] xdp: update xdp_rxq_info's mem type in XDP generic hook Maciej Fijalkowski
2025-10-17 16:45   ` Toke Høiland-Jørgensen
2025-10-20 11:20   ` Jesper Dangaard Brouer
2025-10-20 15:36     ` Alexander Lobakin
2025-10-20 17:53       ` Maciej Fijalkowski
2025-10-22  1:01         ` Jakub Kicinski
2025-10-22 11:22           ` Maciej Fijalkowski
2025-10-22 14:04             ` Jakub Kicinski [this message]
2025-10-17 14:31 ` [PATCH v2 bpf 2/2] veth: update mem type in xdp_buff Maciej Fijalkowski
2025-10-17 16:33   ` Toke Høiland-Jørgensen
2025-10-17 16:52     ` Maciej Fijalkowski
2025-10-17 17:51       ` Toke Høiland-Jørgensen
2025-10-17 18:12         ` Toke Høiland-Jørgensen
2025-10-17 20:08           ` Maciej Fijalkowski
2025-10-20 10:25             ` Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251022070401.0a02452d@kernel.org \
    --to=kuba@kernel.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=hawk@kernel.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=ilias.apalodimas@linaro.org \
    --cc=lorenzo@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=stfomichev@gmail.com \
    --cc=syzbot+ff145014d6b0ce64a173@syzkaller.appspotmail.com \
    --cc=tavip@google.com \
    --cc=toke@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.