[PATCH net] bnxt_en: Fix potential data corruption with HW GRO/LRO

[PATCH net] bnxt_en: Fix potential data corruption with HW GRO/LRO

[Michael Chan]

From: Srijit Bose <srijit.bose@broadcom.com>

Fix the max number of bits passed to find_first_zero_bit() in
bnxt_alloc_agg_idx().  We were incorrectly passing the number of
long words.  find_first_zero_bit() may fail to find a zero bit and
cause a wrong ID to be used.  If the wrong ID is already in use, this
can cause data corruption.  Sometimes an error like this can also be
seen:

bnxt_en 0000:83:00.0 enp131s0np0: TPA end agg_buf 2 != expected agg_bufs 1

Fix it by passing the correct number of bits MAX_TPA_P5.  Add a sanity
BUG_ON() check if find_first_zero_bit() fails.  It should never happen.

Fixes: ec4d8e7cf024 ("bnxt_en: Add TPA ID mapping logic for 57500 chips.")
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Srijit Bose <srijit.bose@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d17d0ea89c36..6704cbbc1b24 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1482,9 +1482,10 @@ static u16 bnxt_alloc_agg_idx(struct bnxt_rx_ring_info *rxr, u16 agg_id)
 	struct bnxt_tpa_idx_map *map = rxr->rx_tpa_idx_map;
 	u16 idx = agg_id & MAX_TPA_P5_MASK;
 
-	if (test_bit(idx, map->agg_idx_bmap))
-		idx = find_first_zero_bit(map->agg_idx_bmap,
-					  BNXT_AGG_IDX_BMAP_SIZE);
+	if (test_bit(idx, map->agg_idx_bmap)) {
+		idx = find_first_zero_bit(map->agg_idx_bmap, MAX_TPA_P5);
+		BUG_ON(idx >= MAX_TPA_P5);
+	}
 	__set_bit(idx, map->agg_idx_bmap);
 	map->agg_id_tbl[agg_id] = idx;
 	return idx;
-- 
2.51.0

Re: [PATCH net] bnxt_en: Fix potential data corruption with HW GRO/LRO

[Vadim Fedorenko]

On 12/24/25 19:11, Michael Chan wrote:
> From: Srijit Bose <srijit.bose@broadcom.com>
> 
> Fix the max number of bits passed to find_first_zero_bit() in
> bnxt_alloc_agg_idx().  We were incorrectly passing the number of
> long words.  find_first_zero_bit() may fail to find a zero bit and
> cause a wrong ID to be used.  If the wrong ID is already in use, this
> can cause data corruption.  Sometimes an error like this can also be
> seen:
> 
> bnxt_en 0000:83:00.0 enp131s0np0: TPA end agg_buf 2 != expected agg_bufs 1
> 
> Fix it by passing the correct number of bits MAX_TPA_P5.  Add a sanity
> BUG_ON() check if find_first_zero_bit() fails.  It should never happen.
> 
> Fixes: ec4d8e7cf024 ("bnxt_en: Add TPA ID mapping logic for 57500 chips.")
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Srijit Bose <srijit.bose@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index d17d0ea89c36..6704cbbc1b24 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -1482,9 +1482,10 @@ static u16 bnxt_alloc_agg_idx(struct bnxt_rx_ring_info *rxr, u16 agg_id)
>   	struct bnxt_tpa_idx_map *map = rxr->rx_tpa_idx_map;
>   	u16 idx = agg_id & MAX_TPA_P5_MASK;
>   
> -	if (test_bit(idx, map->agg_idx_bmap))
> -		idx = find_first_zero_bit(map->agg_idx_bmap,
> -					  BNXT_AGG_IDX_BMAP_SIZE);
> +	if (test_bit(idx, map->agg_idx_bmap)) {
> +		idx = find_first_zero_bit(map->agg_idx_bmap, MAX_TPA_P5);
> +		BUG_ON(idx >= MAX_TPA_P5);
> +	}
>   	__set_bit(idx, map->agg_idx_bmap);
>   	map->agg_id_tbl[agg_id] = idx;
>   	return idx;


The change itself is correct, but it would be great to use DECLARE_BITMAP() in
struct bnxt_tpa_idx_map to completely remove BNXT_AGG_IDX_BMAP_SIZE and avoid
such problems in the future.

Re: [PATCH net] bnxt_en: Fix potential data corruption with HW GRO/LRO

[Leon Romanovsky]

On Wed, Dec 24, 2025 at 11:11:16AM -0800, Michael Chan wrote:
> From: Srijit Bose <srijit.bose@broadcom.com>
> 
> Fix the max number of bits passed to find_first_zero_bit() in
> bnxt_alloc_agg_idx().  We were incorrectly passing the number of
> long words.  find_first_zero_bit() may fail to find a zero bit and
> cause a wrong ID to be used.  If the wrong ID is already in use, this
> can cause data corruption.  Sometimes an error like this can also be
> seen:
> 
> bnxt_en 0000:83:00.0 enp131s0np0: TPA end agg_buf 2 != expected agg_bufs 1
> 
> Fix it by passing the correct number of bits MAX_TPA_P5.  Add a sanity
> BUG_ON() check if find_first_zero_bit() fails.  It should never happen.

Things that should never occur are flagged with WARN_ON(), not BUG_ON().
Using BUG_ON() would unnecessarily crash the system just because something
unexpected happened in the networking driver.

Thanks

> 
> Fixes: ec4d8e7cf024 ("bnxt_en: Add TPA ID mapping logic for 57500 chips.")
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Srijit Bose <srijit.bose@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index d17d0ea89c36..6704cbbc1b24 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -1482,9 +1482,10 @@ static u16 bnxt_alloc_agg_idx(struct bnxt_rx_ring_info *rxr, u16 agg_id)
>  	struct bnxt_tpa_idx_map *map = rxr->rx_tpa_idx_map;
>  	u16 idx = agg_id & MAX_TPA_P5_MASK;
>  
> -	if (test_bit(idx, map->agg_idx_bmap))
> -		idx = find_first_zero_bit(map->agg_idx_bmap,
> -					  BNXT_AGG_IDX_BMAP_SIZE);
> +	if (test_bit(idx, map->agg_idx_bmap)) {
> +		idx = find_first_zero_bit(map->agg_idx_bmap, MAX_TPA_P5);
> +		BUG_ON(idx >= MAX_TPA_P5);
> +	}
>  	__set_bit(idx, map->agg_idx_bmap);
>  	map->agg_id_tbl[agg_id] = idx;
>  	return idx;
> -- 
> 2.51.0
> 
>