RE: [PATCH] net/ice: fix VLAN tag reporting on Rx

["Loftus, Ciara" <ciara.loftus@intel.com>]

> 
> The ice driver expects the first, or outer, VLAN tag in a packet to be
> written to the L2TAG1 field of the descriptor, as configured by the
> l2tsel field when configuring the queue context initially for the
> device. However, when configuring the actual VLAN or QinQ strip
> behaviour, that l2tsel field was changed, sending the single/outer vlan
> tag to the L2TAG2 field in the descriptor. This meant that it was not
> getting picked up correctly by the Rx paths.
> 
> This issue has been around for a long time, but was previously
> partially hidden by the issue fixed in [1], since due to that bug,
> the l2tsel field was not getting overridden in the single-queue case
> (since the single queue was the final queue).
> 
> Fix the issue by just removing the code updating the l2tsel field, and
> leave it as set by default in the initial queue configuration.
> 
> [1] https://github.com/DPDK/dpdk/commit/4cd8c72f6
> 
> Fixes: de5da9d16430 ("net/ice: support double VLAN")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c | 75 ++----------------------------
>  1 file changed, 3 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/net/intel/ice/ice_ethdev.c
> b/drivers/net/intel/ice/ice_ethdev.c
> index 513777e372..4a6e580628 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -4965,49 +4965,12 @@ ice_vsi_config_vlan_stripping(struct ice_vsi *vsi,
> bool ena)
>  	return ret;
>  }
> 
> -/**
> - * ice_vsi_update_l2tsel - update l2tsel field for all Rx rings on this VSI
> - * @vsi: VSI used to update l2tsel on
> - * @l2tsel: l2tsel setting requested
> - *
> - * Use the l2tsel setting to update all of the Rx queue context bits for l2tsel.
> - * This will modify which descriptor field the first offloaded VLAN will be
> - * stripped into.
> - */
> -static void ice_vsi_update_l2tsel(struct ice_vsi *vsi, enum ice_l2tsel l2tsel)
> -{
> -	struct ice_hw *hw = ICE_VSI_TO_HW(vsi);
> -	struct ice_pf *pf = ICE_VSI_TO_PF(vsi);
> -	struct rte_eth_dev_data *dev_data = pf->dev_data;
> -	u32 l2tsel_bit;
> -	uint16_t i;
> -
> -	if (l2tsel == ICE_L2TSEL_EXTRACT_FIRST_TAG_L2TAG2_2ND)
> -		l2tsel_bit = 0;
> -	else
> -		l2tsel_bit = BIT(ICE_L2TSEL_BIT_OFFSET);
> -
> -	for (i = 0; i < dev_data->nb_rx_queues; i++) {
> -		const struct ci_rx_queue *rxq = dev_data->rx_queues[i];
> -		u32 qrx_context_offset;
> -		u32 regval;
> -
> -		qrx_context_offset =
> QRX_CONTEXT(ICE_L2TSEL_QRX_CONTEXT_REG_IDX, rxq->reg_idx);
> -
> -		regval = rd32(hw, qrx_context_offset);
> -		regval &= ~BIT(ICE_L2TSEL_BIT_OFFSET);
> -		regval |= l2tsel_bit;
> -		wr32(hw, qrx_context_offset, regval);
> -	}
> -}
> -
>  /* Configure outer vlan stripping on or off in QinQ mode */
>  static int
>  ice_vsi_config_outer_vlan_stripping(struct ice_vsi *vsi, bool on)
>  {
>  	uint16_t outer_ethertype = vsi->adapter->pf.outer_ethertype;
>  	struct ice_hw *hw = ICE_VSI_TO_HW(vsi);
> -	int err = 0;
> 
>  	if (vsi->vsi_id >= ICE_MAX_NUM_VSIS) {
>  		PMD_DRV_LOG(ERR, "VSI ID exceeds the maximum");
> @@ -5019,41 +4982,9 @@ ice_vsi_config_outer_vlan_stripping(struct ice_vsi
> *vsi, bool on)
>  		return -EOPNOTSUPP;
>  	}
> 
> -	if (on) {
> -		err = ice_vsi_ena_outer_stripping(vsi, outer_ethertype);
> -		if (!err) {
> -			enum ice_l2tsel l2tsel =
> -
> 	ICE_L2TSEL_EXTRACT_FIRST_TAG_L2TAG2_2ND;
> -
> -			/* PF tells the VF that the outer VLAN tag is always
> -			 * extracted to
> VIRTCHNL_VLAN_TAG_LOCATION_L2TAG2_2 and
> -			 * inner is always extracted to
> -			 * VIRTCHNL_VLAN_TAG_LOCATION_L2TAG1. This is
> needed to
> -			 * support outer stripping so the first tag always ends
> -			 * up in L2TAG2_2ND and the second/inner tag, if
> -			 * enabled, is extracted in L2TAG1.
> -			 */
> -			ice_vsi_update_l2tsel(vsi, l2tsel);
> -		}
> -	} else {
> -		err = ice_vsi_dis_outer_stripping(vsi);
> -		if (!err) {
> -			enum ice_l2tsel l2tsel =
> -				ICE_L2TSEL_EXTRACT_FIRST_TAG_L2TAG1;
> -
> -			/* PF tells the VF that the outer VLAN tag is always
> -			 * extracted to
> VIRTCHNL_VLAN_TAG_LOCATION_L2TAG2_2 and
> -			 * inner is always extracted to
> -			 * VIRTCHNL_VLAN_TAG_LOCATION_L2TAG1. This is
> needed to
> -			 * support inner stripping while outer stripping is
> -			 * disabled so that the first and only tag is extracted
> -			 * in L2TAG1.
> -			 */
> -			ice_vsi_update_l2tsel(vsi, l2tsel);
> -		}
> -	}
> -
> -	return err;
> +	return on ?
> +		ice_vsi_ena_outer_stripping(vsi, outer_ethertype) :
> +		ice_vsi_dis_outer_stripping(vsi);
>  }
> 
>  static int
> --
> 2.48.1

You can also remove the following #defines and enum related to the code you have removed as they are not used anywhere else as far as I can see.

#define ICE_L2TSEL_QRX_CONTEXT_REG_IDX	3
#define ICE_L2TSEL_BIT_OFFSET		   23
enum ice_l2tsel {
	ICE_L2TSEL_EXTRACT_FIRST_TAG_L2TAG2_2ND,
	ICE_L2TSEL_EXTRACT_FIRST_TAG_L2TAG1,
};

Acked-by: Ciara Loftus <ciara.loftus@intel.com>