RE: [PATCH v2] net/iavf: fix VLAN offload strip flag

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

> 
> Thanks.
> 
> Just wanted to know  - what are the next steps of the review and approval
> process?
> 
> There is a test suite "ci/intel-Testing" failing now with v2 which passed in v1. (
> We know for sure, there is absolutely no code change between v1 and v2
> except the commit message). Is there a way to trigger the test suite again?

Hi Amiya,

There is no further action required on your end since the failure seems like an issue with the test infrastructure. If the maintainer for the intel next-net tree is happy with the patch they will pull it into the tree.

Thanks,
Ciara

> 
> Thanks
> Amiya
> 
> 
> On Tue, 24 Jun 2025 at 14:49, Loftus, Ciara <mailto:ciara.loftus@intel.com>
> wrote:
> > Subject: Re: [PATCH v2] net/iavf: fix VLAN offload strip flag
> >
> > Hi Ciara
> > Thanks for your effort in reproducing the issue and confirming that the patch
> > works. However, I have taken care of the indentation in the commit message
> > and sent out a v2 patch. Appreciate your review comments.
> >
> > >>>Perhaps we should make the disabling unconditional or even better make
> it
> > depend on if the stripping was enabled although I'm not sure if there's a way
> > to check for this.
> >
> > I understand and agree with your point of disabling vlan_strip after checking
> if
> > the stripping is enabled. But like you mentioned, I'm also not sure if there is
> > any way to know that.
> >
> > However, I think, the current check also does a good job by checking the
> > dev_conf parameter against RTE_ETH_RX_OFFLOAD_VLAN_STRIP and re-
> > disables the vlan_strip after every vlan_add operation.
> 
> Thanks for the v2.
> I think this approach is fine for now, it matches what was already in place for
> VIRTCHNL_VF_OFFLOAD_VLAN(V1).
> A future improvement would be to find a way to determine if the stripping
> was re-enabled and only attempt to disable in that case.
> 
> >
> >
> > Thanks
> > Amiya
> >
> >
> > On Mon, 23 Jun 2025 at 23:41, Amiya Ranjan Mohakud
> > <mailto:mailto:amiyaranjan.mohakud@gmail.com> wrote:
> > For i40e kernel drivers which support either vlan(v1) or vlan(v2)
> > VIRTCHNL OP,it will set strip on when setting filter on. But dpdk
> > side will not change strip flag. To be consistent with dpdk side,
> > explicitly disable strip again.
> >
> > Bugzilla ID:1725
> > Cc: mailto:mailto:stable@dpdk.org
> >
> > v2:
> > - Fixed indentation in commit message
> >
> > Signed-off-by: Amiya Ranjan Mohakud
> > <mailto:mailto:amiyaranjan.mohakud@gmail.com>
> > ---
> >  drivers/net/intel/iavf/iavf_ethdev.c | 48 +++++++++++++++++-----------
> >  1 file changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> > b/drivers/net/intel/iavf/iavf_ethdev.c
> > index b3dacbef84..f93e7bf9ae 100644
> > --- a/drivers/net/intel/iavf/iavf_ethdev.c
> > +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> > @@ -1378,13 +1378,38 @@ iavf_dev_del_mac_addr(struct rte_eth_dev
> > *dev, uint32_t index)
> >         vf->mac_num--;
> >  }
> >
> > +static int
> > +iavf_disable_vlan_strip_ex(struct rte_eth_dev *dev, int on)
> > +{
> > +       /* For i40e kernel drivers which supports both vlan(v1 & v2) VIRTCHNL
> > OP,
> > +        * it will set strip on when setting filter on but dpdk side will not
> > +        * change strip flag. To be consistent with dpdk side, explicitly disable
> > +        * strip again.
> > +        *
> > +        */
> > +       struct iavf_adapter *adapter =
> > +               IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > +       struct rte_eth_conf *dev_conf = &dev->data->dev_conf;
> > +       int err;
> > +
> > +       if (adapter->hw.mac.type == IAVF_MAC_XL710 ||
> > +           adapter->hw.mac.type == IAVF_MAC_VF ||
> > +           adapter->hw.mac.type == IAVF_MAC_X722_VF) {
> > +               if (on && !(dev_conf->rxmode.offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_STRIP)) {
> > +                       err = iavf_disable_vlan_strip(adapter);
> > +                       if (err)
> > +                               return -EIO;
> > +               }
> > +       }
> > +       return 0;
> > +}
> > +
> >  static int
> >  iavf_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
> >  {
> >         struct iavf_adapter *adapter =
> >                 IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> >         struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
> > -       struct rte_eth_conf *dev_conf = &dev->data->dev_conf;
> >         int err;
> >
> >         if (adapter->closed)
> > @@ -1394,7 +1419,8 @@ iavf_dev_vlan_filter_set(struct rte_eth_dev *dev,
> > uint16_t vlan_id, int on)
> >                 err = iavf_add_del_vlan_v2(adapter, vlan_id, on);
> >                 if (err)
> >                         return -EIO;
> > -               return 0;
> > +
> > +               return iavf_disable_vlan_strip_ex(dev, on);
> >         }
> >
> >         if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN))
> > @@ -1404,23 +1430,7 @@ iavf_dev_vlan_filter_set(struct rte_eth_dev
> *dev,
> > uint16_t vlan_id, int on)
> >         if (err)
> >                 return -EIO;
> >
> > -       /* For i40e kernel driver which only supports vlan(v1) VIRTCHNL OP,
> > -        * it will set strip on when setting filter on but dpdk side will not
> > -        * change strip flag. To be consistent with dpdk side, disable strip
> > -        * again.
> > -        *
> > -        * For i40e kernel driver which supports vlan v2, dpdk will invoke vlan v2
> > -        * related function, so it won't go through here.
> > -        */
> > -       if (adapter->hw.mac.type == IAVF_MAC_XL710 ||
> > -           adapter->hw.mac.type == IAVF_MAC_X722_VF) {
> > -               if (on && !(dev_conf->rxmode.offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_STRIP)) {
> > -                       err = iavf_disable_vlan_strip(adapter);
> > -                       if (err)
> > -                               return -EIO;
> > -               }
> > -       }
> > -       return 0;
> > +       return iavf_disable_vlan_strip_ex(dev, on);
> >  }
> >
> >  static void
> > --
> > 2.39.5 (Apple Git-154)