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

["Zhou, YidingX" <yidingx.zhou@intel.com>]

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