intel-wired-lan.osuosl.org archive mirror
 help / color / mirror / Atom feed
From: Kurt Kanzenbach <kurt@linutronix.de>
To: Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-next] igc: Restore default Qbv schedule when changing channels
Date: Fri, 14 Nov 2025 10:01:04 +0100	[thread overview]
Message-ID: <874iqxng33.fsf@jax.kurt.home> (raw)
In-Reply-To: <871pm126fv.fsf@intel.com>

[-- Attachment #1: Type: text/plain, Size: 2590 bytes --]

On Thu Nov 13 2025, Vinicius Costa Gomes wrote:
> Kurt Kanzenbach <kurt@linutronix.de> writes:
>
>> On Wed Nov 12 2025, Vinicius Costa Gomes wrote:
>>> 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?
>>
>> This only happens if a user uses ETF or MQPRIO and a dual/single core
>> system. So I didn't see the need to mark it as a fix.
>>
>
> I still think this is fix material. People can always run stuff in VMs,
> and it makes it easier to have single/dual core systems.

Fair enough.

>
>>>
>>>  - 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.
>>
>> Excellent point. Honestly I'm not sure what to expect when changing the
>> number of queues after a user Qbv schedule is added. For MQPRIO we added
>> a restriction [1] especially for that case. I'm leaning towards the same
>> solution here. What do you think?
>
> Sounds great. Avoiding getting into inconsistent states is better than
> trying to fix it later.

Jup. I'll wait a bit for further comments and then send a v2 with your
and Aleksandr's suggestions addressed.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

      reply	other threads:[~2025-11-14  9:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-07 13:31 [Intel-wired-lan] [PATCH iwl-next] igc: Restore default Qbv schedule when changing channels Kurt Kanzenbach
2025-11-07 15:22 ` Loktionov, Aleksandr
2025-11-10  7:43   ` Kurt Kanzenbach
2025-11-12 21:42 ` Vinicius Costa Gomes
2025-11-13  8:50   ` Kurt Kanzenbach
2025-11-13 17:22     ` Vinicius Costa Gomes
2025-11-14  9:01       ` Kurt Kanzenbach [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874iqxng33.fsf@jax.kurt.home \
    --to=kurt@linutronix.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=vinicius.gomes@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).