[dpdk-dev] [PATCH] net/iavf: fix VLAN insert issue

[Li, Xiaoyun]

> -----Original Message-----
> From: Xing, Beilei <beilei.xing at intel.com>
> Sent: Thursday, February 4, 2021 14:36
> To: Li, Xiaoyun <xiaoyun.li at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>;
> dev at dpdk.org
> Subject: RE: [PATCH] net/iavf: fix VLAN insert issue
> 
> 
> 
> > -----Original Message-----
> > From: Li, Xiaoyun <xiaoyun.li at intel.com>
> > Sent: Thursday, February 4, 2021 11:11 AM
> > To: Wu, Jingjing <jingjing.wu at intel.com>; Xing, Beilei
> > <beilei.xing at intel.com>; dev at dpdk.org
> > Cc: Li, Xiaoyun <xiaoyun.li at intel.com>
> > Subject: [PATCH] net/iavf: fix VLAN insert issue
> >
> > The new VIRTCHNL_VF_OFFLOAD_VLAN_V2 capability allows PF to set the
> > location of TX VLAN insertion.
> >
> > So VF needs to insert VLAN tag according to the location flags.
> >
> > Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li at intel.com>
> > ---
> >  drivers/net/iavf/iavf_rxtx.c | 45
> > +++++++++++++++++++++++++++++++-----
> >  drivers/net/iavf/iavf_rxtx.h |  3 +++
> >  2 files changed, 42 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/iavf/iavf_rxtx.c
> > b/drivers/net/iavf/iavf_rxtx.c index 3d471d9acc..af5a28d84d 100644
> > --- a/drivers/net/iavf/iavf_rxtx.c
> > +++ b/drivers/net/iavf/iavf_rxtx.c
> >
> >  /* Check if the context descriptor is needed for TX offloading */
> > static inline uint16_t -iavf_calc_context_desc(uint64_t flags)
> > +iavf_calc_context_desc(uint64_t flags, uint8_t vlan_flag)
> >  {
> > -	static uint64_t mask = PKT_TX_TCP_SEG;
> > -
> > -	return (flags & mask) ? 1 : 0;
> > +	if (flags & PKT_TX_TCP_SEG)
> > +		return 1;
> > +	if (flags & PKT_TX_VLAN_PKT &&
> > +	    vlan_flag & IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2)
> > +		return 1;
> > +	return 0;
> 
> How about merging if condition?
> 
> if ((flags & PKT_TX_TCP_SEG) ||
>     (flags & PKT_TX_VLAN_PKT &&
>      vlan_flag & IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2))
> 	 return 1;
> else
> 	 return 0;
> 
Both are OK. In case there are new offloads in the future which needs context desc. The previous code looks more clear which offload takes effect?

> >  }
> >