DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask
@ 2020-03-12 17:20 Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership Stephen Hemminger
                   ` (8 more replies)
  0 siblings, 9 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The failsafe, bonding, and netvsc PMD use eth dev port ownership
to control sub devices. These sub devices are hidden in the
normal iteration over ports but some applications use a direct
port mask.  In these cases, user may still (incorrectly) try
to directly use the sub devices.

This patch set introduces a helper function to check if
a port is under ownership. And the follow on patches use that.

IMHO this should not be marked as experimental and done as
a bug fix back to DPDK 18.11 because then it will get picked
up in LTS and users won't run into it on Azure.

An alternative proposed in earlier discussion is to have
each application try and own the port. But this solution would
be more invasive, and does not handle the case if secondary process
exits prematurely without releasing ownership. It makes more
sense to keep the concept of ownership as strictly part of
the device model and not part of the application layer.

Stephen Hemminger (7):
  ethdev: add function to test port ownership
  examples/l2fwd-cat: block attempts to use owned ports
  examples/l3fwd: block attempts to use owned ports
  examples/l3fwd-acl: block attempts to use owned ports
  examples/l3fwd-power: block attempts to use owned ports
  examples/tep_termination: block attempts to use owned ports
  examples/vhost: block attempts to use owned ports

 examples/l2fwd-cat/Makefile              |  2 ++
 examples/l2fwd-cat/l2fwd-cat.c           |  3 +++
 examples/l2fwd-cat/meson.build           |  1 +
 examples/l3fwd-acl/Makefile              |  3 +++
 examples/l3fwd-acl/main.c                |  4 ++++
 examples/l3fwd-acl/meson.build           |  1 +
 examples/l3fwd-power/main.c              |  4 ++++
 examples/l3fwd/Makefile                  |  3 +++
 examples/l3fwd/main.c                    |  4 ++++
 examples/l3fwd/meson.build               |  1 +
 examples/tep_termination/Makefile        |  2 ++
 examples/tep_termination/main.c          |  2 +-
 examples/tep_termination/meson.build     |  1 +
 examples/vhost/main.c                    |  2 +-
 lib/librte_ethdev/rte_ethdev.c           |  9 +++++++++
 lib/librte_ethdev/rte_ethdev.h           | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  3 +++
 17 files changed, 58 insertions(+), 2 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-15  7:45   ` Matan Azrad
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 2/7] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

Applications using a port mask need a method to be able
to test for (and reject) ethdev ports that are in use for
other purposes.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ethdev/rte_ethdev.c           |  9 +++++++++
 lib/librte_ethdev/rte_ethdev.h           | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  3 +++
 3 files changed, 27 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 774c721b3484..38f99f417dba 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -685,6 +685,15 @@ rte_eth_dev_owner_unset(const uint16_t port_id, const uint64_t owner_id)
 	return ret;
 }
 
+int
+rte_eth_dev_is_owned_by(uint16_t port_id, uint64_t owner_id)
+{
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return 0;
+
+	return rte_eth_devices[port_id].data->owner.id == owner_id;
+}
+
 int
 rte_eth_dev_owner_delete(const uint64_t owner_id)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index d1a593ad112a..91a8f9578282 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1530,6 +1530,21 @@ uint64_t rte_eth_find_next_owned_by(uint16_t port_id,
 	     (unsigned int)p < (unsigned int)RTE_MAX_ETHPORTS; \
 	     p = rte_eth_find_next_owned_by(p + 1, o))
 
+/**
+ * Test if a port is owned
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device
+ * @param	owner_id
+ *   The owner identifier.
+ *   RTE_ETH_DEV_NO_OWNER means test if port is not owned.
+ * @return
+ *   - 0 if port is out of range or not owned by owner_id
+ *   - 1 if device is associated with owner_id
+ */
+__rte_experimental
+int rte_eth_dev_is_owned_by(uint16_t port_id, uint64_t owner_id);
+
 /**
  * Iterates over valid ethdev ports.
  *
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 3f32fdecf722..95231fe3410c 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -230,4 +230,7 @@ EXPERIMENTAL {
 
 	# added in 20.02
 	rte_flow_dev_dump;
+
+	# added in 20.05
+	rte_eth_dev_is_owned_by;
 };
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 2/7] examples/l2fwd-cat: block attempts to use owned ports
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-15  7:51   ` Matan Azrad
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 3/7] examples/l3fwd: " Stephen Hemminger
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l2fwd-cat/Makefile    | 2 ++
 examples/l2fwd-cat/l2fwd-cat.c | 3 +++
 examples/l2fwd-cat/meson.build | 1 +
 3 files changed, 6 insertions(+)

diff --git a/examples/l2fwd-cat/Makefile b/examples/l2fwd-cat/Makefile
index b0e53c37e8d8..aa19347c1545 100644
--- a/examples/l2fwd-cat/Makefile
+++ b/examples/l2fwd-cat/Makefile
@@ -20,6 +20,7 @@ static: build/$(APP)-static
 PKGCONF ?= pkg-config
 
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
@@ -65,6 +66,7 @@ endif
 
 EXTRA_CFLAGS += -O3 -g -Wfatal-errors
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -I$(PQOS_INSTALL_PATH)/../include
 
 LDLIBS += -L$(PQOS_INSTALL_PATH)
diff --git a/examples/l2fwd-cat/l2fwd-cat.c b/examples/l2fwd-cat/l2fwd-cat.c
index 6838f288c621..c1c59b2d81f2 100644
--- a/examples/l2fwd-cat/l2fwd-cat.c
+++ b/examples/l2fwd-cat/l2fwd-cat.c
@@ -42,6 +42,9 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 	if (!rte_eth_dev_is_valid_port(port))
 		return -1;
 
+	if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER)) {
+		return -1;
+
 	/* Configure the Ethernet device. */
 	retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
 	if (retval != 0)
diff --git a/examples/l2fwd-cat/meson.build b/examples/l2fwd-cat/meson.build
index 4e2777a03358..5c062c1354e3 100644
--- a/examples/l2fwd-cat/meson.build
+++ b/examples/l2fwd-cat/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 pqos = cc.find_library('pqos', required: false)
 build = pqos.found()
 ext_deps += pqos
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 3/7] examples/l3fwd: block attempts to use owned ports
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 2/7] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 4/7] examples/l3fwd-acl: " Stephen Hemminger
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of l3fwd.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd/Makefile    | 3 +++
 examples/l3fwd/main.c      | 4 ++++
 examples/l3fwd/meson.build | 1 +
 3 files changed, 8 insertions(+)

diff --git a/examples/l3fwd/Makefile b/examples/l3fwd/Makefile
index 59a110d12a49..59cd00abe6e9 100644
--- a/examples/l3fwd/Makefile
+++ b/examples/l3fwd/Makefile
@@ -25,6 +25,8 @@ 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
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
 	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
 
@@ -50,6 +52,7 @@ RTE_TARGET ?= $(notdir $(abspath $(dir $(firstword $(wildcard $(RTE_SDK)/*/.conf
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -I$(SRCDIR)
 CFLAGS += -O3 $(USER_FLAGS)
 CFLAGS += $(WERROR_FLAGS)
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index dda430d68a20..307a490fb552 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -221,6 +221,10 @@ check_port_config(void)
 			printf("port %u is not present on the board\n", portid);
 			return -1;
 		}
+		if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER)) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
diff --git a/examples/l3fwd/meson.build b/examples/l3fwd/meson.build
index ebed3b518c4b..7d72b1b365ae 100644
--- a/examples/l3fwd/meson.build
+++ b/examples/l3fwd/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 deps += ['hash', 'lpm', 'eventdev']
 sources = files(
 	'l3fwd_em.c', 'l3fwd_lpm.c', 'l3fwd_event.c',
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 4/7] examples/l3fwd-acl: block attempts to use owned ports
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
                   ` (2 preceding siblings ...)
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 3/7] examples/l3fwd: " Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 5/7] examples/l3fwd-power: " Stephen Hemminger
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-acl/Makefile    | 3 +++
 examples/l3fwd-acl/main.c      | 4 ++++
 examples/l3fwd-acl/meson.build | 1 +
 3 files changed, 8 insertions(+)

diff --git a/examples/l3fwd-acl/Makefile b/examples/l3fwd-acl/Makefile
index d9909584b1c6..ff14b4fdd593 100644
--- a/examples/l3fwd-acl/Makefile
+++ b/examples/l3fwd-acl/Makefile
@@ -20,7 +20,9 @@ static: build/$(APP)-static
 PKGCONF ?= pkg-config
 
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
+
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
 
@@ -51,6 +53,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
index fa92a2829740..695121bb7bb5 100644
--- a/examples/l3fwd-acl/main.c
+++ b/examples/l3fwd-acl/main.c
@@ -1470,6 +1470,10 @@ check_port_config(void)
 			printf("port %u is not present on the board\n", portid);
 			return -1;
 		}
+		if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER)) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
diff --git a/examples/l3fwd-acl/meson.build b/examples/l3fwd-acl/meson.build
index 7096e00c1073..6fa468b3aa9c 100644
--- a/examples/l3fwd-acl/meson.build
+++ b/examples/l3fwd-acl/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 deps += ['acl', 'lpm', 'hash']
 sources = files(
 	'main.c'
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 5/7] examples/l3fwd-power: block attempts to use owned ports
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
                   ` (3 preceding siblings ...)
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 4/7] examples/l3fwd-acl: " Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 6/7] examples/tep_termination: " Stephen Hemminger
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of l3fwd.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-power/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index c7fe0ec03495..3a8a7277cb0a 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1396,6 +1396,10 @@ check_port_config(void)
 								portid);
 			return -1;
 		}
+		if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER)) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 6/7] examples/tep_termination: block attempts to use owned ports
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
                   ` (4 preceding siblings ...)
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 5/7] examples/l3fwd-power: " Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 7/7] examples/vhost: " Stephen Hemminger
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/tep_termination/Makefile    | 2 ++
 examples/tep_termination/main.c      | 2 +-
 examples/tep_termination/meson.build | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

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..928cd226d25a 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -507,7 +507,7 @@ check_ports_num(unsigned max_nb_ports)
 	}
 
 	for (portid = 0; portid < nb_ports; portid++) {
-		if (!rte_eth_dev_is_valid_port(ports[portid])) {
+		if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER)) {
 			RTE_LOG(INFO, VHOST_PORT,
 				"\nSpecified port ID(%u) is not valid\n",
 				ports[portid]);
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(
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH 7/7] examples/vhost: block attempts to use owned ports
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
                   ` (5 preceding siblings ...)
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 6/7] examples/tep_termination: " Stephen Hemminger
@ 2020-03-12 17:20 ` Stephen Hemminger
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
  8 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-12 17:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/vhost/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index ab649bf147e1..d759581f1904 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -669,7 +669,7 @@ static unsigned check_ports_num(unsigned nb_ports)
 	}
 
 	for (portid = 0; portid < num_ports; portid ++) {
-		if (!rte_eth_dev_is_valid_port(ports[portid])) {
+		if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER)) {
 			RTE_LOG(INFO, VHOST_PORT,
 				"\nSpecified port ID(%u) is not valid\n",
 				ports[portid]);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership Stephen Hemminger
@ 2020-03-15  7:45   ` Matan Azrad
  0 siblings, 0 replies; 31+ messages in thread
From: Matan Azrad @ 2020-03-15  7:45 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: stable

Hi  Stephen

From: Stephen Hemminger
> Applications using a port mask need a method to be able to test for (and
> reject) ethdev ports that are in use for other purposes.
> 

There is already function which is thread safe to get this information:
rte_eth_dev_owner_get.

I don't think we need one more.


> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Cc: matan@mellanox.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_ethdev/rte_ethdev.c           |  9 +++++++++
>  lib/librte_ethdev/rte_ethdev.h           | 15 +++++++++++++++
>  lib/librte_ethdev/rte_ethdev_version.map |  3 +++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 774c721b3484..38f99f417dba 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -685,6 +685,15 @@ rte_eth_dev_owner_unset(const uint16_t port_id,
> const uint64_t owner_id)
>  	return ret;
>  }
> 
> +int
> +rte_eth_dev_is_owned_by(uint16_t port_id, uint64_t owner_id) {
> +	if (!rte_eth_dev_is_valid_port(port_id))
> +		return 0;
> +
> +	return rte_eth_devices[port_id].data->owner.id == owner_id; }
> +
>  int
>  rte_eth_dev_owner_delete(const uint64_t owner_id)  { diff --git
> a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h index
> d1a593ad112a..91a8f9578282 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1530,6 +1530,21 @@ uint64_t rte_eth_find_next_owned_by(uint16_t
> port_id,
>  	     (unsigned int)p < (unsigned int)RTE_MAX_ETHPORTS; \
>  	     p = rte_eth_find_next_owned_by(p + 1, o))
> 
> +/**
> + * Test if a port is owned
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device
> + * @param	owner_id
> + *   The owner identifier.
> + *   RTE_ETH_DEV_NO_OWNER means test if port is not owned.
> + * @return
> + *   - 0 if port is out of range or not owned by owner_id
> + *   - 1 if device is associated with owner_id
> + */
> +__rte_experimental
> +int rte_eth_dev_is_owned_by(uint16_t port_id, uint64_t owner_id);
> +
>  /**
>   * Iterates over valid ethdev ports.
>   *
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map
> b/lib/librte_ethdev/rte_ethdev_version.map
> index 3f32fdecf722..95231fe3410c 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -230,4 +230,7 @@ EXPERIMENTAL {
> 
>  	# added in 20.02
>  	rte_flow_dev_dump;
> +
> +	# added in 20.05
> +	rte_eth_dev_is_owned_by;
>  };
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH 2/7] examples/l2fwd-cat: block attempts to use owned ports
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 2/7] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
@ 2020-03-15  7:51   ` Matan Azrad
  0 siblings, 0 replies; 31+ messages in thread
From: Matan Azrad @ 2020-03-15  7:51 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: stable

Hi Stephen

From: Stephen Hemminger

I think the correct terminology here is (can be the title):
Make the application be port ownership aware.

Some applications may be ownership aware and others not.

> If a ethdev port is in use for a sub device, then it should not be allowed in the
> portmask of application.
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Cc: matan@mellanox.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/l2fwd-cat/Makefile    | 2 ++
>  examples/l2fwd-cat/l2fwd-cat.c | 3 +++
>  examples/l2fwd-cat/meson.build | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/examples/l2fwd-cat/Makefile b/examples/l2fwd-cat/Makefile
> index b0e53c37e8d8..aa19347c1545 100644
> --- a/examples/l2fwd-cat/Makefile
> +++ b/examples/l2fwd-cat/Makefile
> @@ -20,6 +20,7 @@ static: build/$(APP)-static  PKGCONF ?= pkg-config
> 
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
>  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)  LDFLAGS_SHARED =
> $(shell $(PKGCONF) --libs libdpdk)  LDFLAGS_STATIC = -Wl,-Bstatic $(shell
> $(PKGCONF) --static --libs libdpdk) @@ -65,6 +66,7 @@ endif
> 
>  EXTRA_CFLAGS += -O3 -g -Wfatal-errors
> 
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
>  CFLAGS += -I$(PQOS_INSTALL_PATH)/../include
> 
>  LDLIBS += -L$(PQOS_INSTALL_PATH)
> diff --git a/examples/l2fwd-cat/l2fwd-cat.c b/examples/l2fwd-cat/l2fwd-
> cat.c index 6838f288c621..c1c59b2d81f2 100644
> --- a/examples/l2fwd-cat/l2fwd-cat.c
> +++ b/examples/l2fwd-cat/l2fwd-cat.c
> @@ -42,6 +42,9 @@ port_init(uint16_t port, struct rte_mempool
> *mbuf_pool)
>  	if (!rte_eth_dev_is_valid_port(port))
>  		return -1;
> 
> +	if (!rte_eth_dev_is_owned_by(portid, RTE_ETH_DEV_NO_OWNER))
> {
> +		return -1;
> +
>  	/* Configure the Ethernet device. */
>  	retval = rte_eth_dev_configure(port, rx_rings, tx_rings,
> &port_conf);
>  	if (retval != 0)
> diff --git a/examples/l2fwd-cat/meson.build b/examples/l2fwd-
> cat/meson.build index 4e2777a03358..5c062c1354e3 100644
> --- a/examples/l2fwd-cat/meson.build
> +++ b/examples/l2fwd-cat/meson.build
> @@ -6,6 +6,7 @@
>  # To build this example as a standalone application with an already-installed
> # DPDK instance, use 'make'
> 
> +allow_experimental_apis = true
>  pqos = cc.find_library('pqos', required: false)  build = pqos.found()  ext_deps
> += pqos
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
                   ` (6 preceding siblings ...)
  2020-03-12 17:20 ` [dpdk-dev] [PATCH 7/7] examples/vhost: " Stephen Hemminger
@ 2020-03-16 16:09 ` Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned Stephen Hemminger
                     ` (6 more replies)
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
  8 siblings, 7 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The failsafe, bonding, and netvsc PMD use eth dev port ownership
to control sub devices. These sub devices are hidden in the
normal iteration over ports but some applications use a direct
port mask.  In these cases, user may still (incorrectly) try
to directly use the sub devices.

This patch set introduces a helper function to check if
a port is under ownership. And the follow on patches use that.

An alternative proposed in earlier discussion is to have
each application try and own the port. But this solution would
be more invasive, and does not handle the case if secondary process
exits prematurely without releasing ownership. It makes more
sense to keep the concept of ownership as strictly part of
the device model and not part of the application layer.

v2 - rename the helper function and use rte_eth_dev_owner_get

Stephen Hemminger (6):
  rte_ethdev: add function to check if device is owned
  examples/l2fwd-cat: block attempts to use owned ports
  examples/l3fwd: block attempts to use owned ports
  examples/l3fwd-acl: block attempts to use owned ports
  examples/l3fwd-power: block attempts to use owned ports
  examples/tep_termination: block attempts to use owned ports

 examples/l2fwd-cat/Makefile              |  2 ++
 examples/l2fwd-cat/l2fwd-cat.c           |  3 +++
 examples/l2fwd-cat/meson.build           |  1 +
 examples/l3fwd-acl/Makefile              |  3 +++
 examples/l3fwd-acl/main.c                |  4 ++++
 examples/l3fwd-acl/meson.build           |  1 +
 examples/l3fwd-power/main.c              |  4 ++++
 examples/l3fwd/Makefile                  |  3 +++
 examples/l3fwd/main.c                    |  5 +++++
 examples/l3fwd/meson.build               |  1 +
 examples/tep_termination/Makefile        |  2 ++
 examples/tep_termination/main.c          |  2 +-
 examples/tep_termination/meson.build     |  1 +
 lib/librte_ethdev/rte_ethdev.c           | 12 ++++++++++++
 lib/librte_ethdev/rte_ethdev.h           | 13 +++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  3 +++
 16 files changed, 59 insertions(+), 1 deletion(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
@ 2020-03-16 16:09   ` Stephen Hemminger
  2020-04-01 21:42     ` Thomas Monjalon
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 2/6] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
                     ` (5 subsequent siblings)
  6 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

This is a simple helper function to check if device is owned
(being used as a sub-device). It is more convienent than having
applications call rte_eth_dev_owner_get and check the result.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2 - rename the helper function and use rte_eth_dev_owner_get

 lib/librte_ethdev/rte_ethdev.c           | 12 ++++++++++++
 lib/librte_ethdev/rte_ethdev.h           | 13 +++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  3 +++
 3 files changed, 28 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 774c721b3484..6e2d0110f65e 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -737,6 +737,18 @@ rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
 	return ret;
 }
 
+int
+rte_eth_dev_is_owned(uint16_t port_id)
+{
+	struct rte_eth_dev_owner owner;
+	int ret;
+
+	ret = rte_eth_dev_owner_get(port_id, &owner);
+	if (ret == 0)
+		ret = (owner.id != RTE_ETH_DEV_NO_OWNER);
+	return ret;
+}
+
 int
 rte_eth_dev_socket_id(uint16_t port_id)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index d1a593ad112a..3a8a4bcde4b7 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1693,6 +1693,19 @@ __rte_experimental
 int rte_eth_dev_owner_get(const uint16_t port_id,
 		struct rte_eth_dev_owner *owner);
 
+/**
+ * Check if port_id is part of a sub-device
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device
+ * @return
+ *   - 1 if device is associated with an owner
+ *   - 0 if port is not owned
+ *   - negative errno value on error.
+ */
+__rte_experimental
+int rte_eth_dev_is_owned(uint16_t port_id);
+
 /**
  * Get the number of ports which are usable for the application.
  *
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 3f32fdecf722..435df2dba149 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -230,4 +230,7 @@ EXPERIMENTAL {
 
 	# added in 20.02
 	rte_flow_dev_dump;
+
+	# added in 20.05
+	rte_eth_dev_is_owned;
 };
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 2/6] examples/l2fwd-cat: block attempts to use owned ports
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned Stephen Hemminger
@ 2020-03-16 16:09   ` Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 3/6] examples/l3fwd: " Stephen Hemminger
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l2fwd-cat/Makefile    | 2 ++
 examples/l2fwd-cat/l2fwd-cat.c | 3 +++
 examples/l2fwd-cat/meson.build | 1 +
 3 files changed, 6 insertions(+)

diff --git a/examples/l2fwd-cat/Makefile b/examples/l2fwd-cat/Makefile
index b0e53c37e8d8..aa19347c1545 100644
--- a/examples/l2fwd-cat/Makefile
+++ b/examples/l2fwd-cat/Makefile
@@ -20,6 +20,7 @@ static: build/$(APP)-static
 PKGCONF ?= pkg-config
 
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
@@ -65,6 +66,7 @@ endif
 
 EXTRA_CFLAGS += -O3 -g -Wfatal-errors
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -I$(PQOS_INSTALL_PATH)/../include
 
 LDLIBS += -L$(PQOS_INSTALL_PATH)
diff --git a/examples/l2fwd-cat/l2fwd-cat.c b/examples/l2fwd-cat/l2fwd-cat.c
index 6838f288c621..e89b3568bc86 100644
--- a/examples/l2fwd-cat/l2fwd-cat.c
+++ b/examples/l2fwd-cat/l2fwd-cat.c
@@ -42,6 +42,9 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 	if (!rte_eth_dev_is_valid_port(port))
 		return -1;
 
+	if (rte_eth_dev_is_owned(portid))
+		return -1;
+
 	/* Configure the Ethernet device. */
 	retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
 	if (retval != 0)
diff --git a/examples/l2fwd-cat/meson.build b/examples/l2fwd-cat/meson.build
index 4e2777a03358..5c062c1354e3 100644
--- a/examples/l2fwd-cat/meson.build
+++ b/examples/l2fwd-cat/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 pqos = cc.find_library('pqos', required: false)
 build = pqos.found()
 ext_deps += pqos
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 3/6] examples/l3fwd: block attempts to use owned ports
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 2/6] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
@ 2020-03-16 16:09   ` Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 4/6] examples/l3fwd-acl: " Stephen Hemminger
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of l3fwd.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd/Makefile    | 3 +++
 examples/l3fwd/main.c      | 5 +++++
 examples/l3fwd/meson.build | 1 +
 3 files changed, 9 insertions(+)

diff --git a/examples/l3fwd/Makefile b/examples/l3fwd/Makefile
index 59a110d12a49..59cd00abe6e9 100644
--- a/examples/l3fwd/Makefile
+++ b/examples/l3fwd/Makefile
@@ -25,6 +25,8 @@ 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
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
 	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
 
@@ -50,6 +52,7 @@ RTE_TARGET ?= $(notdir $(abspath $(dir $(firstword $(wildcard $(RTE_SDK)/*/.conf
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -I$(SRCDIR)
 CFLAGS += -O3 $(USER_FLAGS)
 CFLAGS += $(WERROR_FLAGS)
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index dda430d68a20..ab717e3bfd2e 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -221,6 +221,11 @@ check_port_config(void)
 			printf("port %u is not present on the board\n", portid);
 			return -1;
 		}
+
+		if (rte_eth_dev_is_owned(portid)) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
diff --git a/examples/l3fwd/meson.build b/examples/l3fwd/meson.build
index ebed3b518c4b..7d72b1b365ae 100644
--- a/examples/l3fwd/meson.build
+++ b/examples/l3fwd/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 deps += ['hash', 'lpm', 'eventdev']
 sources = files(
 	'l3fwd_em.c', 'l3fwd_lpm.c', 'l3fwd_event.c',
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 4/6] examples/l3fwd-acl: block attempts to use owned ports
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 3/6] examples/l3fwd: " Stephen Hemminger
@ 2020-03-16 16:09   ` Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 5/6] examples/l3fwd-power: " Stephen Hemminger
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-acl/Makefile    | 3 +++
 examples/l3fwd-acl/main.c      | 4 ++++
 examples/l3fwd-acl/meson.build | 1 +
 3 files changed, 8 insertions(+)

diff --git a/examples/l3fwd-acl/Makefile b/examples/l3fwd-acl/Makefile
index d9909584b1c6..ff14b4fdd593 100644
--- a/examples/l3fwd-acl/Makefile
+++ b/examples/l3fwd-acl/Makefile
@@ -20,7 +20,9 @@ static: build/$(APP)-static
 PKGCONF ?= pkg-config
 
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
+
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
 
@@ -51,6 +53,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
index fa92a2829740..d9bee9ade323 100644
--- a/examples/l3fwd-acl/main.c
+++ b/examples/l3fwd-acl/main.c
@@ -1470,6 +1470,10 @@ check_port_config(void)
 			printf("port %u is not present on the board\n", portid);
 			return -1;
 		}
+		if (rte_eth_dev_is_owned(portid)) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
diff --git a/examples/l3fwd-acl/meson.build b/examples/l3fwd-acl/meson.build
index 7096e00c1073..6fa468b3aa9c 100644
--- a/examples/l3fwd-acl/meson.build
+++ b/examples/l3fwd-acl/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 deps += ['acl', 'lpm', 'hash']
 sources = files(
 	'main.c'
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 5/6] examples/l3fwd-power: block attempts to use owned ports
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
                     ` (3 preceding siblings ...)
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 4/6] examples/l3fwd-acl: " Stephen Hemminger
@ 2020-03-16 16:09   ` Stephen Hemminger
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 6/6] examples/tep_termination: " Stephen Hemminger
  2020-03-17  8:18   ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Matan Azrad
  6 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of l3fwd.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-power/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index c7fe0ec03495..60f5ddd8fb9e 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1396,6 +1396,10 @@ check_port_config(void)
 								portid);
 			return -1;
 		}
+		if (rte_eth_dev_is_owned(portid)) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 6/6] examples/tep_termination: block attempts to use owned ports
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
                     ` (4 preceding siblings ...)
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 5/6] examples/l3fwd-power: " Stephen Hemminger
@ 2020-03-16 16:09   ` Stephen Hemminger
  2020-03-17  8:18   ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Matan Azrad
  6 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-03-16 16:09 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/tep_termination/Makefile    | 2 ++
 examples/tep_termination/main.c      | 2 +-
 examples/tep_termination/meson.build | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

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..96c2d934cd09 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -507,7 +507,7 @@ check_ports_num(unsigned max_nb_ports)
 	}
 
 	for (portid = 0; portid < nb_ports; portid++) {
-		if (!rte_eth_dev_is_valid_port(ports[portid])) {
+		if (rte_eth_dev_is_owned(portid) != 0) {
 			RTE_LOG(INFO, VHOST_PORT,
 				"\nSpecified port ID(%u) is not valid\n",
 				ports[portid]);
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(
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
                     ` (5 preceding siblings ...)
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 6/6] examples/tep_termination: " Stephen Hemminger
@ 2020-03-17  8:18   ` Matan Azrad
  6 siblings, 0 replies; 31+ messages in thread
From: Matan Azrad @ 2020-03-17  8:18 UTC (permalink / raw)
  To: Stephen Hemminger, dev



From: Stephen Hemminger
> The failsafe, bonding, and netvsc PMD use eth dev port ownership to control
> sub devices. These sub devices are hidden in the normal iteration over ports
> but some applications use a direct port mask.  In these cases, user may still
> (incorrectly) try to directly use the sub devices.
> 
> This patch set introduces a helper function to check if a port is under
> ownership. And the follow on patches use that.
> 
> An alternative proposed in earlier discussion is to have each application try
> and own the port. But this solution would be more invasive, and does not
> handle the case if secondary process exits prematurely without releasing
> ownership. It makes more sense to keep the concept of ownership as strictly
> part of the device model and not part of the application layer.
> 
> v2 - rename the helper function and use rte_eth_dev_owner_get

Series-acked-by: Matan Azrad <matan@mellanox.com>

> Stephen Hemminger (6):
>   rte_ethdev: add function to check if device is owned
>   examples/l2fwd-cat: block attempts to use owned ports
>   examples/l3fwd: block attempts to use owned ports
>   examples/l3fwd-acl: block attempts to use owned ports
>   examples/l3fwd-power: block attempts to use owned ports
>   examples/tep_termination: block attempts to use owned ports
> 
>  examples/l2fwd-cat/Makefile              |  2 ++
>  examples/l2fwd-cat/l2fwd-cat.c           |  3 +++
>  examples/l2fwd-cat/meson.build           |  1 +
>  examples/l3fwd-acl/Makefile              |  3 +++
>  examples/l3fwd-acl/main.c                |  4 ++++
>  examples/l3fwd-acl/meson.build           |  1 +
>  examples/l3fwd-power/main.c              |  4 ++++
>  examples/l3fwd/Makefile                  |  3 +++
>  examples/l3fwd/main.c                    |  5 +++++
>  examples/l3fwd/meson.build               |  1 +
>  examples/tep_termination/Makefile        |  2 ++
>  examples/tep_termination/main.c          |  2 +-
>  examples/tep_termination/meson.build     |  1 +
>  lib/librte_ethdev/rte_ethdev.c           | 12 ++++++++++++
>  lib/librte_ethdev/rte_ethdev.h           | 13 +++++++++++++
>  lib/librte_ethdev/rte_ethdev_version.map |  3 +++
>  16 files changed, 59 insertions(+), 1 deletion(-)
> 
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned
  2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned Stephen Hemminger
@ 2020-04-01 21:42     ` Thomas Monjalon
  2020-04-01 22:24       ` Stephen Hemminger
  0 siblings, 1 reply; 31+ messages in thread
From: Thomas Monjalon @ 2020-04-01 21:42 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, ferruh.yigit, arybchenko

16/03/2020 17:09, Stephen Hemminger:
> This is a simple helper function to check if device is owned
> (being used as a sub-device).

I would suggest not restricting ownership to sub-device case.

> It is more convienent than having
> applications call rte_eth_dev_owner_get and check the result.

Yes it is more convenient but I don't like adding such simple wrapper.

I propose to extend rte_eth_dev_owner_get() behaviour:
if the owner pointer is NULL, the function returns 0 only
if an owner (not RTE_ETH_DEV_NO_OWNER) is found.

So instead of using your wrapper:
	if (rte_eth_dev_is_owned(port_id))
you can use:
	if (rte_eth_dev_owner_get(port_id, NULL) == 0)

[...]
> +int
> +rte_eth_dev_is_owned(uint16_t port_id)
> +{
> +	struct rte_eth_dev_owner owner;
> +	int ret;
> +
> +	ret = rte_eth_dev_owner_get(port_id, &owner);
> +	if (ret == 0)
> +		ret = (owner.id != RTE_ETH_DEV_NO_OWNER);
> +	return ret;
> +}




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned
  2020-04-01 21:42     ` Thomas Monjalon
@ 2020-04-01 22:24       ` Stephen Hemminger
  2020-04-02  8:04         ` Thomas Monjalon
  0 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-01 22:24 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, ferruh.yigit, arybchenko

On Wed, 01 Apr 2020 23:42:44 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 16/03/2020 17:09, Stephen Hemminger:
> > This is a simple helper function to check if device is owned
> > (being used as a sub-device).  
> 
> I would suggest not restricting ownership to sub-device case.
> 
> > It is more convienent than having
> > applications call rte_eth_dev_owner_get and check the result.  
> 
> Yes it is more convenient but I don't like adding such simple wrapper.
> 
> I propose to extend rte_eth_dev_owner_get() behaviour:
> if the owner pointer is NULL, the function returns 0 only
> if an owner (not RTE_ETH_DEV_NO_OWNER) is found.
> 
> So instead of using your wrapper:
> 	if (rte_eth_dev_is_owned(port_id))
> you can use:
> 	if (rte_eth_dev_owner_get(port_id, NULL) == 0)

That is not how rte_eth_dev_owner_get works now.
Passing NULL to it would crash.

And if devices is not owned rte_eth_dev_owner_get() returns 0
and owner is set to a magic value. We could change the ABI for this 
since it is marked experimental. But that seems more risky.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned
  2020-04-01 22:24       ` Stephen Hemminger
@ 2020-04-02  8:04         ` Thomas Monjalon
  0 siblings, 0 replies; 31+ messages in thread
From: Thomas Monjalon @ 2020-04-02  8:04 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, ferruh.yigit, arybchenko

02/04/2020 00:24, Stephen Hemminger:
> On Wed, 01 Apr 2020 23:42:44 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 16/03/2020 17:09, Stephen Hemminger:
> > > This is a simple helper function to check if device is owned
> > > (being used as a sub-device).  
> > 
> > I would suggest not restricting ownership to sub-device case.
> > 
> > > It is more convienent than having
> > > applications call rte_eth_dev_owner_get and check the result.  
> > 
> > Yes it is more convenient but I don't like adding such simple wrapper.
> > 
> > I propose to extend rte_eth_dev_owner_get() behaviour:
> > if the owner pointer is NULL, the function returns 0 only
> > if an owner (not RTE_ETH_DEV_NO_OWNER) is found.
> > 
> > So instead of using your wrapper:
> > 	if (rte_eth_dev_is_owned(port_id))
> > you can use:
> > 	if (rte_eth_dev_owner_get(port_id, NULL) == 0)
> 
> That is not how rte_eth_dev_owner_get works now.
> Passing NULL to it would crash.

Indeed this is why I am proposing to extend it
and fill this case.

> And if devices is not owned rte_eth_dev_owner_get() returns 0
> and owner is set to a magic value. We could change the ABI for this 
> since it is marked experimental. But that seems more risky.

This is not an ABI change.
Just manage a case which was crashing previously.






^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 0/6] checking for owned ports in portmask
  2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
                   ` (7 preceding siblings ...)
  2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
@ 2020-04-02 17:19 ` Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned Stephen Hemminger
                     ` (5 more replies)
  8 siblings, 6 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The failsafe, bonding, and netvsc PMD use eth dev port ownership
to control sub devices. These sub devices are hidden in the
normal iteration over ports but some applications use a direct
port mask.  In these cases, user may still (incorrectly) try
to directly use the sub devices.

This patch set changes the rte_eth_dev_owner_get function
so that it can be used to test for any ownership.
And the follow on patches use that.

An alternative proposed in earlier discussion is to have
each application try and own the port. But this solution would
be more invasive, and does not handle the case if secondary process
exits prematurely without releasing ownership. It makes more
sense to keep the concept of ownership as strictly part of
the device model and not part of the application layer.

v3 - use rte_eth_dev_owner_get directly, and change the function return
     rework port setup in tep_termination example
v2 - rename the helper function and use rte_eth_dev_owner_get

Stephen Hemminger (6):
  rte_ethdev: change rte_eth_dev_owner_get to return error if unowned
  examples/l2fwd-cat: make applicaton aware of port ownership
  examples/l3fwd: make applicaton aware of port ownership
  examples/l3fwd-acl: make applicaton aware of port ownership
  examples/l3fwd-power: make applicaton aware of port ownership
  examples/tep_termination: rework the port setup logic

 drivers/net/failsafe/failsafe_eal.c  |  4 +-
 drivers/net/netvsc/hn_vf.c           | 11 ++---
 examples/l2fwd-cat/Makefile          |  2 +
 examples/l2fwd-cat/l2fwd-cat.c       |  3 ++
 examples/l2fwd-cat/meson.build       |  1 +
 examples/l3fwd-acl/Makefile          |  3 ++
 examples/l3fwd-acl/main.c            |  4 ++
 examples/l3fwd-acl/meson.build       |  1 +
 examples/l3fwd-power/main.c          |  4 ++
 examples/l3fwd/Makefile              |  3 ++
 examples/l3fwd/main.c                |  5 ++
 examples/l3fwd/meson.build           |  1 +
 examples/tep_termination/Makefile    |  2 +
 examples/tep_termination/main.c      | 70 ++++++++++------------------
 examples/tep_termination/meson.build |  1 +
 lib/librte_ethdev/rte_ethdev.c       |  4 +-
 lib/librte_ethdev/rte_ethdev.h       |  6 ++-
 17 files changed, 69 insertions(+), 56 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
@ 2020-04-02 17:19   ` Stephen Hemminger
  2020-04-10 15:45     ` Ferruh Yigit
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 2/6] examples/l2fwd-cat: make applicaton aware of port ownership Stephen Hemminger
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

For applications that want to check if device is owned,
change the return value of rte_eth_dev_owner_get to return -ENOENT
if there is no owner for the device.

Change the two drivers (failsafe and netvsc) that are using
this experimental API to handle the new return value.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/failsafe/failsafe_eal.c |  4 ++--
 drivers/net/netvsc/hn_vf.c          | 11 +++++------
 lib/librte_ethdev/rte_ethdev.c      |  4 +++-
 lib/librte_ethdev/rte_ethdev.h      |  6 ++++--
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index b9fc508673f2..8a1b5e068840 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -63,8 +63,8 @@ fs_bus_init(struct rte_eth_dev *dev)
 			 * The NEW callback tried to take ownership, check
 			 * whether it succeed or didn't.
 			 */
-			rte_eth_dev_owner_get(pid, &pid_owner);
-			if (pid_owner.id != PRIV(dev)->my_owner.id) {
+			if (rte_eth_dev_owner_get(pid, &pid_owner) == 0 &&
+			    pid_owner.id != PRIV(dev)->my_owner.id) {
 				INFO("sub_device %d owner(%s_%016"PRIX64") is not my,"
 				     " owner(%s_%016"PRIX64"), will try again later",
 				     i, pid_owner.name, pid_owner.id,
diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 7a3734cadfa4..b56f60b28a62 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -63,16 +63,15 @@ static int hn_vf_attach(struct hn_data *hv, uint16_t port_id)
 	}
 
 	ret = rte_eth_dev_owner_get(port_id, &owner);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "Can not find owner for port %d", port_id);
-		return ret;
-	}
-
-	if (owner.id != RTE_ETH_DEV_NO_OWNER) {
+	if (ret == 0) {
 		PMD_DRV_LOG(ERR, "Port %u already owned by other device %s",
 			    port_id, owner.name);
 		return -EBUSY;
 	}
+	if (ret != -ENOENT) {
+		PMD_DRV_LOG(ERR, "Can not find owner for port %d", port_id);
+		return ret;
+	}
 
 	ret = rte_eth_dev_owner_set(port_id, &hv->owner);
 	if (ret < 0) {
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 0854ef883270..0c4645cf9832 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -729,7 +729,9 @@ rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
 		RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
 			port_id);
 		ret = -ENODEV;
-	} else {
+	} else if (ethdev->data->owner.id == RTE_ETH_DEV_NO_OWNER) {
+		ret = -ENOENT;
+	} else if (owner != NULL) {
 		rte_memcpy(owner, &ethdev->data->owner, sizeof(*owner));
 	}
 
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index d1a593ad112a..0e59d39de54c 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1685,9 +1685,11 @@ int rte_eth_dev_owner_delete(const uint64_t owner_id);
  * @param	port_id
  *  The port identifier.
  * @param	owner
- *  The owner structure pointer to fill.
+ *  The owner structure pointer to fill (optional can be NULL)
  * @return
- *  0 on success, negative errno value on error..
+ *  0 on success,
+ *  -ENODEV if port_id is not valid
+ *  -ENOENT if the device is ownerless.
  */
 __rte_experimental
 int rte_eth_dev_owner_get(const uint16_t port_id,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 2/6] examples/l2fwd-cat: make applicaton aware of port ownership
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned Stephen Hemminger
@ 2020-04-02 17:19   ` Stephen Hemminger
  2020-04-10 15:47     ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 3/6] examples/l3fwd: " Stephen Hemminger
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l2fwd-cat/Makefile    | 2 ++
 examples/l2fwd-cat/l2fwd-cat.c | 3 +++
 examples/l2fwd-cat/meson.build | 1 +
 3 files changed, 6 insertions(+)

diff --git a/examples/l2fwd-cat/Makefile b/examples/l2fwd-cat/Makefile
index b0e53c37e8d8..aa19347c1545 100644
--- a/examples/l2fwd-cat/Makefile
+++ b/examples/l2fwd-cat/Makefile
@@ -20,6 +20,7 @@ static: build/$(APP)-static
 PKGCONF ?= pkg-config
 
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
@@ -65,6 +66,7 @@ endif
 
 EXTRA_CFLAGS += -O3 -g -Wfatal-errors
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -I$(PQOS_INSTALL_PATH)/../include
 
 LDLIBS += -L$(PQOS_INSTALL_PATH)
diff --git a/examples/l2fwd-cat/l2fwd-cat.c b/examples/l2fwd-cat/l2fwd-cat.c
index 6838f288c621..e370efeaf1f8 100644
--- a/examples/l2fwd-cat/l2fwd-cat.c
+++ b/examples/l2fwd-cat/l2fwd-cat.c
@@ -42,6 +42,9 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 	if (!rte_eth_dev_is_valid_port(port))
 		return -1;
 
+	if (rte_eth_dev_owner_get(port, NULL) == 0)
+		return -1;
+
 	/* Configure the Ethernet device. */
 	retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
 	if (retval != 0)
diff --git a/examples/l2fwd-cat/meson.build b/examples/l2fwd-cat/meson.build
index 4e2777a03358..5c062c1354e3 100644
--- a/examples/l2fwd-cat/meson.build
+++ b/examples/l2fwd-cat/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 pqos = cc.find_library('pqos', required: false)
 build = pqos.found()
 ext_deps += pqos
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 3/6] examples/l3fwd: make applicaton aware of port ownership
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 2/6] examples/l2fwd-cat: make applicaton aware of port ownership Stephen Hemminger
@ 2020-04-02 17:19   ` Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 4/6] examples/l3fwd-acl: " Stephen Hemminger
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of l3fwd.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd/Makefile    | 3 +++
 examples/l3fwd/main.c      | 5 +++++
 examples/l3fwd/meson.build | 1 +
 3 files changed, 9 insertions(+)

diff --git a/examples/l3fwd/Makefile b/examples/l3fwd/Makefile
index 59a110d12a49..59cd00abe6e9 100644
--- a/examples/l3fwd/Makefile
+++ b/examples/l3fwd/Makefile
@@ -25,6 +25,8 @@ 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
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
 	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
 
@@ -50,6 +52,7 @@ RTE_TARGET ?= $(notdir $(abspath $(dir $(firstword $(wildcard $(RTE_SDK)/*/.conf
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -I$(SRCDIR)
 CFLAGS += -O3 $(USER_FLAGS)
 CFLAGS += $(WERROR_FLAGS)
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index dda430d68a20..110d4cc109d1 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -221,6 +221,11 @@ check_port_config(void)
 			printf("port %u is not present on the board\n", portid);
 			return -1;
 		}
+
+		if (rte_eth_dev_owner_get(portid, NULL) == 0) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
diff --git a/examples/l3fwd/meson.build b/examples/l3fwd/meson.build
index ebed3b518c4b..7d72b1b365ae 100644
--- a/examples/l3fwd/meson.build
+++ b/examples/l3fwd/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 deps += ['hash', 'lpm', 'eventdev']
 sources = files(
 	'l3fwd_em.c', 'l3fwd_lpm.c', 'l3fwd_event.c',
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 4/6] examples/l3fwd-acl: make applicaton aware of port ownership
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 3/6] examples/l3fwd: " Stephen Hemminger
@ 2020-04-02 17:19   ` Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 5/6] examples/l3fwd-power: " Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 6/6] examples/tep_termination: rework the port setup logic Stephen Hemminger
  5 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of application.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-acl/Makefile    | 3 +++
 examples/l3fwd-acl/main.c      | 4 ++++
 examples/l3fwd-acl/meson.build | 1 +
 3 files changed, 8 insertions(+)

diff --git a/examples/l3fwd-acl/Makefile b/examples/l3fwd-acl/Makefile
index d9909584b1c6..ff14b4fdd593 100644
--- a/examples/l3fwd-acl/Makefile
+++ b/examples/l3fwd-acl/Makefile
@@ -20,7 +20,9 @@ static: build/$(APP)-static
 PKGCONF ?= pkg-config
 
 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
+
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
 
@@ -51,6 +53,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
index fa92a2829740..dbdea8db241a 100644
--- a/examples/l3fwd-acl/main.c
+++ b/examples/l3fwd-acl/main.c
@@ -1470,6 +1470,10 @@ check_port_config(void)
 			printf("port %u is not present on the board\n", portid);
 			return -1;
 		}
+		if (rte_eth_dev_owner_get(portid, NULL) == 0) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
diff --git a/examples/l3fwd-acl/meson.build b/examples/l3fwd-acl/meson.build
index 7096e00c1073..6fa468b3aa9c 100644
--- a/examples/l3fwd-acl/meson.build
+++ b/examples/l3fwd-acl/meson.build
@@ -6,6 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'
 
+allow_experimental_apis = true
 deps += ['acl', 'lpm', 'hash']
 sources = files(
 	'main.c'
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 5/6] examples/l3fwd-power: make applicaton aware of port ownership
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
                     ` (3 preceding siblings ...)
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 4/6] examples/l3fwd-acl: " Stephen Hemminger
@ 2020-04-02 17:19   ` Stephen Hemminger
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 6/6] examples/tep_termination: rework the port setup logic Stephen Hemminger
  5 siblings, 0 replies; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, matan, stable

If a ethdev port is in use for a sub device, then it should not
be allowed in the portmask of l3fwd.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: matan@mellanox.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-power/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index c7fe0ec03495..186d6ac8e271 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1396,6 +1396,10 @@ check_port_config(void)
 								portid);
 			return -1;
 		}
+		if (rte_eth_dev_owner_get(portid, NULL) == 0) {
+			printf("port %u is already in use\n", portid);
+			return -1;
+		}
 	}
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 6/6] examples/tep_termination: rework the port setup logic
  2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
                     ` (4 preceding siblings ...)
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 5/6] examples/l3fwd-power: " Stephen Hemminger
@ 2020-04-02 17:19   ` Stephen Hemminger
  2020-04-10 16:16     ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  5 siblings, 1 reply; 31+ messages in thread
From: Stephen Hemminger @ 2020-04-02 17:19 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, jijiang.liu, stable

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned Stephen Hemminger
@ 2020-04-10 15:45     ` Ferruh Yigit
  0 siblings, 0 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-04-10 15:45 UTC (permalink / raw)
  To: Stephen Hemminger, dev

On 4/2/2020 6:19 PM, Stephen Hemminger wrote:
> For applications that want to check if device is owned,
> change the return value of rte_eth_dev_owner_get to return -ENOENT
> if there is no owner for the device.
> 
> Change the two drivers (failsafe and netvsc) that are using
> this experimental API to handle the new return value.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

<...>

> @@ -1685,9 +1685,11 @@ int rte_eth_dev_owner_delete(const uint64_t owner_id);
>   * @param	port_id
>   *  The port identifier.
>   * @param	owner
> - *  The owner structure pointer to fill.
> + *  The owner structure pointer to fill (optional can be NULL)
>   * @return
> - *  0 on success, negative errno value on error..
> + *  0 on success,
> + *  -ENODEV if port_id is not valid
> + *  -ENOENT if the device is ownerless.

Not sure if port not having an owner is error, I understand the motivation but
what about send a special value, like 1, instead of error:
0: get owner success, port owned
1: get owner success, port not owned
-ENODEV: get owner failed, port_id is not valid


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 2/6] examples/l2fwd-cat: make applicaton aware of port ownership
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 2/6] examples/l2fwd-cat: make applicaton aware of port ownership Stephen Hemminger
@ 2020-04-10 15:47     ` Ferruh Yigit
  0 siblings, 0 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-04-10 15:47 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: matan, stable

On 4/2/2020 6:19 PM, Stephen Hemminger wrote:
> If a ethdev port is in use for a sub device, then it should not
> be allowed in the portmask of application.
> 
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Cc: matan@mellanox.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

<...>

> @@ -42,6 +42,9 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
>  	if (!rte_eth_dev_is_valid_port(port))
>  		return -1;
>  
> +	if (rte_eth_dev_owner_get(port, NULL) == 0)
> +		return -1;

'port_init' is called by the loop:

 RTE_ETH_FOREACH_DEV(portid)
    if (port_init(portid, mbuf_pool) != 0)

And 'RTE_ETH_FOREACH_DEV' already iterates only on ports not owned, so this
check is redundant.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 6/6] examples/tep_termination: rework the port setup logic
  2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 6/6] examples/tep_termination: rework the port setup logic Stephen Hemminger
@ 2020-04-10 16:16     ` Ferruh Yigit
  0 siblings, 0 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-04-10 16:16 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: jijiang.liu, stable

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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2020-04-10 16:16 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 17:20 [dpdk-dev] [PATCH 0/7] checking for owned ports in portmask Stephen Hemminger
2020-03-12 17:20 ` [dpdk-dev] [PATCH 1/7] ethdev: add function to test port ownership Stephen Hemminger
2020-03-15  7:45   ` Matan Azrad
2020-03-12 17:20 ` [dpdk-dev] [PATCH 2/7] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
2020-03-15  7:51   ` Matan Azrad
2020-03-12 17:20 ` [dpdk-dev] [PATCH 3/7] examples/l3fwd: " Stephen Hemminger
2020-03-12 17:20 ` [dpdk-dev] [PATCH 4/7] examples/l3fwd-acl: " Stephen Hemminger
2020-03-12 17:20 ` [dpdk-dev] [PATCH 5/7] examples/l3fwd-power: " Stephen Hemminger
2020-03-12 17:20 ` [dpdk-dev] [PATCH 6/7] examples/tep_termination: " Stephen Hemminger
2020-03-12 17:20 ` [dpdk-dev] [PATCH 7/7] examples/vhost: " Stephen Hemminger
2020-03-16 16:09 ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Stephen Hemminger
2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 1/6] rte_ethdev: add function to check if device is owned Stephen Hemminger
2020-04-01 21:42     ` Thomas Monjalon
2020-04-01 22:24       ` Stephen Hemminger
2020-04-02  8:04         ` Thomas Monjalon
2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 2/6] examples/l2fwd-cat: block attempts to use owned ports Stephen Hemminger
2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 3/6] examples/l3fwd: " Stephen Hemminger
2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 4/6] examples/l3fwd-acl: " Stephen Hemminger
2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 5/6] examples/l3fwd-power: " Stephen Hemminger
2020-03-16 16:09   ` [dpdk-dev] [PATCH v2 6/6] examples/tep_termination: " Stephen Hemminger
2020-03-17  8:18   ` [dpdk-dev] [PATCH v2 0/6] check for owned ports in portmask Matan Azrad
2020-04-02 17:19 ` [dpdk-dev] [PATCH v3 0/6] checking " Stephen Hemminger
2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 1/6] rte_ethdev: change rte_eth_dev_owner_get to return error if unowned Stephen Hemminger
2020-04-10 15:45     ` Ferruh Yigit
2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 2/6] examples/l2fwd-cat: make applicaton aware of port ownership Stephen Hemminger
2020-04-10 15:47     ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 3/6] examples/l3fwd: " Stephen Hemminger
2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 4/6] examples/l3fwd-acl: " Stephen Hemminger
2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 5/6] examples/l3fwd-power: " Stephen Hemminger
2020-04-02 17:19   ` [dpdk-dev] [PATCH v3 6/6] examples/tep_termination: rework the port setup logic Stephen Hemminger
2020-04-10 16:16     ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).