Re: [PATCH net-next v2] net: stmmac: improve TSO/GSO queue selection

[Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>]

[-- Attachment #1: Type: text/plain, Size: 3056 bytes --]

> On Thu, 13 Aug 2026 22:36:29 +0200 Lorenzo Bianconi wrote:
> > On Aug 12, Jakub Kicinski wrote:
> > > On Wed, 12 Aug 2026 18:11:31 +0200 Lorenzo Bianconi wrote:  
> > > > please drop this version, I will post v3 to fix some pending issues.  
> > > 
> > > FTR Russell was trying to fix TSO in this driver too, before giving up
> > > (on us?). The direction he was following of clearing the TSO caps in
> > > ndo_features_check and letting the stack GSO instead of all the weird
> > > hacks this driver has seemed much more sane. But maybe I'm missing
> > > something TBS specific here   
> > 
> > thx for the pointers. I reviewed Russell's commits and I guess we have two
> > options here:
> > 
> > - manages all the TSO/GSO checks in ndo_features_check() (stmmac_features_check())
> >   and disable TSO/GSO if the selected queue does not support checksum offload
> >   or it has TBS enabled. In this case I guess we can drop ndo_select_queue()
> >   callback completely (it does not make sense to me to always use queue 0 for
> >   TSO/GSO packets, e.g. it does not allow proper mqprio offload).
> >   Please note this approach would introduce some performance regressions with
> >   respect of the previous implementation.
> > - implements TSO/GSO checks in ndo_select_queue() callback
> >   (stmmac_select_queue()) in order to keep TSO/GSO enabled if the selected
> >   queue supports it and at the same time do not always use queue 0 for TSO/GSO
> >   packets (proper qdisc offload). Please note this is patch I am proposing.
> > 
> > What do you think?
> 
> I don't know this driver, or how it's used.
> 
> From the commit message it sounds like "if the packet wants TSO and 
> the queue has a scheduler enabled - take the packet to another queue".
> Presumably you know why the TBS is enabled and whether the packet
> should or should not be going to that queue in the first place if
> you're sending the patch? What is the use case?

Reviewing the driver codebase, TBS and TSO can't be enabled at the same time
for a specific queue (TBS is not enabled via DTS but it is hardcoded in the
driver). Moreover, for some devices (e.g. dwmac-intel, dwmac-mediatek, ecc.)
TBS is enabled at bootstrap for Q1, Q2 .. Qn, (Q0 is never a TBS queue) while
for others (e.g. dwmac-stm32, dwmac-sunxi, ecc.) TBS is not enabled at all.
The current codebase forces TSO/GSO packets to Q0 (since the current driver
guarantees Q0 is a TSO queue). However this approach does not work with qdisc
offload (e.g. mqprio).
I think it is clean to just move the TSO logic in ndo_features_check() and drop
ndo_select_queue() (previous option 1) but I guess this could introduce a
performance regression with respect to the current codebase (we could disable TSO).
If we move the TSO logic in ndo_select_queue() (previous option 2), we could avoid
forcing the TSO/GSO traffic to Q0 if the selected queue supports it and
at the same time avoid any performance regression with respect to the current
driver.

Regards,
Lorenzo


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]