> -----Original Message-----
> From: Xing, Beilei <beilei.x...@intel.com>
> Sent: Thursday, February 4, 2021 14:36
> To: Li, Xiaoyun <xiaoyun...@intel.com>; Wu, Jingjing <jingjing...@intel.com>;
> dev@dpdk.org
> Subject: RE: [PATCH] net/iavf: fix VLAN insert issue
>
>
>
> > -----Original Message-----
> > From: Li, Xiaoyun <xiaoyun...@intel.com>
> > Sent: Thursday, February 4, 2021 11:11 AM
> > To: Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei
> > <beilei.x...@intel.com>; dev@dpdk.org
> > Cc: Li, Xiaoyun <xiaoyun...@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...@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?
> > }
> >
