| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250213184613.cqc2zhj2wkaf5hn7@skbuf>
Date: Thu, 13 Feb 2025 20:46:13 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Kurt Kanzenbach <kurt@...utronix.de>
Cc: "Abdul Rahim, Faizal" <faizal.abdul.rahim@...ux.intel.com>,
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>,
Andrew Halaney <ahalaney@...hat.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 Thu, Feb 13, 2025 at 03:12:35PM +0100, Kurt Kanzenbach wrote:
> On Thu Feb 13 2025, Abdul Rahim, Faizal wrote:
> > On 13/2/2025 9:00 pm, Vladimir Oltean wrote:
> >> On Thu, Feb 13, 2025 at 08:54:18PM +0800, Abdul Rahim, Faizal wrote:
> >>>> Well, my idea was to move the current mqprio offload implementation from
> >>>> legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio
> >>>> can share the same code (with or without fpe). I have a draft patch
> >>>> ready for that. What do you think about it?
> >>>
> >>> Hi Kurt,
> >>>
> >>> I’m okay with including it in this series and testing fpe + mqprio, but I’m
> >>> not sure if others might be concerned about adding different functional
> >>> changes in this fpe series.
> >>>
> >>> Hi Vladimir,
> >>> Any thoughts on this ?
> >>
> >> Well, what do you think of my split proposal from here, essentially
> >> drawing the line for the first patch set at just ethtool mm?
> >> https://lore.kernel.org/netdev/20250213110653.iqy5magn27jyfnwh@skbuf/
> >>
> >
> > Honestly, after reconsidering, I’d prefer to keep the current series as is
> > (without Kurt’s patch), assuming you’re okay with enabling mqprio + fpe
> > later rather than at the same time as taprio + fpe. There likely won’t be
> > any additional work needed for mqprio + fpe after Kurt’s patch is accepted,
> > since it will mostly reuse the taprio code flow.
>
> I think so. After switching the Tx mode mqprio will basically be a
> special case of taprio with a dummy Qbv schedule. Also the driver
> currently rejects mqprio with hardware offloading and preemptible_tcs
> set. So, I do not see any issues in merging your fpe series first. I can
> handle the mqprio part afterwards.
>
> Thanks,
> Kurt
Currently, igc sets tc_taprio_caps :: broken_mqprio = true, meaning that
higher scheduling priority is given to smaller TXQ indices. This is a
special case, as normally speaking, higher scheduler priority is given
to higher traffic classes, both in mqprio and in normal taprio (see
taprio_dequeue_txq_priority() vs taprio_dequeue_tc_priority()).
In commit 9f3297511dae ("igc: Add MQPRIO offload support") you document
the intended mqprio usage pattern:
tc qdisc replace dev ${INTERFACE} handle 100 parent root mqprio num_tc 4 \
map 0 0 0 0 0 1 2 3 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 \
hw 1
Applying the transformations described in
https://man7.org/linux/man-pages/man8/tc-mqprio.8.html, it looks like this:
┌────┬────┬───────┐
│Prio│ tc │ queue │
├────┼────┼───────┤
│ 0 │ 0 │ 0 │
│ 1 │ 0 │ 0 │
│ 2 │ 0 │ 0 │
│ 3 │ 0 │ 0 │
│ 4 │ 0 │ 0 │
│ 5 │ 1 │ 1 │
│ 6 │ 2 │ 2 │
│ 7 │ 3 │ 3 │
│ 8 │ 0 │ 0 │
│ 9 │ 0 │ 0 │
│ 10 │ 0 │ 0 │
│ 11 │ 0 │ 0 │
│ 12 │ 0 │ 0 │
│ 13 │ 0 │ 0 │
│ 14 │ 0 │ 0 │
│ 15 │ 0 │ 0 │
└────┴────┴───────┘
In this model, prio 7 goes to TXQ 3, and since I assume prio 7 is a high
priority, it makes me think TXQ 3 is the highest priority queue (I don't
have a lot of spare time to search for i216 documentation and enlighten
myself).
Then we have Faizal's example from patch 7/9:
https://lore.kernel.org/netdev/20250210070207.2615418-8-faizal.abdul.rahim@linux.intel.com/
a) 1:1 TC-to-Queue Mapping
$ sudo tc qdisc replace dev enp1s0 parent root handle 100 \
taprio num_tc 4 map 3 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 \
queues 1@0 1@1 1@2 1@3 base-time 0 sched-entry S F 100000 \
fp E E P P
b) Non-1:1 TC-to-Queue Mapping
$ sudo tc qdisc replace dev enp1s0 parent root handle 100 \
taprio num_tc 3 map 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 2
queues 2@0 1@2 1@3
fp E E P
┌────┬────┬───────┐ ┌────┬────┬────────┐
│Prio│ tc │ queue │ │Prio│ tc │ queue │
├────┼────┼───────┤ ├────┼────┼────────┤
│ 0 │ 3 │ 3 │ │ 0 │ 2 │ 3 │
│ 1 │ 2 │ 2 │ │ 1 │ 1 │ 2 │
│ 2 │ 1 │ 1 │ │ 2 │ 0 │ 0 or 1 │
│ 3 │ 0 │ 0 │ │ 3 │ 2 │ 3 │
│ 4 │ 3 │ 3 │ │ 4 │ 2 │ 3 │
│ 5 │ 3 │ 3 │ │ 5 │ 2 │ 3 │
│ 6 │ 3 │ 3 │ │ 6 │ 2 │ 3 │
│ 7 │ 3 │ 3 │ │ 7 │ 2 │ 3 │
│ 8 │ 3 │ 3 │ │ 8 │ 2 │ 3 │
│ 9 │ 3 │ 3 │ │ 9 │ 2 │ 3 │
│ 10 │ 3 │ 3 │ │ 10 │ 2 │ 3 │
│ 11 │ 3 │ 3 │ │ 11 │ 2 │ 3 │
│ 12 │ 3 │ 3 │ │ 12 │ 2 │ 3 │
│ 13 │ 3 │ 3 │ │ 13 │ 2 │ 3 │
│ 14 │ 3 │ 3 │ │ 14 │ 2 │ 3 │
│ 15 │ 3 │ 3 │ │ 15 │ 2 │ 3 │
└────┴────┴───────┘ └────┴────┴────────┘
case a case b
In these cases, Faizal leaves us a hint that the preemptible traffic
classes are the ones with the lower scheduling priority (TC2 and TC3 in
case a, TC2 in case b). Here, the lower scheduling priority traffic
classes are mapped to the higher numbered TX queues, which basically
matches the broken_mqprio description.
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? 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.
Powered by blists - more mailing lists