Re: [PATCH] net: macb: fix race condition in TX queue initialization

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 20 Feb 2026 15:27:23 +0100 Ingo Rohloff wrote:
> Subject: Re: [PATCH] net: macb: fix race condition in TX queue initialization
> Date: Thu, 19 Feb 2026 14:43:38 -0800
> 
> > > On Wed, 18 Feb 2026 11:43:25 +0100 Ingo Rohloff wrote:
> > > Commit bf9cf80cab81e ("net: macb: Fix tx/rx malfunction after phy link
> > > down and up") initializes queue->tx_head and queue->tx_tail to zero
> > > without holding queue->tx_ptr_lock.  
> 
> > That commit does look quite buggy, but I'm not sure whether the fix is
> > effective.  
> 
> Yes: I myself think that what I posted is definitely not enough.
> 
> I am not sure if I can come up with a real fix though:
> 
> Looking at ethernet drivers which use both:
>    * phylink_mac_ops->mac_link_down()
>    * net_device_ops->ndo_open()
> the most commonly used strategy seems to be:
> * When net_device_ops->ndo_open() is called:
>   - Setup ring buffers
>   - Call netif_tx_start_all_queues()
> * DO NOT call netif_tx_stop_all_queues() or similar when
>   phylink_mac_ops->mac_link_down() is called
> * DO NOT re-initialize ring buffers when
>   phylink_mac_ops->mac_link_up() is called.
> 
> So: Leave the ring buffers alone in
> phylink_mac_ops->mac_link_up() and phylink_mac_ops->mac_link_down().
> 
> The driver in macb_main.c seems to be one of the exceptions
> which calls netif_tx_stop_all_queues() in macb_mac_link_down() and
> netif_tx_wake_all_queues() in macb_mac_link_up().
> 
> My problem is:
> I don't understand why the OTHER drivers don't do it like this.
> What's the expectation of the hardware, if I fill the TX ring buffer
> with TX skb's even if the link is down?
> Is the hardware supposed to throw all these TX entries away
> and still produce completion or error(?) interrupts?
> Or is this handled, because drivers which use "phylink",
> automatically call netif_carrier_on() and netif_carrier_off() via
> "phylink" when the link changes up/down state?
> 
> 
> If you know the expected behavior:
> Could you teach me what's expected ?
> 
> If I understand the expected behavior I might be able to come up
> with an idea what the macb driver is supposed to do :-)

Before I tell you what I think - a disclosure that I have limited
experience with embedded devices, so take what I say with a grain
of salt. I mostly work on integrated NIC designs, day to day.

Normally the MAC has a "flush" bit of some sort which makes it consume
all the packets sent to the device / placed on the xmit queues.
With this mode enabled the device completes the transfers to the host
as if the send was successful, but the packets go nowhere.
This is necessary because we don't want to suddenly send a burst of
potentially extremely old packets when the link finally comes back up.

The driver is not expected to stop the Tx rings when link goes down.

I suppose macb either lacks such flush functionality or it's broken?
That'd explain why it tries to reset the ring manually.

Note that for integrated NICs the link is usually managed by FW running
in the NIC, so you won't see the driver do anything in response to the
link down.