| 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
| ||
|
Message-ID: <IA3PR11MB89867DF16F7438B2DFF1D1C1E5D6A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Tue, 18 Nov 2025 07:44:32 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, "Nguyen, Anthony L"
<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
<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>, "Gomes, Vinicius" <vinicius.gomes@...el.com>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH iwl-net v2] igc: Restore default Qbv schedule when
changing channels
> -----Original Message-----
> From: Kurt Kanzenbach <kurt@...utronix.de>
> Sent: Tuesday, November 18, 2025 8:29 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Kitszel,
> Przemyslaw <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>; Gomes, Vinicius
> <vinicius.gomes@...el.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@...el.com>; intel-wired-lan@...ts.osuosl.org;
> netdev@...r.kernel.org; Kurt Kanzenbach <kurt@...utronix.de>
> 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 <kurt@...utronix.de>
> ---
> 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-
> v1-1-42415562d0f8@...utronix.de
> ---
> 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.
> + if (adapter->flags & IGC_FLAG_TSN_ANY_ENABLED)
> return -EINVAL;
>
> /* Verify the number of channels doesn't exceed hw limits */
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index
> 728d7ca5338bf27c3ce50a2a497b238c38cfa338..21e67e7534562ccfa7bad8769bba
> fc866ecbc85f 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7761,6 +7761,11 @@ int igc_reinit_queues(struct igc_adapter
> *adapter)
> if (netif_running(netdev))
> err = igc_open(netdev);
>
> + if (!err) {
> + /* Restore default IEEE 802.1Qbv schedule after queue
> reinit */
> + igc_tsn_clear_schedule(adapter);
> + }
> +
> return err;
> }
>
>
> ---
> base-commit: ca412f25d6b2c21f69a6cf12da062e0be4a5f45e
> change-id: 20251107-igc_mqprio_channels-2329166e898b
>
> Best regards,
> --
> Kurt Kanzenbach <kurt@...utronix.de>
With the best regards
Alex
Powered by blists - more mailing lists