[v5] net/iavf: revert fix VLAN insertion

Message ID 20221113163031.1975916-1-yidingx.zhou@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [v5] net/iavf: revert fix VLAN insertion |

Checks

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

Commit Message

Yiding Zhou Nov. 13, 2022, 4:30 p.m. UTC
  The vector Tx path does not support VLAN insertion via the L2TAG2 field,
but the scalar path supports. The earlier commit was to force to select
scalar path as soon as kernel driver requests to use L2TAG2. That logic is
incorrect. Because other case like VLAN offloading not required but scalar
path selected would have a significant performance drop.

Therefore the following commit was reverted accordingly.

commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")

After reverting this commit, the AVX512 Tx path would insert the VLAN tag
into the wrong location(inner of QinQ) when the kernel driver requested
L2TAG2. This is inconsistent with the behavior of PF(outer of QinQ).

It is currently known that ice kernel drivers newer than 1.8.9 will request
the use of L2TAG2. User can set parameter '--force-max-simd-bitwidth' to
64/128/256 to avoid this issue.

Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
Cc: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
v5: move the document
v4: document this issue as kown issue and add some commit log
v3: rebase and change commit log
---
 doc/guides/nics/intel_vf.rst            | 13 +++++++++++++
 drivers/net/iavf/iavf_rxtx_vec_common.h |  3 ---
 2 files changed, 13 insertions(+), 3 deletions(-)
  

Comments

Qi Zhang Nov. 14, 2022, 12:52 a.m. UTC | #1
> -----Original Message-----
> From: Zhou, YidingX <yidingx.zhou@intel.com>
> Sent: Monday, November 14, 2022 12:31 AM
> To: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; ktraynor@redhat.com; Zhou,
> YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: [PATCH v5] net/iavf: revert fix VLAN insertion
> 
> The vector Tx path does not support VLAN insertion via the L2TAG2 field, but
> the scalar path supports. The earlier commit was to force to select scalar
> path as soon as kernel driver requests to use L2TAG2. That logic is incorrect.
> Because other case like VLAN offloading not required but scalar path selected
> would have a significant performance drop.
> 
> Therefore the following commit was reverted accordingly.
> 
> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> After reverting this commit, the AVX512 Tx path would insert the VLAN tag
> into the wrong location(inner of QinQ) when the kernel driver requested
> L2TAG2. This is inconsistent with the behavior of PF(outer of QinQ).
> 
> It is currently known that ice kernel drivers newer than 1.8.9 will request the
> use of L2TAG2. User can set parameter '--force-max-simd-bitwidth' to
> 64/128/256 to avoid this issue.
> 
> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
  

Patch

diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst
index d582f831da..edbda275c1 100644
--- a/doc/guides/nics/intel_vf.rst
+++ b/doc/guides/nics/intel_vf.rst
@@ -704,3 +704,16 @@  i40e: Vlan filtering of VF
 For i40e driver 2.17.15, configuring VLAN filters from the DPDK VF is unsupported.
 When applying VLAN filters on the VF it must first be configured from the
 corresponding PF.
+
+ice: VF inserts VLAN tag incorrectly on AVX-512 Tx path
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+When the kernel driver requests the VF to use the L2TAG2 field of the Tx context
+descriptor to insert the hardware offload VLAN tag, AVX-512 Tx path cannot handle
+this case correctly due to its lack of support for the Tx context descriptor.
+
+The VLAN tag will be inserted to the wrong location(inner of QinQ) on AVX-512 Tx
+path. That is inconsistent with the behavior of PF(outer of QinQ). The ice kernel
+driver's version newer than 1.8.9 requests to use L2TAG2 and has this issue.
+
+Set the parameter `--force-max-simd-bitwidth` as 64/128/256 to avoid selecting
+AVX-512 Tx path.
diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h
index 4ab22c6b2b..a59cb2ceee 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_common.h
+++ b/drivers/net/iavf/iavf_rxtx_vec_common.h
@@ -253,9 +253,6 @@  iavf_tx_vec_queue_default(struct iavf_tx_queue *txq)
 	if (txq->offloads & IAVF_TX_NO_VECTOR_FLAGS)
 		return -1;
 
-	if (txq->vlan_flag == IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2)
-		return -1;
-
 	if (txq->offloads & IAVF_TX_VECTOR_OFFLOAD)
 		return IAVF_VECTOR_OFFLOAD_PATH;