Re: [PATCH iwl-next] igc: Restore default Qbv schedule when changing channels

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Hi,

Kurt Kanzenbach <kurt@linutronix.de> writes:

> 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.
>
> 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.
>

Couple of questions:
 - Would it make sense to mark this patch as a fix?

 - What would happen if the user added a Qbv schedule (not the default
   one) and then changed the number of queues? My concern is that 'tc
   qdisc' would show the custom user schedule and the hardware would be
   "running" the default schedule, this inconsistency is not ideal. In
   any case, it would be a separate patch.


> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
>  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..e4200fcb2682ccd5b57abb0d2b8e4eb30df117df 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);
> +
>  	return err;
>  }
>  
>
> ---
> base-commit: 6fc33710cd6c55397e606eeb544bdf56ee87aae5
> change-id: 20251107-igc_mqprio_channels-2329166e898b
>
> Best regards,
> -- 
> Kurt Kanzenbach <kurt@linutronix.de>
>


Cheers,
-- 
Vinicius