Re: [PATCH net-next] gve: Introduce a way to disable queue formats

[Leon Romanovsky <leon@kernel.org>]

On Fri, Jan 27, 2023 at 11:07:44AM -0800, Jeroen de Borst wrote:
> The device is capable of simultaneously supporting multiple
> queue formats. With this change the driver can deliberately
> pick a queue format.
> 
> Signed-off-by: Jeroen de Borst <jeroendb@google.com>
> ---
>  drivers/net/ethernet/google/gve/gve.h         | 30 +++++++++-
>  drivers/net/ethernet/google/gve/gve_adminq.c  | 35 +++++++-----
>  drivers/net/ethernet/google/gve/gve_ethtool.c | 57 ++++++++++++-------
>  drivers/net/ethernet/google/gve/gve_main.c    | 26 ++++++++-
>  4 files changed, 110 insertions(+), 38 deletions(-)

<...>

>  	/* Interrupt coalescing settings */
>  	u32 tx_coalesce_usecs;
> @@ -609,7 +613,7 @@ enum gve_service_task_flags_bit {
>  	GVE_PRIV_FLAGS_DO_RESET			= 1,
>  	GVE_PRIV_FLAGS_RESET_IN_PROGRESS	= 2,
>  	GVE_PRIV_FLAGS_PROBE_IN_PROGRESS	= 3,
> -	GVE_PRIV_FLAGS_DO_REPORT_STATS = 4,
> +	GVE_PRIV_FLAGS_DO_REPORT_STATS		= 4,

Not relevant change.

>  };

<...>

>  static u32 gve_get_priv_flags(struct net_device *netdev)
>  {
>  	struct gve_priv *priv = netdev_priv(netdev);
> -	u32 ret_flags = 0;
> -
> -	/* Only 1 flag exists currently: report-stats (BIT(O)), so set that flag. */
> -	if (priv->ethtool_flags & BIT(0))
> -		ret_flags |= BIT(0);
> -	return ret_flags;
> +	return priv->ethtool_flags & GVE_PRIV_FLAGS_MASK;

It it possible to get ethtool_flags which has enabled bits other than
GVE_PRIV_FLAGS_MASK?

I would expect that gve_set_priv_flags() won't allow setting of such bits.

>  }
>  
>  static int gve_set_priv_flags(struct net_device *netdev, u32 flags)
>  {
>  	struct gve_priv *priv = netdev_priv(netdev);
> -	u64 ori_flags, new_flags;
> +	u32 ori_flags;
> +	enum gve_queue_format new_format;

Reversed Xmas tree, please.

> +
> +	/* Make sure there is a queue format to use */
> +	if ((priv->ethtool_formats & flags) == 0) {
> +		dev_err(&priv->pdev->dev,
> +			"All available queue formats disabled\n");
> +		return -EINVAL;
> +	}

Thanks