> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf
> Of Kurt Kanzenbach
> Sent: Friday, November 7, 2025 2:32 PM
> To: Nguyen, Anthony L <[email protected]>; Kitszel,
> Przemyslaw <[email protected]>
> Cc: Andrew Lunn <[email protected]>; David S. Miller
> <[email protected]>; Eric Dumazet <[email protected]>; Jakub
> Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Sebastian
> Andrzej Siewior <[email protected]>; Gomes, Vinicius
> <[email protected]>; [email protected];
> [email protected]; Kurt Kanzenbach <[email protected]>
> Subject: [Intel-wired-lan] [PATCH iwl-next] igc: Restore default Qbv
> schedule when changing channels
> 
> The MQPRIO (and ETF) offload utilizes the TSN Tx mode. This mode is
> always coupled to 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.
> 
I'd recommend to explain abbreviations in the commit message:
  “Multi‑Queue Priority (MQPRIO)”
  “Earliest TxTime First (ETF)”
  “Time‑Sensitive Networking (TSN)”
  “Qbv” → “IEEE 802.1Qbv time‑aware shaper (Qbv)”

> However, the following sequence of events lead to Tx issues:
> 
>  - Boot a dual core system
>    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 amount
> of channels.
> 
> Signed-off-by: Kurt Kanzenbach <[email protected]>
> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index
> 728d7ca5338bf27c3ce50a2a497b238c38cfa338..e4200fcb2682ccd5b57abb0d2b8e
> 4eb30df117df 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7761,6 +7761,8 @@ int igc_reinit_queues(struct igc_adapter
> *adapter)
>       if (netif_running(netdev))
>               err = igc_open(netdev);
> 
> +     igc_tsn_clear_schedule(adapter);
> +
I'm afraid you need to guard the helper call on success (or when open wasn’t 
attempted)
Because call made even when igc_open() fails.
As written, igc_tsn_clear_schedule(adapter); executes unconditionally, even if 
igc_open()
returned an error (e.g., rings not fully set up, device not ready).
That could program TSN/Qbv registers while the device is in a failed/partially 
initialized state.
Isn't it?

       if (!netif_running(netdev) || !err) {
               /* Restore default IEEE 802.1Qbv schedule after queue reinit */
               igc_tsn_clear_schedule(adapter);
       }


>       return err;
>  }
> 
> 
> ---
> base-commit: 6fc33710cd6c55397e606eeb544bdf56ee87aae5
> change-id: 20251107-igc_mqprio_channels-2329166e898b
> 
> Best regards,
> --
> Kurt Kanzenbach <[email protected]>

Reply via email to