Thread (5 messages) 5 messages, 2 authors, 3d ago

RE: [PATCH iwl-net 1/2] igc: Fix Tx stop threshold to cover empty frame descriptors

flat view

From: Loktionov, Aleksandr <hidden>
Date: 2026-10-05 10:24:56
Also in: intel-wired-lan, lkml

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Benoit DE RANCOURT <redacted>
Sent: Sunday, October 4, 2026 5:29 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
Przemyslaw [off-list ref]; intel-wired-
lan@lists.osuosl.org
Cc: netdev@vger.kernel.org; Andrew Lunn <andrew+netdev@lunn.ch>; David
S. Miller [off-list ref]; Eric Dumazet [off-list ref];
Jakub Kicinski [off-list ref]; Paolo Abeni [off-list ref];
Gomes, Vinicius [off-list ref]; Sasha Neftin
[off-list ref]; linux-kernel@vger.kernel.org; Benoit DE
RANCOURT [off-list ref]
Subject: [PATCH iwl-net 1/2] igc: Fix Tx stop threshold to cover empty
frame descriptors

Commit db0b124f02ba ("igc: Enhance Qbv scheduling by using first flag
bit") raised the number of free descriptors that igc_xmit_frame_ring()
requires before mapping a packet from count + 3 to count + 5, to leave
room for the empty frame (one context and one data descriptor) that
may be inserted ahead of a launch time packet. DESC_NEEDED was left at
MAX_SKB_FRAGS + 4. igc_tx_map() uses it to stop the queue in advance,
and TX_WAKE_THRESHOLD is derived from it.

A queue left running after a transmit therefore only guarantees
MAX_SKB_FRAGS + 4 free descriptors, while the next skb with a linear
part and MAX_SKB_FRAGS fragments, each fitting in one data descriptor,
requires MAX_SKB_FRAGS + 6. igc_xmit_frame_ring() then returns
NETDEV_TX_BUSY, which the queue stop logic is meant to prevent.

With TSO this happens routinely. On an I226-V (8086:125c) running
7.2.8 with MAX_SKB_FRAGS = 17, routed and locally generated TCP
traffic on a CPU-saturated router produced 10890 NETDEV_TX_BUSY
returns in three runs of 180 seconds, all for TSO skbs with 16 or 17
fragments, with
21 or 22 free descriptors in all but 5 cases. Together with deferred
tail writes (xmit_more), these returns led to 25 Tx timeouts and
adapter resets in the same runs:

  igc 0000:08:00.0 terra: NETDEV WATCHDOG: CPU: 1: transmit queue 2
timed out 5353 ms
  igc 0000:08:00.0 terra: Reset adapter

The following patch addresses the lost tail write itself.

Raise DESC_NEEDED to MAX_SKB_FRAGS + 6 to match the admission check.
The additional reservation is currently unconditional, including when
launch time is disabled; match that existing admission policy in the
proactive stop threshold. With MAX_SKB_FRAGS = 17, the queue is now
stopped below 23 free descriptors instead of 21. This also raises the
completion-based wake threshold from 42 to 46 free descriptors,
preserving the existing two-times-stop-threshold policy. Throughput
and latency effects were not measured.

With this change alone, three runs of the same test on 7.2.8 produced
no NETDEV_TX_BUSY and no Tx timeout over 8.8M NETDEV_TX_OK returns,
including 613k TSO skbs with 17 fragments.

As before commit db0b124f02ba, an skb whose linear part or fragments
exceed IGC_MAX_DATA_PER_TXD may still need more descriptors than
DESC_NEEDED accounts for.

Fixes: db0b124f02ba ("igc: Enhance Qbv scheduling by using first flag
bit")
Cc: stable@vger.kernel.org
Assisted-by: LLM bpftrace
Signed-off-by: Benoit DE RANCOURT <redacted>
---
 drivers/net/ethernet/intel/igc/igc.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc.h
b/drivers/net/ethernet/intel/igc/igc.h
index 17f213cc93e4..f1efadbd9f74 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -575,9 +575,13 @@ enum igc_boards {
 #define IGC_MAX_TXD_PWR		15
 #define IGC_MAX_DATA_PER_TXD	BIT(IGC_MAX_TXD_PWR)

-/* Tx Descriptors needed, worst case */
 #define TXD_USE_COUNT(S)	DIV_ROUND_UP((S), IGC_MAX_DATA_PER_TXD)
-#define DESC_NEEDED	(MAX_SKB_FRAGS + 4)
+
+/* Tx descriptor budget for a head and MAX_SKB_FRAGS fragments,
+ * each fitting in one data descriptor: one context descriptor,
+ * two descriptors for an optional empty frame, and two spare
entries.
+ */
+#define DESC_NEEDED	(MAX_SKB_FRAGS + 6)

 struct igc_rx_buffer {
 	union {

base-commit: a83267db14681b3be481e02a4d5a38177507006c
--
2.55.0
Reviewed-by: Aleksandr Loktionov <redacted>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help