Re: [PATCH net-next v4 06/15] gve: introduce ctrl ops to set vectors and Qs

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Refactoring of device info leaves `priv->default_num_queues` uninitialized, bypassing the device's suggested queue limits.
--

--- Patch [6]: [PATCH net-next v4 06/15] gve: introduce ctrl ops to set vectors and Qs ---
commit 69a42c3a205dae467269f4679a52b92e85d73da9
Author: Harshitha Ramamurthy <[email protected]>

gve: introduce ctrl ops to set vectors and Qs

This patch introduces control operations for Adminq mode by attaching the
adminq_ prefix to the existing functions placed in the gve_adminq.c file.
It adds set_num_queues and set_num_ntfy_blks to the control operations to
set maximum and default TX/RX queues in the initialization flow.

> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index fc5482215588a..d0b7ce8ff4c5f 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -1616,7 +1616,7 @@ int gve_set_num_ntfy_blks(struct gve_priv *priv)
>  	return 0;
>  }
>  
> -void gve_set_num_queues(struct gve_priv *priv)
> +void gve_adminq_set_num_queues(struct gve_priv *priv)
>  {
>  	priv->tx_cfg.max_queues =
>  		min_t(int, priv->tx_cfg.max_queues, priv->num_ntfy_blks / 2);

[Severity: Medium]
Are we certain priv->default_num_queues is properly initialized before being
used further down in this function?

Looking at device probe initialization, it appears a prior refactoring parses
the device descriptor into priv->device_info.default_tx_queues via
gve_fill_device_info(), but the explicit assignment to
priv->default_num_queues was removed:

drivers/net/ethernet/google/gve/gve_adminq.c:gve_fill_device_info() {
	...
	default_num_queues = be16_to_cpu(descriptor->default_num_queues);
	device_info->default_tx_queues = default_num_queues;
	device_info->default_rx_queues = default_num_queues;
	...
}

Because priv->default_num_queues evaluates to 0, does this check fail in
gve_adminq_set_num_queues():

drivers/net/ethernet/google/gve/gve_adminq.c:gve_adminq_set_num_queues() {
	...
	if (priv->default_num_queues > 0) {
		priv->tx_cfg.num_queues = min_t(int, priv->default_num_queues,
						priv->tx_cfg.num_queues);
	...
}

Would this unconditionally allocate the absolute maximum number of queues
available (up to priv->num_ntfy_blks / 2) and completely bypass the default
limits suggested by the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6