Subject: Re: [iwl-next v1 1/3] ice: support max_io_eqs for subfunction

From: Simon Horman <[email-masked]>
Date: Wed, 6 Nov 2024 09:53:50 +0000

On Thu, Oct 31, 2024 at 07:00:07AM +0100, Michal Swiatkowski wrote:
> Implement get and set for the maximum IO event queues for SF.
> It is used to derive the maximum number of Rx/Tx queues on subfunction
> device.
> 
> If the value isn't set when activating set it to the low default value.
> 
> Reviewed-by: Sridhar Samudrala <[email-masked]>
> Signed-off-by: Michal Swiatkowski <[email-masked]>
> ---
>  drivers/net/ethernet/intel/ice/devlink/port.c | 37 +++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice.h          |  2 +
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/devlink/port.c b/drivers/net/ethernet/intel/ice/devlink/port.c

...

> @@ -548,6 +575,14 @@ ice_activate_dynamic_port(struct ice_dynamic_port *dyn_port,
>  	if (dyn_port->active)
>  		return 0;
>  
> +	if (!dyn_port->vsi->max_io_eqs) {
> +		err = ice_devlink_port_fn_max_io_eqs_set(&dyn_port->devlink_port,
> +							 ICE_SF_DEFAULT_EQS,
> +							 extack);

Hi Michal,

I am a little confused about the relationship between this,
where ICE_SF_DEFAULT_EQS is 8, and the following check in
ice_devlink_port_fn_max_io_eqs_set().

	if (max_io_eqs > num_online_cpus()) {
		NL_SET_ERR_MSG_MOD(extack, "Supplied value out of range");
		return -EINVAL;
	}

What is the behaviour on systems with more than 8 online CPUs?

> +		if (err)
> +			return err;
> +	}
> +
>  	err = ice_sf_eth_activate(dyn_port, extack);
>  	if (err)
>  		return err;

...

> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 70d5294a558c..ca0739625d3b 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -109,6 +109,7 @@
>  #define ICE_Q_WAIT_MAX_RETRY	(5 * ICE_Q_WAIT_RETRY_LIMIT)
>  #define ICE_MAX_LG_RSS_QS	256
>  #define ICE_INVAL_Q_INDEX	0xffff
> +#define ICE_SF_DEFAULT_EQS	8
>  
>  #define ICE_MAX_RXQS_PER_TC		256	/* Used when setting VSI context per TC Rx queues */
>  

...