[v7,2/5] examples/l3fwd: move l3fwd routes to common header

Message ID 20210406111106.2020555-3-conor.walsh@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series examples/l3fwd: add FIB lookup method to l3fwd |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Conor Walsh April 6, 2021, 11:11 a.m. UTC
  To prevent code duplication from the addition of lookup methods
the routes specified in lpm should be moved to a common header.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 examples/l3fwd/l3fwd_common_route.h | 48 +++++++++++++++++++
 examples/l3fwd/l3fwd_lpm.c          | 74 +++++++----------------------
 2 files changed, 65 insertions(+), 57 deletions(-)
 create mode 100644 examples/l3fwd/l3fwd_common_route.h
  

Comments

David Marchand April 15, 2021, 2:38 p.m. UTC | #1
On Tue, Apr 6, 2021 at 1:11 PM Conor Walsh <conor.walsh@intel.com> wrote:
>
> To prevent code duplication from the addition of lookup methods
> the routes specified in lpm should be moved to a common header.
>
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
>  examples/l3fwd/l3fwd_common_route.h | 48 +++++++++++++++++++
>  examples/l3fwd/l3fwd_lpm.c          | 74 +++++++----------------------
>  2 files changed, 65 insertions(+), 57 deletions(-)
>  create mode 100644 examples/l3fwd/l3fwd_common_route.h
>
> diff --git a/examples/l3fwd/l3fwd_common_route.h b/examples/l3fwd/l3fwd_common_route.h
> new file mode 100644
> index 0000000000..7f0125a8a5
> --- /dev/null
> +++ b/examples/l3fwd/l3fwd_common_route.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Intel Corporation
> + */
> +
> +#include <stdint.h>
> +#include <rte_ip.h>
> +
> +struct ipv4_l3fwd_common_route {

Why "common" ? ipv4_l3fwd_route is enough.
Plus, this is what we reference in the doc.

doc/guides/sample_app_ug/l3_forward.rst:            if ((1 <<
ipv4_l3fwd_route_array[i].if_out & enabled_port_mask) == 0)
doc/guides/sample_app_ug/l3_forward.rst:            ret =
rte_lpm_add(ipv4_l3fwd_lookup_struct[socketid],
ipv4_l3fwd_route_array[i].ip,
doc/guides/sample_app_ug/l3_forward.rst:
     ipv4_l3fwd_route_array[i].depth,
ipv4_l3fwd_route_array[i].if_out);
doc/guides/sample_app_ug/l3_forward.rst:
(unsigned)ipv4_l3fwd_route_array[i].ip,
ipv4_l3fwd_route_array[i].depth, ipv4_l3fwd_route_array[i].if_out);


> +       uint32_t ip;
> +       uint8_t  depth;
> +       uint8_t  if_out;
> +};



> +
> +struct ipv6_l3fwd_common_route {
> +       uint8_t ip[16];
> +       uint8_t  depth;

Unneeded double space.


> +       uint8_t  if_out;
> +};
> +
> +/*
> + * 198.18.0.0/16 are set aside for RFC2544 benchmarking (RFC5735).
> + * 198.18.{0-7}.0/24 = Port {0-7}
> + */
> +static const struct ipv4_l3fwd_common_route ipv4_l3fwd_common_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},
> +       {RTE_IPV4(198, 18, 3, 0), 24, 3},
> +       {RTE_IPV4(198, 18, 4, 0), 24, 4},
> +       {RTE_IPV4(198, 18, 5, 0), 24, 5},
> +       {RTE_IPV4(198, 18, 6, 0), 24, 6},
> +       {RTE_IPV4(198, 18, 7, 0), 24, 7},
> +};

With static symbols, we end up with duplicating info in lpm / fib.
$ nm build/examples/dpdk-l3fwd |grep ipv4_l3fwd_common_route_array
000000000100c260 r ipv4_l3fwd_common_route_array
000000000100c4c0 r ipv4_l3fwd_common_route_array

You can declare global symbols in a common header, and define them in main.c.


> +
> +/*
> + * 2001:200::/48 is IANA reserved range for IPv6 benchmarking (RFC5180).
> + * 2001:200:0:{0-7}::/64 = Port {0-7}
> + */
> +static const struct ipv6_l3fwd_common_route ipv6_l3fwd_common_route_array[] = {
> +       {{32, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 0},
> +       {{32, 1, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 1},
> +       {{32, 1, 2, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 2},
> +       {{32, 1, 2, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 3},
> +       {{32, 1, 2, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 4},
> +       {{32, 1, 2, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 5},
> +       {{32, 1, 2, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 6},
> +       {{32, 1, 2, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 7},
> +};
  

Patch

diff --git a/examples/l3fwd/l3fwd_common_route.h b/examples/l3fwd/l3fwd_common_route.h
new file mode 100644
index 0000000000..7f0125a8a5
--- /dev/null
+++ b/examples/l3fwd/l3fwd_common_route.h
@@ -0,0 +1,48 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+
+#include <stdint.h>
+#include <rte_ip.h>
+
+struct ipv4_l3fwd_common_route {
+	uint32_t ip;
+	uint8_t  depth;
+	uint8_t  if_out;
+};
+
+struct ipv6_l3fwd_common_route {
+	uint8_t ip[16];
+	uint8_t  depth;
+	uint8_t  if_out;
+};
+
+/*
+ * 198.18.0.0/16 are set aside for RFC2544 benchmarking (RFC5735).
+ * 198.18.{0-7}.0/24 = Port {0-7}
+ */
+static const struct ipv4_l3fwd_common_route ipv4_l3fwd_common_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},
+	{RTE_IPV4(198, 18, 3, 0), 24, 3},
+	{RTE_IPV4(198, 18, 4, 0), 24, 4},
+	{RTE_IPV4(198, 18, 5, 0), 24, 5},
+	{RTE_IPV4(198, 18, 6, 0), 24, 6},
+	{RTE_IPV4(198, 18, 7, 0), 24, 7},
+};
+
+/*
+ * 2001:200::/48 is IANA reserved range for IPv6 benchmarking (RFC5180).
+ * 2001:200:0:{0-7}::/64 = Port {0-7}
+ */
+static const struct ipv6_l3fwd_common_route ipv6_l3fwd_common_route_array[] = {
+	{{32, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 0},
+	{{32, 1, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 1},
+	{{32, 1, 2, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 2},
+	{{32, 1, 2, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 3},
+	{{32, 1, 2, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 4},
+	{{32, 1, 2, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 5},
+	{{32, 1, 2, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 6},
+	{{32, 1, 2, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 7},
+};
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 1cfaf36572..818cf717d1 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -30,47 +30,7 @@ 
 #include "l3fwd.h"
 #include "l3fwd_event.h"
 
-struct ipv4_l3fwd_lpm_route {
-	uint32_t ip;
-	uint8_t  depth;
-	uint8_t  if_out;
-};
-
-struct ipv6_l3fwd_lpm_route {
-	uint8_t ip[16];
-	uint8_t  depth;
-	uint8_t  if_out;
-};
-
-/*
- * 198.18.0.0/16 are set aside for RFC2544 benchmarking (RFC5735).
- * 198.18.{0-7}.0/24 = Port {0-7}
- */
-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},
-	{RTE_IPV4(198, 18, 3, 0), 24, 3},
-	{RTE_IPV4(198, 18, 4, 0), 24, 4},
-	{RTE_IPV4(198, 18, 5, 0), 24, 5},
-	{RTE_IPV4(198, 18, 6, 0), 24, 6},
-	{RTE_IPV4(198, 18, 7, 0), 24, 7},
-};
-
-/*
- * 2001:200::/48 is IANA reserved range for IPv6 benchmarking (RFC5180).
- * 2001:200:0:{0-7}::/64 = Port {0-7}
- */
-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}, 64, 0},
-	{{32, 1, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 1},
-	{{32, 1, 2, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 2},
-	{{32, 1, 2, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 3},
-	{{32, 1, 2, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 4},
-	{{32, 1, 2, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 5},
-	{{32, 1, 2, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 6},
-	{{32, 1, 2, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 0}, 64, 7},
-};
+#include "l3fwd_common_route.h"
 
 #define IPV4_L3FWD_LPM_MAX_RULES         1024
 #define IPV4_L3FWD_LPM_NUMBER_TBL8S (1 << 8)
@@ -485,18 +445,18 @@  setup_lpm(const int socketid)
 			socketid);
 
 	/* populate the LPM table */
-	for (i = 0; i < RTE_DIM(ipv4_l3fwd_lpm_route_array); i++) {
+	for (i = 0; i < RTE_DIM(ipv4_l3fwd_common_route_array); i++) {
 		struct in_addr in;
 
 		/* skip unused ports */
-		if ((1 << ipv4_l3fwd_lpm_route_array[i].if_out &
+		if ((1 << ipv4_l3fwd_common_route_array[i].if_out &
 				enabled_port_mask) == 0)
 			continue;
 
 		ret = rte_lpm_add(ipv4_l3fwd_lpm_lookup_struct[socketid],
-			ipv4_l3fwd_lpm_route_array[i].ip,
-			ipv4_l3fwd_lpm_route_array[i].depth,
-			ipv4_l3fwd_lpm_route_array[i].if_out);
+			ipv4_l3fwd_common_route_array[i].ip,
+			ipv4_l3fwd_common_route_array[i].depth,
+			ipv4_l3fwd_common_route_array[i].if_out);
 
 		if (ret < 0) {
 			rte_exit(EXIT_FAILURE,
@@ -504,11 +464,11 @@  setup_lpm(const int socketid)
 				i, socketid);
 		}
 
-		in.s_addr = htonl(ipv4_l3fwd_lpm_route_array[i].ip);
+		in.s_addr = htonl(ipv4_l3fwd_common_route_array[i].ip);
 		printf("LPM: Adding route %s / %d (%d)\n",
 		       inet_ntop(AF_INET, &in, abuf, sizeof(abuf)),
-			ipv4_l3fwd_lpm_route_array[i].depth,
-			ipv4_l3fwd_lpm_route_array[i].if_out);
+			ipv4_l3fwd_common_route_array[i].depth,
+			ipv4_l3fwd_common_route_array[i].if_out);
 	}
 
 	/* create the LPM6 table */
@@ -525,17 +485,17 @@  setup_lpm(const int socketid)
 			socketid);
 
 	/* populate the LPM table */
-	for (i = 0; i < RTE_DIM(ipv6_l3fwd_lpm_route_array); i++) {
+	for (i = 0; i < RTE_DIM(ipv6_l3fwd_common_route_array); i++) {
 
 		/* skip unused ports */
-		if ((1 << ipv6_l3fwd_lpm_route_array[i].if_out &
+		if ((1 << ipv6_l3fwd_common_route_array[i].if_out &
 				enabled_port_mask) == 0)
 			continue;
 
 		ret = rte_lpm6_add(ipv6_l3fwd_lpm_lookup_struct[socketid],
-			ipv6_l3fwd_lpm_route_array[i].ip,
-			ipv6_l3fwd_lpm_route_array[i].depth,
-			ipv6_l3fwd_lpm_route_array[i].if_out);
+			ipv6_l3fwd_common_route_array[i].ip,
+			ipv6_l3fwd_common_route_array[i].depth,
+			ipv6_l3fwd_common_route_array[i].if_out);
 
 		if (ret < 0) {
 			rte_exit(EXIT_FAILURE,
@@ -544,10 +504,10 @@  setup_lpm(const int socketid)
 		}
 
 		printf("LPM: Adding route %s / %d (%d)\n",
-		       inet_ntop(AF_INET6, ipv6_l3fwd_lpm_route_array[i].ip,
+		       inet_ntop(AF_INET6, ipv6_l3fwd_common_route_array[i].ip,
 				 abuf, sizeof(abuf)),
-		       ipv6_l3fwd_lpm_route_array[i].depth,
-		       ipv6_l3fwd_lpm_route_array[i].if_out);
+		       ipv6_l3fwd_common_route_array[i].depth,
+		       ipv6_l3fwd_common_route_array[i].if_out);
 	}
 }