> 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: sta...@dpdk.org
>
> 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);
> }
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)
