Re: [dpdk-dev] [PATCH] net: introduce big and little endian types

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

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, December 8, 2016 3:07 PM
> To: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; Wiles,
> Keith <keith.wiles@intel.com>; Morten Brørup <mb@smartsharesystems.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>
> Subject: Re: [PATCH] net: introduce big and little endian types
> 
> On Thu, Dec 08, 2016 at 10:30:05AM +0100, Nélio Laranjeiro wrote:
> > Hi all,
> >
> > Following previous discussions, I would like to gather requirements for
> > v2, currently we have:
> >
> > 1. Introduction of new typedefs.
> > 2. Modification of network headers.
> > 3. Modification of rte_*_to_*() functions.
> >
> > Point 1. seems not to be an issue, everyone seems to agree on the fact
> > having those types could help to document some parts of the code.
> >
> No objection here
> 
> > Point 2. does not cause any ABI change as it is only a documentation
> > commit, not sure if anyone disagrees about this.
> >
> I have an objection here, and I think it was stated by others previously.  While
> its fine to offer endian encoded types so that developers can use them
> expediently, I don't like the idea of coding them into network headers
> specifically.  I assert that because network headers represent multiple views of
> network data (both network byte order if the data is taken off the wire and cpu
> byte order if its translated.  To implement such a network header change
> efficiently what you would need is something like the following:
> 
> struct rte_ip_network_hdr {
> 	rte_le_u32 dst;
> 	rte_le_u32 src;
> 	...
> };
> 
> struct rte_ip_cpu_hdr {
> 	rte_cpu_u32 dst;
> 	rte_cpu_u32 src;
> 	...
> };
> 
> where rte_cpu_* is defined to a big endian or little endian type based on the
> cpu being targeted.
> 
> Then of course you need to define translation macros to do all the appropriate
> conversions convieniently (or you need to do specific translations on the
> network byte order as needed, which may lead to lots of repeated conversions).
> Regardless, this seems to be unscalable. Endian types are the sort of thing that
> you should only use sparingly, not by default.

+1

> 
> > Point 3. documentation commit most people are uncomfortable with.
> > I propose to drop it from v2.
> >
> > Any objection to this plan?
> >
> > --
> > Nélio Laranjeiro
> > 6WIND
> >