[dpdk-dev] [PATCH 0/5] l3fwd and lpm cleanups

[dpdk-dev] [PATCH 0/5] l3fwd and lpm cleanups

[Stephen Hemminger]

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(-)

-- 
2.20.1

[dpdk-dev] [PATCH 1/5] lpm: make ipv6 address immutable

[Stephen Hemminger]

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 <stephen@networkplumber.org>
---
 lib/librte_lpm/rte_lpm6.c | 8 ++++----
 lib/librte_lpm/rte_lpm6.h | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index c46e557e2389..5ea818abd6e9 100644
--- a/lib/librte_lpm/rte_lpm6.c
+++ b/lib/librte_lpm/rte_lpm6.c
@@ -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;
diff --git a/lib/librte_lpm/rte_lpm6.h b/lib/librte_lpm/rte_lpm6.h
index 37dfb20249a8..042991f8cdb7 100644
--- a/lib/librte_lpm/rte_lpm6.h
+++ b/lib/librte_lpm/rte_lpm6.h
@@ -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.
-- 
2.20.1

[dpdk-dev] [PATCH 2/5] examples/l3fwd: use RTE_DIM

[Stephen Hemminger]

Use the standard RTE_DIM macro to compute array size.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd/l3fwd_lpm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 349de2703cd1..b6802d63ba1d 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -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)
-- 
2.20.1

[dpdk-dev] [PATCH 3/5] examples/l3fwd: make lookup struct static

[Stephen Hemminger]

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

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd/l3fwd_lpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index b6802d63ba1d..486f19963fc4 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -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)
-- 
2.20.1

[dpdk-dev] [PATCH 4/5] examples/l3fwd: make route array constant

[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 <stephen@networkplumber.org>
---
 examples/l3fwd/l3fwd_lpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 486f19963fc4..30f6385419ee 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -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},
-- 
2.20.1

[dpdk-dev] [PATCH 5/5] examples/l3fwd: improve readability for destination lookup

[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 <stephen@networkplumber.org>
---
 examples/l3fwd/l3fwd_lpm.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 30f6385419ee..bd6e7fe03d77 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -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
-- 
2.20.1

Re: [dpdk-dev] [PATCH 0/5] l3fwd and lpm cleanups

[Thomas Monjalon]

26/01/2020 02:09, Stephen Hemminger:
> 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.