Re: [PATCH net-next v2] net: stmmac: improve TSO/GSO queue selection
Jakub Kicinski <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.infradead.lists.linux-arm-kernel |
|---|---|
| Message-ID | <[email protected]> |
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.