* [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully
@ 2023-07-20 1:04 Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 1/3] eth: bnxt: move and rename reset helpers Jakub Kicinski
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-07-20 1:04 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski
bnxt trusts the events generated by the device which may lead to kernel
crashes. These are extremely rare but they do happen. For a while
I thought crashing may be intentional, because device reporting invalid
completions should never happen, and having a core dump could be useful
if it does. But in practice I haven't found any clues in the core dumps,
and panic_on_warn exists.
Series was tested by forcing the recovery path manually. Because of
how rare the real crashes are I can't confirm it works for the actual
device errors until it's been widely deployed.
v2:
- factor out the reset scheduling
- also add a check on the XDP path
v1: https://lore.kernel.org/all/20230710205611.1198878-1-kuba@kernel.org/
Jakub Kicinski (3):
eth: bnxt: move and rename reset helpers
eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work()
eth: bnxt: handle invalid Tx completions more gracefully
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 154 ++++++++++--------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 4 +
3 files changed, 91 insertions(+), 70 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 1/3] eth: bnxt: move and rename reset helpers
2023-07-20 1:04 [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
@ 2023-07-20 1:04 ` Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 2/3] eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work() Jakub Kicinski
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-07-20 1:04 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski
Move the reset helpers, subsequent patches will need some
of them on the Tx path.
While at it rename bnxt_sched_reset(), on more recent chips
it schedules a queue reset, instead of a fuller reset.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 72 +++++++++++------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d84ded8db93d..417554a4c837 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -293,6 +293,38 @@ static void bnxt_db_cq(struct bnxt *bp, struct bnxt_db_info *db, u32 idx)
BNXT_DB_CQ(db, idx);
}
+static void bnxt_queue_fw_reset_work(struct bnxt *bp, unsigned long delay)
+{
+ if (!(test_bit(BNXT_STATE_IN_FW_RESET, &bp->state)))
+ return;
+
+ if (BNXT_PF(bp))
+ queue_delayed_work(bnxt_pf_wq, &bp->fw_reset_task, delay);
+ else
+ schedule_delayed_work(&bp->fw_reset_task, delay);
+}
+
+static void bnxt_queue_sp_work(struct bnxt *bp)
+{
+ if (BNXT_PF(bp))
+ queue_work(bnxt_pf_wq, &bp->sp_task);
+ else
+ schedule_work(&bp->sp_task);
+}
+
+static void bnxt_sched_reset_rxr(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
+{
+ if (!rxr->bnapi->in_reset) {
+ rxr->bnapi->in_reset = true;
+ if (bp->flags & BNXT_FLAG_CHIP_P5)
+ set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
+ else
+ set_bit(BNXT_RST_RING_SP_EVENT, &bp->sp_event);
+ bnxt_queue_sp_work(bp);
+ }
+ rxr->rx_next_cons = 0xffff;
+}
+
const u16 bnxt_lhint_arr[] = {
TX_BD_FLAGS_LHINT_512_AND_SMALLER,
TX_BD_FLAGS_LHINT_512_TO_1023,
@@ -1234,38 +1266,6 @@ static int bnxt_discard_rx(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
return 0;
}
-static void bnxt_queue_fw_reset_work(struct bnxt *bp, unsigned long delay)
-{
- if (!(test_bit(BNXT_STATE_IN_FW_RESET, &bp->state)))
- return;
-
- if (BNXT_PF(bp))
- queue_delayed_work(bnxt_pf_wq, &bp->fw_reset_task, delay);
- else
- schedule_delayed_work(&bp->fw_reset_task, delay);
-}
-
-static void bnxt_queue_sp_work(struct bnxt *bp)
-{
- if (BNXT_PF(bp))
- queue_work(bnxt_pf_wq, &bp->sp_task);
- else
- schedule_work(&bp->sp_task);
-}
-
-static void bnxt_sched_reset(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
-{
- if (!rxr->bnapi->in_reset) {
- rxr->bnapi->in_reset = true;
- if (bp->flags & BNXT_FLAG_CHIP_P5)
- set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
- else
- set_bit(BNXT_RST_RING_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
- rxr->rx_next_cons = 0xffff;
-}
-
static u16 bnxt_alloc_agg_idx(struct bnxt_rx_ring_info *rxr, u16 agg_id)
{
struct bnxt_tpa_idx_map *map = rxr->rx_tpa_idx_map;
@@ -1320,7 +1320,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
netdev_warn(bp->dev, "TPA cons %x, expected cons %x, error code %x\n",
cons, rxr->rx_next_cons,
TPA_START_ERROR_CODE(tpa_start1));
- bnxt_sched_reset(bp, rxr);
+ bnxt_sched_reset_rxr(bp, rxr);
return;
}
/* Store cfa_code in tpa_info to use in tpa_end
@@ -1844,7 +1844,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
if (rxr->rx_next_cons != 0xffff)
netdev_warn(bp->dev, "RX cons %x != expected cons %x\n",
cons, rxr->rx_next_cons);
- bnxt_sched_reset(bp, rxr);
+ bnxt_sched_reset_rxr(bp, rxr);
if (rc1)
return rc1;
goto next_rx_no_prod_no_len;
@@ -1882,7 +1882,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
!(bp->fw_cap & BNXT_FW_CAP_RING_MONITOR)) {
netdev_warn_once(bp->dev, "RX buffer error %x\n",
rx_err);
- bnxt_sched_reset(bp, rxr);
+ bnxt_sched_reset_rxr(bp, rxr);
}
}
goto next_rx_no_len;
@@ -2329,7 +2329,7 @@ static int bnxt_async_event_process(struct bnxt *bp,
goto async_event_process_exit;
}
rxr = bp->bnapi[grp_idx]->rx_ring;
- bnxt_sched_reset(bp, rxr);
+ bnxt_sched_reset_rxr(bp, rxr);
goto async_event_process_exit;
}
case ASYNC_EVENT_CMPL_EVENT_ID_ECHO_REQUEST: {
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next v2 2/3] eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work()
2023-07-20 1:04 [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 1/3] eth: bnxt: move and rename reset helpers Jakub Kicinski
@ 2023-07-20 1:04 ` Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 3/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-07-20 1:04 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski
Most callers of bnxt_queue_sp_work() set a bit to indicate what work
to perform right before calling it. Pass it to the function instead.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 61 ++++++++++-------------
1 file changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 417554a4c837..7b545d2a98b4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -304,7 +304,7 @@ static void bnxt_queue_fw_reset_work(struct bnxt *bp, unsigned long delay)
schedule_delayed_work(&bp->fw_reset_task, delay);
}
-static void bnxt_queue_sp_work(struct bnxt *bp)
+static void __bnxt_queue_sp_work(struct bnxt *bp)
{
if (BNXT_PF(bp))
queue_work(bnxt_pf_wq, &bp->sp_task);
@@ -312,6 +312,12 @@ static void bnxt_queue_sp_work(struct bnxt *bp)
schedule_work(&bp->sp_task);
}
+static void bnxt_queue_sp_work(struct bnxt *bp, unsigned int event)
+{
+ set_bit(event, &bp->sp_event);
+ __bnxt_queue_sp_work(bp);
+}
+
static void bnxt_sched_reset_rxr(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
{
if (!rxr->bnapi->in_reset) {
@@ -320,7 +326,7 @@ static void bnxt_sched_reset_rxr(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
else
set_bit(BNXT_RST_RING_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ __bnxt_queue_sp_work(bp);
}
rxr->rx_next_cons = 0xffff;
}
@@ -2384,7 +2390,7 @@ static int bnxt_async_event_process(struct bnxt *bp,
default:
goto async_event_process_exit;
}
- bnxt_queue_sp_work(bp);
+ __bnxt_queue_sp_work(bp);
async_event_process_exit:
return 0;
}
@@ -2413,8 +2419,7 @@ static int bnxt_hwrm_handler(struct bnxt *bp, struct tx_cmp *txcmp)
}
set_bit(vf_id - bp->pf.first_vf_id, bp->pf.vf_event_bmap);
- set_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ bnxt_queue_sp_work(bp, BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT);
break;
case CMPL_BASE_TYPE_HWRM_ASYNC_EVENT:
@@ -11031,8 +11036,7 @@ static void bnxt_set_rx_mode(struct net_device *dev)
if (mask != vnic->rx_mask || uc_update || mc_update) {
vnic->rx_mask = mask;
- set_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ bnxt_queue_sp_work(bp, BNXT_RX_MASK_SP_EVENT);
}
}
@@ -11597,8 +11601,7 @@ static void bnxt_tx_timeout(struct net_device *dev, unsigned int txqueue)
struct bnxt *bp = netdev_priv(dev);
netdev_err(bp->dev, "TX timeout detected, starting reset task!\n");
- set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ bnxt_queue_sp_work(bp, BNXT_RESET_TASK_SP_EVENT);
}
static void bnxt_fw_health_check(struct bnxt *bp)
@@ -11635,8 +11638,7 @@ static void bnxt_fw_health_check(struct bnxt *bp)
return;
fw_reset:
- set_bit(BNXT_FW_EXCEPTION_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ bnxt_queue_sp_work(bp, BNXT_FW_EXCEPTION_SP_EVENT);
}
static void bnxt_timer(struct timer_list *t)
@@ -11653,21 +11655,15 @@ static void bnxt_timer(struct timer_list *t)
if (bp->fw_cap & BNXT_FW_CAP_ERROR_RECOVERY)
bnxt_fw_health_check(bp);
- if (BNXT_LINK_IS_UP(bp) && bp->stats_coal_ticks) {
- set_bit(BNXT_PERIODIC_STATS_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
+ if (BNXT_LINK_IS_UP(bp) && bp->stats_coal_ticks)
+ bnxt_queue_sp_work(bp, BNXT_PERIODIC_STATS_SP_EVENT);
- if (bnxt_tc_flower_enabled(bp)) {
- set_bit(BNXT_FLOW_STATS_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
+ if (bnxt_tc_flower_enabled(bp))
+ bnxt_queue_sp_work(bp, BNXT_FLOW_STATS_SP_EVENT);
#ifdef CONFIG_RFS_ACCEL
- if ((bp->flags & BNXT_FLAG_RFS) && bp->ntp_fltr_count) {
- set_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
+ if ((bp->flags & BNXT_FLAG_RFS) && bp->ntp_fltr_count)
+ bnxt_queue_sp_work(bp, BNXT_RX_NTP_FLTR_SP_EVENT);
#endif /*CONFIG_RFS_ACCEL*/
if (bp->link_info.phy_retry) {
@@ -11675,21 +11671,17 @@ static void bnxt_timer(struct timer_list *t)
bp->link_info.phy_retry = false;
netdev_warn(bp->dev, "failed to update phy settings after maximum retries.\n");
} else {
- set_bit(BNXT_UPDATE_PHY_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ bnxt_queue_sp_work(bp, BNXT_UPDATE_PHY_SP_EVENT);
}
}
- if (test_bit(BNXT_STATE_L2_FILTER_RETRY, &bp->state)) {
- set_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
+ if (test_bit(BNXT_STATE_L2_FILTER_RETRY, &bp->state))
+ bnxt_queue_sp_work(bp, BNXT_RX_MASK_SP_EVENT);
if ((bp->flags & BNXT_FLAG_CHIP_P5) && !bp->chip_rev &&
- netif_carrier_ok(dev)) {
- set_bit(BNXT_RING_COAL_NOW_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
+ netif_carrier_ok(dev))
+ bnxt_queue_sp_work(bp, BNXT_RING_COAL_NOW_SP_EVENT);
+
bnxt_restart_timer:
mod_timer(&bp->timer, jiffies + bp->current_interval);
}
@@ -12968,8 +12960,7 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb,
bp->ntp_fltr_count++;
spin_unlock_bh(&bp->ntp_fltr_lock);
- set_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
+ bnxt_queue_sp_work(bp, BNXT_RX_NTP_FLTR_SP_EVENT);
return new_fltr->sw_id;
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next v2 3/3] eth: bnxt: handle invalid Tx completions more gracefully
2023-07-20 1:04 [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 1/3] eth: bnxt: move and rename reset helpers Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 2/3] eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work() Jakub Kicinski
@ 2023-07-20 1:04 ` Jakub Kicinski
2023-07-20 6:59 ` [PATCH net-next v2 0/3] " Michael Chan
2023-07-21 3:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-07-20 1:04 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, michael.chan, Jakub Kicinski
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>
--
v2:
- factor out the reset scheduling to a helper
- add the check in XDP as well
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 25 ++++++++++++++++++-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +++
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 4 +++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7b545d2a98b4..a3bbd13c070f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -331,6 +331,22 @@ static void bnxt_sched_reset_rxr(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
rxr->rx_next_cons = 0xffff;
}
+void bnxt_sched_reset_txr(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
+ int idx)
+{
+ struct bnxt_napi *bnapi = txr->bnapi;
+
+ if (bnapi->tx_fault)
+ return;
+
+ 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, idx);
+ WARN_ON_ONCE(1);
+ bnapi->tx_fault = 1;
+ bnxt_queue_sp_work(bp, BNXT_RESET_TASK_SP_EVENT);
+}
+
const u16 bnxt_lhint_arr[] = {
TX_BD_FLAGS_LHINT_512_AND_SMALLER,
TX_BD_FLAGS_LHINT_512_TO_1023,
@@ -690,6 +706,11 @@ 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)) {
+ bnxt_sched_reset_txr(bp, txr, i);
+ return;
+ }
+
tx_bytes += skb->len;
if (tx_buf->is_push) {
@@ -2576,7 +2597,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 +9450,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..9d16757e27fe 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
@@ -2329,6 +2330,8 @@ int bnxt_get_avail_msix(struct bnxt *bp, int num);
int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init);
void bnxt_tx_disable(struct bnxt *bp);
void bnxt_tx_enable(struct bnxt *bp);
+void bnxt_sched_reset_txr(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
+ int idx);
void bnxt_report_link(struct bnxt *bp);
int bnxt_update_link(struct bnxt *bp, bool chng_link_state);
int bnxt_hwrm_set_pause(struct bnxt *);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index 4efa5fe6972b..5b6fbdc4dc40 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -149,6 +149,7 @@ void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
tx_buf->action = 0;
tx_buf->xdpf = NULL;
} else if (tx_buf->action == XDP_TX) {
+ tx_buf->action = 0;
rx_doorbell_needed = true;
last_tx_cons = tx_cons;
@@ -158,6 +159,9 @@ void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
tx_buf = &txr->tx_buf_ring[tx_cons];
page_pool_recycle_direct(rxr->page_pool, tx_buf->page);
}
+ } else {
+ bnxt_sched_reset_txr(bp, txr, i);
+ return;
}
tx_cons = NEXT_TX(tx_cons);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully
2023-07-20 1:04 [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
` (2 preceding siblings ...)
2023-07-20 1:04 ` [PATCH net-next v2 3/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
@ 2023-07-20 6:59 ` Michael Chan
2023-07-21 3:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Michael Chan @ 2023-07-20 6:59 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni
On Wed, Jul 19, 2023 at 6:05 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> bnxt trusts the events generated by the device which may lead to kernel
> crashes. These are extremely rare but they do happen. For a while
> I thought crashing may be intentional, because device reporting invalid
> completions should never happen, and having a core dump could be useful
> if it does. But in practice I haven't found any clues in the core dumps,
> and panic_on_warn exists.
>
> Series was tested by forcing the recovery path manually. Because of
> how rare the real crashes are I can't confirm it works for the actual
> device errors until it's been widely deployed.
>
> v2:
> - factor out the reset scheduling
> - also add a check on the XDP path
> v1: https://lore.kernel.org/all/20230710205611.1198878-1-kuba@kernel.org/
>
> Jakub Kicinski (3):
> eth: bnxt: move and rename reset helpers
> eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work()
> eth: bnxt: handle invalid Tx completions more gracefully
>
For the series,
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
I have already done the patches to preserve the ring error counters
and it's being tested internally. I will add the tx reset counter
when I post the patches. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully
2023-07-20 1:04 [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
` (3 preceding siblings ...)
2023-07-20 6:59 ` [PATCH net-next v2 0/3] " Michael Chan
@ 2023-07-21 3:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-07-21 3:30 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, michael.chan
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 19 Jul 2023 18:04:37 -0700 you wrote:
> bnxt trusts the events generated by the device which may lead to kernel
> crashes. These are extremely rare but they do happen. For a while
> I thought crashing may be intentional, because device reporting invalid
> completions should never happen, and having a core dump could be useful
> if it does. But in practice I haven't found any clues in the core dumps,
> and panic_on_warn exists.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/3] eth: bnxt: move and rename reset helpers
https://git.kernel.org/netdev/net-next/c/fea2993aecd7
- [net-next,v2,2/3] eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work()
https://git.kernel.org/netdev/net-next/c/9b1a00fda61e
- [net-next,v2,3/3] eth: bnxt: handle invalid Tx completions more gracefully
https://git.kernel.org/netdev/net-next/c/2b56b3d99241
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-21 3:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-20 1:04 [PATCH net-next v2 0/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 1/3] eth: bnxt: move and rename reset helpers Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 2/3] eth: bnxt: take the bit to set as argument of bnxt_queue_sp_work() Jakub Kicinski
2023-07-20 1:04 ` [PATCH net-next v2 3/3] eth: bnxt: handle invalid Tx completions more gracefully Jakub Kicinski
2023-07-20 6:59 ` [PATCH net-next v2 0/3] " Michael Chan
2023-07-21 3:30 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.