Subject: Re: [PATCH iwl-next v3 06/13] ice: support turning on/off the parser's double vlan mode

From: Simon Horman <[email-masked]>
Date: Mon, 22 Jul 2024 16:02:34 +0100

On Wed, Jul 10, 2024 at 02:40:08PM -0600, Ahmed Zaki wrote:
> From: Junfeng Guo <[email-masked]>
> 
> Add API ice_parser_dvm_set() to support turning on/off the parser's double
> vlan mode.
> 
> Reviewed-by: Marcin Szycik <[email-masked]>
> Signed-off-by: Qi Zhang <[email-masked]>
> Signed-off-by: Junfeng Guo <[email-masked]>
> Co-developed-by: Ahmed Zaki <[email-masked]>
> Signed-off-by: Ahmed Zaki <[email-masked]>
> ---
>  drivers/net/ethernet/intel/ice/ice_parser.c | 78 ++++++++++++++++++++-
>  drivers/net/ethernet/intel/ice/ice_parser.h | 18 +++++
>  2 files changed, 93 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_parser.c b/drivers/net/ethernet/intel/ice/ice_parser.c

...

>  static void ice_parse_lbl_item(struct ice_hw *hw, u16 idx, void *item,
> -			       void *data, int size)
> +			       void *data, int __maybe_unused size)
>  {
> -	memcpy(item, data, size);
> +	struct ice_lbl_item *lbl_item = (struct ice_lbl_item *)item;
> +	struct ice_lbl_item *lbl_data = (struct ice_lbl_item *)data;

nit: Explicitly casting void * is unnecessary.

> +
> +	lbl_item->idx = lbl_data->idx;
> +	memcpy(lbl_item->label, lbl_data->label, sizeof(lbl_item->label));
> +
> +	if (strstarts(lbl_item->label, ICE_LBL_BST_DVM))
> +		lbl_item->type = ICE_LBL_BST_TYPE_DVM;
> +	else if (strstarts(lbl_item->label, ICE_LBL_BST_SVM))
> +		lbl_item->type = ICE_LBL_BST_TYPE_SVM;
>  
>  	if (hw->debug_mask & ICE_DBG_PARSER)
> -		ice_lbl_dump(hw, (struct ice_lbl_item *)item);
> +		ice_lbl_dump(hw, lbl_item);
>  }

...

> +static void ice_bst_dvm_set(struct ice_parser *psr, enum ice_lbl_type type,
> +			    bool on)
> +{
> +	u16 i = 0;
> +
> +	while (true) {
> +		struct ice_bst_tcam_item *item;
> +		u8 key;
> +
> +		item = ice_bst_tcam_search(psr->bst_tcam_table,
> +					   psr->bst_lbl_table,
> +					   type, &i);
> +		if (!item)
> +			break;
> +
> +		key = (on ? ICE_BT_VLD_KEY : ICE_BT_INV_KEY);

nit: these parentheses seem unnecessary

> +		item->key[ICE_BT_VM_OFF] = key;
> +		item->key_inv[ICE_BT_VM_OFF] = key;
> +		i++;
> +	}
> +}

...