Message-ID: <3e458cbe-a251-4f25-b264-6d1d441604c7@linux.intel.com>
Date: Wed, 18 Jun 2025 14:35:25 +0800
From: "Abdul Rahim, Faizal" <faizal.abdul.rahim@...ux.intel.com>
To: Vladimir Oltean <vladimir.oltean@....com>, Paolo Abeni <pabeni@...hat.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, faizal.abdul.rahim@...el.com,
 chwee.lin.choong@...el.com, horms@...nel.org, vitaly.lifshits@...el.com,
 dima.ruinskiy@...el.com, Mor Bar-Gabay <morx.bar.gabay@...el.com>,
 davem@...emloft.net, edumazet@...gle.com, andrew+netdev@...n.ch,
 netdev@...r.kernel.org, kuba@...nel.org
Subject: Re: [PATCH net-next 5/7] igc: add private flag to reverse TX queue
 priority in TSN mode


Hi Vladimir,

Thanks for your feedback.

On 17/6/2025 8:17 pm, Vladimir Oltean wrote:
> Hi Paolo,
> 
> On Tue, Jun 17, 2025 at 12:06:14PM +0200, Paolo Abeni wrote:
>> On 6/11/25 8:03 PM, Tony Nguyen wrote:
>>> To harmonize TX queue priority behavior between taprio and mqprio, and
>>> to fix these issues without breaking long-standing taprio use cases,
>>> this patch adds a new private flag, called reverse-tsn-txq-prio, to
>>> reverse the TX queue priority. It makes queue 3 the highest and queue 0
>>> the lowest, reusing the TX arbitration logic already used by mqprio.
>> Isn't the above quite the opposite of what Vladimir asked in
>> https://lore.kernel.org/all/20250214113815.37ttoor3isrt34dg@skbuf/ ?
>>
>> """
>> I would expect that for uniform behavior, you would force the users a
>> little bit to adopt the new TX scheduling mode in taprio, otherwise any
>> configuration with preemptible traffic classes would be rejected by the
>> driver.
>> """
>>
>> I don't see him commenting on later version, @Vladimir: does this fits you?
> 
> Indeed, sorry for disappearing from the patch review process.
> 
> I don't see the discrepancy between what Faizal implemented and what we
> discussed. Specifically on the bit you quoted - patch "igc: add
> preemptible queue support in taprio" refuses taprio schedules with
> preemptible TCs if the user hasn't explicitly opted into
> IGC_FLAG_TSN_REVERSE_TXQ_PRIO. If that private flag isn't set,
> everything works as currently documented, just the new features are
> gated.
> 
> The name of the private flag is debatable IMHO, because it's taprio
> specific and the name doesn't reflect that (mqprio uses the "reverse"
> priority assignment to TX queues by default, and this flag doesn't
> change that). Also, "reverse" compared to what? Both operating modes can
Compared to the default Tx queue priority in TSN mode, where Tx q0 has the 
highest priority and q3 the lowest. My thinking behind the naming was based 
on how the relevant register fields are configured.

Snippet of i226 documentation:
"While in TSN mode each transmit queue is assigned a priority level by the 
TxQ_Priority fields in the TxARB register"
"TxQ_Priority_0: The transmit queue that is assigned as priority 0 (highest 
priority). Default is queue 0"
"TxQ_Priority_3: The transmit queue that is assigned as priority 3 (lowest 
priority). Default is queue 3."

> equally be named "reverse". 
True —  mqprio already uses the reverse mapping by default even without 
this private flag, which could feel inconsistent.

> Maybe "taprio-standard-txq-priority" would
> have been clearer regarding what the flag really does.
I’m okay with that suggestion, though I’m not sure it makes things clearer. 
What’s considered “standard” may depend on context — for IGC users who 
haven’t worked with other NICs, the existing default priority mapping might 
feel like the standard. I’m definitely in favor of improving readability 
and maintenance, but I’m still unsure what name best reflects that balance.

Powered by blists - more mailing lists