Message-ID: <afa50e3a-914b-46b6-8401-0589b6099f68@linux.intel.com>
Date: Fri, 14 Feb 2025 19:20:08 +0800
From: "Abdul Rahim, Faizal" <faizal.abdul.rahim@...ux.intel.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: Kurt Kanzenbach <kurt@...utronix.de>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 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>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Simon Horman <horms@...nel.org>, Russell King <linux@...linux.org.uk>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>, Furong Xu <0x1207@...il.com>,
 Russell King <rmk+kernel@...linux.org.uk>,
 Serge Semin <fancer.lancer@...il.com>,
 Xiaolei Wang <xiaolei.wang@...driver.com>,
 Suraj Jaiswal <quic_jsuraj@...cinc.com>,
 Kory Maincent <kory.maincent@...tlin.com>, Gal Pressman <gal@...dia.com>,
 Jesper Nilsson <jesper.nilsson@...s.com>,
 Choong Yong Liang <yong.liang.choong@...ux.intel.com>,
 Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
 Vinicius Costa Gomes <vinicius.gomes@...el.com>,
 intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org
Subject: Re: [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption
 feature in IGC



On 14/2/2025 6:22 pm, Vladimir Oltean wrote:
> Faizal,
> 
> On Fri, Feb 14, 2025 at 05:43:19PM +0800, Abdul Rahim, Faizal wrote:
>>>> Hi Kurt & Vladimir,
>>>>
>>>> After reading Vladimir's reply on tc, hw queue, and socket priority mapping
>>>> for both taprio and mqprio, I agree they should follow the same priority
>>>> scheme for consistency—both in code and command usage (i.e., taprio,
>>>> mqprio, and fpe in both configurations). Since igc_tsn_tx_arb() ensures a
>>>> standard mapping of tc, socket priority, and hardware queue priority, I'll
>>>> enable taprio to use igc_tsn_tx_arb() in a separate patch submission.
>>>
>>> There's one point to consider here: igc_tsn_tx_arb() changes the mapping
>>> between priorities and Tx queues. I have no idea how many people rely on
>>> the fact that queue 0 has always the highest priority. For example, it
>>> will change the Tx behavior for schedules which open multiple traffic
>>> classes at the same time. Users may notice.
>>
>> Yeah, I was considering the impact on existing users too. I hadn’t given it
>> much thought initially and figured they’d just need to adapt to the changes,
>> but now that I think about it, properly communicating this would be tough.
>> taprio on igc (i225, i226) has been around for a while, so a lot of users
>> would be affected.
>>
>>> OTOH changing mqprio to the broken_mqprio model is easy, because AFAIK
>>> there's only one customer using this.
>>>
>>
>> Hmmmm, now I’m leaning toward keeping taprio as is (hw queue 0 highest
>> priority) and having mqprio follow the default priority scheme (aka
>> broken_mqprio). Even though it’s not the norm, the impact doesn’t seem worth
>> the gain. Open to hearing others' thoughts.
> 
> Kurt is right, you need to think about your users, but it isn't only that.
> Intel puts out a lot of user-facing TSN technical documentation for Linux,
> and currently, they have a hard time adapting it to other vendors, because
> of Intel specific peculiarities such as this one. I would argue that for
> being one of the most visible vendors from the Linux TSN space, you also
> have a duty to the rest of the community of not pushing users away from
> established conventions.
> 
> It's unfair that a past design mistake would stifle further evolution of
> the driver in the correct direction, so I don't think we should let that
> happen. I was thinking the igc driver should have a driver-specific
> opt-in flag which users explicitly have to set in order to get the
> conventional TX scheduling behavior in taprio (the one from mqprio).
> Public Intel documentation would be updated to present the differences
> between the old and the new mode, and to recommend opting into the new
> mode. By default, the current behavior is maintained, thus not breaking
> any user.  Something like an ethtool priv flag seems adequate for this.
> 
> Understandably, many network maintainers will initially dislike this,
> but you will have to be persistent and explain the ways in which having
> this priv flag is better than not having it. Normally they will respect
> those reasons more than they dislike driver-specific priv flags, which,
> let's be honest, are way too often abused for adding custom behavior.
> Here the situation is different, the custom behavior already exists, it
> just doesn't have a name and there's no way of turning it off.

Okay. I can look into this in a separate patch submission, but just an 
FYI—this adds another dependency to the second part of the igc fpe 
submission (preemptible tc on taprio + mqprio). This new patch 
(driver-specific priv flag to control 2 different priority scheme) would 
need to be accepted first before the second part of igc fpe can be submitted.

Powered by blists - more mailing lists