Re: [PATCH net-next v2 02/15] gve: refactor initialization with helper functions

[Harshitha Ramamurthy]

On Tue, Jun 2, 2026 at 4:59 PM Harshitha Ramamurthy
<hramamurthy@xxxxxxxxxx> wrote:
>
> In the interest of commonizing code, refactor gve_probe()
> and gve_init_priv() with a few helper functions that can
> be expanded and utilized in upcoming patches that add the
> mailbox ABI to the driver. The helper functions are:
>
> - gve_set_num_ntfy_blks()
> - gve_set_num_queues()
>
> Reorder code to combine lines that accomplish a similar objective
> like setting defaults. Move setting HW-GRO and UDP GSO support out
> of an Adminq method into gve_init_priv().
>
> These changes are just code movement, no functional change.
>
> Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>
> Reviewed-by: Jordan Rhee <jordanrhee@xxxxxxxxxx>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/google/gve/gve_adminq.c |   8 --
>  drivers/net/ethernet/google/gve/gve_main.c   | 108 +++++++++++--------
>  2 files changed, 65 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 70ffed8b52c3..dfeabddfb396 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -1117,14 +1117,6 @@ int gve_adminq_describe_device(struct gve_priv *priv)
>
>         gve_set_default_rss_sizes(priv);
>
> -       /* DQO supports HW-GRO and UDP_GSO */
> -       if (gve_is_dqo(priv)) {
> -               u64 additional_features = NETIF_F_GRO_HW | NETIF_F_GSO_UDP_L4;
> -
> -               priv->dev->hw_features |= additional_features;
> -               priv->dev->features |= additional_features;
> -       }
> -
>         priv->max_registered_pages =
>                                 be64_to_cpu(descriptor->max_registered_pages);
>         mtu = be16_to_cpu(descriptor->mtu);
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 30bf6df4ebc5..b9542ef36b29 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -2398,65 +2398,32 @@ static const struct xdp_metadata_ops gve_xdp_metadata_ops = {
>         .xmo_rx_timestamp       = gve_xdp_rx_timestamp,
>  };
>
> -static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
> +static int gve_set_num_ntfy_blks(struct gve_priv *priv)
>  {
>         int num_ntfy;
> -       int err;
> -
> -       /* Set up the adminq */
> -       err = gve_adminq_alloc(&priv->pdev->dev, priv);
> -       if (err) {
> -               dev_err(&priv->pdev->dev,
> -                       "Failed to alloc admin queue: err=%d\n", err);
> -               return err;
> -       }
> -
> -       err = gve_verify_driver_compatibility(priv);
> -       if (err) {
> -               dev_err(&priv->pdev->dev,
> -                       "Could not verify driver compatibility: err=%d\n", err);
> -               goto err;
> -       }
> -
> -       priv->num_registered_pages = 0;
> -
> -       if (skip_describe_device)
> -               goto setup_device;
>
> -       priv->queue_format = GVE_QUEUE_FORMAT_UNSPECIFIED;
> -       /* Get the initial information we need from the device */
> -       err = gve_adminq_describe_device(priv);
> -       if (err) {
> -               dev_err(&priv->pdev->dev,
> -                       "Could not get device information: err=%d\n", err);
> -               goto err;
> -       }
> -       priv->dev->mtu = priv->dev->max_mtu;
>         num_ntfy = pci_msix_vec_count(priv->pdev);
>         if (num_ntfy <= 0) {
>                 dev_err(&priv->pdev->dev,
>                         "could not count MSI-x vectors: err=%d\n", num_ntfy);
> -               err = num_ntfy;
> -               goto err;
> +               return num_ntfy;
>         } else if (num_ntfy < GVE_MIN_MSIX) {
>                 dev_err(&priv->pdev->dev, "gve needs at least %d MSI-x vectors, but only has %d\n",
>                         GVE_MIN_MSIX, num_ntfy);
> -               err = -EINVAL;
> -               goto err;
> +               return -EINVAL;
>         }
>
> -       /* Big TCP is only supported on DQO */
> -       if (!gve_is_gqi(priv))
> -               netif_set_tso_max_size(priv->dev, GVE_DQO_TX_MAX);
> -
> -       priv->rx_copybreak = GVE_DEFAULT_RX_COPYBREAK;
>         /* gvnic has one Notification Block per MSI-x vector, except for the
>          * management vector
>          */
>         priv->num_ntfy_blks = (num_ntfy - 1) & ~0x1;
>         priv->mgmt_msix_idx = priv->num_ntfy_blks;
> -       priv->numa_node = dev_to_node(&priv->pdev->dev);
>
> +       return 0;
> +}
> +
> +static void gve_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);
>         priv->rx_cfg.max_queues =
> @@ -2470,18 +2437,73 @@ static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
>                 priv->rx_cfg.num_queues = min_t(int, priv->default_num_queues,
>                                                 priv->rx_cfg.num_queues);
>         }
> -       priv->tx_cfg.num_xdp_queues = 0;
>
>         dev_info(&priv->pdev->dev, "TX queues %d, RX queues %d\n",
>                  priv->tx_cfg.num_queues, priv->rx_cfg.num_queues);
>         dev_info(&priv->pdev->dev, "Max TX queues %d, Max RX queues %d\n",
>                  priv->tx_cfg.max_queues, priv->rx_cfg.max_queues);
> +}
> +
> +static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
> +{
> +       int err;
> +
> +       /* Set up the adminq */
> +       err = gve_adminq_alloc(&priv->pdev->dev, priv);
> +       if (err) {
> +               dev_err(&priv->pdev->dev,
> +                       "Failed to alloc admin queue: err=%d\n", err);
> +               return err;
> +       }
> +
> +       priv->num_registered_pages = 0;
> +
> +       if (skip_describe_device)
> +               goto setup_device;
> +
> +       err = gve_verify_driver_compatibility(priv);
> +       if (err) {
> +               dev_err(&priv->pdev->dev,
> +                       "Could not verify driver compatibility: err=%d\n", err);
> +               goto err;
> +       }

Addressing Sashiko's report:
https://sashiko.dev/#/patchset/20260602235900.3050782-1-hramamurthy%40google.com

"Does moving this driver compatibility check below the early-exit bypass the
critical capability handshake during reset recovery?"

Yes, this needs to also be called in the reset path. Will fix in v3.


> +
> +       /* Get the initial information we need from the device */
> +       priv->queue_format = GVE_QUEUE_FORMAT_UNSPECIFIED;
> +       err = gve_adminq_describe_device(priv);
> +       if (err) {
> +               dev_err(&priv->pdev->dev,
> +                       "Could not get device information: err=%d\n", err);
> +               goto err;
> +       }
> +
> +       err = gve_set_num_ntfy_blks(priv);
> +       if (err) {
> +               dev_err(&priv->pdev->dev,
> +                       "Could not setup notify blocks: err=%d\n", err);
> +               goto err;
> +       }
> +
> +       gve_set_num_queues(priv);
> +
> +       if (gve_is_dqo(priv)) {
> +               /* DQO supports HW-GRO and UDP_GSO */
> +               u64 additional_features = NETIF_F_GRO_HW | NETIF_F_GSO_UDP_L4;
> +
> +               priv->dev->hw_features |= additional_features;
> +               priv->dev->features |= additional_features;
>
> -       if (!gve_is_gqi(priv)) {
>                 priv->tx_coalesce_usecs = GVE_TX_IRQ_RATELIMIT_US_DQO;
>                 priv->rx_coalesce_usecs = GVE_RX_IRQ_RATELIMIT_US_DQO;
> +
> +               /* Big TCP is only supported on DQO */
> +               netif_set_tso_max_size(priv->dev, GVE_DQO_TX_MAX);
>         }
>
> +       priv->dev->mtu = priv->dev->max_mtu;
> +       priv->numa_node = dev_to_node(&priv->pdev->dev);
> +       priv->tx_cfg.num_xdp_queues = 0;
> +       priv->rx_copybreak = GVE_DEFAULT_RX_COPYBREAK;
>         priv->ts_config.tx_type = HWTSTAMP_TX_OFF;
>         priv->ts_config.rx_filter = HWTSTAMP_FILTER_NONE;
>
> --
> 2.54.0.1013.g208068f2d8-goog
>