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

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

[Kurt Kanzenbach]

The Multi Queue Priority (MQPRIO) and Earliest TxTime First (ETF) offload
utilizes the Time Sensitive Networking (TSN) Tx mode. This 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).

 - 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

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@linutronix.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@linutronix.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..43aea9e53e1e79b304c2a7e41fa7d52dc956bffc 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 */
+	if (adapter->flags & IGC_FLAG_TSN_ANY_ENABLED)
 		return -EINVAL;
 
 	/* Verify the number of channels doesn't exceed hw limits */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 728d7ca5338bf27c3ce50a2a497b238c38cfa338..21e67e7534562ccfa7bad8769bbafc866ecbc85f 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7761,6 +7761,11 @@ int igc_reinit_queues(struct igc_adapter *adapter)
 	if (netif_running(netdev))
 		err = igc_open(netdev);
 
+	if (!err) {
+		/* Restore default IEEE 802.1Qbv schedule after queue reinit */
+		igc_tsn_clear_schedule(adapter);
+	}
+
 	return err;
 }
 

---
base-commit: ca412f25d6b2c21f69a6cf12da062e0be4a5f45e
change-id: 20251107-igc_mqprio_channels-2329166e898b

Best regards,
-- 
Kurt Kanzenbach <kurt@linutronix.de>

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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kurt Kanzenbach <kurt@linutronix.de>
> Sent: Tuesday, November 18, 2025 8:29 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Sebastian
> Andrzej Siewior <bigeasy@linutronix.de>; Gomes, Vinicius
> <vinicius.gomes@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; Kurt Kanzenbach <kurt@linutronix.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@linutronix.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@linutronix.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.

> +	if (adapter->flags & IGC_FLAG_TSN_ANY_ENABLED)
>  		return -EINVAL;
> 
>  	/* Verify the number of channels doesn't exceed hw limits */
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index
> 728d7ca5338bf27c3ce50a2a497b238c38cfa338..21e67e7534562ccfa7bad8769bba
> fc866ecbc85f 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7761,6 +7761,11 @@ int igc_reinit_queues(struct igc_adapter
> *adapter)
>  	if (netif_running(netdev))
>  		err = igc_open(netdev);
> 
> +	if (!err) {
> +		/* Restore default IEEE 802.1Qbv schedule after queue
> reinit */
> +		igc_tsn_clear_schedule(adapter);
> +	}
> +
>  	return err;
>  }
> 
> 
> ---
> base-commit: ca412f25d6b2c21f69a6cf12da062e0be4a5f45e
> change-id: 20251107-igc_mqprio_channels-2329166e898b
> 
> Best regards,
> --
> Kurt Kanzenbach <kurt@linutronix.de>


With the best regards
Alex

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

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 4253 bytes --]

On Tue Nov 18 2025, Loktionov, Aleksandr wrote:
>> -----Original Message-----
>> From: Kurt Kanzenbach <kurt@linutronix.de>
>> Sent: Tuesday, November 18, 2025 8:29 AM
>> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
>> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
>> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Sebastian
>> Andrzej Siewior <bigeasy@linutronix.de>; Gomes, Vinicius
>> <vinicius.gomes@intel.com>; Loktionov, Aleksandr
>> <aleksandr.loktionov@intel.com>; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; Kurt Kanzenbach <kurt@linutronix.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@linutronix.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@linutronix.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]