On Thu, Aug 07, 2025 at 02:12:37PM +0100, Loftus, Ciara wrote:
> >
> > 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)
> > -{
<snip
> > -
> > - return err;
> > + return on ?
> > + ice_vsi_ena_outer_stripping(vsi, outer_ethertype) :
v> > + 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>
>
Sure. We can always reintroduce those defines again if it proves necessary
in future, but for now, I'm happy enough to remove all unused code! I'll do
up a new revision.
/Bruce
