All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Vazquez <brianvv@google.com>
To: Naman Gulati <namangulati@google.com>
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	 Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	 Alexander Lobakin <aleksander.lobakin@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	 "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	 Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH iwl-net] idpf: fix Rx payload buffer leak when header buffer allocation fails
Date: Wed, 4 Nov 2026 18:14:48 +0000	[thread overview]
Message-ID: <aut0VPowjLkQL5Oe@google.com> (raw)
In-Reply-To: <20261104013606.1527767-1-namangulati@google.com>

On Wed, Nov 04, 2026 at 01:36:06AM +0000, Naman Gulati wrote:
> With header split, idpf_rx_update_bufq_desc() allocates the payload
> buffer for a buffer ID first and its header buffer second. If the header
> buffer allocation fails, it returns -ENOMEM and leaves the new payload
> buffer in bufq->buf[buf_id]. idpf_rx_clean_refillq() then stops without
> consuming the refill queue entry, and the next NAPI poll retries the
> same buffer ID, again starting with the payload buffer.
> libeth_rx_alloc() overwrites bufq->buf[buf_id], and the earlier payload
> buffer is never returned to its page_pool.
> 
> Each such failure leaks one payload buffer. The pool's inflight count
> never drops to zero, so when the queues are re-created or the device
> goes away, the pool never finishes shutting down and
> page_pool_release_retry() keeps warning about a stalled pool shutdown.
> 
> This happens when the header buffer pool runs dry while the payload pool
> still has pages, for example under memory pressure: a header page holds
> several header buffers and only goes back to its pool once the skbs
> using all of them have been freed.
> 
> Release the payload buffer when the header buffer cannot be allocated,
> as ice_alloc_rx_bufs() already does, so that the retry starts from an
> empty slot. idpf_rx_page_rel() also clears the slot, so the buffer queue
> teardown cannot release the buffer a second time.
> 
> Fixes: 90912f9f4f2d ("idpf: convert header split mode to libeth + napi_build_skb()")
> Signed-off-by: Naman Gulati <namangulati@google.com>

Tested on an idpf PF (8086:11df), net-queue dev-queue (da74aaf50018),
TCP data split on, 16 TCP flows of Rx traffic:

1. A local debug hook makes the header libeth_rx_alloc() in
   idpf_rx_update_bufq_desc() return DMA_MAPPING_ERROR on every 20th
   call, 2000 times in total.
2. Recreate the Rx queues: ethtool -G eth1 rx 1024; ethtool -G eth1 rx 2048
3. After 120 s, check with tools/net/ynl/ynltool: ynltool page-pool stats

Without the patch:
    eth1[3] page pools: 253 (zombies: 13)
        refs: 150913 bytes: 618139648 (refs: 2000 bytes: 8192000)

With the patch:
    eth1[3] page pools: 240 (zombies: 0)
        refs: 152521 bytes: 624726016 (refs: 0 bytes: 0)

One page stuck per forced failure without the fix (2000), none with it.

Tested-by: Brian Vazquez <brianvv@google.com>

> ---
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 885df6294ad94..7383d8241ead5 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -3745,8 +3745,10 @@ static int idpf_rx_update_bufq_desc(struct idpf_buf_queue *bufq, u32 buf_id,
>  	fq.truesize = bufq->hdr_truesize;
>  
>  	addr = libeth_rx_alloc(&fq, buf_id);
> -	if (addr == DMA_MAPPING_ERROR)
> +	if (addr == DMA_MAPPING_ERROR) {
> +		idpf_rx_page_rel(&bufq->buf[buf_id]);
>  		return -ENOMEM;
> +	}
>  
>  	buf_desc->hdr_addr = cpu_to_le64(addr);
>  
> -- 
> 2.56.0.398.g882cbad275-goog
> 

  parent reply	other threads:[~2026-11-04 18:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04  1:36 [PATCH iwl-net] idpf: fix Rx payload buffer leak when header buffer allocation fails Naman Gulati
2026-11-04  1:38 ` netdev-bot+sinfo
2026-11-04  1:49   ` Naman Gulati
2026-11-04 18:14 ` Brian Vazquez [this message]
2026-11-05 18:05 ` Alexander Lobakin

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=aut0VPowjLkQL5Oe@google.com \
    --to=brianvv@google.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=namangulati@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.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.