[PATCH] net/iavf: revert fix VLAN insertion

[PATCH] net/iavf: revert fix VLAN insertion

[Yiding Zhou]

The patch to be reverted forces to select normal Tx path when kernel driver
tells that L2TAG2 is required, it results in a lot of performance loss.

We should support Tx context descriptor on vector path to handle the L2TAG2
case.

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

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
 drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
 1 file changed, 3 deletions(-)

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;
 
-- 
2.34.1

Re: [PATCH] net/iavf: revert fix VLAN insertion

[Kevin Traynor]

On 18/10/2022 11:26, Yiding Zhou wrote:
> The patch to be reverted forces to select normal Tx path when kernel driver
> tells that L2TAG2 is required, it results in a lot of performance loss.
> 
> We should support Tx context descriptor on vector path to handle the L2TAG2
> case.
> 

Hi. Was the original fix in 0d58caa7d6d1 not needed? or is it just being 
reverted because of performance issues and the original functional issue 
still remains? It is not very clear in the commit message. It would be 
good to write what the status of the original issue with this patch 
applied is in the commit message.

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

Better to add DPDK style tags here, so they can be picked up by scripts, 
thanks. i.e.

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

> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
>   drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> 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;
>   

[PATCH v2] net/iavf: revert fix VLAN insertion

[Yiding Zhou]

When the kernel driver tells to use the L2TAG2 field for VLAN insertion,
the context descriptor needs to be used. There is an issue on the vector Tx
path, because it does not support the context descriptor.

The previous commit forces to select normal path to avoid the above issue,
but it results in a performance loss of around 40%. So it needs to be
reverted and the original issue needed to be fixed by rework.

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

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

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
 drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
 1 file changed, 3 deletions(-)

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;
 
-- 
2.34.1

Re: [PATCH v2] net/iavf: revert fix VLAN insertion

[Kevin Traynor]

On 19/10/2022 08:54, Yiding Zhou wrote:
> When the kernel driver tells to use the L2TAG2 field for VLAN insertion,
> the context descriptor needs to be used. There is an issue on the vector Tx
> path, because it does not support the context descriptor.
> 
> The previous commit forces to select normal path to avoid the above issue,
> but it results in a performance loss of around 40%. So it needs to be
> reverted and the original issue needed to be fixed by rework.
> 

Thank you, that is a much clearer explanation.

Now on the approach, the commit being reverted says:
"When the driver tells the VF to insert VLAN tag using the L2TAG2 field,
vector Tx path does not use Tx context descriptor and would cause VLAN 
tag inserted into the wrong location."

So it means this revert is solving a performance regression, but 
re-introducing the functional issue above.

Is there a correct fix for the original issue sent that can be applied 
too? If not, wouldn't it be better to wait until it is before doing the 
revert?

> To reverts
> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
>   drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> 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;
>   

RE: [PATCH v2] net/iavf: revert fix VLAN insertion

[Zhou, YidingX]

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Wednesday, October 19, 2022 4:53 PM
> To: Zhou, YidingX <yidingx.zhou@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion
> 
> On 19/10/2022 08:54, Yiding Zhou wrote:
> > When the kernel driver tells to use the L2TAG2 field for VLAN
> > insertion, the context descriptor needs to be used. There is an issue
> > on the vector Tx path, because it does not support the context descriptor.
> >
> > The previous commit forces to select normal path to avoid the above
> > issue, but it results in a performance loss of around 40%. So it needs
> > to be reverted and the original issue needed to be fixed by rework.
> >
> 
> Thank you, that is a much clearer explanation.
> 
> Now on the approach, the commit being reverted says:
> "When the driver tells the VF to insert VLAN tag using the L2TAG2 field, vector
> Tx path does not use Tx context descriptor and would cause VLAN tag inserted
> into the wrong location."
> 
> So it means this revert is solving a performance regression, but re-introducing
> the functional issue above.
> 
> Is there a correct fix for the original issue sent that can be applied too? If not,
> wouldn't it be better to wait until it is before doing the revert?
> 

Sorry, there is no correct fix yet.
We plan to support context descriptor on vector path to fix the original issue, 
It may take more time and cannot be completed within this cycle.

> > To reverts
> > commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> >
> > Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> >
> > Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> > ---
> >   drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
> >   1 file changed, 3 deletions(-)
> >
> > 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;
> >

Re: [PATCH v2] net/iavf: revert fix VLAN insertion

[Kevin Traynor]

On 20/10/2022 02:33, Zhou, YidingX wrote:
> 
> 
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Wednesday, October 19, 2022 4:53 PM
>> To: Zhou, YidingX <yidingx.zhou@intel.com>; dev@dpdk.org
>> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion
>>
>> On 19/10/2022 08:54, Yiding Zhou wrote:
>>> When the kernel driver tells to use the L2TAG2 field for VLAN
>>> insertion, the context descriptor needs to be used. There is an issue
>>> on the vector Tx path, because it does not support the context descriptor.
>>>
>>> The previous commit forces to select normal path to avoid the above
>>> issue, but it results in a performance loss of around 40%. So it needs
>>> to be reverted and the original issue needed to be fixed by rework.
>>>
>>
>> Thank you, that is a much clearer explanation.
>>
>> Now on the approach, the commit being reverted says:
>> "When the driver tells the VF to insert VLAN tag using the L2TAG2 field, vector
>> Tx path does not use Tx context descriptor and would cause VLAN tag inserted
>> into the wrong location."
>>
>> So it means this revert is solving a performance regression, but re-introducing
>> the functional issue above.
>>
>> Is there a correct fix for the original issue sent that can be applied too? If not,
>> wouldn't it be better to wait until it is before doing the revert?
>>
> 
> Sorry, there is no correct fix yet.
> We plan to support context descriptor on vector path to fix the original issue,
> It may take more time and cannot be completed within this cycle.
> 

ok, but you didn't answer the second question.

"When the driver tells the VF to insert VLAN tag using the L2TAG2 field, 
vector Tx path does not use Tx context descriptor and would cause VLAN 
tag inserted into the wrong location."

Please explain your justification for (re-)introducing this bug?

Why is better to get (corrupt?) packets with incorrect VLAN tags than 
lose performance for this case? Or have I mis-interpreted the patches.


>>> To reverts
>>> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>>>
>>> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>>>
>>> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
>>> ---
>>>    drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>>>    1 file changed, 3 deletions(-)
>>>
>>> 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;
>>>
> 

RE: [PATCH v2] net/iavf: revert fix VLAN insertion

[Zhou, YidingX]

> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Wednesday, October 19, 2022 4:53 PM
> >> To: Zhou, YidingX <yidingx.zhou@intel.com>; dev@dpdk.org
> >> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion
> >>
> >> On 19/10/2022 08:54, Yiding Zhou wrote:
> >>> When the kernel driver tells to use the L2TAG2 field for VLAN
> >>> insertion, the context descriptor needs to be used. There is an
> >>> issue on the vector Tx path, because it does not support the context
> descriptor.
> >>>
> >>> The previous commit forces to select normal path to avoid the above
> >>> issue, but it results in a performance loss of around 40%. So it
> >>> needs to be reverted and the original issue needed to be fixed by rework.
> >>>
> >>
> >> Thank you, that is a much clearer explanation.
> >>
> >> Now on the approach, the commit being reverted says:
> >> "When the driver tells the VF to insert VLAN tag using the L2TAG2
> >> field, vector Tx path does not use Tx context descriptor and would
> >> cause VLAN tag inserted into the wrong location."
> >>
> >> So it means this revert is solving a performance regression, but
> >> re-introducing the functional issue above.
> >>
> >> Is there a correct fix for the original issue sent that can be
> >> applied too? If not, wouldn't it be better to wait until it is before doing the
> revert?
> >>
> >
> > Sorry, there is no correct fix yet.
> > We plan to support context descriptor on vector path to fix the
> > original issue, It may take more time and cannot be completed within this
> cycle.
> >
> 
> ok, but you didn't answer the second question.
> 
> "When the driver tells the VF to insert VLAN tag using the L2TAG2 field, vector
> Tx path does not use Tx context descriptor and would cause VLAN tag inserted
> into the wrong location."
> 
> Please explain your justification for (re-)introducing this bug?
> 
> Why is better to get (corrupt?) packets with incorrect VLAN tags than lose
> performance for this case? Or have I mis-interpreted the patches.
> 
> 
Thanks for your review.
I agree with you.  It should not re-introduce functional issue .
This revert is not needed. I will resubmit a new patch for the performance loss.

> >>> To reverts
> >>> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> >>>
> >>> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> >>>
> >>> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> >>> ---
> >>>    drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
> >>>    1 file changed, 3 deletions(-)
> >>>
> >>> 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;
> >>>
> >

Re: [PATCH v2] net/iavf: revert fix VLAN insertion

[Kevin Traynor]

re-adding dev mailing list

On 03/11/2022 10:51, Kevin Traynor wrote:
> On 03/11/2022 10:38, Zhou, YidingX wrote:
>> Hi, Kevin
>>
>> According to suggestion, we did many deep investigation and various attempts, unfortunately that the performance drop(about 40%) caused by
>> the previous commit:
>> 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>> Still can not be resolved in this cycle due to tight schedule.
>>
>> Because the performance drop is too serious and the scope of impact is relatively large, we think the previous commit is a mistake.
> 
> ok, I'm trying to understand why a performance drop is worse than some
> corrupt packets.
> 
> What is the scope of the performance drop? Does the scope impact more
> cases than just when the L2TAG2 is used?
> 
> IOW, is it a functional issue in a small use case, and performance issue
> in more use cases? If so, I can understand you wanting to revert.
> 
>> For the original bug, we plan to fix it in the next cycle (by supporting L2TAG2 on the vector path).
>>
> 
> With the revert, is there a way to disable use of L2TAG2 being used
> while it is incorrect? At very least, the issue should be
> documented/bugzilla so a user can know what doesn't work correctly.
> 
>> So we are expecting to revert the above commit from main branch and it should not be merged to stable branch.
> 
> It has already been merged but if the decision for main branch is to
> revert, then I can revert on stable also.
> 
> thanks,
> Kevin.
> 
>> Sorry I'm a little late in explaining the situation. Your understanding would be appreciated.
>>
>> /Yiding
>>
>>> -----Original Message-----
>>> From: Zhou, YidingX
>>> Sent: Friday, October 21, 2022 10:43 AM
>>> To: Kevin Traynor <ktraynor@redhat.com>; dev@dpdk.org
>>> Subject: RE: [PATCH v2] net/iavf: revert fix VLAN insertion
>>>
>>>
>>>
>>>>>> -----Original Message-----
>>>>>> From: Kevin Traynor <ktraynor@redhat.com>
>>>>>> Sent: Wednesday, October 19, 2022 4:53 PM
>>>>>> To: Zhou, YidingX <yidingx.zhou@intel.com>; dev@dpdk.org
>>>>>> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion
>>>>>>
>>>>>> On 19/10/2022 08:54, Yiding Zhou wrote:
>>>>>>> When the kernel driver tells to use the L2TAG2 field for VLAN
>>>>>>> insertion, the context descriptor needs to be used. There is an
>>>>>>> issue on the vector Tx path, because it does not support the
>>>>>>> context
>>>> descriptor.
>>>>>>>
>>>>>>> The previous commit forces to select normal path to avoid the
>>>>>>> above issue, but it results in a performance loss of around 40%.
>>>>>>> So it needs to be reverted and the original issue needed to be fixed by
>>> rework.
>>>>>>>
>>>>>>
>>>>>> Thank you, that is a much clearer explanation.
>>>>>>
>>>>>> Now on the approach, the commit being reverted says:
>>>>>> "When the driver tells the VF to insert VLAN tag using the L2TAG2
>>>>>> field, vector Tx path does not use Tx context descriptor and would
>>>>>> cause VLAN tag inserted into the wrong location."
>>>>>>
>>>>>> So it means this revert is solving a performance regression, but
>>>>>> re-introducing the functional issue above.
>>>>>>
>>>>>> Is there a correct fix for the original issue sent that can be
>>>>>> applied too? If not, wouldn't it be better to wait until it is
>>>>>> before doing the
>>>> revert?
>>>>>>
>>>>>
>>>>> Sorry, there is no correct fix yet.
>>>>> We plan to support context descriptor on vector path to fix the
>>>>> original issue, It may take more time and cannot be completed within
>>>>> this
>>>> cycle.
>>>>>
>>>>
>>>> ok, but you didn't answer the second question.
>>>>
>>>> "When the driver tells the VF to insert VLAN tag using the L2TAG2
>>>> field, vector Tx path does not use Tx context descriptor and would
>>>> cause VLAN tag inserted into the wrong location."
>>>>
>>>> Please explain your justification for (re-)introducing this bug?
>>>>
>>>> Why is better to get (corrupt?) packets with incorrect VLAN tags than
>>>> lose performance for this case? Or have I mis-interpreted the patches.
>>>>
>>>>
>>> Thanks for your review.
>>> I agree with you.  It should not re-introduce functional issue .
>>> This revert is not needed. I will resubmit a new patch for the performance loss.
>>>
>>>>>>> To reverts
>>>>>>> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>>>>>>>
>>>>>>> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>>>>>>>
>>>>>>> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
>>>>>>> ---
>>>>>>>      drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>>>>>>>      1 file changed, 3 deletions(-)
>>>>>>>
>>>>>>> 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;
>>>>>>>
>>>>>
>>
> 

Re: [PATCH v2] net/iavf: revert fix VLAN insertion

[Kevin Traynor]

On 03/11/2022 14:06, Yang, Qiming wrote:
> Hi, Kevin
> 
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Thursday, November 3, 2022 6:52 PM
>> To: Zhou, YidingX <yidingx.zhou@intel.com>
>> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion
>>
>> On 03/11/2022 10:38, Zhou, YidingX wrote:
>>> Hi, Kevin
>>>
>>> According to suggestion, we did many deep investigation and various
>>> attempts, unfortunately that the performance drop(about 40%) caused by
>> the previous commit:
>>> 0d58caa7d6d1 ("net/iavf: fix VLAN insertion") Still can not be
>>> resolved in this cycle due to tight schedule.
>>>
>>> Because the performance drop is too serious and the scope of impact is
>> relatively large, we think the previous commit is a mistake.
>>
>> ok, I'm trying to understand why a performance drop is worse than some
>> corrupt packets.
>>
>> What is the scope of the performance drop? Does the scope impact more
>> cases than just when the L2TAG2 is used?
>>
>> IOW, is it a functional issue in a small use case, and performance issue in
>> more use cases? If so, I can understand you wanting to revert.
>>
> 
> For the original issue, it is QinQ insert function can't work in vector path, because we don't support this function in vector path. And after revert the patch, user still can use QinQ insert by set the Tx function to normal path as a workaround.
> But with this unreasonable patch, all the case using vector mode will have 40% performance drop, the drop is too high to accept.
> So I think we should revert it first and enable the QinQ insert in vector path in next release after well design and full performance test.
> 

ok, thanks for the explanation. I will revert on 21.11 branch when it 
reverted on main.

Kevin.

> Qiming
> 
>>> For the original bug, we plan to fix it in the next cycle (by supporting
>> L2TAG2 on the vector path).
>>>
>>
>> With the revert, is there a way to disable use of L2TAG2 being used while it is
>> incorrect? At very least, the issue should be documented/bugzilla so a user
>> can know what doesn't work correctly.
>>
>>> So we are expecting to revert the above commit from main branch and it
>> should not be merged to stable branch.
>>
>> It has already been merged but if the decision for main branch is to revert,
>> then I can revert on stable also.
>>
>> thanks,
>> Kevin.
>>
>>> Sorry I'm a little late in explaining the situation. Your understanding would
>> be appreciated.
>>>
>>> /Yiding
>>>
>>>> -----Original Message-----
>>>> From: Zhou, YidingX
>>>> Sent: Friday, October 21, 2022 10:43 AM
>>>> To: Kevin Traynor <ktraynor@redhat.com>; dev@dpdk.org
>>>> Subject: RE: [PATCH v2] net/iavf: revert fix VLAN insertion
>>>>
>>>>
>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Kevin Traynor <ktraynor@redhat.com>
>>>>>>> Sent: Wednesday, October 19, 2022 4:53 PM
>>>>>>> To: Zhou, YidingX <yidingx.zhou@intel.com>; dev@dpdk.org
>>>>>>> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion
>>>>>>>
>>>>>>> On 19/10/2022 08:54, Yiding Zhou wrote:
>>>>>>>> When the kernel driver tells to use the L2TAG2 field for VLAN
>>>>>>>> insertion, the context descriptor needs to be used. There is an
>>>>>>>> issue on the vector Tx path, because it does not support the
>>>>>>>> context
>>>>> descriptor.
>>>>>>>>
>>>>>>>> The previous commit forces to select normal path to avoid the
>>>>>>>> above issue, but it results in a performance loss of around 40%.
>>>>>>>> So it needs to be reverted and the original issue needed to be
>>>>>>>> fixed by
>>>> rework.
>>>>>>>>
>>>>>>>
>>>>>>> Thank you, that is a much clearer explanation.
>>>>>>>
>>>>>>> Now on the approach, the commit being reverted says:
>>>>>>> "When the driver tells the VF to insert VLAN tag using the L2TAG2
>>>>>>> field, vector Tx path does not use Tx context descriptor and would
>>>>>>> cause VLAN tag inserted into the wrong location."
>>>>>>>
>>>>>>> So it means this revert is solving a performance regression, but
>>>>>>> re-introducing the functional issue above.
>>>>>>>
>>>>>>> Is there a correct fix for the original issue sent that can be
>>>>>>> applied too? If not, wouldn't it be better to wait until it is
>>>>>>> before doing the
>>>>> revert?
>>>>>>>
>>>>>>
>>>>>> Sorry, there is no correct fix yet.
>>>>>> We plan to support context descriptor on vector path to fix the
>>>>>> original issue, It may take more time and cannot be completed
>>>>>> within this
>>>>> cycle.
>>>>>>
>>>>>
>>>>> ok, but you didn't answer the second question.
>>>>>
>>>>> "When the driver tells the VF to insert VLAN tag using the L2TAG2
>>>>> field, vector Tx path does not use Tx context descriptor and would
>>>>> cause VLAN tag inserted into the wrong location."
>>>>>
>>>>> Please explain your justification for (re-)introducing this bug?
>>>>>
>>>>> Why is better to get (corrupt?) packets with incorrect VLAN tags
>>>>> than lose performance for this case? Or have I mis-interpreted the
>> patches.
>>>>>
>>>>>
>>>> Thanks for your review.
>>>> I agree with you.  It should not re-introduce functional issue .
>>>> This revert is not needed. I will resubmit a new patch for the performance
>> loss.
>>>>
>>>>>>>> To reverts
>>>>>>>> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>>>>>>>>
>>>>>>>> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
>>>>>>>>
>>>>>>>> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
>>>>>>>> ---
>>>>>>>>      drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>>>>>>>>      1 file changed, 3 deletions(-)
>>>>>>>>
>>>>>>>> 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;
>>>>>>>>
>>>>>>
>>>
> 

[PATCH v3] net/iavf: revert fix VLAN insertion

[Yiding Zhou]

The vector Tx path does not support VLAN insertion via the L2TAG2 field,
but the scalar path supports. The previous 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 needs to revert.

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

After this commit reverted, the user can select scalar path with the
parameter '--force-max-simd-bitwidth' if necessary.

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

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
v3: rebase and change commit log
---
 drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
 1 file changed, 3 deletions(-)

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;
 
-- 
2.34.1

RE: [PATCH v3] net/iavf: revert fix VLAN insertion

[Zhou, YidingX]

Hi 

> -----Original Message-----
> From: Zhou, YidingX <yidingx.zhou@intel.com>
> Sent: Friday, November 4, 2022 2:10 PM
> To: dev@dpdk.org
> Cc: ktraynor@redhat.com; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> YidingX <yidingx.zhou@intel.com>
> Subject: [PATCH v3] 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 previous 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 needs to revert.
> 
> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> After this commit reverted, the user can select scalar path with the parameter
> '--force-max-simd-bitwidth' if necessary.
> 
> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
> v3: rebase and change commit log
> ---
>  drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> 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;
> 
> --
> 2.34.1

RE: [PATCH v3] net/iavf: revert fix VLAN insertion

[Zhang, Qi Z]

> -----Original Message-----
> From: Yiding Zhou <yidingx.zhou@intel.com>
> Sent: Friday, November 4, 2022 2:10 PM
> To: dev@dpdk.org
> Cc: ktraynor@redhat.com; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> YidingX <yidingx.zhou@intel.com>
> Subject: [PATCH v3] 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 previous commit was to force to select scalar
> path as soon as kernel driver requests to use L2TAG2.

In which situation, that kernel driver will request 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 needs to revert.

What will happen, if kernel driver request to use L2TAG2, but still vector path is selected?

> 
> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> After this commit reverted, the user can select scalar path with the
> parameter '--force-max-simd-bitwidth' if necessary.
> 
> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
> v3: rebase and change commit log
> ---
>  drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> 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;
> 
> --
> 2.34.1

RE: [PATCH v3] net/iavf: revert fix VLAN insertion

[Zhou, YidingX]

> > Subject: [PATCH v3] 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 previous commit was to force
> > to select scalar path as soon as kernel driver requests to use L2TAG2.
> 
> In which situation, that kernel driver will request to use L2TAG2?

According to my tests, this happens when the kernel driver version is newer than 1.8.9

> >
> > 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 needs to revert.
> 
> What will happen, if kernel driver request to use L2TAG2, but still vector path is
> selected?
> 
The VLAN tag will be inserted to wrong location (inner of QinQ),  and this behavior is inconsistent with PF (outer).

> >
> > commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> >
> > After this commit reverted, the user can select scalar path with the
> > parameter '--force-max-simd-bitwidth' if necessary.
> >
> > Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion")
> >
> > Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> > ---
> > v3: rebase and change commit log
> > ---
> >  drivers/net/iavf/iavf_rxtx_vec_common.h | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > 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;
> >
> > --
> > 2.34.1

RE: [PATCH v3] net/iavf: revert fix VLAN insertion

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhou, YidingX <yidingx.zhou@intel.com>
> Sent: Thursday, November 10, 2022 10:10 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: ktraynor@redhat.com; Yang, Qiming <qiming.yang@intel.com>
> Subject: RE: [PATCH v3] net/iavf: revert fix VLAN insertion
> 
> > > Subject: [PATCH v3] 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 previous commit was to
> > > force to select scalar path as soon as kernel driver requests to use L2TAG2.
> >
> > In which situation, that kernel driver will request to use L2TAG2?
> 
> According to my tests, this happens when the kernel driver version is newer
> than 1.8.9

> 
> > >
> > > 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 needs to revert.
> >
> > What will happen, if kernel driver request to use L2TAG2, but still
> > vector path is selected?
> >
> The VLAN tag will be inserted to wrong location (inner of QinQ),  and this
> behavior is inconsistent with PF (outer).

Ok, I assume we will have above limitation after applying this patch, right?
If that's true we'd better claim this in the commit log as well as document it as a knowing issue.

[PATCH v4] net/iavf: revert fix VLAN insertion

[Yiding Zhou]

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>
---
v4: document this issue as kown issue and add some commit log
v3: rebase and change commit log
---
 doc/guides/rel_notes/known_issues.rst   | 23 +++++++++++++++++++++++
 drivers/net/iavf/iavf_rxtx_vec_common.h |  3 ---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/known_issues.rst b/doc/guides/rel_notes/known_issues.rst
index 570550843a..1f73b17716 100644
--- a/doc/guides/rel_notes/known_issues.rst
+++ b/doc/guides/rel_notes/known_issues.rst
@@ -906,3 +906,26 @@ Vhost multi-queue reconnection failed with QEMU version 4.2.0 to 5.1.0
 
 **Driver/Module**:
    Virtual Device Poll Mode Driver (PMD).
+
+IAVF inserts VLAN tag incorrectly on AVX-512 Tx path
+----------------------------------------------------------------------
+
+**Description**
+   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.
+
+**Implication**
+   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.
+
+**Resolution/Workaround**:
+   Set the parameter `--force-max-simd-bitwidth` as 64/128/256 to avoid selecting AVX-512
+   Tx path
+
+**Affected Environment/Platform**:
+   ALL.
+
+**Driver/Module**:
+   Poll Mode Driver (PMD).
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;
 
-- 
2.34.1

[PATCH v5] net/iavf: revert fix VLAN insertion

[Yiding Zhou]

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

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;
 
-- 
2.34.1

RE: [PATCH v5] net/iavf: revert fix VLAN insertion

[Zhang, Qi Z]

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