From: Jakub Kicinski <kuba@kernel.org>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com,
michael.chan@broadcom.com, Jakub Kicinski <kuba@kernel.org>
Subject: [PATCH net-next 3/3] eth: bnxt: handle invalid Tx completions more gracefully
Date: Mon, 10 Jul 2023 13:56:11 -0700 [thread overview]
Message-ID: <20230710205611.1198878-4-kuba@kernel.org> (raw)
In-Reply-To: <20230710205611.1198878-1-kuba@kernel.org>
Invalid Tx completions should never happen (tm) but when they do
they crash the host, because driver blindly trusts that there is
a valid skb pointer on the ring.
The completions I've seen appear to be some form of FW / HW
miscalculation or staleness, they have typical (small) values
(<100), but they are most often higher than number of queued
descriptors. They usually happen after boot.
Instead of crashing print a warning and schedule a reset.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 +++++++++++++-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b8ddad33b01a..bfa56f35d2e0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -690,6 +690,16 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
skb = tx_buf->skb;
tx_buf->skb = NULL;
+ if (unlikely(!skb)) {
+ netdev_err(bp->dev, "Invalid Tx completion (ring:%d tx_pkts:%d cons:%u prod:%u i:%d)",
+ txr->txq_index, bnapi->tx_pkts,
+ txr->tx_cons, txr->tx_prod, i);
+ WARN_ON_ONCE(1);
+ bnapi->tx_fault = 1;
+ bnxt_queue_sp_work(bp, BNXT_RESET_TASK_SP_EVENT);
+ return;
+ }
+
tx_bytes += skb->len;
if (tx_buf->is_push) {
@@ -2576,7 +2586,7 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
static void __bnxt_poll_work_done(struct bnxt *bp, struct bnxt_napi *bnapi)
{
- if (bnapi->tx_pkts) {
+ if (bnapi->tx_pkts && !bnapi->tx_fault) {
bnapi->tx_int(bp, bnapi, bnapi->tx_pkts);
bnapi->tx_pkts = 0;
}
@@ -9429,6 +9439,8 @@ static void bnxt_enable_napi(struct bnxt *bp)
struct bnxt_napi *bnapi = bp->bnapi[i];
struct bnxt_cp_ring_info *cpr;
+ bnapi->tx_fault = 0;
+
cpr = &bnapi->cp_ring;
if (bnapi->in_reset)
cpr->sw_stats.rx.rx_resets++;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 080e73496066..08ce9046bfd2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1008,6 +1008,7 @@ struct bnxt_napi {
int);
int tx_pkts;
u8 events;
+ u8 tx_fault:1;
u32 flags;
#define BNXT_NAPI_FLAG_XDP 0x1
--
2.41.0
next prev parent reply other threads:[~2023-07-10 20:56 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-10 20:56 [PATCH net-next 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
2023-07-10 20:56 ` [PATCH net-next 1/3] eth: bnxt: move and rename reset helpers Jakub Kicinski
2023-07-10 20:56 ` [PATCH net-next 2/3] eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work() Jakub Kicinski
2023-07-10 20:56 ` Jakub Kicinski [this message]
2023-07-11 8:00 ` [PATCH net-next 3/3] eth: bnxt: handle invalid Tx completions more gracefully Michael Chan
2023-07-12 0:01 ` Michael Chan
2023-07-12 1:09 ` Jakub Kicinski
2023-07-12 4:11 ` Michael Chan
2023-07-12 4:24 ` Jakub Kicinski
2023-07-12 4:50 ` Michael Chan
2023-07-12 16:35 ` Jakub Kicinski
2023-07-11 10:10 ` Paolo Abeni
2023-07-12 1:19 ` Jakub Kicinski
2023-07-12 6:50 ` Paolo Abeni
2023-07-12 16:34 ` Jakub Kicinski
2023-07-12 20:31 ` Paolo Abeni
2023-07-12 20:50 ` Jakub Kicinski
2023-07-10 21:44 ` [PATCH net-next 0/3] " Michael Chan
2023-07-11 0:38 ` 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=20230710205611.1198878-4-kuba@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=michael.chan@broadcom.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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).