diff mbox series

[v3,6/6] examples/tep_termination: rework the port setup logic

Message ID 20200402171953.13356-7-stephen@networkplumber.org (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series checking for owned ports in portmask |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Stephen Hemminger April 2, 2020, 5:19 p.m. UTC 
  The handling of ports in this application had many problems.
It was checking for things that can never happen with current
DPDK library (like rte_ethdev_avail_count() >= RTE_MAX_ETHPORTS)
and it was not checking if the port was owned and should
not be used directly. Also the variable nb_ports was used
as both local and global variable.

Fix by rewriting the initialization logic to iterate over
valid ports and check if there are any leftovers.

Fixes: a50245ede72a ("examples/tep_term: initialize VXLAN sample")
Cc: jijiang.liu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/tep_termination/Makefile    |  2 +
 examples/tep_termination/main.c      | 70 ++++++++++------------------
 examples/tep_termination/meson.build |  1 +
 3 files changed, 28 insertions(+), 45 deletions(-)

Comments

Ferruh Yigit April 10, 2020, 4:16 p.m. UTC | #1 
On 4/2/2020 6:19 PM, Stephen Hemminger wrote:
> The handling of ports in this application had many problems.
> It was checking for things that can never happen with current
> DPDK library (like rte_ethdev_avail_count() >= RTE_MAX_ETHPORTS)
> and it was not checking if the port was owned and should
> not be used directly. Also the variable nb_ports was used
> as both local and global variable.
> 
> Fix by rewriting the initialization logic to iterate over
> valid ports and check if there are any leftovers.
> 
> Fixes: a50245ede72a ("examples/tep_term: initialize VXLAN sample")
> Cc: jijiang.liu@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/tep_termination/Makefile    |  2 +
>  examples/tep_termination/main.c      | 70 ++++++++++------------------
>  examples/tep_termination/meson.build |  1 +
>  3 files changed, 28 insertions(+), 45 deletions(-)
> 
> diff --git a/examples/tep_termination/Makefile b/examples/tep_termination/Makefile
> index 645112498d33..f55331fb24c2 100644
> --- a/examples/tep_termination/Makefile
> +++ b/examples/tep_termination/Makefile
> @@ -26,6 +26,7 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
>  LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
>  LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
>  
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
>  CFLAGS += -Wno-deprecated-declarations
>  
>  build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
> @@ -61,6 +62,7 @@ endif
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
>  CFLAGS += -Wno-deprecated-declarations
> +CFLAGS += -DALLOW_EXPERIMENTAL_API

Why 'ALLOW_EXPERIMENTAL_API' added in this patch, what is the experimental API
started to use?

<...>

>  
> -	for (portid = 0; portid < nb_ports; portid++) {
> -		if (!rte_eth_dev_is_valid_port(ports[portid])) {
> -			RTE_LOG(INFO, VHOST_PORT,
> -				"\nSpecified port ID(%u) is not valid\n",
> -				ports[portid]);
> -			ports[portid] = INVALID_PORT_ID;

'INVALID_PORT_ID' macro seems can be removed now.
diff mbox series

Patch

diff --git a/examples/tep_termination/Makefile b/examples/tep_termination/Makefile
index 645112498d33..f55331fb24c2 100644
--- a/examples/tep_termination/Makefile
+++ b/examples/tep_termination/Makefile
@@ -26,6 +26,7 @@  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -Wno-deprecated-declarations
 
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
@@ -61,6 +62,7 @@  endif
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 CFLAGS += -Wno-deprecated-declarations
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 include $(RTE_SDK)/mk/rte.extapp.mk
 endif
diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
index ab956ad7ce96..beb3ff7d78e0 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -149,8 +149,6 @@  static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
 static unsigned lcore_ids[RTE_MAX_LCORE];
 uint16_t ports[RTE_MAX_ETHPORTS];
 
-static unsigned nb_ports; /**< The number of ports specified in command line */
-
 /* ethernet addresses of ports */
 struct rte_ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
 
@@ -268,7 +266,6 @@  tep_termination_parse_args(int argc, char **argv)
 {
 	int opt, ret;
 	int option_index;
-	unsigned i;
 	const char *prgname = argv[0];
 	static struct option long_option[] = {
 		{CMD_LINE_OPT_NB_DEVICES, required_argument, NULL, 0},
@@ -474,18 +471,6 @@  tep_termination_parse_args(int argc, char **argv)
 		}
 	}
 
-	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
-		if (enabled_port_mask & (1 << i))
-			ports[nb_ports++] = (uint8_t)i;
-	}
-
-	if ((nb_ports ==  0) || (nb_ports > MAX_SUP_PORTS)) {
-		RTE_LOG(INFO, VHOST_PORT, "Current enabled port number is %u,"
-			"but only %u port can be enabled\n", nb_ports,
-			MAX_SUP_PORTS);
-		return -1;
-	}
-
 	return 0;
 }
 
@@ -493,28 +478,29 @@  tep_termination_parse_args(int argc, char **argv)
  * Update the global var NB_PORTS and array PORTS
  * according to system ports number and return valid ports number
  */
-static unsigned
-check_ports_num(unsigned max_nb_ports)
+static unsigned int
+get_ports(void)
 {
-	unsigned valid_nb_ports = nb_ports;
-	unsigned portid;
-
-	if (nb_ports > max_nb_ports) {
-		RTE_LOG(INFO, VHOST_PORT, "\nSpecified port number(%u) "
-			" exceeds total system port number(%u)\n",
-			nb_ports, max_nb_ports);
-		nb_ports = max_nb_ports;
+	uint32_t port_mask = enabled_port_mask;
+	unsigned int valid_nb_ports = 0;
+	uint16_t portid;
+
+	RTE_ETH_FOREACH_DEV(portid) {
+		uint32_t mask = 1u << portid;
+
+		if ((mask & port_mask) == 0)
+			continue;
+
+		port_mask &= ~mask;
+		ports[valid_nb_ports++] = portid;
 	}
 
-	for (portid = 0; portid < nb_ports; portid++) {
-		if (!rte_eth_dev_is_valid_port(ports[portid])) {
-			RTE_LOG(INFO, VHOST_PORT,
-				"\nSpecified port ID(%u) is not valid\n",
-				ports[portid]);
-			ports[portid] = INVALID_PORT_ID;
-			valid_nb_ports--;
-		}
+	if (port_mask != 0) {
+		RTE_LOG(INFO, VHOST_PORT,
+			"\nInvalid ports %#x specified in portmask\n",
+			port_mask);
 	}
+
 	return valid_nb_ports;
 }
 
@@ -1123,7 +1109,7 @@  main(int argc, char *argv[])
 {
 	struct rte_mempool *mbuf_pool = NULL;
 	unsigned lcore_id, core_id = 0;
-	unsigned nb_ports, valid_nb_ports;
+	unsigned int valid_nb_ports;
 	int ret;
 	uint16_t portid;
 	uint16_t queue_id;
@@ -1148,20 +1134,14 @@  main(int argc, char *argv[])
 	/* set the number of swithcing cores available */
 	nb_switching_cores = rte_lcore_count()-1;
 
-	/* Get the number of physical ports. */
-	nb_ports = rte_eth_dev_count_avail();
-
-	/*
-	 * Update the global var NB_PORTS and global array PORTS
-	 * and get value of var VALID_NB_PORTS according to system ports number
-	 */
-	valid_nb_ports = check_ports_num(nb_ports);
-
-	if ((valid_nb_ports == 0) || (valid_nb_ports > MAX_SUP_PORTS)) {
+	/* Set ports[] array based on system ports and port mask */
+	valid_nb_ports = get_ports();
+	if (valid_nb_ports == 0 || valid_nb_ports > MAX_SUP_PORTS) {
 		rte_exit(EXIT_FAILURE, "Current enabled port number is %u,"
-			"but only %u port can be enabled\n", nb_ports,
+			"but only %u port can be enabled\n", valid_nb_ports,
 			MAX_SUP_PORTS);
 	}
+
 	/* Create the mbuf pool. */
 	mbuf_pool = rte_pktmbuf_pool_create(
 			"MBUF_POOL",
diff --git a/examples/tep_termination/meson.build b/examples/tep_termination/meson.build
index f65d689802a3..ddadf1c2aede 100644
--- a/examples/tep_termination/meson.build
+++ b/examples/tep_termination/meson.build
@@ -9,6 +9,7 @@ 
 if not is_linux
 	build = false
 endif
+allow_experimental_apis = true
 deps += ['hash', 'vhost']
 cflags += '-Wno-deprecated-declarations'
 sources = files(