All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Harshitha Ramamurthy" <hramamurthy@google.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH net-next v4 06/15] gve: introduce ctrl ops to set vectors and Qs
Date: Sat, 15 Aug 2026 02:14:37 +0000	[thread overview]
Message-ID: <20260815021438.48A091F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260814021406.3044324-7-hramamurthy@google.com>

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 <hramamurthy@google.com>

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/20260814021406.3044324-1-hramamurthy@google.com?part=6

  reply	other threads:[~2026-08-15  2:14 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14  2:13 [PATCH net-next v4 00/15] gve: AdminQ mode related refactors Harshitha Ramamurthy
2026-08-14  2:13 ` [PATCH net-next v4 01/15] gve: don't pass in unused parameter to gve_adminq_free Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:13 ` [PATCH net-next v4 02/15] gve: refactor initialization with helper functions Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:13 ` [PATCH net-next v4 03/15] gve: add a few helper functions to set device properties Harshitha Ramamurthy
2026-08-14  2:13 ` [PATCH net-next v4 04/15] gve: add struct gve_device_info to hold " Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:13 ` [PATCH net-next v4 05/15] gve: introduce control plane operations structure Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:13 ` [PATCH net-next v4 06/15] gve: introduce ctrl ops to set vectors and Qs Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot [this message]
2026-08-14  2:13 ` [PATCH net-next v4 07/15] gve: introduce gve_adminq_get_device_properties() Harshitha Ramamurthy
2026-08-14  2:13 ` [PATCH net-next v4 08/15] gve: refactor gve_init_priv for reset path Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:13 ` [PATCH net-next v4 09/15] gve: simplify reset logic Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:14 ` [PATCH net-next v4 10/15] gve: add gve_ctrl_ops for gve initialization/teardown sequences Harshitha Ramamurthy
2026-08-14  2:14 ` [PATCH net-next v4 11/15] gve: split up notify block allocation and setup paths Harshitha Ramamurthy
2026-08-14  2:14 ` [PATCH net-next v4 12/15] gve: introduce new methods to handle IRQ doorbells Harshitha Ramamurthy
2026-08-14  2:14 ` [PATCH net-next v4 13/15] gve: setup and teardown management interrupts Harshitha Ramamurthy
2026-08-14  2:14 ` [PATCH net-next v4 14/15] gve: add ctrl ops to for queue operations Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot
2026-08-14  2:14 ` [PATCH net-next v4 15/15] gve: add link status/speed ctrl ops Harshitha Ramamurthy
2026-08-15  2:14   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260815021438.48A091F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=hramamurthy@google.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.