Thread (12 messages) 12 messages, 3 authors, 2021-08-13

Re: [PATCH net v2 3/4] bnxt: make sure xmit_more + errors does not miss doorbells

flat view

From: Michael Chan <michael.chan@broadcom.com>
Date: 2021-08-11 22:36:48

On Wed, Aug 11, 2021 at 2:38 PM Jakub Kicinski [off-list ref] wrote:
quoted hunk ↗ jump to hunk
---
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");
You forgot to remove this.
quoted hunk ↗ jump to hunk
                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;
I think we should remove the setting of tx_buf->skb to NULL in the
tx_dma_error path since we are setting it here now.
quoted hunk ↗ jump to hunk
+       if (txr->kick_pending)
+               bnxt_txr_db_kick(bp, txr, txr->tx_prod);
        return NETDEV_TX_OK;
 }

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help