Netdev Archive on lore.kernel.org help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org> To: davem@davemloft.net Cc: michael.chan@broadcom.com, huangjw@broadcom.com, eddie.wai@broadcom.com, prashant@broadcom.com, gospo@broadcom.com, netdev@vger.kernel.org, edwin.peer@broadcom.com, Jakub Kicinski <kuba@kernel.org> Subject: [PATCH net v2 3/4] bnxt: make sure xmit_more + errors does not miss doorbells Date: Wed, 11 Aug 2021 14:37:48 -0700 [thread overview] Message-ID: <20210811213749.3276687-4-kuba@kernel.org> (raw) In-Reply-To: <20210811213749.3276687-1-kuba@kernel.org> skbs are freed on error and not put on the ring. We may, however, be in a situation where we're freeing the last skb of a batch, and there is a doorbell ring pending because of xmit_more() being true earlier. Make sure we ring the door bell in such situations. Since errors are rare don't pay attention to xmit_more() and just always flush the pending frames. The busy case should be safe to be left alone because it can only happen if start_xmit races with completions and they both enable the queue. In that case the kick can't be pending. Noticed while reading the code. Fixes: 4d172f21cefe ("bnxt_en: Implement xmit_more.") Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- v2: - netdev_warn() -> netif_warn() [Edwin] - use correct prod value [Michael] --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 36 +++++++++++++++-------- drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 + 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 52f5c8405e76..79bbd6ec7ef7 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -72,7 +72,8 @@ #include "bnxt_debugfs.h" #define BNXT_TX_TIMEOUT (5 * HZ) -#define BNXT_DEF_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_HW) +#define BNXT_DEF_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_HW | \ + NETIF_MSG_TX_ERR) MODULE_LICENSE("GPL"); MODULE_DESCRIPTION("Broadcom BCM573xx network driver"); @@ -367,6 +368,13 @@ static u16 bnxt_xmit_get_cfa_action(struct sk_buff *skb) return md_dst->u.port_info.port_id; } +static void bnxt_txr_db_kick(struct bnxt *bp, struct bnxt_tx_ring_info *txr, + u16 prod) +{ + bnxt_db_write(bp, &txr->tx_db, prod); + txr->kick_pending = 0; +} + static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) { struct bnxt *bp = netdev_priv(dev); @@ -396,6 +404,8 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) free_size = bnxt_tx_avail(bp, txr); if (unlikely(free_size < skb_shinfo(skb)->nr_frags + 2)) { netif_tx_stop_queue(txq); + if (net_ratelimit() && txr->kick_pending) + netif_warn(bp, tx_err, dev, "bnxt: ring busy!\n"); return NETDEV_TX_BUSY; } @@ -516,21 +526,16 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) normal_tx: if (length < BNXT_MIN_PKT_SIZE) { pad = BNXT_MIN_PKT_SIZE - length; - if (skb_pad(skb, pad)) { + if (skb_pad(skb, pad)) /* SKB already freed. */ - tx_buf->skb = NULL; - return NETDEV_TX_OK; - } + goto tx_kick_pending; length = BNXT_MIN_PKT_SIZE; } mapping = dma_map_single(&pdev->dev, skb->data, len, DMA_TO_DEVICE); - if (unlikely(dma_mapping_error(&pdev->dev, mapping))) { - dev_kfree_skb_any(skb); - tx_buf->skb = NULL; - return NETDEV_TX_OK; - } + if (unlikely(dma_mapping_error(&pdev->dev, mapping))) + goto tx_free; dma_unmap_addr_set(tx_buf, mapping, mapping); flags = (len << TX_BD_LEN_SHIFT) | TX_BD_TYPE_LONG_TX_BD | @@ -617,13 +622,15 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) txr->tx_prod = prod; if (!netdev_xmit_more() || netif_xmit_stopped(txq)) - bnxt_db_write(bp, &txr->tx_db, prod); + bnxt_txr_db_kick(bp, txr, prod); + else + txr->kick_pending = 1; tx_done: if (unlikely(bnxt_tx_avail(bp, txr) <= MAX_SKB_FRAGS + 1)) { if (netdev_xmit_more() && !tx_buf->is_push) - bnxt_db_write(bp, &txr->tx_db, prod); + bnxt_txr_db_kick(bp, txr, prod); netif_tx_stop_queue(txq); @@ -661,7 +668,12 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) PCI_DMA_TODEVICE); } +tx_free: dev_kfree_skb_any(skb); +tx_kick_pending: + tx_buf->skb = NULL; + if (txr->kick_pending) + bnxt_txr_db_kick(bp, txr, txr->tx_prod); return NETDEV_TX_OK; } diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h index 9c3324e76ff7..7b989b6e4f6e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h @@ -799,6 +799,7 @@ struct bnxt_tx_ring_info { u16 tx_prod; u16 tx_cons; u16 txq_index; + u8 kick_pending; struct bnxt_db_info tx_db; struct tx_bd *tx_desc_ring[MAX_TX_PAGES]; -- 2.31.1
next prev parent reply other threads:[~2021-08-11 21:38 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-11 21:37 [PATCH net v2 0/4] bnxt: Tx NAPI disabling resiliency improvements Jakub Kicinski 2021-08-11 21:37 ` [PATCH net v2 1/4] bnxt: don't lock the tx queue from napi poll Jakub Kicinski 2021-08-11 21:37 ` [PATCH net v2 2/4] bnxt: disable napi before canceling DIM Jakub Kicinski 2021-08-11 21:37 ` Jakub Kicinski [this message] 2021-08-11 22:36 ` [PATCH net v2 3/4] bnxt: make sure xmit_more + errors does not miss doorbells Michael Chan 2021-08-11 22:44 ` Jakub Kicinski 2021-08-11 23:00 ` Michael Chan 2021-08-11 23:16 ` Jakub Kicinski 2021-08-11 23:38 ` Michael Chan 2021-08-12 6:51 ` Michael Chan 2021-08-13 8:35 ` David Laight 2021-08-11 21:37 ` [PATCH net v2 4/4] bnxt: count Tx drops Jakub Kicinski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210811213749.3276687-4-kuba@kernel.org \ --to=kuba@kernel.org \ --cc=davem@davemloft.net \ --cc=eddie.wai@broadcom.com \ --cc=edwin.peer@broadcom.com \ --cc=gospo@broadcom.com \ --cc=huangjw@broadcom.com \ --cc=michael.chan@broadcom.com \ --cc=netdev@vger.kernel.org \ --cc=prashant@broadcom.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).