From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aaron Conole Subject: Re: [PATCH 1/8] lpm: Fix pointer aliasing issues Date: Thu, 25 Feb 2016 17:00:13 -0500 Message-ID: References: <1456426121-21423-1-git-send-email-aconole@redhat.com> <1456426121-21423-2-git-send-email-aconole@redhat.com> <20160225213054.GA14936@bricha3-MOBL3> Mime-Version: 1.0 Content-Type: text/plain Cc: dev@dpdk.org To: Bruce Richardson 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) In-Reply-To: <20160225213054.GA14936@bricha3-MOBL3> (Bruce Richardson's message of "Thu, 25 Feb 2016 21:30:54 +0000") List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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