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 3/4] bnxt: make sure xmit_more + errors does not miss doorbells
Date: Wed, 11 Aug 2021 12:32:38 -0700 [thread overview]
Message-ID: <20210811193239.3155396-4-kuba@kernel.org> (raw)
In-Reply-To: <20210811193239.3155396-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 ring should never be busy given the queue is stopped in advance
so add a warning there and ignore the busy case.
Noticed while reading the code.
Fixes: 4d172f21cefe ("bnxt_en: Implement xmit_more.")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 33 +++++++++++++++--------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 2c0240ee2105..b80ed556c28b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -367,6 +367,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 +403,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())
+ netdev_warn(dev, "bnxt: ring busy!\n");
return NETDEV_TX_BUSY;
}
@@ -516,21 +525,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 +621,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 +667,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, 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 19:32 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-11 19:32 [PATCH net 0/4] bnxt: Tx NAPI disabling resiliency improvements Jakub Kicinski
2021-08-11 19:32 ` [PATCH net 1/4] bnxt: don't lock the tx queue from napi poll Jakub Kicinski
2021-08-11 20:13 ` Edwin Peer
2021-08-11 20:42 ` Michael Chan
2021-08-11 21:09 ` Jakub Kicinski
2021-08-11 19:32 ` [PATCH net 2/4] bnxt: disable napi before cancelling DIM Jakub Kicinski
2021-08-11 19:32 ` Jakub Kicinski [this message]
2021-08-11 21:12 ` [PATCH net 3/4] bnxt: make sure xmit_more + errors does not miss doorbells Michael Chan
2021-08-11 19:32 ` [PATCH net 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=20210811193239.3155396-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: link
Be 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).