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

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang
> Sent: Friday, November 28, 2014 10:33 AM
> To: Olivier MATZ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields
> 
> Hi Olivier,
> 
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > Sent: Friday, November 28, 2014 5:37 PM
> > To: Liu, Jijiang; dev at 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.
> >
> I agree.
>  "packet is IPv4" is ok for me, too.
> The comment "Required for L4 checksum offload or TSO" is not added by me,  I should have thought you added it during developing
> TSO.
> Anyway, we came to an agreement for  PKT_TX_IPV6/4 meaning, I will change  the two flags comments.
> 
> 

Well, I still prefer them to be mutually exclusive.
Even better, if we can squeeze these 3 flags into 2 bits.
Would save us 2 bits, plus might be handy, as in the PMD you can do:

switch (ol_flags & TX_L3_MASK) {
    case TX_IPV4:
       ...
       break;
    case TX_IPV6:
       ...
       break;
    case TX_IP_CKSUM:
       ...
       break;
}

For the upper layer, I think there would be no big difference, what ways we will choose.

Konstantin