Re: [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC

[Vladimir Oltean <vladimir.oltean@nxp.com>]

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.