[PATCH iwl-net v2 1/2] i40e: fix set_ringparam error path freeing live Tx rings
Aleksandr Loktionov <[email protected]>
| Newsgroups | org.osuosl.intel-wired-lan,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
The free_tx: error label in i40e_set_ringparam() is supposed to release
the temporary tx_rings[] array built before i40e_down() is called.
Instead it passes vsi->tx_rings[i] to i40e_free_tx_resources() - a
pointer to the live, NAPI-active ring - rather than &tx_rings[i].
i40e_free_tx_resources() unconditionally NULLs out ring->desc and
ring->tx_bi. Because i40e_down() has not run at this point, NAPI is
still scheduled and the next i40e_clean_tx_irq() call hits a NULL
descriptor pointer:
BUG: unable to handle page fault for address: 0000000000002000
RIP: i40e_napi_poll (i40e_txrx.c:942 i40e_txrx.c:2769)
RAX: 0000000000000000 RBX: ffff8d0a53ea9800
The trigger is ethtool -G with a TX descriptor count change (so
tx_rings[] is allocated) followed by i40e_alloc_rx_buffers() returning
failure, e.g. under memory pressure.
Pass &tx_rings[i] instead so the temporary rings' DMA descriptors and
software buffer arrays are freed, leaving the live VSI rings intact.
Reproduced on real hardware (XL710/XXV710) by forcing the Rx alloc
failure deterministically instead of relying on real memory pressure;
confirmed the box hangs without this fix and runs clean with it,
including repeated ring resizes and normal (non-error) resize paths.
Verified via git blame that commit 74608d17fe29 ("i40e: add support
for XDP_TX action") introduced the incorrect vsi->tx_rings[i] pointer
at this call site.
Fixes: 74608d17fe29 ("i40e: add support for XDP_TX action")
Cc: [email protected]
Cc: Simon Horman <[email protected]>
Signed-off-by: Aleksandr Loktionov <[email protected]>
---
v2: No functional change to the fix itself. During review, a related
but distinct pre-existing hazard was pointed out: freeing a
temporary tx_rings[] clone with i40e_free_tx_resources() also
resets BQL/dql state for the live queue it was cloned from
(i40e_clean_tx_ring() -> netdev_tx_reset_queue()), since the
clone keeps the same ring->netdev/queue_index. That hazard
predates this bug (Fixes: 9f65e15b4f98, 2013) and is also
reachable from the i40e_setup_tx_descriptors() failure path
that this patch does not touch, so it is fixed separately in
patch 2/2 of this series rather than folded in here.
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 3da9ec4..6d2b076 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2249,7 +2249,7 @@ static int i40e_set_ringparam(struct net_device *netdev,
if (tx_rings) {
for (i = 0; i < tx_alloc_queue_pairs; i++) {
if (i40e_active_tx_ring_index(vsi, i))
- i40e_free_tx_resources(vsi->tx_rings[i]);
+ i40e_free_tx_resources(&tx_rings[i]);
}
kfree(tx_rings);
tx_rings = NULL;
--
2.52.0