On Tue Nov 18 2025, Loktionov, Aleksandr wrote: >> -----Original Message----- >> From: Kurt Kanzenbach <[email protected]> >> Sent: Tuesday, November 18, 2025 8:29 AM >> 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]>; Loktionov, Aleksandr >> <[email protected]>; [email protected]; >> [email protected]; Kurt Kanzenbach <[email protected]> >> 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 <[email protected]> >> --- >> 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- >> [email protected] >> --- >> 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
signature.asc
Description: PGP signature
