netdev - Re: [PATCH iwl-next] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87ldkblyhd.fsf@intel.com>
Date: Wed, 12 Nov 2025 13:42:06 -0800
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, Tony Nguyen
 <anthony.l.nguyen@...el.com>, Przemek Kitszel
 <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>, intel-wired-lan@...ts.osuosl.org,
 netdev@...r.kernel.org, Kurt Kanzenbach <kurt@...utronix.de>
Subject: Re: [PATCH iwl-next] igc: Restore default Qbv schedule when
 changing channels

Hi,

Kurt Kanzenbach <kurt@...utronix.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@...utronix.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@...utronix.de>
>


Cheers,
-- 
Vinicius