Commit 73011773 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

eth: bnxt: fix counting packets discarded due to OOM and netpoll 



I added OOM and netpoll discard counters, naively assuming that
the cpr pointer is pointing to a common completion ring.
Turns out that is usually *a* completion ring but not *the*
completion ring which bnapi->cp_ring points to. bnapi->cp_ring
is where the stats are read from, so we end up reporting 0
thru ethtool -S and qstat even though the drop events have happened.
Make 100% sure we're recording statistics in the correct structure.

Fixes: 907fd4a2 ("bnxt: count discards due to memory allocation errors")
Reviewed-by: default avatarMichael Chan <michael.chan@broadcom.com>
Link: https://lore.kernel.org/r/20240424002148.3937059-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent c04d1b9e

drivers/net/ethernet/broadcom/bnxt/bnxt.c

+18 −26
Original line number Diff line number Diff line
@@ -1778,7 +1778,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp, 
		skb = bnxt_copy_skb(bnapi, data_ptr, len, mapping);

		if (!skb) {

			bnxt_abort_tpa(cpr, idx, agg_bufs);

			cpr->sw_stats.rx.rx_oom_discards += 1;

			cpr->bnapi->cp_ring.sw_stats.rx.rx_oom_discards += 1;

			return NULL;

		}

	} else {
@@ -1788,7 +1788,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp, 
		new_data = __bnxt_alloc_rx_frag(bp, &new_mapping, GFP_ATOMIC);

		if (!new_data) {

			bnxt_abort_tpa(cpr, idx, agg_bufs);

			cpr->sw_stats.rx.rx_oom_discards += 1;

			cpr->bnapi->cp_ring.sw_stats.rx.rx_oom_discards += 1;

			return NULL;

		}
@@ -1804,7 +1804,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp, 
		if (!skb) {

			skb_free_frag(data);

			bnxt_abort_tpa(cpr, idx, agg_bufs);

			cpr->sw_stats.rx.rx_oom_discards += 1;

			cpr->bnapi->cp_ring.sw_stats.rx.rx_oom_discards += 1;

			return NULL;

		}

		skb_reserve(skb, bp->rx_offset);
@@ -1815,7 +1815,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp, 
		skb = bnxt_rx_agg_pages_skb(bp, cpr, skb, idx, agg_bufs, true);

		if (!skb) {

			/* Page reuse already handled by bnxt_rx_pages(). */

			cpr->sw_stats.rx.rx_oom_discards += 1;

			cpr->bnapi->cp_ring.sw_stats.rx.rx_oom_discards += 1;

			return NULL;

		}

	}
@@ -2094,11 +2094,8 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, 
			u32 frag_len = bnxt_rx_agg_pages_xdp(bp, cpr, &xdp,

							     cp_cons, agg_bufs,

							     false);

			if (!frag_len) {

				cpr->sw_stats.rx.rx_oom_discards += 1;

				rc = -ENOMEM;

				goto next_rx;

			}

			if (!frag_len)

				goto oom_next_rx;

		}

		xdp_active = true;

	}
@@ -2121,9 +2118,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, 
				else

					bnxt_xdp_buff_frags_free(rxr, &xdp);

			}

			cpr->sw_stats.rx.rx_oom_discards += 1;

			rc = -ENOMEM;

			goto next_rx;

			goto oom_next_rx;

		}

	} else {

		u32 payload;
@@ -2134,29 +2129,21 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, 
			payload = 0;

		skb = bp->rx_skb_func(bp, rxr, cons, data, data_ptr, dma_addr,

				      payload | len);

		if (!skb) {

			cpr->sw_stats.rx.rx_oom_discards += 1;

			rc = -ENOMEM;

			goto next_rx;

		}

		if (!skb)

			goto oom_next_rx;

	}



	if (agg_bufs) {

		if (!xdp_active) {

			skb = bnxt_rx_agg_pages_skb(bp, cpr, skb, cp_cons, agg_bufs, false);

			if (!skb) {

				cpr->sw_stats.rx.rx_oom_discards += 1;

				rc = -ENOMEM;

				goto next_rx;

			}

			if (!skb)

				goto oom_next_rx;

		} else {

			skb = bnxt_xdp_build_skb(bp, skb, agg_bufs, rxr->page_pool, &xdp, rxcmp1);

			if (!skb) {

				/* we should be able to free the old skb here */

				bnxt_xdp_buff_frags_free(rxr, &xdp);

				cpr->sw_stats.rx.rx_oom_discards += 1;

				rc = -ENOMEM;

				goto next_rx;

				goto oom_next_rx;

			}

		}

	}
@@ -2234,6 +2221,11 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, 
	*raw_cons = tmp_raw_cons;



	return rc;



oom_next_rx:

	cpr->bnapi->cp_ring.sw_stats.rx.rx_oom_discards += 1;

	rc = -ENOMEM;

	goto next_rx;

}



/* In netpoll mode, if we are using a combined completion ring, we need to
@@ -2280,7 +2272,7 @@ static int bnxt_force_rx_discard(struct bnxt *bp, 
	}

	rc = bnxt_rx_pkt(bp, cpr, raw_cons, event);

	if (rc && rc != -EBUSY)

		cpr->sw_stats.rx.rx_netpoll_discards += 1;

		cpr->bnapi->cp_ring.sw_stats.rx.rx_netpoll_discards += 1;

	return rc;

}