Skip to content

Commit

Permalink
bnxt_en: Fix TX timeout when TX ring size is set to the smallest
Browse files Browse the repository at this point in the history
The smallest TX ring size we support must fit a TX SKB with MAX_SKB_FRAGS
+ 1.  Because the first TX BD for a packet is always a long TX BD, we
need an extra TX BD to fit this packet.  Define BNXT_MIN_TX_DESC_CNT with
this value to make this more clear.  The current code uses a minimum
that is off by 1.  Fix it using this constant.

The tx_wake_thresh to determine when to wake up the TX queue is half the
ring size but we must have at least BNXT_MIN_TX_DESC_CNT for the next
packet which may have maximum fragments.  So the comparison of the
available TX BDs with tx_wake_thresh should be >= instead of > in the
current code.  Otherwise, at the smallest ring size, we will never wake
up the TX queue and will cause TX timeout.

Fixes: c0c050c ("bnxt_en: New Broadcom ethernet driver.")
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadocm.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Michael Chan authored and davem330 committed Sep 20, 2021
1 parent 563f23b commit 5bed8b0
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 5 deletions.
8 changes: 4 additions & 4 deletions drivers/net/ethernet/broadcom/bnxt/bnxt.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ static bool bnxt_txr_netif_try_stop_queue(struct bnxt *bp,
* netif_tx_queue_stopped().
*/
smp_mb();
if (bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh) {
if (bnxt_tx_avail(bp, txr) >= bp->tx_wake_thresh) {
netif_tx_wake_queue(txq);
return false;
}
Expand Down Expand Up @@ -764,7 +764,7 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
smp_mb();

if (unlikely(netif_tx_queue_stopped(txq)) &&
bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh &&
bnxt_tx_avail(bp, txr) >= bp->tx_wake_thresh &&
READ_ONCE(txr->dev_state) != BNXT_DEV_STATE_CLOSING)
netif_tx_wake_queue(txq);
}
Expand Down Expand Up @@ -2416,7 +2416,7 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
if (TX_CMP_TYPE(txcmp) == CMP_TYPE_TX_L2_CMP) {
tx_pkts++;
/* return full budget so NAPI will complete. */
if (unlikely(tx_pkts > bp->tx_wake_thresh)) {
if (unlikely(tx_pkts >= bp->tx_wake_thresh)) {
rx_pkts = budget;
raw_cons = NEXT_RAW_CMP(raw_cons);
if (budget)
Expand Down Expand Up @@ -3640,7 +3640,7 @@ static int bnxt_init_tx_rings(struct bnxt *bp)
u16 i;

bp->tx_wake_thresh = max_t(int, bp->tx_ring_size / 2,
MAX_SKB_FRAGS + 1);
BNXT_MIN_TX_DESC_CNT);

for (i = 0; i < bp->tx_nr_rings; i++) {
struct bnxt_tx_ring_info *txr = &bp->tx_ring[i];
Expand Down
5 changes: 5 additions & 0 deletions drivers/net/ethernet/broadcom/bnxt/bnxt.h
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,11 @@ struct nqe_cn {
#define BNXT_MAX_RX_JUM_DESC_CNT (RX_DESC_CNT * MAX_RX_AGG_PAGES - 1)
#define BNXT_MAX_TX_DESC_CNT (TX_DESC_CNT * MAX_TX_PAGES - 1)

/* Minimum TX BDs for a TX packet with MAX_SKB_FRAGS + 1. We need one extra
* BD because the first TX BD is always a long BD.
*/
#define BNXT_MIN_TX_DESC_CNT (MAX_SKB_FRAGS + 2)

#define RX_RING(x) (((x) & ~(RX_DESC_CNT - 1)) >> (BNXT_PAGE_SHIFT - 4))
#define RX_IDX(x) ((x) & (RX_DESC_CNT - 1))

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ static int bnxt_set_ringparam(struct net_device *dev,

if ((ering->rx_pending > BNXT_MAX_RX_DESC_CNT) ||
(ering->tx_pending > BNXT_MAX_TX_DESC_CNT) ||
(ering->tx_pending <= MAX_SKB_FRAGS))
(ering->tx_pending < BNXT_MIN_TX_DESC_CNT))
return -EINVAL;

if (netif_running(dev))
Expand Down

0 comments on commit 5bed8b0

Please sign in to comment.