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

[Loftus, Ciara]

>
> 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 at dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson at 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 at intel.com>