Re: [PATCH net v4 1/4] octeon_ep: fix race conditions in ndo_get_stats64

[Jakub Kicinski]

On Thu, 2 Jan 2025 03:22:43 -0800 Shinas Rasheed wrote:
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> index 549436efc204..a452ee3b9a98 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> @@ -995,16 +995,14 @@ static void octep_get_stats64(struct net_device *netdev,
>  	struct octep_device *oct = netdev_priv(netdev);
>  	int q;
>  
> -	if (netif_running(netdev))
> -		octep_ctrl_net_get_if_stats(oct,
> -					    OCTEP_CTRL_NET_INVALID_VFID,
> -					    &oct->iface_rx_stats,
> -					    &oct->iface_tx_stats);
> -
>  	tx_packets = 0;
>  	tx_bytes = 0;
>  	rx_packets = 0;
>  	rx_bytes = 0;
> +
> +	if (!netif_running(netdev))
> +		return;

So we'll provide no stats when the device is down? That's not correct.
The driver should save the stats from the freed queues (somewhere in
the oct structure). Also please mention how this is synchronized
against netif_running() changing its state, device may get closed while
we're running..
-- 
pw-bot: cr