DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Richardson, Bruce" <bruce.richardson@intel.com>
To: "Wiles, Keith" <keith.wiles@intel.com>,
	"Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>
Cc: "Morten Brørup" <mb@smartsharesystems.com>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	DPDK <dev@dpdk.org>, "Olivier Matz" <olivier.matz@6wind.com>,
	"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"Adrien Mazarguil" <adrien.mazarguil@6wind.com>
Subject: Re: [dpdk-dev] [PATCH] net: introduce big and little endian types
Date: Tue, 6 Dec 2016 16:36:13 +0000	[thread overview]
Message-ID: <59AF69C657FD0841A61C55336867B5B035B53857@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <D1B9E864-EE9F-424D-8EE4-D62AC3B4FE90@intel.com>

> -----Original Message-----
> From: Wiles, Keith
> Sent: Tuesday, December 6, 2016 4:32 PM
> To: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: Morten Brørup <mb@smartsharesystems.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; DPDK <dev@dpdk.org>; Olivier Matz
> <olivier.matz@6wind.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Adrien
> Mazarguil <adrien.mazarguil@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH] net: introduce big and little endian types
> 
> 
> > On Dec 6, 2016, at 10:28 AM, Nélio Laranjeiro
> <nelio.laranjeiro@6wind.com> wrote:
> >
> > Hi all,
> >
> > On Tue, Dec 06, 2016 at 04:34:07PM +0100, Morten Brørup wrote:
> >> Hi all,
> >>
> >> Being a big fan of strong typing, I really like the concept of
> >> explicit endian types. Especially if type mismatches can be caught at
> >> compile time.
> >
> > +1,
> >
> >> However, I think it is too late! That train left the station when the
> >> rest of the world - including libraries and headers that might be
> >> linked with a DPDK application - decided to use implicit big endian
> >> types for network protocols, and has been doing so for decades. And,
> >> with all respect, I don't think the DPDK community has the momentum
> >> required to change this tradition outside the community.
> >
> > I don't think, those types can be use from now on to help new API to
> > expose explicitly the type they are handling.  For older ones, it can
> > come in a second step, even if there are not so numerous.  Only few of
> > them touches the network types.
> >
> >> Furthermore: If not enforced throughout DPDK (and beyond), it might
> >> confuse more than it helps.
> >
> > The current situation is more confusing,  nobody at any layer can rely
> > on a precise information, at each function entry we need to verify if
> > the callee has already handled the job.  The only solution is to
> > browse the code to have this information.
> >
> > Think about any function manipulating network headers (like flow
> > director or rte_flow) from the API down to the PMD, it may take a lot
> > of time to know at the end if the data is CPU or network ordered, with
> > those types it takes less than a second.
> 
> Still Documentation should handle this problem without code and ABI
> changes.
> 

While my additional suggestion of compiler-enforced endian correctness may break the ABI (though even that is not certain since parameters would be the same size, just from a compiler syntax analysis side the result would be different, I think), if I'm reading it correctly, Neilo's original suggestion of just using typedefs for big or little endian won't affect the ABI as the underlying types are exactly the same as before. Only the type name has changed to document for the user the endianness the expected data is to be. In effect, the original suggestion is a documentation patch - just the code is the documentation.

Regards,
/Bruce


  reply	other threads:[~2016-12-06 16:36 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-09 15:04 Nelio Laranjeiro
2016-12-05 10:09 ` Ananyev, Konstantin
2016-12-05 12:06   ` Nélio Laranjeiro
2016-12-06 11:23     ` Ananyev, Konstantin
2016-12-06 11:55       ` Bruce Richardson
2016-12-06 12:41         ` Ananyev, Konstantin
2016-12-06 13:34           ` Bruce Richardson
2016-12-06 14:45             ` Ananyev, Konstantin
2016-12-06 14:56               ` Wiles, Keith
2016-12-06 15:34                 ` Morten Brørup
2016-12-06 16:28                   ` Nélio Laranjeiro
2016-12-06 16:31                     ` Wiles, Keith
2016-12-06 16:36                       ` Richardson, Bruce [this message]
2016-12-06 17:00                     ` Ananyev, Konstantin
2016-12-06 17:29                       ` Neil Horman
2016-12-06 13:14         ` Nélio Laranjeiro
2016-12-06 13:30           ` Bruce Richardson
2016-12-06 14:06     ` Wiles, Keith
2016-12-08  9:30 ` Nélio Laranjeiro
2016-12-08 13:59   ` Wiles, Keith
2016-12-08 16:06     ` Thomas Monjalon
2016-12-08 15:07   ` Neil Horman
2016-12-08 15:10     ` Ananyev, Konstantin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=59AF69C657FD0841A61C55336867B5B035B53857@IRSMSX103.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=keith.wiles@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=mb@smartsharesystems.com \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=olivier.matz@6wind.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).