Re: [PATCH net-next v5 07/12] gve: add gve_ctrl_ops for gve initialization/teardown sequences

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The ethtool callback `gve_set_rxfh` bypasses the `gve_ctrl_ops` abstraction by calling `gve_adminq_configure_rss` directly.
--

commit 10b1b0685ff2bac01a7188b6c0dc5c2a0e9dae42
Author: Joshua Washington <joshwash@google.com>

gve: add gve_ctrl_ops for gve initialization/teardown sequences

Driver initialization and teardown involve a number of control plane
operations that need to be defined for gve_probe to operate in both
mailbox and adminq modes.

> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index acd09e9f416c1..5ad7a8f9bbddb 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
[ ... ]
> @@ -1815,6 +1820,7 @@ static int gve_xdp(struct net_device *dev, struct netdev_bpf *xdp)
>  
>  int gve_init_rss_config(struct gve_priv *priv, u16 num_queues)
>  {
> +	const struct gve_ctrl_ops *ops = priv->ctrl_ops;
>  	struct gve_rss_config *rss_config = &priv->rss_config;
>  	struct ethtool_rxfh_param rxfh = {0};
>  	u16 i;
> @@ -1830,15 +1836,17 @@ int gve_init_rss_config(struct gve_priv *priv, u16 num_queues)
>  
>  	rxfh.hfunc = ETH_RSS_HASH_TOP;
>  
> -	return gve_adminq_configure_rss(priv, &rxfh);
> +	return ops->configure_rss(priv, &rxfh);
>  }

[Severity: Medium]
While this function is updated to use the new ops->configure_rss abstraction,
does gve_set_rxfh() in drivers/net/ethernet/google/gve/gve_ethtool.c also need
to be updated?

It appears to still call gve_adminq_configure_rss() directly:

drivers/net/ethernet/google/gve/gve_ethtool.c:gve_set_rxfh() {
    ...
	if (!priv->rss_key_size || !priv->rss_lut_size)
		return -EOPNOTSUPP;

	err = gve_adminq_configure_rss(priv, rxfh);
    ...
}

Should this also be calling ops->configure_rss() to prevent bypassing the
abstraction and potentially causing undefined behavior when alternative control
plane modes are used?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903215606.31633-1-hramamurthy@google.com?part=7