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

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

[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@intel.com>
Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Michal Kubiak <michal.kubiak@intel.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);
-- 
2.45.2

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

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1557 bytes --]



On 8/8/2025 8:53 AM, Michal Kubiak wrote:
> 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@intel.com>
> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Michal Kubiak <michal.kubiak@intel.com>
> ---

Thanks for posting this. I had a version locally but never got around to
posting it and forgot... :D

>  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);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Paul Menzel]

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@intel.com>
> Suggested-by: Paul Menzel <pmenzel@molgen.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@intel.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@molgen.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?

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

[Jason Xing]

On Sat, Aug 9, 2025 at 5:38 PM Paul Menzel <pmenzel@molgen.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@intel.com>
> > Suggested-by: Paul Menzel <pmenzel@molgen.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@intel.com>

Reviewed-by: Jason Xing <kerneljasonxing@gmail.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@molgen.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

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

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 3283 bytes --]



On 8/9/2025 6:08 AM, Jason Xing wrote:
> On Sat, Aug 9, 2025 at 5:38 PM Paul Menzel <pmenzel@molgen.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@intel.com>
>>> Suggested-by: Paul Menzel <pmenzel@molgen.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@intel.com>
> 
> Reviewed-by: Jason Xing <kerneljasonxing@gmail.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@molgen.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 :)
> 

Exactly this. An end user is unlikely to know the difference between the
buffer allocated vs page allocated failures. Even if they see the
counters going up, how would they know that the counter going up was a bug?

This counter really is just for user visibility into a problem
occurring. Reporting the right counter is critical from a developer
perspective as we might use the fact that one but not the other is going
up in an attempt to isolate issues.. but the end user is unlikely to
ever notice without code inspection.

> Thanks,
> Jason
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

RE: [Intel-wired-lan] [PATCH iwl-net] ice: fix incorrect counter for buffer allocation failures

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Michal Kubiak
> Sent: Friday, August 8, 2025 5:53 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>;
> netdev@vger.kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; Lobakin, Aleksander
> <aleksander.lobakin@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kubiak, Michal
> <michal.kubiak@intel.com>; Paul Menzel <pmenzel@molgen.mpg.de>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: fix incorrect counter
> for buffer allocation failures
> 
> 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@intel.com>
> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Michal Kubiak <michal.kubiak@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.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);
> --
> 2.45.2

RE: [Intel-wired-lan] [PATCH iwl-net] ice: fix incorrect counter for buffer allocation failures

[Singh, PriyaX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Michal Kubiak
> Sent: Friday, August 8, 2025 9:23 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>;
> netdev@vger.kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>;
> Lobakin, Aleksander <aleksander.lobakin@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kubiak, Michal <michal.kubiak@intel.com>;
> Paul Menzel <pmenzel@molgen.mpg.de>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: fix incorrect counter for buffer
> allocation failures
> 
> 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@intel.com>
> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Michal Kubiak <michal.kubiak@intel.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);
> --
> 2.45.2

Tested-by: Priya Singh <priyax.singh@intel.com>

RE: [Intel-wired-lan] [PATCH iwl-net] ice: fix incorrect counter for buffer allocation failures

[Singh, PriyaX]

> 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@intel.com>
> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Michal Kubiak <michal.kubiak@intel.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);
> --
> 2.45.2
> 
> Tested-by: Priya Singh <priyax.singh@intel.com>