Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ
> Sent: Friday, November 28, 2014 9:37 AM
> To: Liu, Jijiang; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields
> 
> Hi Jijiang,
> 
> On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> >  /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. */
> >  #define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> >
> >  /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. */
> >  #define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> 
> The description still does not match what we discussed. Either we
> have PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum
> offload", or  "packet is IPv4". I prefer the second one, but whatever
> the choice is, the comments must be coherent.
> 
> > -#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
> > +/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
> > +#define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
> > +
> > +/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
> > +#define PKT_TX_OUTER_IPV6    (1ULL << 59)
> 
> I think we should have the same flags with the same meanings for
> inner and outer:
> 
> - PKT_TX_IPV4, PKT_TX_IP_CKSUM, PKT_TX_IPV6
> - PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6

Good point.
I still think: should we squeeze each of these triple in 2 bits?
Same as we doing for L4 checksum flags:

#define PKT_TX_IP_CKSUM   (1ULL << X)
#define PKT_TX_IPV6                (2ULL << X)
#define PKT_TX_IPV4                (3ULL << X)

Same of outer flags.
Of course it means that they need to be mutually exclusive.

> 
> In your patch there is no PKT_TX_OUTER_IPV4 flag.
> 
> Regards,
> Olivier