netdev - RE: [PATCH iwl-net v2] igc: Restore default Qbv schedule when changing channels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87ldk37j6v.fsf@jax.kurt.home>
Date: Tue, 18 Nov 2025 11:02:00 +0100
From: Kurt Kanzenbach <kurt@...utronix.de>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, "Nguyen, Anthony
 L" <anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
 <przemyslaw.kitszel@...el.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Sebastian Andrzej
 Siewior <bigeasy@...utronix.de>, "Gomes, Vinicius"
 <vinicius.gomes@...el.com>, "intel-wired-lan@...ts.osuosl.org"
 <intel-wired-lan@...ts.osuosl.org>, "netdev@...r.kernel.org"
 <netdev@...r.kernel.org>
Subject: RE: [PATCH iwl-net v2] igc: Restore default Qbv schedule when
 changing channels

On Tue Nov 18 2025, Loktionov, Aleksandr wrote:
>> -----Original Message-----
>> From: Kurt Kanzenbach <kurt@...utronix.de>
>> Sent: Tuesday, November 18, 2025 8:29 AM
>> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@...el.com>
>> Cc: Andrew Lunn <andrew+netdev@...n.ch>; David S. Miller
>> <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>; Jakub
>> Kicinski <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Sebastian
>> Andrzej Siewior <bigeasy@...utronix.de>; Gomes, Vinicius
>> <vinicius.gomes@...el.com>; Loktionov, Aleksandr
>> <aleksandr.loktionov@...el.com>; intel-wired-lan@...ts.osuosl.org;
>> netdev@...r.kernel.org; Kurt Kanzenbach <kurt@...utronix.de>
>> Subject: [PATCH iwl-net v2] igc: Restore default Qbv schedule when
>> changing channels
>> 
>> The Multi Queue Priority (MQPRIO) and Earliest TxTime First (ETF)
>> offload utilizes the Time Sensitive Networking (TSN) Tx mode. This
> With two items (“MQPRIO and ETF”), the noun phrase is plural; verb must be utilize.
> BTW kernel docs and code commonly use “Multi‑queue”  with hyphen: "The Multi-queue Priority (MQPRIO) ..."
>
> s/Multi Queue/Multi-queue/
> s/offload/offloads/
> s/utilizes/utilize/
>
>> mode is always coupled to IEEE 802.1Qbv time aware shaper (Qbv).
>> Therefore, the driver sets a default Qbv schedule of all gates opened
>> and a cycle time of 1s. This schedule is set during probe.
>> 
>> However, the following sequence of events lead to Tx issues:
>> 
>>  - Boot a dual core system
>>    igc_probe():
>>      igc_tsn_clear_schedule():
>>        -> Default Schedule is set
>>        Note: At this point the driver has allocated two Tx/Rx queues,
>> because
>>        there are only two CPU(s).
> Use standard plural: 'CPUs'
>
>> 
>>  - ethtool -L enp3s0 combined 4
>>    igc_ethtool_set_channels():
>>      igc_reinit_queues()
>>        -> Default schedule is gone, per Tx ring start and end time are
>> zero
>> 
>>   - tc qdisc replace dev enp3s0 handle 100 parent root mqprio \
>>       num_tc 4 map 3 3 2 2 0 1 1 1 3 3 3 3 3 3 3 3 \
>>       queues 1@0 1@1 1@2 1@3 hw 1
>>     igc_tsn_offload_apply():
>>       igc_tsn_enable_offload():
>>         -> Writes zeros to IGC_STQT(i) and IGC_ENDQT(i) -> Boom
> Please avoid colloquialism in commit logs. Suggest:
> "... IGC_STQT(i) and IGC_ENDQT(i), causing TX to stall/fail."
>
>
>> 
>> Therefore, restore the default Qbv schedule after changing the number
>> of channels.
>> 
>> Furthermore, add a restriction to not allow queue reconfiguration when
>> TSN/Qbv is enabled, because it may lead to inconsistent states.
>> 
>> Fixes: c814a2d2d48f ("igc: Use default cycle 'start' and 'end' values
>> for queues")
>> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
>> ---
>> Changes in v2:
>> - Explain abbreviations (Aleksandr)
>> - Only clear schedule if no error happened (Aleksandr)
>> - Add restriction to avoid inconsistent states (Vinicius)
>> - Target net tree (Vinicius)
>> - Link to v1: https://lore.kernel.org/r/20251107-igc_mqprio_channels-
>> v1-1-42415562d0f8@...utronix.de
>> ---
>>  drivers/net/ethernet/intel/igc/igc_ethtool.c | 4 ++--
>>  drivers/net/ethernet/intel/igc/igc_main.c    | 5 +++++
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> index
>> bb783042d1af9c86f18fc033fa4c3e75abb0efe8..43aea9e53e1e79b304c2a7e41fa7
>> d52dc956bffc 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> @@ -1561,8 +1561,8 @@ static int igc_ethtool_set_channels(struct
>> net_device *netdev,
>>  	if (ch->other_count != NON_Q_VECTORS)
>>  		return -EINVAL;
>> 
>> -	/* Do not allow channel reconfiguration when mqprio is enabled
>> */
>> -	if (adapter->strict_priority_enable)
>> +	/* Do not allow channel reconfiguration when any TSN Qdisc is
>> enabled */
> Kernel consistently spells the queuing discipline as “qdisc” (lowercase) in comments, Kconfig, and docs.

Sure, will update the changelog and comment in next version.

Thanks,
Kurt

Download attachment "signature.asc" of type "application/pgp-signature" (862 bytes)