Skip to contentSkip to conversation

l3fwd and lpm cleanups

PATCH In review Stephen Hemminger 2 participants 7 messages Jan 26, 2020 – Feb 16, 2020

lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover l3fwd and lpm cleanups

While working on l3fwd, saw some minor things that should be cleaned up.

Stephen Hemminger (5):
lpm: make ipv6 address immutable
examples/l3fwd: use RTE_DIM
examples/l3fwd: make lookup struct static examples/l3fwd: make route array constant examples/l3fwd: improve readability for destination lookup

 examples/l3fwd/l3fwd_lpm.c | 42 ++++++++++++++++++++------------------
 lib/librte_lpm/rte_lpm6.c  |  8 ++++----
 lib/librte_lpm/rte_lpm6.h  |  6 +++---
 3 files changed, 29 insertions(+), 27 deletions(-)
Signature · 1 line
2.20.1
author
#1 lore raw

PATCH 1/5 lpm: make ipv6 address immutable

Both the table setup and lookup do no modify their arguments. Therefore the parameter should be constant.

This is not actually an API breakage since programs can be recompiled without change. This is not an ABI breakage because old programs will still run.

Signed-off-by Stephen Hemminger <[email protected]>
---
 lib/librte_lpm/rte_lpm6.c | 8 ++++----
 lib/librte_lpm/rte_lpm6.h | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)
lib/librte_lpm/rte_lpm6.c +4 -4
index c46e557e2389..5ea818abd6e9 100644
@@ -854,8 +854,8 @@ simulate_add(struct rte_lpm6 *lpm, const uint8_t *masked_ip, uint8_t depth)
  * Add a route
  */
 int
-rte_lpm6_add(struct rte_lpm6 *lpm, uint8_t *ip, uint8_t depth,
-	uint32_t next_hop)
+rte_lpm6_add(struct rte_lpm6 *lpm, const uint8_t *ip, uint8_t depth,
+	     uint32_t next_hop)
 {
 	struct rte_lpm6_tbl_entry *tbl;
 	struct rte_lpm6_tbl_entry *tbl_next = NULL;

@@ -913,7 +913,7 @@ rte_lpm6_add(struct rte_lpm6 *lpm, uint8_t *ip, uint8_t depth,
  */
 static inline int
 lookup_step(const struct rte_lpm6 *lpm, const struct rte_lpm6_tbl_entry *tbl,
-		const struct rte_lpm6_tbl_entry **tbl_next, uint8_t *ip,
+		const struct rte_lpm6_tbl_entry **tbl_next, const uint8_t *ip,
 		uint8_t first_byte, uint32_t *next_hop)
 {
 	uint32_t tbl8_index, tbl_entry;

@@ -943,7 +943,7 @@ lookup_step(const struct rte_lpm6 *lpm, const struct rte_lpm6_tbl_entry *tbl,
  * Looks up an IP
  */
 int
-rte_lpm6_lookup(const struct rte_lpm6 *lpm, uint8_t *ip,
+rte_lpm6_lookup(const struct rte_lpm6 *lpm, const uint8_t *ip,
 		uint32_t *next_hop)
 {
 	const struct rte_lpm6_tbl_entry *tbl;
lib/librte_lpm/rte_lpm6.h +3 -3
index 37dfb20249a8..042991f8cdb7 100644
@@ -94,8 +94,8 @@ rte_lpm6_free(struct rte_lpm6 *lpm);
  *   0 on success, negative value otherwise
  */
 int
-rte_lpm6_add(struct rte_lpm6 *lpm, uint8_t *ip, uint8_t depth,
-		uint32_t next_hop);
+rte_lpm6_add(struct rte_lpm6 *lpm, const uint8_t *ip, uint8_t depth,
+	     uint32_t next_hop);
 
 /**
  * Check if a rule is present in the LPM table,

@@ -171,7 +171,7 @@ rte_lpm6_delete_all(struct rte_lpm6 *lpm);
  *   -EINVAL for incorrect arguments, -ENOENT on lookup miss, 0 on lookup hit
  */
 int
-rte_lpm6_lookup(const struct rte_lpm6 *lpm, uint8_t *ip, uint32_t *next_hop);
+rte_lpm6_lookup(const struct rte_lpm6 *lpm, const uint8_t *ip, uint32_t *next_hop);
 
 /**
  * Lookup multiple IP addresses in an LPM table.
Signature · 1 line
2.20.1
author
#2 lore raw

PATCH 2/5 examples/l3fwd: use RTE_DIM

In reply to Stephen Hemminger

Use the standard RTE_DIM macro to compute array size.

Signed-off-by Stephen Hemminger <[email protected]>
---
 examples/l3fwd/l3fwd_lpm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
examples/l3fwd/l3fwd_lpm.c +2 -4
index 349de2703cd1..b6802d63ba1d 100644
@@ -65,10 +65,8 @@ static struct ipv6_l3fwd_lpm_route ipv6_l3fwd_lpm_route_array[] = {
 	{{32, 1, 2, 0, 0, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0}, 48, 7},
 };
 
-#define IPV4_L3FWD_LPM_NUM_ROUTES \
-	(sizeof(ipv4_l3fwd_lpm_route_array) / sizeof(ipv4_l3fwd_lpm_route_array[0]))
-#define IPV6_L3FWD_LPM_NUM_ROUTES \
-	(sizeof(ipv6_l3fwd_lpm_route_array) / sizeof(ipv6_l3fwd_lpm_route_array[0]))
+#define IPV4_L3FWD_LPM_NUM_ROUTES RTE_DIM(ipv4_l3fwd_lpm_route_array)
+#define IPV6_L3FWD_LPM_NUM_ROUTES RTE_DIM(ipv6_l3fwd_lpm_route_array)
 
 #define IPV4_L3FWD_LPM_MAX_RULES         1024
 #define IPV4_L3FWD_LPM_NUMBER_TBL8S (1 << 8)
Signature · 1 line
2.20.1
author
#3 lore raw

PATCH 3/5 examples/l3fwd: make lookup struct static

In reply to Stephen Hemminger

The lookup structure is only used in the lpm code and does not have to be global.

Signed-off-by Stephen Hemminger <[email protected]>
---
 examples/l3fwd/l3fwd_lpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
examples/l3fwd/l3fwd_lpm.c +2 -2
index b6802d63ba1d..486f19963fc4 100644
@@ -73,8 +73,8 @@ static struct ipv6_l3fwd_lpm_route ipv6_l3fwd_lpm_route_array[] = {
 #define IPV6_L3FWD_LPM_MAX_RULES         1024
 #define IPV6_L3FWD_LPM_NUMBER_TBL8S (1 << 16)
 
-struct rte_lpm *ipv4_l3fwd_lpm_lookup_struct[NB_SOCKETS];
-struct rte_lpm6 *ipv6_l3fwd_lpm_lookup_struct[NB_SOCKETS];
+static struct rte_lpm *ipv4_l3fwd_lpm_lookup_struct[NB_SOCKETS];
+static struct rte_lpm6 *ipv6_l3fwd_lpm_lookup_struct[NB_SOCKETS];
 
 static inline uint16_t
 lpm_get_ipv4_dst_port(void *ipv4_hdr, uint16_t portid, void *lookup_struct)
Signature · 1 line
2.20.1
author
#4 lore raw

PATCH 4/5 examples/l3fwd: make route array constant

In reply to Stephen Hemminger

The initial route setup array is unmodified by the lpm code and can be made constant. This depends on earlier patch to fix the rte_lpm6 to use const.

Signed-off-by Stephen Hemminger <[email protected]>
---
 examples/l3fwd/l3fwd_lpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
examples/l3fwd/l3fwd_lpm.c +2 -2
index 486f19963fc4..30f6385419ee 100644
@@ -42,7 +42,7 @@ struct ipv6_l3fwd_lpm_route {
 };
 
 /* 198.18.0.0/16 are set aside for RFC2544 benchmarking (RFC5735). */
-static struct ipv4_l3fwd_lpm_route ipv4_l3fwd_lpm_route_array[] = {
+static const struct ipv4_l3fwd_lpm_route ipv4_l3fwd_lpm_route_array[] = {
 	{RTE_IPV4(198, 18, 0, 0), 24, 0},
 	{RTE_IPV4(198, 18, 1, 0), 24, 1},
 	{RTE_IPV4(198, 18, 2, 0), 24, 2},

@@ -54,7 +54,7 @@ static struct ipv4_l3fwd_lpm_route ipv4_l3fwd_lpm_route_array[] = {
 };
 
 /* 2001:0200::/48 is IANA reserved range for IPv6 benchmarking (RFC5180) */
-static struct ipv6_l3fwd_lpm_route ipv6_l3fwd_lpm_route_array[] = {
+static const struct ipv6_l3fwd_lpm_route ipv6_l3fwd_lpm_route_array[] = {
 	{{32, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, 48, 0},
 	{{32, 1, 2, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0}, 48, 1},
 	{{32, 1, 2, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0}, 48, 2},
Signature · 1 line
2.20.1
author
#5 lore raw

PATCH 5/5 examples/l3fwd: improve readability for destination lookup

In reply to Stephen Hemminger

The functions to lookup ipv4 and ipv6 were both using opaque pointers (void *) when they should use a typed pointer instead. The ip headers are not modified during lookup.

Get rid of unnecessary cast on the return from the function. Replace complex trigraph expression with simple if to improve readability.

Signed-off-by Stephen Hemminger <[email protected]>
---
 examples/l3fwd/l3fwd_lpm.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)
examples/l3fwd/l3fwd_lpm.c +16 -12
index 30f6385419ee..bd6e7fe03d77 100644
@@ -77,27 +77,31 @@ static struct rte_lpm *ipv4_l3fwd_lpm_lookup_struct[NB_SOCKETS];
 static struct rte_lpm6 *ipv6_l3fwd_lpm_lookup_struct[NB_SOCKETS];
 
 static inline uint16_t
-lpm_get_ipv4_dst_port(void *ipv4_hdr, uint16_t portid, void *lookup_struct)
+lpm_get_ipv4_dst_port(const struct rte_ipv4_hdr *ipv4_hdr,
+		      uint16_t portid,
+		      struct rte_lpm *ipv4_l3fwd_lookup_struct)
 {
+	uint32_t dst_ip = rte_be_to_cpu_32(ipv4_hdr->dst_addr);
 	uint32_t next_hop;
-	struct rte_lpm *ipv4_l3fwd_lookup_struct =
-		(struct rte_lpm *)lookup_struct;
 
-	return (uint16_t) ((rte_lpm_lookup(ipv4_l3fwd_lookup_struct,
-		rte_be_to_cpu_32(((struct rte_ipv4_hdr *)ipv4_hdr)->dst_addr),
-		&next_hop) == 0) ? next_hop : portid);
+	if (rte_lpm_lookup(ipv4_l3fwd_lookup_struct, dst_ip, &next_hop) == 0)
+		return next_hop;
+	else
+		return portid;
 }
 
 static inline uint16_t
-lpm_get_ipv6_dst_port(void *ipv6_hdr, uint16_t portid, void *lookup_struct)
+lpm_get_ipv6_dst_port(const struct rte_ipv6_hdr *ipv6_hdr,
+		      uint16_t portid,
+		      struct rte_lpm6 *ipv6_l3fwd_lookup_struct)
 {
+	const uint8_t *dst_ip = ipv6_hdr->dst_addr;
 	uint32_t next_hop;
-	struct rte_lpm6 *ipv6_l3fwd_lookup_struct =
-		(struct rte_lpm6 *)lookup_struct;
 
-	return (uint16_t) ((rte_lpm6_lookup(ipv6_l3fwd_lookup_struct,
-			((struct rte_ipv6_hdr *)ipv6_hdr)->dst_addr,
-			&next_hop) == 0) ?  next_hop : portid);
+	if (rte_lpm6_lookup(ipv6_l3fwd_lookup_struct, dst_ip, &next_hop) == 0)
+		return next_hop;
+	else
+		return portid;
 }
 
 static __rte_always_inline uint16_t
Signature · 1 line
2.20.1
#6 lore raw
In reply to Stephen Hemminger

26/01/2020 02:09, Stephen Hemminger:

Quoted 8 lines

While working on l3fwd, saw some minor things that should be cleaned up.

Stephen Hemminger (5):
lpm: make ipv6 address immutable
examples/l3fwd: use RTE_DIM
examples/l3fwd: make lookup struct static examples/l3fwd: make route array constant examples/l3fwd: improve readability for destination lookup

Applied (except RTE_DIM patch already applied), thanks.