From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from na01-bn1-obe.outbound.protection.outlook.com (mail-bn1on0142.outbound.protection.outlook.com [157.56.110.142]) by dpdk.org (Postfix) with ESMTP id 9041E9AEC for ; Fri, 13 Feb 2015 04:42:30 +0100 (CET) Received: from CY1PR0301MB0684.namprd03.prod.outlook.com (25.160.158.155) by CY1PR0301MB0681.namprd03.prod.outlook.com (25.160.158.151) with Microsoft SMTP Server (TLS) id 15.1.75.20; Fri, 13 Feb 2015 03:42:28 +0000 Received: from CY1PR0301MB0684.namprd03.prod.outlook.com ([25.160.158.155]) by CY1PR0301MB0684.namprd03.prod.outlook.com ([25.160.158.155]) with mapi id 15.01.0075.002; Fri, 13 Feb 2015 03:42:28 +0000 From: Xuelin Shi To: Bruce Richardson Thread-Topic: [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion. Thread-Index: AQHQRrWKNEvxd4tb9Eag948YJx4Yb5zt6RuA Date: Fri, 13 Feb 2015 03:42:28 +0000 Message-ID: References: <1423635179-24903-1-git-send-email-xuelin.shi@freescale.com> <20150212111745.GA10216@bricha3-MOBL3> In-Reply-To: <20150212111745.GA10216@bricha3-MOBL3> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [123.151.195.51] authentication-results: intel.com; dkim=none (message not signed) header.d=none; x-microsoft-antispam: BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB0681; x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:; SRVR:CY1PR0301MB0681; x-forefront-prvs: 0486A0CB86 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(13464003)(24454002)(51704005)(164054003)(54356999)(99286002)(2950100001)(110136001)(86362001)(92566002)(19580405001)(76176999)(2656002)(77156002)(76576001)(74316001)(66066001)(102836002)(106116001)(87936001)(46102003)(33656002)(62966003)(50986999)(19580395003)(122556002)(2900100001)(40100003); DIR:OUT; SFP:1102; SCL:1; SRVR:CY1PR0301MB0681; H:CY1PR0301MB0684.namprd03.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Feb 2015 03:42:28.1825 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0301MB0681 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: Fri, 13 Feb 2015 03:42:31 -0000 Hi, Needs more consideration. RTE_LPM_VALID_EXT_ENTRY_BITMASK is defined as 0x0030, also little endian as= sumed. Seems like the struct bit field also need some position conversion. I would like to send v2 patch to fix that. Thanks, Shi Xuelin > -----Original Message----- > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Thursday, February 12, 2015 19:18 > To: Shi Xuelin-B29237 > Cc: thomas.monjalon@6wind.com; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] librte_lpm: use field access instead of > type conversion. >=20 > 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 =3D (uint16_t)tbl_entry; > > next_hop =3D (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 =3D (ip >> 8); > > uint16_t tbl_entry; > > + struct rte_lpm_tbl8_entry *entry; > > > > /* DEBUG: Check user input arguments. */ > > RTE_LPM_RETURN_IF_TRUE(((lpm =3D=3D NULL) || (next_hop =3D=3D NULL)), > > -EINVAL); @@ -290,8 +291,10 @@ rte_lpm_lookup(struct rte_lpm *lpm, > uint32_t ip, uint8_t *next_hop) > > tbl_entry =3D *(const uint16_t *)&lpm->tbl8[tbl8_index]; > > } > > > > - *next_hop =3D (uint8_t)tbl_entry; > > - return (tbl_entry & RTE_LPM_LOOKUP_SUCCESS) ? 0 : -ENOENT; > > + entry =3D (struct rte_lpm_tbl8_entry *)&tbl_entry; > > + *next_hop =3D 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? >=20 > /Bruce >=20 > 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 =3D (uint8_t)ip + > ((uint8_t)tbl_entry * > RTE_LPM_TBL8_GROUP_NUM_ENTRIES); >=20 > - tbl_entry =3D *(const uint16_t *)&lpm->tbl8[tbl8_index]; > + tbl_entry =3D rte_cpu_to_le_16( > + *(const uint16_t > + *)&lpm->tbl8[tbl8_index]); > } >=20 > *next_hop =3D (uint8_t)tbl_entry;