From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 37FAEA00E6 for ; Thu, 16 May 2019 18:36:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9758B5A6A; Thu, 16 May 2019 18:36:49 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 13FF95942 for ; Thu, 16 May 2019 18:36:47 +0200 (CEST) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 May 2019 09:36:47 -0700 X-ExtLoop1: 1 Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.96]) by orsmga008.jf.intel.com with SMTP; 16 May 2019 09:36:45 -0700 Received: by (sSMTP sendmail emulation); Thu, 16 May 2019 17:36:44 +0100 Date: Thu, 16 May 2019 17:36:43 +0100 From: Bruce Richardson To: Stephen Hemminger Cc: dev@dpdk.org Message-ID: <20190516163643.GC632@bricha3-MOBL.ger.corp.intel.com> References: <20190515221952.21959-1-stephen@networkplumber.org> <20190515221952.21959-5-stephen@networkplumber.org> <20190516160337.GA632@bricha3-MOBL.ger.corp.intel.com> <20190516090652.5ad965f4@hermes.lan> <20190516160745.GB632@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190516160745.GB632@bricha3-MOBL.ger.corp.intel.com> User-Agent: Mutt/1.11.4 (2019-03-13) Subject: Re: [dpdk-dev] [RFC 4/4] net/ether: use bitops to speedup comparison 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, May 16, 2019 at 05:07:45PM +0100, Bruce Richardson wrote: > On Thu, May 16, 2019 at 09:06:52AM -0700, Stephen Hemminger wrote: > > On Thu, 16 May 2019 17:03:37 +0100 > > Bruce Richardson wrote: > > > > > On Wed, May 15, 2019 at 03:19:52PM -0700, Stephen Hemminger wrote: > > > > Using bit operations like or and xor is faster than a loop > > > > on all architectures. Really just explicit unrolling. > > > > > > > > Similar cast to uint16 unaligned is already done in > > > > other functions here. > > > > > > > > Signed-off-by: Stephen Hemminger > > > > --- > > > > lib/librte_net/rte_ether.h | 17 +++++++---------- > > > > 1 file changed, 7 insertions(+), 10 deletions(-) > > > > > > > Rather than casting to unaligned values, which gives compiler warnings in > > > some cases, I believe we should just mark the ethernet addresses as always > > > being 2-byte aligned and simplify things. [unless we have a good use case > > > where we won't have 2-byte alignment???]. > > > > > > See patch: http://patches.dpdk.org/patch/53482/ > > > > > > Regards, > > > /Bruce > > > > I agree. Then you could also remove the unaligned_uint16_t that > > already exists in rte_ether.h > > > > Do you want me to put your patch in my series? > > Sure, feel free. > I've just found another problem in this area. Compiling l2fwd with gcc 9, I get: main.c:164:54: warning: taking address of packed member of ‘struct ether_hdr’ may result in an unaligned pointer value [-Waddress-of-packed-member] 164 | ether_addr_copy(&l2fwd_ports_eth_addr[dest_portid], ð->s_addr); | Looking at some of the structures, it appears that not all the packet attributes may be necessary. For example, while AFAIK there are not absolute guarantees about structure padding, for all compilers I remember using the following changes are safe: diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h index 8090b7c01..7d9f34791 100644 --- a/lib/librte_net/rte_ether.h +++ b/lib/librte_net/rte_ether.h @@ -57,7 +57,7 @@ extern "C" { struct ether_addr { /** Addr bytes in tx order */ uint8_t addr_bytes[ETHER_ADDR_LEN] __rte_aligned(2); -} __attribute__((__packed__)); +}; #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */ #define ETHER_GROUP_ADDR 0x01 /**< Multicast or broadcast Eth. address. */ @@ -273,7 +273,7 @@ struct ether_hdr { struct ether_addr d_addr; /**< Destination address. */ struct ether_addr s_addr; /**< Source address. */ uint16_t ether_type; /**< Frame type. */ -} __attribute__((__packed__)); +}; /** * Ethernet VLAN Header. @@ -283,7 +283,7 @@ struct ether_hdr { struct vlan_hdr { uint16_t vlan_tci; /**< Priority (3) + CFI (1) + Identifier Code (12) */ uint16_t eth_proto;/**< Ethernet type of encapsulated frame. */ -} __attribute__((__packed__)); +}; /** * VXLAN protocol header. I think we therefore should consider removing those packed attributes to avoid application warnings. /Bruce