[v4] net/i40e: restore disable double VLAN by default

Message ID 20220707170434.2159759-1-kevinx.liu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [v4] net/i40e: restore disable double VLAN by default |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Kevin Liu July 7, 2022, 5:04 p.m. UTC
  Previously, QinQ is enabled by default and can't be disabled,
but there'll be performance drop if QinQ is enabled.

So, disable QinQ by default.

Fixes: ae97b8b89826 ("net/i40e: fix error disable double VLAN")
Signed-off-by: Kevin Liu <kevinx.liu@intel.com>

---
v2: update doc and refine commit log
---
v3: refine commit log
---
v4: update doc
---
 doc/guides/nics/i40e.rst       | 13 ++++++++-----
 drivers/net/i40e/i40e_ethdev.c | 12 ------------
 2 files changed, 8 insertions(+), 17 deletions(-)
  

Comments

Xing, Beilei July 8, 2022, 12:21 a.m. UTC | #1
> -----Original Message-----
> From: Liu, KevinX <kevinx.liu@intel.com>
> Sent: Friday, July 8, 2022 1:05 AM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Liu, KevinX
> <kevinx.liu@intel.com>
> Subject: [PATCH v4] net/i40e: restore disable double VLAN by default
> 
> Previously, QinQ is enabled by default and can't be disabled, but there'll be
> performance drop if QinQ is enabled.
> 
> So, disable QinQ by default.
> 
> Fixes: ae97b8b89826 ("net/i40e: fix error disable double VLAN")
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> 
> ---
> v2: update doc and refine commit log
> ---
> v3: refine commit log
> ---
> v4: update doc
> ---
>  doc/guides/nics/i40e.rst       | 13 ++++++++-----
>  drivers/net/i40e/i40e_ethdev.c | 12 ------------
>  2 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index
> 85fdc4944d..d5938fa8e4 100644
> --- a/doc/guides/nics/i40e.rst
> +++ b/doc/guides/nics/i40e.rst
> @@ -969,11 +969,14 @@ it will fail and return the info "Conflict with the first
> rule's input set",  which means the current rule's input set conflicts with the first
> rule's.
>  Remove the first rule if want to change the input set of the PCTYPE.
> 
> -Disable QinQ is not supported when FW >= 8.4 -
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> -
> -If upgrade FW to version 8.4 and higher, enable QinQ by default and disable
> QinQ is not supported.
> -
> +Vlan related Features miss when FW >= 8.4
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +If FW version >= 8.4, there'll be some Vlan related issues:
> +1. TCI input set for QinQ  is invalid.
> +2. Fail to configure TPID for QinQ.
> +3. Need to enable QinQ before enabling Vlan filter.
> +4. Fail to strip outer Vlan.
> 
>  Example of getting best performance with l3fwd example
>  ------------------------------------------------------
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 684e095026..117dd85c11 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -4027,12 +4027,6 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> mask)
>  	}
> 
>  	if (mask & RTE_ETH_VLAN_EXTEND_MASK) {
> -		/* Double VLAN not allowed to be disabled.*/
> -		if (pf->fw8_3gt && !(rxmode->offloads &
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> -			PMD_DRV_LOG(WARNING,
> -				"Disable double VLAN is not allowed after
> firmwarev8.3!");
> -			return 0;
> -		}
>  		i = 0;
>  		num = vsi->mac_num;
>  		mac_filter = rte_zmalloc("mac_filter_info_data",
> @@ -6296,7 +6290,6 @@ int i40e_vsi_cfg_inner_vlan_stripping(struct i40e_vsi
> *vsi, bool on)  static int  i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
> -	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> >dev_private);
>  	struct rte_eth_dev_data *data = dev->data;
>  	int ret;
>  	int mask = 0;
> @@ -6307,11 +6300,6 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)
>  	       RTE_ETH_VLAN_FILTER_MASK |
>  	       RTE_ETH_VLAN_EXTEND_MASK;
> 
> -	/* Double VLAN be enabled by default.*/
> -	if (pf->fw8_3gt) {
> -		struct rte_eth_rxmode *rxmode = &dev->data-
> >dev_conf.rxmode;
> -		rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> -	}
>  	ret = i40e_vlan_offload_set(dev, mask);
>  	if (ret) {
>  		PMD_DRV_LOG(INFO, "Failed to update vlan offload");
> --
> 2.34.1

Acked-by: Beilei Xing <beilei.xing@intel.com>
  
Qi Zhang July 8, 2022, 12:41 a.m. UTC | #2
> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Friday, July 8, 2022 8:22 AM
> To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> Cc: Zhang, Yuying <yuying.zhang@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>
> Subject: RE: [PATCH v4] net/i40e: restore disable double VLAN by default
> 
> 
> 
> > -----Original Message-----
> > From: Liu, KevinX <kevinx.liu@intel.com>
> > Sent: Friday, July 8, 2022 1:05 AM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Yuying
> > <yuying.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Liu,
> > KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH v4] net/i40e: restore disable double VLAN by default
> >
> > Previously, QinQ is enabled by default and can't be disabled, but
> > there'll be performance drop if QinQ is enabled.
> >
> > So, disable QinQ by default.
> >
> > Fixes: ae97b8b89826 ("net/i40e: fix error disable double VLAN")

The commit log of above patch has been refined 
Correct it to 
Fixes: 5bd74df1dbf5 ("net/i40e: fix QinQ enablement")

> Acked-by: Beilei Xing <beilei.xing@intel.com>

Applied to dpdk-next-net-intel.
Thanks
Qi
  
Thomas Monjalon July 11, 2022, 4:50 p.m. UTC | #3
07/07/2022 19:04, Kevin Liu:
> +Vlan related Features miss when FW >= 8.4
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +If FW version >= 8.4, there'll be some Vlan related issues:
> +1. TCI input set for QinQ  is invalid.
> +2. Fail to configure TPID for QinQ.
> +3. Need to enable QinQ before enabling Vlan filter.
> +4. Fail to strip outer Vlan.

Did you check the HTML rendering?
A blank line is missing before the list.

Also you could use #. for automatic list numbering.

I'll fix when pulling.
  

Patch

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index 85fdc4944d..d5938fa8e4 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -969,11 +969,14 @@  it will fail and return the info "Conflict with the first rule's input set",
 which means the current rule's input set conflicts with the first rule's.
 Remove the first rule if want to change the input set of the PCTYPE.
 
-Disable QinQ is not supported when FW >= 8.4
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-If upgrade FW to version 8.4 and higher, enable QinQ by default and disable QinQ is not supported.
-
+Vlan related Features miss when FW >= 8.4
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If FW version >= 8.4, there'll be some Vlan related issues:
+1. TCI input set for QinQ  is invalid.
+2. Fail to configure TPID for QinQ.
+3. Need to enable QinQ before enabling Vlan filter.
+4. Fail to strip outer Vlan.
 
 Example of getting best performance with l3fwd example
 ------------------------------------------------------
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 684e095026..117dd85c11 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -4027,12 +4027,6 @@  i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	}
 
 	if (mask & RTE_ETH_VLAN_EXTEND_MASK) {
-		/* Double VLAN not allowed to be disabled.*/
-		if (pf->fw8_3gt && !(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
-			PMD_DRV_LOG(WARNING,
-				"Disable double VLAN is not allowed after firmwarev8.3!");
-			return 0;
-		}
 		i = 0;
 		num = vsi->mac_num;
 		mac_filter = rte_zmalloc("mac_filter_info_data",
@@ -6296,7 +6290,6 @@  int i40e_vsi_cfg_inner_vlan_stripping(struct i40e_vsi *vsi, bool on)
 static int
 i40e_dev_init_vlan(struct rte_eth_dev *dev)
 {
-	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_eth_dev_data *data = dev->data;
 	int ret;
 	int mask = 0;
@@ -6307,11 +6300,6 @@  i40e_dev_init_vlan(struct rte_eth_dev *dev)
 	       RTE_ETH_VLAN_FILTER_MASK |
 	       RTE_ETH_VLAN_EXTEND_MASK;
 
-	/* Double VLAN be enabled by default.*/
-	if (pf->fw8_3gt) {
-		struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
-		rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
-	}
 	ret = i40e_vlan_offload_set(dev, mask);
 	if (ret) {
 		PMD_DRV_LOG(INFO, "Failed to update vlan offload");