Re: [PATCH 1/2] net: thunderbolt: Call tbnet_stop() upon shutdown

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Tue, Apr 28, 2026 at 06:12:24PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: thunderbolt: Call tbnet_stop() upon shutdown
>     
> This commit updates the shutdown callback for the Thunderbolt network service
> driver to call tbnet_stop() if the interface is running. This ensures the
> service drivers release the rings upon shutdown.
> 
> > diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> > index 7aae5d915a1e..631af237998d 100644
> > --- a/drivers/net/thunderbolt/main.c
> > +++ b/drivers/net/thunderbolt/main.c
> > @@ -1417,7 +1417,10 @@ static void tbnet_remove(struct tb_service *svc)
> >  
> >  static void tbnet_shutdown(struct tb_service *svc)
> >  {
> > -	tbnet_tear_down(tb_service_get_drvdata(svc), true);
> > +	struct tbnet *net = tb_service_get_drvdata(svc);
> > +
> > +	if (netif_running(net->dev))
> > +		tbnet_stop(net->dev);
> 
> Does calling tbnet_stop() directly bypass the network stack teardown sequence?

This is called when the machine is rebooted or shut down to quiesce the
device. I don't think the network stack has anything similar?

> tbnet_stop() appears to be the driver's ndo_stop callback. By bypassing
> dev_close(), netif_tx_disable() is not called and the device's transmit queues
> remain active.

tbnet_stop() calls tbnet_tear_down() that calls netif_stop_queue() which
should stop the queue.

> tbnet_stop() destroys the DMA rings and explicitly sets net->tx_ring.ring
> to NULL:
> 
> drivers/net/thunderbolt/main.c:tbnet_stop() {
>     ...
>     tb_ring_free(net->tx_ring.ring);
>     net->tx_ring.ring = NULL;
>     ...
> }
> 
> If a concurrent transmission occurs, will tbnet_start_xmit() access the null
> pointer and cause a crash?

It should not happen see above.

> Additionally, since this does not hold the rtnl_lock(), could this race
> with concurrent administrative state changes (like 'ip link set dev down')
> and lead to a double-execution of tbnet_stop()?

As far as I can tell no because when these hooks are called userspace is
not running anymore (we are rebooting or shutting down).