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

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

[Amiya Ranjan Mohakud]

 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: stable@dpdk.org

Signed-off-by: Amiya Ranjan Mohakud <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)

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

[Loftus, Ciara]

> Subject: [PATCH] net/iavf: fix VLAN offload strip flag
> 
>  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: stable@dpdk.org
> 
> Signed-off-by: Amiya Ranjan Mohakud <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);
>  }

Hi,

Thanks for the patch. I reproduced the issue it aims to resolve and confirm the patch resolves it.
I noticed when testing that even if the vf command in the iavf_add_del_vlan_v2 function fails, the stripping may still be enabled. However, we only re-disable it if the iavf_add_del_vlan_v2 function was successful. 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.
With test-pmd I use "rx_vlan add all <port_id>" and it fails on adding vlan 17 but still triggers the stripping to be enabled.

If you are posting a v2 please check the indentation on the commit message, it looks like there is some unnecessary whitespace.

Thanks,
Ciara

> 
>  static void
> --
> 2.39.5 (Apple Git-154)

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

[Amiya Ranjan Mohakud]

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: stable@dpdk.org

v2:
- Fixed indentation in commit message

Signed-off-by: Amiya Ranjan Mohakud <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)

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

[Amiya Ranjan Mohakud]

[-- Attachment #1: Type: text/plain, Size: 4738 bytes --]

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
Amiya


On Mon, 23 Jun 2025 at 23:41, Amiya Ranjan Mohakud <
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: stable@dpdk.org
>
> v2:
> - Fixed indentation in commit message
>
> Signed-off-by: Amiya Ranjan Mohakud <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)
>
>

[-- Attachment #2: Type: text/html, Size: 7128 bytes --]

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

[Loftus, Ciara]

> Subject: [PATCH v2] net/iavf: fix VLAN offload strip flag
> 
> 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: stable@dpdk.org
> 
> v2:
> - Fixed indentation in commit message
> 
> Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>

Thanks for the v2.

Reviewed-by: Ciara Loftus <ciara.loftus@intel.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)

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

[Loftus, Ciara]

> 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: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:stable@dpdk.org
> 
> v2:
> - Fixed indentation in commit message
> 
> Signed-off-by: Amiya Ranjan Mohakud
> <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)

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

[Amiya Ranjan Mohakud]

[-- Attachment #1: Type: text/plain, Size: 5831 bytes --]

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?

Thanks
Amiya


On Tue, 24 Jun 2025 at 14:49, Loftus, Ciara <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: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:stable@dpdk.org
> >
> > v2:
> > - Fixed indentation in commit message
> >
> > Signed-off-by: Amiya Ranjan Mohakud
> > <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)
>

[-- Attachment #2: Type: text/html, Size: 8327 bytes --]

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

[Loftus, Ciara]

> 
> 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)

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

[Bruce Richardson]

On Wed, Jun 25, 2025 at 02:01:15PM +0000, Loftus, Ciara wrote:
> > 
> > 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
> 
Agree test failures look like false positives, as they are errors with the
PF driver and virtio driver, while this patch changes the VF driver.

Patch applied to dpdk-next-net-intel.

Thanks,
/Bruce

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

[Amiya Ranjan Mohakud]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

Thanks Bruce and Ciara for the review/acceptance of the patch.

Thanks
Amiya


On Wed, 25 Jun 2025 at 21:31, Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Wed, Jun 25, 2025 at 02:01:15PM +0000, Loftus, Ciara wrote:
> > >
> > > 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
> >
> Agree test failures look like false positives, as they are errors with the
> PF driver and virtio driver, while this patch changes the VF driver.
>
> Patch applied to dpdk-next-net-intel.
>
> Thanks,
> /Bruce
>

[-- Attachment #2: Type: text/html, Size: 1932 bytes --]

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

[Amiya Ranjan Mohakud]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

Just an additional note: It needs to be back ported to all LTS releases
till v22.11. Hope it will be taken care of.

Thanks
Amiya


On Thu, 26 Jun 2025 at 09:04, Amiya Ranjan Mohakud <
amiyaranjan.mohakud@gmail.com> wrote:

> Thanks Bruce and Ciara for the review/acceptance of the patch.
>
> Thanks
> Amiya
>
>
> On Wed, 25 Jun 2025 at 21:31, Bruce Richardson <bruce.richardson@intel.com>
> wrote:
>
>> On Wed, Jun 25, 2025 at 02:01:15PM +0000, Loftus, Ciara wrote:
>> > >
>> > > 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
>> >
>> Agree test failures look like false positives, as they are errors with the
>> PF driver and virtio driver, while this patch changes the VF driver.
>>
>> Patch applied to dpdk-next-net-intel.
>>
>> Thanks,
>> /Bruce
>>
>

[-- Attachment #2: Type: text/html, Size: 2788 bytes --]

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

[Gao, DaxueX]

Hi Amiya,

The dpdk patch you submitted has caused the following issues. Please help confirm if it is a bug.
Subsequent bug tracking will be conducted on bugzilla. If you have any questions, please add commnet on Bugzilla.
 
DPDK patch: https://patchwork.dpdk.org/project/dpdk/patch/20250623181136.48239-1-amiyaranjan.mohakud@gmail.com/

Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=1735

Thanks,
Daxue Gao


> -----Original Message-----
> From: Loftus, Ciara <ciara.loftus@intel.com>
> Sent: 2025年6月24日 17:19
> To: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>;
> dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [PATCH v2] net/iavf: fix VLAN offload strip flag
> 
> > Subject: [PATCH v2] net/iavf: fix VLAN offload strip flag
> >
> > 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: stable@dpdk.org
> >
> > v2:
> > - Fixed indentation in commit message
> >
> > Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>
> 
> Thanks for the v2.
> 
> Reviewed-by: Ciara Loftus <ciara.loftus@intel.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)

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

[Amiya Ranjan Mohakud]

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: stable@dpdk.org

v3:
- Fixed the vlan(v2) scenario by calling appropriate api
- Addresses Bugzilla ID: 1735
v2:
- Fixed indentation in commit message

Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 52 ++++++++++++++++++----------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index b3dacbef84..845d773594 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1379,14 +1379,43 @@ iavf_dev_del_mac_addr(struct rte_eth_dev *dev, uint32_t index)
 }
 
 static int
-iavf_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
+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 iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 	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)) {
+			if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2)
+				err = iavf_config_vlan_strip_v2(adapter, false);
+			else
+				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);
+	int err;
+
 	if (adapter->closed)
 		return -EIO;
 
@@ -1394,7 +1423,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 +1434,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)

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

[Amiya Ranjan Mohakud]

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: stable@dpdk.org

v3:
- Fixed the vlan(v2) scenario by calling appropriate api
- Addresses Bugzilla ID: 1735
v2:
- Fixed indentation in commit message

Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 52 ++++++++++++++++++----------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index b3dacbef84..845d773594 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1379,14 +1379,43 @@ iavf_dev_del_mac_addr(struct rte_eth_dev *dev, uint32_t index)
 }
 
 static int
-iavf_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
+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 iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 	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)) {
+			if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2)
+				err = iavf_config_vlan_strip_v2(adapter, false);
+			else
+				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);
+	int err;
+
 	if (adapter->closed)
 		return -EIO;
 
@@ -1394,7 +1423,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 +1434,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)

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

[Amiya Ranjan Mohakud]

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: stable@dpdk.org

v3:
- Fixed the vlan(v2) scenario by calling appropriate api
- Addresses Bugzilla ID: 1735
v2:
- Fixed indentation in commit message

Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index c33fdd9069..335a8126c4 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1388,6 +1388,7 @@ iavf_disable_vlan_strip_ex(struct rte_eth_dev *dev, 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;
 
@@ -1395,7 +1396,10 @@ iavf_disable_vlan_strip_ex(struct rte_eth_dev *dev, int on)
 	    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 (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2)
+				err = iavf_config_vlan_strip_v2(adapter, false);
+			else
+				err = iavf_disable_vlan_strip(adapter);
 			if (err)
 				return -EIO;
 		}
-- 
2.39.5 (Apple Git-154)

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

[Loftus, Ciara]

> Subject: [PATCH v3] net/iavf: fix VLAN offload strip flag
> 
> 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: stable@dpdk.org
> 
> v3:
> - Fixed the vlan(v2) scenario by calling appropriate api
> - Addresses Bugzilla ID: 1735
> v2:
> - Fixed indentation in commit message
> 
> Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>

Hi Amiya,

Thanks for the patch.
Since your first patch was already merged into the main branch, this patch is considered a new patch (v1).
If you submit another revision, it should be labelled v2.

Since it's a new patch, it needs a new commit message. Something like:

net/iavf: fix VLAN strip disabling for v2 capability

Ensure the correct virtchnl op is called for disabling vlan stripping by checking if the device supports either v1 or v2 vlan capability and choosing the op accordingly.

You should include the below line in the commit message which indicates it is a fix for the first patch:
Fixes: 3bfad066f9b4 ("net/iavf: fix VLAN strip setting after enabling filter")

Thanks,
Ciara

> ---
>  drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> b/drivers/net/intel/iavf/iavf_ethdev.c
> index c33fdd9069..335a8126c4 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -1388,6 +1388,7 @@ iavf_disable_vlan_strip_ex(struct rte_eth_dev
> *dev, 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;
> 
> @@ -1395,7 +1396,10 @@ iavf_disable_vlan_strip_ex(struct rte_eth_dev
> *dev, int on)
>  	    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 (vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_OFFLOAD_VLAN_V2)
> +				err = iavf_config_vlan_strip_v2(adapter, false);
> +			else
> +				err = iavf_disable_vlan_strip(adapter);
>  			if (err)
>  				return -EIO;
>  		}
> --
> 2.39.5 (Apple Git-154)

Re: [PATCH v3] net/iavf: fix VLAN offload strip flag

[Amiya Ranjan Mohakud]

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Sure Ciara. Thanks.

I will raise a fresh new patch for bug Id: 1735. That would be better.

Thanks
Amiya


On Thu, 3 Jul 2025 at 13:48, Loftus, Ciara <ciara.loftus@intel.com> wrote:

> > Subject: [PATCH v3] net/iavf: fix VLAN offload strip flag
> >
> > 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: stable@dpdk.org
> >
> > v3:
> > - Fixed the vlan(v2) scenario by calling appropriate api
> > - Addresses Bugzilla ID: 1735
> > v2:
> > - Fixed indentation in commit message
> >
> > Signed-off-by: Amiya Ranjan Mohakud <amiyaranjan.mohakud@gmail.com>
>
> Hi Amiya,
>
> Thanks for the patch.
> Since your first patch was already merged into the main branch, this patch
> is considered a new patch (v1).
> If you submit another revision, it should be labelled v2.
>
> Since it's a new patch, it needs a new commit message. Something like:
>
> net/iavf: fix VLAN strip disabling for v2 capability
>
> Ensure the correct virtchnl op is called for disabling vlan stripping by
> checking if the device supports either v1 or v2 vlan capability and
> choosing the op accordingly.
>
> You should include the below line in the commit message which indicates it
> is a fix for the first patch:
> Fixes: 3bfad066f9b4 ("net/iavf: fix VLAN strip setting after enabling
> filter")
>
> Thanks,
> Ciara
>
> > ---
> >  drivers/net/intel/iavf/iavf_ethdev.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> > b/drivers/net/intel/iavf/iavf_ethdev.c
> > index c33fdd9069..335a8126c4 100644
> > --- a/drivers/net/intel/iavf/iavf_ethdev.c
> > +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> > @@ -1388,6 +1388,7 @@ iavf_disable_vlan_strip_ex(struct rte_eth_dev
> > *dev, 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;
> >
> > @@ -1395,7 +1396,10 @@ iavf_disable_vlan_strip_ex(struct rte_eth_dev
> > *dev, int on)
> >           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 (vf->vf_res->vf_cap_flags &
> > VIRTCHNL_VF_OFFLOAD_VLAN_V2)
> > +                             err = iavf_config_vlan_strip_v2(adapter,
> false);
> > +                     else
> > +                             err = iavf_disable_vlan_strip(adapter);
> >                       if (err)
> >                               return -EIO;
> >               }
> > --
> > 2.39.5 (Apple Git-154)
>
>

[-- Attachment #2: Type: text/html, Size: 4579 bytes --]