From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 390E32B9E for ; Thu, 25 Feb 2016 23:00:16 +0100 (CET) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 92B6DC09FA87; Thu, 25 Feb 2016 22:00:15 +0000 (UTC) Received: from aconole-fed23 (dhcp-25-213.bos.redhat.com [10.18.25.213]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1PM0Eru019623 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 25 Feb 2016 17:00:14 -0500 From: Aaron Conole To: Bruce Richardson References: <1456426121-21423-1-git-send-email-aconole@redhat.com> <1456426121-21423-2-git-send-email-aconole@redhat.com> <20160225213054.GA14936@bricha3-MOBL3> Date: Thu, 25 Feb 2016 17:00:13 -0500 In-Reply-To: <20160225213054.GA14936@bricha3-MOBL3> (Bruce Richardson's message of "Thu, 25 Feb 2016 21:30:54 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 1/8] lpm: Fix pointer aliasing issues X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Feb 2016 22:00:16 -0000 Hi Bruce, Bruce Richardson writes: > On Thu, Feb 25, 2016 at 01:48:34PM -0500, Aaron Conole wrote: >> The current implementation attempts to use a uint16_t to alias the lpm table >> structures. Such aliasing can break optimizer performance. This patch uses >> union type indirection and adds static inline functions for performing the >> aliasing. >> >> Signed-off-by: Aaron Conole >> --- >> lib/librte_lpm/rte_lpm.h | 53 +++++++++++++++++++++++++++++++++++++----------- >> 1 file changed, 41 insertions(+), 12 deletions(-) >> >> diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h >> index c299ce2..eae6ff1 100644 >> --- a/lib/librte_lpm/rte_lpm.h >> +++ b/lib/librte_lpm/rte_lpm.h >> @@ -157,6 +157,33 @@ struct rte_lpm { >> }; >> >> /** >> + * Convert from tbl_entry types to integer types >> + */ >> +static inline uint16_t >> +rte_lpm_tbl24_entry_to_uint16(const struct rte_lpm_tbl24_entry *entry) >> +{ >> + union { >> + uint16_t i; >> + struct rte_lpm_tbl24_entry s; >> + } tbl_entry_u; >> + >> + tbl_entry_u.s = *entry; >> + return tbl_entry_u.i; >> +} >> + >> +static inline uint16_t >> +rte_lpm_tbl8_entry_to_uint16(const struct rte_lpm_tbl8_entry *entry) >> +{ >> + union { >> + uint16_t i; >> + struct rte_lpm_tbl8_entry s; >> + } tbl_entry_u; >> + >> + tbl_entry_u.s = *entry; >> + return tbl_entry_u.i; >> +} >> + > > These two new functions could be reduced to one with the help of patch: > http://dpdk.org/dev/patchwork/patch/9087/ > > Anyone care to go back and review or ack that patch for me and simplify all > the lpm code just that little bit? I definitely ack that patch; I also want to apply it in my local testing setup and test a bit so you can add my Tested-by, but it looks sane on the surface. Thanks so much for the review! > /Bruce