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

[Jakub Kicinski <kuba@kernel.org>]

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?

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() 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?

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()?

>  }
>  
>  static int tbnet_suspend(struct device *dev)
-- 
pw-bot: cr