netdev - Re: [PATCH iwl-net] ice: fix incorrect counter for buffer allocation failures

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL+tcoAwKcy-E6LkLhwvKA9+es5RuFmg4+kPZ8dV08-s-VopPA@mail.gmail.com>
Date: Sat, 9 Aug 2025 21:08:57 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: Michal Kubiak <michal.kubiak@...el.com>, intel-wired-lan@...ts.osuosl.org, 
	maciej.fijalkowski@...el.com, netdev@...r.kernel.org, 
	przemyslaw.kitszel@...el.com, jacob.e.keller@...el.com, 
	aleksander.lobakin@...el.com, anthony.l.nguyen@...el.com
Subject: Re: [PATCH iwl-net] ice: fix incorrect counter for buffer allocation failures

On Sat, Aug 9, 2025 at 5:38 PM Paul Menzel <pmenzel@...gen.mpg.de> wrote:
>
> Dear Michal,
>
>
> Thank you for your patch.
>
>
> Am 08.08.25 um 17:53 schrieb Michal Kubiak:
> > Currently, the driver increments `alloc_page_failed` when buffer allocation fails
> > in `ice_clean_rx_irq()`. However, this counter is intended for page allocation
> > failures, not buffer allocation issues.
> >
> > This patch corrects the counter by incrementing `alloc_buf_failed` instead,
> > ensuring accurate statistics reporting for buffer allocation failures.
> >
> > Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
> > Reported-by: Jacob Keller <jacob.e.keller@...el.com>
> > Suggested-by: Paul Menzel <pmenzel@...gen.mpg.de>
>
> Thank you, but I merely asked to send in the patch separately and didn’t
> spot the error. So, I’d remove the tag, but you add the one at the end.
>
> > Signed-off-by: Michal Kubiak <michal.kubiak@...el.com>

Reviewed-by: Jason Xing <kerneljasonxing@...il.com>

> > ---
> >   drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > index 93907ab2eac7..1b1ebfd347ef 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > @@ -1337,7 +1337,7 @@ static int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
> >                       skb = ice_construct_skb(rx_ring, xdp);
> >               /* exit if we failed to retrieve a buffer */
> >               if (!skb) {
> > -                     rx_ring->ring_stats->rx_stats.alloc_page_failed++;
> > +                     rx_ring->ring_stats->rx_stats.alloc_buf_failed++;
> >                       xdp_verdict = ICE_XDP_CONSUMED;
> >               }
> >               ice_put_rx_mbuf(rx_ring, xdp, ntc, xdp_verdict);
>
> Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>
>
>
> Kind regards,
>
> Paul
>
>
> PS: A little off-topic: As this code is present since v6.3-rc1, I
> wonder, why this has not been causing any user visible issues in the
> last two years. Can somebody explain this?
>

>From my limited experience, upgrading to the new kernel (like v6.x) is
not an easy thing. Plus not that many people monitor the driver
counter on the machine with the ice driver loaded. Sometimes we
neglect this error because it doesn't harm the real and overall
workload even when the allocation fails. Things like this sometimes
happen in other areas :)

Thanks,
Jason