From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id AEA8F7F1C for ; Thu, 12 Feb 2015 12:17:50 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 12 Feb 2015 03:13:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,565,1418112000"; d="scan'208";a="665349286" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.46]) by fmsmga001.fm.intel.com with SMTP; 12 Feb 2015 03:17:45 -0800 Received: by (sSMTP sendmail emulation); Thu, 12 Feb 2015 11:17:45 +0025 Date: Thu, 12 Feb 2015 11:17:45 +0000 From: Bruce Richardson To: xuelin.shi@freescale.com Message-ID: <20150212111745.GA10216@bricha3-MOBL3> References: <1423635179-24903-1-git-send-email-xuelin.shi@freescale.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423635179-24903-1-git-send-email-xuelin.shi@freescale.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion. 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, 12 Feb 2015 11:17:51 -0000 On Wed, Feb 11, 2015 at 02:12:59PM +0800, xuelin.shi@freescale.com wrote: > From: Xuelin Shi > > struct tbl_entry{ > uint8_t next_hop; > uint8_t valid :1; > uint8_t valid_group :1; > uint8_t depth :6 > } > uint16_t tbl = (uint16_t)tbl_entry; > next_hop = (uint8_t)tbl; > > next_hop cannot get the correct value of the field > if the cpu arch is BIG_ENDIAN. > > change it to field access. > > Signed-off-by: Xuelin Shi > --- > lib/librte_lpm/rte_lpm.h | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h > index 586300b..1af150c 100644 > --- a/lib/librte_lpm/rte_lpm.h > +++ b/lib/librte_lpm/rte_lpm.h > @@ -273,6 +273,7 @@ rte_lpm_lookup(struct rte_lpm *lpm, uint32_t ip, uint8_t *next_hop) > { > unsigned tbl24_index = (ip >> 8); > uint16_t tbl_entry; > + struct rte_lpm_tbl8_entry *entry; > > /* DEBUG: Check user input arguments. */ > RTE_LPM_RETURN_IF_TRUE(((lpm == NULL) || (next_hop == NULL)), -EINVAL); > @@ -290,8 +291,10 @@ rte_lpm_lookup(struct rte_lpm *lpm, uint32_t ip, uint8_t *next_hop) > tbl_entry = *(const uint16_t *)&lpm->tbl8[tbl8_index]; > } > > - *next_hop = (uint8_t)tbl_entry; > - return (tbl_entry & RTE_LPM_LOOKUP_SUCCESS) ? 0 : -ENOENT; > + entry = (struct rte_lpm_tbl8_entry *)&tbl_entry; > + *next_hop = entry->next_hop; > + > + return (entry->valid) ? 0 : -ENOENT; > } > > /** > -- > 1.9.1 > I've run a quick test using "lpm_autotest" inside the test app, and on my (Intel) platform, this patch has a small (but none-the-less significant) performance regression. How about the below as an alternative fix? /Bruce diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index 586300b..de6f1cb 100644 --- a/lib/librte_lpm/rte_lpm.h +++ b/lib/librte_lpm/rte_lpm.h @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -287,7 +288,8 @@ rte_lpm_lookup(struct rte_lpm *lpm, uint32_t ip, uint8_t *next_hop) unsigned tbl8_index = (uint8_t)ip + ((uint8_t)tbl_entry * RTE_LPM_TBL8_GROUP_NUM_ENTRIES); - tbl_entry = *(const uint16_t *)&lpm->tbl8[tbl8_index]; + tbl_entry = rte_cpu_to_le_16( + *(const uint16_t *)&lpm->tbl8[tbl8_index]); } *next_hop = (uint8_t)tbl_entry;