Re: [PATCH net] gve: Fix use of netif_carrier_ok()

[Simon Horman <horms@kernel.org>]

On Thu, Aug 01, 2024 at 01:56:19PM -0700, Praveen Kaligineedi wrote:
> GVE driver wrongly relies on netif_carrier_ok() to check the
> interface administrative state when resources are being
> allocated/deallocated for queue(s). netif_carrier_ok() needs
> to be replaced with netif_running() for all such cases.
> 
> Administrative state is the result of "ip link set dev <dev>
> up/down". It reflects whether the administrator wants to use
> the device for traffic and the corresponding resources have
> been allocated.
> 
> Fixes: 5f08cd3d6423 ("gve: Alloc before freeing when adjusting queues")
> Signed-off-by: Praveen Kaligineedi <pkaligineedi@google.com>
> Reviewed-by: Shailend Chand <shailend@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>

...

> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c

...

> @@ -1847,7 +1847,7 @@ int gve_adjust_queues(struct gve_priv *priv,
>  	rx_alloc_cfg.qcfg = &new_rx_config;
>  	tx_alloc_cfg.num_rings = new_tx_config.num_queues;
>  
> -	if (netif_carrier_ok(priv->dev)) {
> +	if (netif_running(priv->dev)) {
>  		err = gve_adjust_config(priv, &tx_alloc_cfg, &rx_alloc_cfg);
>  		return err;
>  	}

Hi Praveen,

Not for this patch, but I am curious to know if this check is needed at
all, because gve_adjust_queues only seems to be called from
gve_set_channels if netif_running() (previously netif_carrier_ok()) is true

...