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

Abdul Rahim, Faizal
2025-02-14T09:43:31Z

On 14/2/2025 4:56 pm, Kurt Kanzenbach wrote:
> On Fri Feb 14 2025, Abdul Rahim, Faizal wrote:
>> On 14/2/2025 3:12 am, Kurt Kanzenbach wrote:
>>> On Thu Feb 13 2025, Vladimir Oltean wrote:
>>>> So, confusingly to me, it seems like one operating mode is fundamentally
>>>> different from the other, and something will have to change if both will
>>>> be made to behave the same. What will change? You say mqprio will behave
>>>> like taprio, but I think if anything, mqprio is the one which does the
>>>> right thing, in igc_tsn_tx_arb(), and taprio seems to use the default Tx
>>>> arbitration scheme?
>>>
>>> Correct. taprio is using the default scheme. mqprio configures it to
>>> what ever the user provided (in igc_tsn_tx_arb()).
>>>
>>>> I don't think I'm on the same page as you guys, because to me, it is
>>>> just odd that the P traffic classes would be the first ones with
>>>> mqprio, but the last ones with taprio.
>>>
>>> I think we are on the same page here. At the end both have to behave the
>>> same. Either by using igc_tsn_tx_arb() for taprio too or only using the
>>> default scheme for both (and thereby keeping broken_mqprio). Whatever
>>> Faizal implements I'll match the behavior with mqprio.
>>>
>>
>> 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.