[dpdk-dev] [RFC PATCH] ethdev: fix ports enumeration

[Thomas Monjalon <thomas@monjalon.net>]

Some DPDK applications wrongly assume these requirements:
    - no hotplug, i.e. ports are never detached
    - all allocated ports are available to the application

Such application iterates over ports by its own mean.
The most common pattern is to request the port count and
assume ports with index in the range [0..count[ can be used.

There are three consequences when using such wrong design:
    - new ports having an index higher than the port count won't be seen
    - old ports being detached (RTE_ETH_DEV_UNUSED) can be seen as ghosts
    - failsafe sub-devices (RTE_ETH_DEV_DEFERRED) will be seen by the application

Such mistake will be less common with growing hotplug awareness.
All applications and examples inside this repository - except testpmd -
must be fixed to use the iterator RTE_ETH_FOREACH_DEV.

In order to fix this common mistake in all external applications,
the function rte_eth_dev_count is deprecated and replaced by
the new functions rte_eth_dev_count_avail and rte_eth_dev_count_total.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

---

This RFC fixes only procinfo and skeleton example.
Next version of this patch should fix all occurences.

This RFC gather all ideas in one patch, but next version
will be more detailed and split.
---
 app/proc_info/main.c                    |  2 +-
 examples/skeleton/Makefile              |  3 +++
 examples/skeleton/basicfwd.c            | 12 ++++--------
 lib/librte_ether/rte_ethdev.c           | 18 ++++++++++++++++++
 lib/librte_ether/rte_ethdev.h           | 23 +++++++++++++++++++++++
 lib/librte_ether/rte_ethdev_version.map |  4 +++-
 6 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2f53e3caa..d508db957 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -636,7 +636,7 @@ main(int argc, char **argv)
 	if (enabled_port_mask == 0)
 		enabled_port_mask = 0xffff;
 
-	for (i = 0; i < nb_ports; i++) {
+	RTE_ETH_FOREACH_DEV(i) {
 		if (enabled_port_mask & (1 << i)) {
 			if (enable_stats)
 				nic_stats_display(i);
diff --git a/examples/skeleton/Makefile b/examples/skeleton/Makefile
index bd980ec9b..a4a1860cb 100644
--- a/examples/skeleton/Makefile
+++ b/examples/skeleton/Makefile
@@ -23,6 +23,8 @@ CFLAGS += -O3 $(shell pkg-config --cflags libdpdk)
 LDFLAGS_SHARED = $(shell pkg-config --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell pkg-config --static --libs libdpdk)
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
 	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
 
@@ -48,6 +50,7 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS)
 
 # workaround for a gcc bug with noreturn attribute
diff --git a/examples/skeleton/basicfwd.c b/examples/skeleton/basicfwd.c
index e62cc0a59..1c1eb08ef 100644
--- a/examples/skeleton/basicfwd.c
+++ b/examples/skeleton/basicfwd.c
@@ -42,9 +42,6 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 	struct rte_eth_dev_info dev_info;
 	struct rte_eth_txconf txconf;
 
-	if (port >= rte_eth_dev_count())
-		return -1;
-
 	rte_eth_dev_info_get(port, &dev_info);
 	if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
 		port_conf.txmode.offloads |=
@@ -106,14 +103,13 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 static __attribute__((noreturn)) void
 lcore_main(void)
 {
-	const uint16_t nb_ports = rte_eth_dev_count();
 	uint16_t port;
 
 	/*
 	 * Check that the port is on the same NUMA node as the polling thread
 	 * for best performance.
 	 */
-	for (port = 0; port < nb_ports; port++)
+	RTE_ETH_FOREACH_DEV(port)
 		if (rte_eth_dev_socket_id(port) > 0 &&
 				rte_eth_dev_socket_id(port) !=
 						(int)rte_socket_id())
@@ -130,7 +126,7 @@ lcore_main(void)
 		 * Receive packets on a port and forward them on the paired
 		 * port. The mapping is 0 -> 1, 1 -> 0, 2 -> 3, 3 -> 2, etc.
 		 */
-		for (port = 0; port < nb_ports; port++) {
+		RTE_ETH_FOREACH_DEV(port) {
 
 			/* Get burst of RX packets, from first port of pair. */
 			struct rte_mbuf *bufs[BURST_SIZE];
@@ -174,7 +170,7 @@ main(int argc, char *argv[])
 	argv += ret;
 
 	/* Check that there is an even number of ports to send/receive on. */
-	nb_ports = rte_eth_dev_count();
+	nb_ports = rte_eth_dev_count_avail();
 	if (nb_ports < 2 || (nb_ports & 1))
 		rte_exit(EXIT_FAILURE, "Error: number of ports must be even\n");
 
@@ -186,7 +182,7 @@ main(int argc, char *argv[])
 		rte_exit(EXIT_FAILURE, "Cannot create mbuf pool\n");
 
 	/* Initialize all ports. */
-	for (portid = 0; portid < nb_ports; portid++)
+	RTE_ETH_FOREACH_DEV(portid)
 		if (port_init(portid, mbuf_pool) != 0)
 			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu16 "\n",
 					portid);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0590f0c10..583427559 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -529,6 +529,12 @@ rte_eth_dev_get_sec_ctx(uint8_t port_id)
 
 uint16_t
 rte_eth_dev_count(void)
+{
+	return rte_eth_dev_count_avail();
+}
+
+uint16_t
+rte_eth_dev_count_avail(void)
 {
 	uint16_t p;
 	uint16_t count;
@@ -541,6 +547,18 @@ rte_eth_dev_count(void)
 	return count;
 }
 
+uint16_t
+rte_eth_dev_count_total(void)
+{
+	uint16_t port, count = 0;
+
+	for (port = 0; port < RTE_MAX_ETHPORTS; port++)
+		if (rte_eth_devices[port].state != RTE_ETH_DEV_UNUSED)
+			count++;
+
+	return count;
+}
+
 int
 rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
 {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 036153306..cba37fe34 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1362,8 +1362,31 @@ int __rte_experimental rte_eth_dev_owner_get(const uint16_t port_id,
  * @return
  *   - The total number of usable Ethernet devices.
  */
+__rte_deprecated
 uint16_t rte_eth_dev_count(void);
 
+/**
+ * Get the number of ports which are usable for the application.
+ *
+ * These devices must be iterated by using the macro
+ * ``RTE_ETH_FOREACH_DEV`` or ``RTE_ETH_FOREACH_DEV_OWNED_BY``
+ * to deal with non-contiguous ranges of devices.
+ *
+ * @return
+ *   The count of available Ethernet devices.
+ */
+uint16_t __rte_experimental rte_eth_dev_count_avail(void);
+
+/**
+ * Get the total number of ports which are allocated.
+ *
+ * Some devices may not be available for the application.
+ *
+ * @return
+ *   The total count of Ethernet devices.
+ */
+uint16_t __rte_experimental rte_eth_dev_count_total(void);
+
 /**
  * Attach a new Ethernet device specified by arguments.
  *
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index 87f02fb74..c9a18ab5a 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -207,6 +207,8 @@ DPDK_18.02 {
 EXPERIMENTAL {
 	global:
 
+	rte_eth_dev_count_avail;
+	rte_eth_dev_count_total;
 	rte_eth_dev_is_removed;
 	rte_eth_dev_owner_delete;
 	rte_eth_dev_owner_get;
@@ -229,4 +231,4 @@ EXPERIMENTAL {
 	rte_mtr_stats_read;
 	rte_mtr_stats_update;
 
-} DPDK_17.11;
+} DPDK_18.02;
-- 
2.16.2