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