>
> 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: sta...@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richard...@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.lof...@intel.com>
