* [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion.
@ 2015-02-11 6:12 xuelin.shi
2015-02-12 11:17 ` Bruce Richardson
0 siblings, 1 reply; 3+ messages in thread
From: xuelin.shi @ 2015-02-11 6:12 UTC (permalink / raw)
To: thomas.monjalon; +Cc: dev
From: Xuelin Shi <xuelin.shi@freescale.com>
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 <xuelin.shi@freescale.com>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion. 2015-02-11 6:12 [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion xuelin.shi @ 2015-02-12 11:17 ` Bruce Richardson 2015-02-13 3:42 ` Xuelin Shi 0 siblings, 1 reply; 3+ messages in thread From: Bruce Richardson @ 2015-02-12 11:17 UTC (permalink / raw) To: xuelin.shi; +Cc: dev On Wed, Feb 11, 2015 at 02:12:59PM +0800, xuelin.shi@freescale.com wrote: > From: Xuelin Shi <xuelin.shi@freescale.com> > > 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 <xuelin.shi@freescale.com> > --- > 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 <stdint.h> #include <stdlib.h> #include <rte_branch_prediction.h> +#include <rte_byteorder.h> #include <rte_memory.h> #include <rte_common.h> #include <rte_common_vect.h> @@ -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; ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion. 2015-02-12 11:17 ` Bruce Richardson @ 2015-02-13 3:42 ` Xuelin Shi 0 siblings, 0 replies; 3+ messages in thread From: Xuelin Shi @ 2015-02-13 3:42 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev Hi, Needs more consideration. RTE_LPM_VALID_EXT_ENTRY_BITMASK is defined as 0x0030, also little endian assumed. 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. > > On Wed, Feb 11, 2015 at 02:12:59PM +0800, xuelin.shi@freescale.com wrote: > > From: Xuelin Shi <xuelin.shi@freescale.com> > > > > 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 <xuelin.shi@freescale.com> > > --- > > 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 <stdint.h> > #include <stdlib.h> > #include <rte_branch_prediction.h> > +#include <rte_byteorder.h> > #include <rte_memory.h> > #include <rte_common.h> > #include <rte_common_vect.h> > @@ -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; ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-02-13 3:42 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-02-11 6:12 [dpdk-dev] [PATCH] librte_lpm: use field access instead of type conversion xuelin.shi 2015-02-12 11:17 ` Bruce Richardson 2015-02-13 3:42 ` Xuelin Shi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).