[PATCH net-next] net: stmmac: fix .ndo_fix_features()

[Russell King (Oracle)]

On Tue, Feb 24, 2026 at 05:30:37PM -0800, Jakub Kicinski wrote:
> On Mon, 23 Feb 2026 11:24:51 +0000 Russell King (Oracle) wrote:
> > netdev features documentation requires that .ndo_fix_features() is
> > stateless: it shouldn't modify driver state. Yet, stmmac_fix_features()
> > does exactly that, changing whether GSO frames are processed by the
> > driver.
> > 
> > Move this code to stmmac_set_features() instead, which is the correct
> > place for it. We don't need to check whether TSO is supported; this
> > is already handled via the setup of netdev->hw_features, and we are
> > guaranteed that if netdev->hw_features indicates that a feature is
> > not supported, .ndo_set_features() won't be called with it set.
> 
> No lies detected, but is this enough? The whole TSO enablement 
> looks quite wobbly (as you mentioned in another email IIRC). 
> 
> Only stmmac_hw_setup() actually calls stmmac_enable_tso(). 
> And stmmac_set_features() does not call stmmac_hw_setup().

Looking deeper, dwmac4_enable_tso() has two paths depending on the "en"
flag, both of which are commented as "enable TSO".

Then, looking at the "Enable TSO" block of code in stmmac_hw_setup(),
it looks to me like we can end up with some queues that have TSO
enabled, and others which don't (because TBS has been enabled on the
queue.) As far as I'm aware, the network layer doesn't support
per-queue TSO.

We can't call stmmac_hw_setup() from any path that we care about any
state being preserved - it issues a software reset which causes
everything, including the PTP clock, to be reset and all state then
needs to be reloaded.

Lastly, we don't need to disable the TSE bit (via stmmac_enable_tso()
when we disable TSO, because there's a bit in the descriptors that
also needs to be set for the packet to undergo TSO. However, if the
system undergoes a suspend/resume with TSO disabled by the user, we'll
call stmmac_hw_setup(), which as I note in the previous paragraph will
reset all state, including the TSE bit. stmmac_enable_tso() won't be
called, and thus the TSE bit will remain clear. If the user
subsequently re-enables TSO, then we'll queue packets for TSO but
the hardware won't have that enabled...

That's a problem today. So, while you're right to point this out (I
had missed it) it's a separate problem to the one being addressed
in this patch. In fact, I think there's two new issues that have now
been identified here.

1. The lack of setting TSE on resume if TSO is supported but but has
   been disabled by the user.
2. The interaction of TBS and TSO feature where some channels can
   support TSO and others not, which I don't think can be supported.
   So, I think if we have _any_ channel with TBS enabled, we have to
   disable TSO on the entire netdev.

I think these need to be separate patches, and this one is still
valid on its own.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!