[dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Thursday, December 04, 2014 1:48 PM
> To: Ananyev, Konstantin; Zhang, Helin; Liu, Jijiang; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM
> 
> Hi,
> 
> On 12/04/2014 11:19 AM, Ananyev, Konstantin wrote:
> >>> 1/ (Jijiang's patch)
> >>> PKT_TX_IP_CKSUM  /* packet is IPv4, and we want hw cksum */
> >>> PKT_TX_IPV6      /* packet is IPv6 */
> >>> PKT_TX_IPV4      /* packet is IPv4, and we don't want hw cksum */
> >>>
> >>> with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive
> >>>
> >>> and
> >>>
> >>> 2/
> >>> PKT_TX_IP_CKSUM  /* we want hw IP cksum */
> >>> PKT_TX_IPV6      /* packet is IPv6 */
> >>> PKT_TX_IPV4      /* packet is IPv4 */
> >> There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the
> >> same bit of 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware
> >> checksum offload is needed or not.
> >
> > Yes, 'PKT_TX_IPV4_CSUM is an alias to PKT_TX_IP_CKSUM  and we are going to remove it.
> >
> >> It seems that we do not need 'PKT_TX_IPV6_CSUM'.
> >
> > No one even planned it.
> >
> >> 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I guess
> >> other features should not be contained in it, according to its name.
> >>
> >> So here I got the option 3:
> >> PKT_TX_IPV4_CKSUM  /* we want hw IPv4 cksum */
> >> PKT_TX_IPV6      /* packet is IPv6 */
> >> PKT_TX_IPV4      /* packet is IPv4 */
> >
> > Hmm, and how this is different from what we have now in the Jijiang's patch?
> > Except that you renamed PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM?
> 
> I think it's more like solution 2 with a renaming. And it is more
> coherent to always have "IPV4" on all flag names.

About renaming PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM: I don't mind.
But that means we would have to update all the drivers and apps that using PKT_TX_IP_CKSUM.
Again there are customers that using that flag in their apps right now.
They would have to change it too.
For me -  too much hassle for such small nit.

Konstantin

> 
> Regards,
> Olivier