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

["Ruinskiy, Dima" <dima.ruinskiy@intel.com>]

On 05/10/2026 13:22, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Benoit DE RANCOURT <b2rancourt@gmail.com>
>> Sent: Sunday, October 4, 2026 5:29 PM
>> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>; intel-wired-
>> lan@lists.osuosl.org
>> Cc: netdev@vger.kernel.org; Andrew Lunn <andrew+netdev@lunn.ch>; David
>> S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@kernel.org>;
>> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
>> Gomes, Vinicius <vinicius.gomes@intel.com>; Sasha Neftin
>> <sasha.neftin@intel.com>; linux-kernel@vger.kernel.org; Benoit DE
>> RANCOURT <b2rancourt@gmail.com>
>> 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 <b2rancourt@gmail.com>
>> ---
>>   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 <aleksandr.loktionov@intel.com>

Reviewed-by: Dima Ruinskiy <dima.ruinskiy@intel.com>