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 739B4A00E6 for ; Thu, 16 May 2019 19:05:04 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 766F95F1A; Thu, 16 May 2019 19:05:03 +0200 (CEST) Received: from mail-pg1-f193.google.com (mail-pg1-f193.google.com [209.85.215.193]) by dpdk.org (Postfix) with ESMTP id 36CA15F14 for ; Thu, 16 May 2019 19:05:02 +0200 (CEST) Received: by mail-pg1-f193.google.com with SMTP id t22so1852234pgi.10 for ; Thu, 16 May 2019 10:05:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=HMbIc96cqpE1lKDTNynP70hLJ6Guf4Pn9M+Dc3yHwD0=; b=TmG57m+vihz/j2QZrBVG9IjAyDh2s2LaScyg9eybC9FmQOb90/qncTETvLdFK3BAsF 4I6o19v7R2ZAAsOcFuINTz13AJfcPG/Uo/s0DJE0JXzTziS8x0Dc7CVf/Hf7AEz38qDq P91I0e0CKcA/0N+K2+aAyTnFsoea+jmJwSUDqhQCwAD03q5Gbal+PDICygP91dJ8xLgZ OwMWkEbyea3bfi2wX60t9Cg+6kL1L6wgffCfwYvKN28IEcFvLi1jUa4FCeuz4zK+bjR5 9S3jY/MCJEFb7WThpAQtahbkaC0mrAew8tWLLfJK2cd1Xai3jy0Iy69AeKzHLdrlVA1N auqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=HMbIc96cqpE1lKDTNynP70hLJ6Guf4Pn9M+Dc3yHwD0=; b=WyHV+DSp91ezm/h4j8D8KNibaXLTWbVCAh5Z2F0ZHCPFyKcQ6gTq77YPBylnOdBv0Q /DHkjvcNnZKVdiHOHU8f/4tq8v19Zq/a4hDHuavIO1/dQXu+t2fHLGQuw5QlM3GDg0Rn /HeGOq8eDmAEzw0RTtwVqNuExorntk/kjKW1qmpcMnDCMBnsSDTOT5IgspPqI/+tlwkl RF99j47klQG4c9OEnMdaM1UQcOaUnKSc30ArX/l7G1YG70Pr5mKcvuadS2YMYbJFJUTx aBc9yZngEmnu6G6DuH90NflSw3EKPkfycGNA0Ol06XeeVDzPHpdwNU9ztsoZN4U7zDqg 1VSw== X-Gm-Message-State: APjAAAXZt+q2BqVRaSRgDVRL83s+N7GI9Ep9IgvQ4sviin54kIn8vali deeOE4DM7YXiRekcjm4Hwv6Fmp4raDk= X-Google-Smtp-Source: APXvYqzXY7dReB92Hgqg4zuLkR4ptYoE9UkQEGenxeQUB47mY6APR2E0o1YUY67JQeNz1ZdMN5GefA== X-Received: by 2002:a63:e52:: with SMTP id 18mr52411904pgo.3.1558026301183; Thu, 16 May 2019 10:05:01 -0700 (PDT) Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id a9sm10657357pgw.72.2019.05.16.10.05.00 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 16 May 2019 10:05:01 -0700 (PDT) Date: Thu, 16 May 2019 10:04:54 -0700 From: Stephen Hemminger To: Bruce Richardson Cc: dev@dpdk.org Message-ID: <20190516100454.36e7bc88@hermes.lan> In-Reply-To: <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> <20190516163643.GC632@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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, 16 May 2019 17:36:43 +0100 Bruce Richardson wrote: > 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: =20 > > > On Thu, 16 May 2019 17:03:37 +0100 > > > Bruce Richardson wrote: > > > =20 > > > > On Wed, May 15, 2019 at 03:19:52PM -0700, Stephen Hemminger wrote: = =20 > > > > > Using bit operations like or and xor is faster than a loop > > > > > on all architectures. Really just explicit unrolling. > > > > >=20 > > > > > Similar cast to uint16 unaligned is already done in > > > > > other functions here. > > > > >=20 > > > > > Signed-off-by: Stephen Hemminger > > > > > --- > > > > > lib/librte_net/rte_ether.h | 17 +++++++---------- > > > > > 1 file changed, 7 insertions(+), 10 deletions(-) > > > > > =20 > > > > Rather than casting to unaligned values, which gives compiler warni= ngs 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 us= e case > > > > where we won't have 2-byte alignment???]. > > > >=20 > > > > See patch: http://patches.dpdk.org/patch/53482/ > > > >=20 > > > > Regards, > > > > /Bruce =20 > > >=20 > > > I agree. Then you could also remove the unaligned_uint16_t that > > > already exists in rte_ether.h > > >=20 > > > Do you want me to put your patch in my series? =20 > >=20 > > Sure, feel free. > > =20 >=20 > I've just found another problem in this area. Compiling l2fwd with gcc 9,= I > get: >=20 > main.c:164:54: warning: taking address of packed member of =E2=80=98struc= t ether_hdr=E2=80=99 may result in an unaligned pointer value [-Waddress-of= -packed-member] > 164 | ether_addr_copy(&l2fwd_ports_eth_addr[dest_portid], ð->s_addr= ); > |=20 >=20 > 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: >=20 > 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__)); > +}; >=20 > #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. = */ > #define ETHER_GROUP_ADDR 0x01 /**< Multicast or broadcast Eth. add= ress. */ > @@ -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__)); > +}; >=20 > /** > * 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__)); > +}; >=20 > /** > * VXLAN protocol header. >=20 > I think we therefore should consider removing those packed attributes to > avoid application warnings. >=20 > /Bruce Agree if structure is naturally packed, adding packed attribute is a mistak= e.