Re: [PATCH net-next v5 3/3] net: stmmac: add tc mqprio hw queue priority offload

[Daniel Thompson <daniel@riscstar.com>]

On Mon, Oct 12, 2026 at 07:02:39PM +0200, Paolo Abeni wrote:
> On 10/8/26 17:31, Daniel Thompson wrote:
> > On Tue, Oct 06, 2026 at 12:46:27PM +0200, Lorenzo Bianconi wrote:
> > > Implement the offload of the tc mqprio hw queue priority in the stmmac
> > > driver. When the mqprio qdisc is configured in channel mode, the MTL TX
> > > scheduler is switched to strict priority, and the PSTQX/PSTC priority
> > > bitmask of each TX queue is programmed from the set of frame priorities
> > > mapped to the owning traffic class (qopt->prio_tc_map).
> > >
> > > The channel mode offload requires the DCB hw feature and a 1:1 TC to TX
> > > queue mapping, with a single queue per TC. Configurations with AVB queues
> > > are rejected, since forcing strict priority conflicts with the CBS
> > > algorithm. The CBS offload is refused while the strict-priority scheduler
> > > is active, and so is a channel count change that would enable an AVB
> > > queue.
> >
> > I don't think these CBS offload checks are necessary. It's quite reasonable
> > to mix strict priority on low priority queues with CBS on higher
> > priority queues.
> >
> > In fact on the hardware I have (TC956x) the lower priority queues don't
> > implement CBS so mixing AVB and DCB queuing (whether SP or ETS) isn't
> > just reasonable, it's pretty much unavoidable when using CBS!
> >
> > As a concrete example of combining mqprio and CBS, the following works well on
> > TC956x (when I tested an older version of your patch):
> >
> > --- cut here ---
> > tc qdisc replace dev eth0 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 mode channel hw 1
> >
> > # No CBS for TxQ1 means we can't offload this (h/ware stays in SP mode)
> > tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
> >      idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 \
> >      offload 0
> >
> > tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
> >      idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 \
> >      offload 1
> >
> > tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
> >      idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 \
> >      offload 1
> > --- cut here ---
> Given your later tested-by, I assume you are looking for future possible
> improvement?

Not sure how it appeared in the thread order but the Tested-by was not
later chronologically (at that point I had only checked mqprio
*without* CBS applied to some queues).

Direct answer:

I'm pretty sure it's *possible* that the new CBS checks could trigger
spuriously on an existing platform where the queues are statically
configured via DT. However do we *care* about these cases? I think
than can only be reached when are attempt to dynamically reconfigure
the statically configured queues. That would be unusual... and made
even less likely to happen because some parts of dynamic configuration
were broken before Lorenzo's patches anyway.

Indirect answer:

I don't know if the new checks can cause a meaningful regression but
Lorenzo has shared a patch[1] to remove the checks. Taking that patch
is probably better then debating! The new patch is late in the dev cycle
but should be low risk since AFAICT that patch only removes checks
added by this one.

[1] https://lore.kernel.org/all/20261012-stmmac-mqprio-cbs-combo-v1-1-831e323eb4ef@oss.qualcomm.com/


> Also it's not clear which revision you are referring to: AFAICS this
> patch is (attempting to) refusing CBS + SP since v1?!?

Oops. I guess I might have pulled a pre-LKML version from a git repo
and forgotten that!


Daniel.