[v2,012/148] net/ice/base: clean up __ice_aq_get_set_rss_lut()

From: Ian Stokes <ian.stokes@intel.com>

Refactor __ice_aq_get_set_rss_lut():
get:
 - make use params->lut_size only as a size of params->lut;
 - return LUT size via params->lut_size;
set:
 - remove option to set RSS LUT smaller than available
   (eg forbid PF LUT sized 512);
both:
 - clean up code.

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ice/base/ice_adminq_cmd.h |  17 +++-
 drivers/net/ice/base/ice_common.c     | 126 ++++++++++++++------------
 drivers/net/ice/base/ice_common.h     |   1 +
 3 files changed, 82 insertions(+), 62 deletions(-)
  

On 6/12/24 17:00, Anatoly Burakov wrote:
> From: Ian Stokes <ian.stokes@intel.com>
> 
> Refactor __ice_aq_get_set_rss_lut():
> get:
>   - make use params->lut_size only as a size of params->lut;
>   - return LUT size via params->lut_size;
> set:
>   - remove option to set RSS LUT smaller than available
>     (eg forbid PF LUT sized 512);
> both:
>   - clean up code.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>

I have not much interest in DPDK, but when you CC me on code authored by
me, it would be nice to also apply proper authorship (the From: line).

> ---
>   drivers/net/ice/base/ice_adminq_cmd.h |  17 +++-
>   drivers/net/ice/base/ice_common.c     | 126 ++++++++++++++------------
>   drivers/net/ice/base/ice_common.h     |   1 +
>   3 files changed, 82 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/net/ice/base/ice_adminq_cmd.h b/drivers/net/ice/base/ice_adminq_cmd.h
> index 56c9a4813e..a6a8210dd5 100644
> --- a/drivers/net/ice/base/ice_adminq_cmd.h
> +++ b/drivers/net/ice/base/ice_adminq_cmd.h
> @@ -2225,6 +2225,19 @@ struct ice_aqc_get_set_rss_keys {
>   	u8 extended_hash_key[ICE_AQC_GET_SET_RSS_KEY_DATA_HASH_KEY_SIZE];
>   };
>   
> +enum ice_lut_type {
> +	ICE_LUT_VSI = 0,
> +	ICE_LUT_PF = 1,
> +	ICE_LUT_GLOBAL = 2,
> +	ICE_LUT_TYPE_MASK = 3
> +};
> +
> +enum ice_lut_size {
> +	ICE_LUT_VSI_SIZE = 64,
> +	ICE_LUT_GLOBAL_SIZE = 512,
> +	ICE_LUT_PF_SIZE = 2048,
> +};
> +
>   /* Get/Set RSS LUT (indirect 0x0B05/0x0B03) */
>   struct ice_aqc_get_set_rss_lut {
>   #define ICE_AQC_GSET_RSS_LUT_VSI_VALID	BIT(15)
> @@ -2233,7 +2246,7 @@ struct ice_aqc_get_set_rss_lut {
>   	__le16 vsi_id;
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S	0
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_M	\
> -				(0x3 << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S)
> +	(ICE_LUT_TYPE_MASK << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S)
>   
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_VSI	 0
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF	 1
> @@ -2241,7 +2254,7 @@ struct ice_aqc_get_set_rss_lut {
>   
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S	 2
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M	 \
> -				(0x3 << ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S)
> +	(ICE_LUT_TYPE_MASK << ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S)
>   
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128	 128
>   #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128_FLAG 0
> diff --git a/drivers/net/ice/base/ice_common.c b/drivers/net/ice/base/ice_common.c
> index 1fe9bebe7d..314d1bffb4 100644
> --- a/drivers/net/ice/base/ice_common.c
> +++ b/drivers/net/ice/base/ice_common.c
> @@ -4127,6 +4127,51 @@ ice_aq_read_topo_dev_nvm(struct ice_hw *hw,
>   	return 0;
>   }
>   
> +static u16 ice_lut_type_to_size(u16 lut_type)
> +{
> +	switch (lut_type) {
> +	case ICE_LUT_VSI:
> +		return ICE_LUT_VSI_SIZE;
> +	case ICE_LUT_GLOBAL:
> +		return ICE_LUT_GLOBAL_SIZE;
> +	case ICE_LUT_PF:
> +		return ICE_LUT_PF_SIZE;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static u16 ice_lut_size_to_flag(u16 lut_size)
> +{
> +	u16 f = 0;
> +
> +	switch (lut_size) {
> +	case ICE_LUT_GLOBAL_SIZE:
> +		f = ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512_FLAG;
> +		break;
> +	case ICE_LUT_PF_SIZE:
> +		f = ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K_FLAG;
> +		break;
> +	default:
> +		break;
> +	}
> +	return f << ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S;
> +}
> +
> +int ice_lut_size_to_type(int lut_size)
> +{
> +	switch (lut_size) {
> +	case ICE_LUT_VSI_SIZE:
> +		return ICE_LUT_VSI;
> +	case ICE_LUT_GLOBAL_SIZE:
> +		return ICE_LUT_GLOBAL;
> +	case ICE_LUT_PF_SIZE:
> +		return ICE_LUT_PF;
> +	default:
> +		return -1;
> +	}
> +}
> +
>   /**
>    * __ice_aq_get_set_rss_lut
>    * @hw: pointer to the hardware structure
> @@ -4138,7 +4183,7 @@ ice_aq_read_topo_dev_nvm(struct ice_hw *hw,
>   static int
>   __ice_aq_get_set_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params *params, bool set)
>   {
> -	u16 flags = 0, vsi_id, lut_type, lut_size, glob_lut_idx, vsi_handle;
> +	u16 flags, vsi_id, lut_type, lut_size, glob_lut_idx = 0, vsi_handle;
>   	struct ice_aqc_get_set_rss_lut *cmd_resp;
>   	struct ice_aq_desc desc;
>   	int status;
> @@ -4149,16 +4194,22 @@ __ice_aq_get_set_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params
>   
>   	vsi_handle = params->vsi_handle;
>   	lut = params->lut;
> -
> -	if (!ice_is_vsi_valid(hw, vsi_handle) || !lut)
> -		return ICE_ERR_PARAM;
> -
> -	lut_size = params->lut_size;
>   	lut_type = params->lut_type;
> -	glob_lut_idx = params->global_lut_id;
> -	vsi_id = ice_get_hw_vsi_num(hw, vsi_handle);
> -
> +	lut_size = ice_lut_type_to_size(lut_type);
>   	cmd_resp = &desc.params.get_set_rss_lut;
> +	if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_GLOBAL)
> +		glob_lut_idx = params->global_lut_id;
> +
> +	if (!lut || !lut_size || !ice_is_vsi_valid(hw, vsi_handle))
> +		return ICE_ERR_PARAM;
> +
> +	if (lut_size > params->lut_size)
> +		return ICE_ERR_INVAL_SIZE;
> +
> +	if (set && lut_size != params->lut_size)
> +		return ICE_ERR_PARAM;
> +
> +	vsi_id = ice_get_hw_vsi_num(hw, vsi_handle);
>   
>   	if (set) {
>   		ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_set_rss_lut);
> @@ -4172,61 +4223,16 @@ __ice_aq_get_set_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params
>   					ICE_AQC_GSET_RSS_LUT_VSI_ID_M) |
>   				       ICE_AQC_GSET_RSS_LUT_VSI_VALID);
>   
> -	switch (lut_type) {
> -	case ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_VSI:
> -	case ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF:
> -	case ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_GLOBAL:
> -		flags |= ((lut_type << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S) &
> -			  ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_M);
> -		break;
> -	default:
> -		status = ICE_ERR_PARAM;
> -		goto ice_aq_get_set_rss_lut_exit;
> -	}
> +	flags = ice_lut_size_to_flag(lut_size) |
> +		 ((lut_type << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S) &
> +		  ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_M) |
> +		 ((glob_lut_idx << ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_S) &
> +		  ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_M);
>   
> -	if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_GLOBAL) {
> -		flags |= ((glob_lut_idx << ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_S) &
> -			  ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_M);
> -
> -		if (!set)
> -			goto ice_aq_get_set_rss_lut_send;
> -	} else if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF) {
> -		if (!set)
> -			goto ice_aq_get_set_rss_lut_send;
> -	} else {
> -		goto ice_aq_get_set_rss_lut_send;
> -	}
> -
> -	/* LUT size is only valid for Global and PF table types */
> -	switch (lut_size) {
> -	case ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128:
> -		flags |= (ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128_FLAG <<
> -			  ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S) &
> -			 ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M;
> -		break;
> -	case ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512:
> -		flags |= (ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512_FLAG <<
> -			  ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S) &
> -			 ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M;
> -		break;
> -	case ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K:
> -		if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF) {
> -			flags |= (ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K_FLAG <<
> -				  ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S) &
> -				 ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M;
> -			break;
> -		}
> -		/* fall-through */
> -	default:
> -		status = ICE_ERR_PARAM;
> -		goto ice_aq_get_set_rss_lut_exit;
> -	}
> -
> -ice_aq_get_set_rss_lut_send:
>   	cmd_resp->flags = CPU_TO_LE16(flags);
>   	status = ice_aq_send_cmd(hw, &desc, lut, lut_size, NULL);
>   
> -ice_aq_get_set_rss_lut_exit:
> +	params->lut_size = LE16_TO_CPU(desc.datalen);
>   	return status;
>   }
>   
> diff --git a/drivers/net/ice/base/ice_common.h b/drivers/net/ice/base/ice_common.h
> index 2aeb1a520a..fe99b56f8b 100644
> --- a/drivers/net/ice/base/ice_common.h
> +++ b/drivers/net/ice/base/ice_common.h
> @@ -101,6 +101,7 @@ ice_write_tx_drbell_q_ctx(struct ice_hw *hw,
>   			  struct ice_tx_drbell_q_ctx *tx_drbell_q_ctx,
>   			  u32 tx_drbell_q_index);
>   
> +int ice_lut_size_to_type(int lut_size);
>   int
>   ice_aq_get_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params *get_params);
>   int
  

Avoid use of function names in title. This could be "improve RSS LUT
function", perhaps?

On Wed, Jun 12, 2024 at 04:00:06PM +0100, Anatoly Burakov wrote:
> From: Ian Stokes <ian.stokes@intel.com>
> 
> Refactor __ice_aq_get_set_rss_lut():
> get:
>  - make use params->lut_size only as a size of params->lut;
>  - return LUT size via params->lut_size;
> set:
>  - remove option to set RSS LUT smaller than available
>    (eg forbid PF LUT sized 512);
> both:
>  - clean up code.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> ---
  

On 6/13/2024 8:17 AM, Przemek Kitszel wrote:
> On 6/12/24 17:00, Anatoly Burakov wrote:
>> From: Ian Stokes <ian.stokes@intel.com>
>>
>> Refactor __ice_aq_get_set_rss_lut():
>> get:
>>   - make use params->lut_size only as a size of params->lut;
>>   - return LUT size via params->lut_size;
>> set:
>>   - remove option to set RSS LUT smaller than available
>>     (eg forbid PF LUT sized 512);
>> both:
>>   - clean up code.
>>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> 
> I have not much interest in DPDK, but when you CC me on code authored by
> me, it would be nice to also apply proper authorship (the From: line).
> 
Hi,

Yes, apologies for that, it's an artifact of the process we were using 
to create these patches. Will be addressed for all patches in next 
revisions.