On Fri Nov 07 2025, Loktionov, Aleksandr wrote: >> -----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?
igc_tsn_clear_schedule() does not write any registers. It just sets the default parameters. The actual programming is done later by igc_tsn_enable_offload(). Thanks, Kurt
signature.asc
Description: PGP signature
