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

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 17 Aug 2026 04:22:37 +0200 Lorenzo Bianconi wrote:
> > > 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.

Sounds like we don't have a clear understanding of the Intel / TBS use.
I'd rather risk introducing a regression for them, and have someone who
actually knows this system help with the fix than accumulate
incompatible workarounds.