From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 5963D2A66 for ; Tue, 6 Dec 2016 18:29:48 +0100 (CET) Received: from cpe-2606-a000-111b-40ed-7aac-c0ff-fec2-933b.dyn6.twc.com ([2606:a000:111b:40ed:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1cEJYf-0002u2-JT; Tue, 06 Dec 2016 12:29:40 -0500 Date: Tue, 6 Dec 2016 12:29:27 -0500 From: Neil Horman To: "Ananyev, Konstantin" Cc: =?iso-8859-1?Q?N=E9lio?= Laranjeiro , Morten =?iso-8859-1?Q?Br=F8rup?= , "Wiles, Keith" , "Richardson, Bruce" , "dev@dpdk.org" , Olivier Matz , "Lu, Wenzhuo" , Adrien Mazarguil Message-ID: <20161206172927.GA18116@hmsreliant.think-freely.org> References: <20161205120603.GL21794@autoinstall.dev.6wind.com> <2601191342CEEE43887BDE71AB9772583F0E4632@irsmsx105.ger.corp.intel.com> <20161206115502.GA12224@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583F0E46DC@irsmsx105.ger.corp.intel.com> <20161206133427.GB15416@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583F0E47E1@irsmsx105.ger.corp.intel.com> <7E1BBBA1-F384-4235-8A82-4B0D6DC0889C@intel.com> <98CBD80474FA8B44BF855DF32C47DC359EAA47@smartserver.smartshare.dk> <20161206162807.GS21794@autoinstall.dev.6wind.com> <2601191342CEEE43887BDE71AB9772583F0E4AC3@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E4AC3@irsmsx105.ger.corp.intel.com> User-Agent: Mutt/1.7.1 (2016-10-04) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH] net: introduce big and little endian types X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Dec 2016 17:29:48 -0000 On Tue, Dec 06, 2016 at 05:00:05PM +0000, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: Nélio Laranjeiro [mailto:nelio.laranjeiro@6wind.com] > > Sent: Tuesday, December 6, 2016 4:28 PM > > To: Morten Brørup > > Cc: Wiles, Keith ; Ananyev, Konstantin ; Richardson, Bruce > > ; dev@dpdk.org; Olivier Matz ; Lu, Wenzhuo ; Adrien > > Mazarguil > > Subject: Re: [dpdk-dev] [PATCH] net: introduce big and little endian types > > > > 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. > > Hmm, suppose I have such piece of code: > > struct ipv4_hdr iph; > iph.total_length = val0; > iph.packet_id = val1; > ... > iph.dst_addr = valn; > > hton_whole_ipv4_hdr_at_once(&iph); > > How I suppose to indicate via these new types that > hton_whole_ipv4_hdr_at_once() expects ipv4_hdr fields in host byte order? > Other than just putting it into the doc? > Konstantin > You aren't. Your implication is correct in that, if you have a struct that needs to store data in either cpu endian order or in network endian order, its best to leave endian tagging out of the type, because it will by definition be wrong at least 50% of the team, leading to more confusion than it rectifies That isn't to say that endian coded types don't have a place. They're often fairly useful when reading hardware registers and the like, or if a speciic bit of data is to be extracted from a network payload. But in the use case above, no, its not useful. The best idea it seems is to provide the types, as well as endian-typed and untyped variants to the endian conversion routines, so that developers can choose what works best for them. Since the conversions are all macros with casts anyway, it should be a no harm no foul addition in my mind. To be clear, I don't support the patch as it exists, but I would support a variant that just offered the types and conversion routines. Neil > > > >