[dpdk-dev] [PATCH 0/3] deprecate attach and detach functions

[dpdk-dev] [PATCH 0/3] deprecate attach and detach functions

[Andrew Rybchenko]

As discussed in [1] EAL hotplug should be used directly to add/remove devices.

app/pdump changes are build-tested only.

[1] http://mails.dpdk.org/archives/dev/2018-July/107507.html

Andrew Rybchenko (3):
  app/pdump: use hotplug add instead of attach
  ethdev: deprecate attach and detach functions
  eal: deprecate device attach and detach functions

 app/pdump/main.c                        | 94 ++++++++++++++++++-------
 app/test-pmd/Makefile                   |  1 +
 app/test-pmd/meson.build                |  1 +
 doc/guides/rel_notes/deprecation.rst    | 12 ++++
 lib/librte_eal/common/include/rte_dev.h |  2 +
 lib/librte_ethdev/rte_ethdev.h          |  2 +
 6 files changed, 88 insertions(+), 24 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 1/3] app/pdump: use hotplug add instead of attach

[Andrew Rybchenko]

rte_eth_dev_attach() is to be deprecated.

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 app/pdump/main.c | 94 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 24 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 6bcf8c498..ac2287124 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -38,8 +38,9 @@
 #define PDUMP_MSIZE_ARG "mbuf-size"
 #define PDUMP_NUM_MBUFS_ARG "total-num-mbufs"
 
-#define VDEV_PCAP "net_pcap_%s_%d,tx_pcap=%s"
-#define VDEV_IFACE "net_pcap_%s_%d,tx_iface=%s"
+#define VDEV_NAME_FMT "net_pcap_%s_%d"
+#define VDEV_PCAP_ARGS_FMT "tx_pcap=%s"
+#define VDEV_IFACE_ARGS_FMT "tx_iface=%s"
 #define TX_STREAM_SIZE 64
 
 #define MP_NAME "pdump_pool_%d"
@@ -570,6 +571,7 @@ create_mp_ring_vdev(void)
 	uint16_t portid;
 	struct pdump_tuples *pt = NULL;
 	struct rte_mempool *mbuf_pool = NULL;
+	char vdev_name[SIZE];
 	char vdev_args[SIZE];
 	char ring_name[SIZE];
 	char mempool_name[SIZE];
@@ -619,17 +621,28 @@ create_mp_ring_vdev(void)
 			}
 
 			/* create vdevs */
+			snprintf(vdev_name, sizeof(vdev_name),
+				 VDEV_NAME_FMT, RX_STR, i);
 			(pt->rx_vdev_stream_type == IFACE) ?
-			snprintf(vdev_args, SIZE, VDEV_IFACE, RX_STR, i,
-			pt->rx_dev) :
-			snprintf(vdev_args, SIZE, VDEV_PCAP, RX_STR, i,
-			pt->rx_dev);
-			if (rte_eth_dev_attach(vdev_args, &portid) < 0) {
+			snprintf(vdev_args, sizeof(vdev_args),
+				 VDEV_IFACE_ARGS_FMT, pt->rx_dev) :
+			snprintf(vdev_args, sizeof(vdev_args),
+				 VDEV_PCAP_ARGS_FMT, pt->rx_dev);
+			if (rte_eal_hotplug_add("vdev", vdev_name,
+						vdev_args) < 0) {
 				cleanup_rings();
 				rte_exit(EXIT_FAILURE,
 					"vdev creation failed:%s:%d\n",
 					__func__, __LINE__);
 			}
+			if (rte_eth_dev_get_port_by_name(vdev_name,
+							 &portid) != 0) {
+				rte_eal_hotplug_remove("vdev", vdev_name);
+				cleanup_rings();
+				rte_exit(EXIT_FAILURE,
+					"cannot find added vdev %s:%s:%d\n",
+					vdev_name, __func__, __LINE__);
+			}
 			pt->rx_vdev_id = portid;
 
 			/* configure vdev */
@@ -638,18 +651,29 @@ create_mp_ring_vdev(void)
 			if (pt->single_pdump_dev)
 				pt->tx_vdev_id = portid;
 			else {
-				(pt->tx_vdev_stream_type == IFACE) ?
-				snprintf(vdev_args, SIZE, VDEV_IFACE, TX_STR, i,
-				pt->tx_dev) :
-				snprintf(vdev_args, SIZE, VDEV_PCAP, TX_STR, i,
-				pt->tx_dev);
-				if (rte_eth_dev_attach(vdev_args,
-							&portid) < 0) {
+				snprintf(vdev_name, sizeof(vdev_name),
+					 VDEV_NAME_FMT, TX_STR, i);
+				(pt->rx_vdev_stream_type == IFACE) ?
+				snprintf(vdev_args, sizeof(vdev_args),
+					 VDEV_IFACE_ARGS_FMT, pt->tx_dev) :
+				snprintf(vdev_args, sizeof(vdev_args),
+					 VDEV_PCAP_ARGS_FMT, pt->tx_dev);
+				if (rte_eal_hotplug_add("vdev", vdev_name,
+							vdev_args) < 0) {
 					cleanup_rings();
 					rte_exit(EXIT_FAILURE,
 						"vdev creation failed:"
 						"%s:%d\n", __func__, __LINE__);
 				}
+				if (rte_eth_dev_get_port_by_name(vdev_name,
+						&portid) != 0) {
+					rte_eal_hotplug_remove("vdev",
+							       vdev_name);
+					cleanup_rings();
+					rte_exit(EXIT_FAILURE,
+						"cannot find added vdev %s:%s:%d\n",
+						vdev_name, __func__, __LINE__);
+				}
 				pt->tx_vdev_id = portid;
 
 				/* configure vdev */
@@ -667,17 +691,28 @@ create_mp_ring_vdev(void)
 					rte_strerror(rte_errno));
 			}
 
+			snprintf(vdev_name, sizeof(vdev_name),
+				 VDEV_NAME_FMT, RX_STR, i);
 			(pt->rx_vdev_stream_type == IFACE) ?
-			snprintf(vdev_args, SIZE, VDEV_IFACE, RX_STR, i,
-				pt->rx_dev) :
-			snprintf(vdev_args, SIZE, VDEV_PCAP, RX_STR, i,
-				pt->rx_dev);
-			if (rte_eth_dev_attach(vdev_args, &portid) < 0) {
+			snprintf(vdev_args, sizeof(vdev_args),
+				 VDEV_IFACE_ARGS_FMT, pt->rx_dev) :
+			snprintf(vdev_args, sizeof(vdev_args),
+				 VDEV_PCAP_ARGS_FMT, pt->rx_dev);
+			if (rte_eal_hotplug_add("vdev", vdev_name,
+						vdev_args) < 0) {
 				cleanup_rings();
 				rte_exit(EXIT_FAILURE,
 					"vdev creation failed:%s:%d\n",
 					__func__, __LINE__);
 			}
+			if (rte_eth_dev_get_port_by_name(vdev_name,
+							 &portid) != 0) {
+				rte_eal_hotplug_remove("vdev", vdev_name);
+				cleanup_rings();
+				rte_exit(EXIT_FAILURE,
+					"cannot find added vdev %s:%s:%d\n",
+					vdev_name, __func__, __LINE__);
+			}
 			pt->rx_vdev_id = portid;
 			/* configure vdev */
 			configure_vdev(pt->rx_vdev_id);
@@ -693,16 +728,27 @@ create_mp_ring_vdev(void)
 					rte_strerror(rte_errno));
 			}
 
+			snprintf(vdev_name, sizeof(vdev_name),
+				 VDEV_NAME_FMT, TX_STR, i);
 			(pt->tx_vdev_stream_type == IFACE) ?
-			snprintf(vdev_args, SIZE, VDEV_IFACE, TX_STR, i,
-				pt->tx_dev) :
-			snprintf(vdev_args, SIZE, VDEV_PCAP, TX_STR, i,
-				pt->tx_dev);
-			if (rte_eth_dev_attach(vdev_args, &portid) < 0) {
+			snprintf(vdev_args, sizeof(vdev_args),
+				 VDEV_IFACE_ARGS_FMT, pt->tx_dev) :
+			snprintf(vdev_args, sizeof(vdev_args),
+				 VDEV_PCAP_ARGS_FMT, pt->tx_dev);
+			if (rte_eal_hotplug_add("vdev", vdev_name,
+						vdev_args) < 0) {
 				cleanup_rings();
 				rte_exit(EXIT_FAILURE,
 					"vdev creation failed\n");
 			}
+			if (rte_eth_dev_get_port_by_name(vdev_name,
+							 &portid) != 0) {
+				rte_eal_hotplug_remove("vdev", vdev_name);
+				cleanup_rings();
+				rte_exit(EXIT_FAILURE,
+					"cannot find added vdev %s:%s:%d\n",
+					vdev_name, __func__, __LINE__);
+			}
 			pt->tx_vdev_id = portid;
 
 			/* configure vdev */
-- 
2.17.1

[dpdk-dev] [PATCH 2/3] ethdev: deprecate attach and detach functions

[Andrew Rybchenko]

These functions are buggy from the very beginning and should not be used.
Generic EAL hotplug mechanisms should be used instead.

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 app/test-pmd/Makefile                | 1 +
 app/test-pmd/meson.build             | 1 +
 doc/guides/rel_notes/deprecation.rst | 7 +++++++
 lib/librte_ethdev/rte_ethdev.h       | 2 ++
 4 files changed, 11 insertions(+)

diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index a5a827bbd..ae9897510 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -13,6 +13,7 @@ APP = testpmd
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -Wno-deprecated-declarations
 
 #
 # all source are stored in SRCS-y
diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
index 4c9fbbfda..861e157ea 100644
--- a/app/test-pmd/meson.build
+++ b/app/test-pmd/meson.build
@@ -4,6 +4,7 @@
 # override default name to drop the hyphen
 name = 'testpmd'
 allow_experimental_apis = true
+cflags += '-Wno-deprecated-declarations'
 sources = files('cmdline.c',
 	'cmdline_flow.c',
 	'cmdline_mtr.c',
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 8df41cb6a..fc3bf1a0b 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -72,6 +72,13 @@ Deprecation Notices
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
 
+* ethdev: In v18.11 ``rte_eth_dev_attach()`` and ``rte_eth_dev_detach()``
+  will be removed.
+  Hotplug functions ``rte_eal_hotplug_add()`` and ``rte_eal_hotplug_remove()``
+  should be used instread.
+  Function ``rte_eth_dev_get_port_by_name()`` may be used to find
+  identifier of the added port.
+
 * pdump: As we changed to use generic IPC, some changes in APIs and structure
   are expected in subsequent release.
 
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index f5f593b31..46c569a7f 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1433,6 +1433,7 @@ uint16_t __rte_experimental rte_eth_dev_count_total(void);
  * @return
  *  0 on success and port_id is filled, negative on error
  */
+__rte_deprecated
 int rte_eth_dev_attach(const char *devargs, uint16_t *port_id);
 
 /**
@@ -1448,6 +1449,7 @@ int rte_eth_dev_attach(const char *devargs, uint16_t *port_id);
  * @return
  *  0 on success and devname is filled, negative on error
  */
+__rte_deprecated
 int rte_eth_dev_detach(uint16_t port_id, char *devname);
 
 /**
-- 
2.17.1

[dpdk-dev] [PATCH 3/3] eal: deprecate device attach and detach functions

[Andrew Rybchenko]

Hotplug functions should be used directly to add and remove devices.

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/rel_notes/deprecation.rst    | 5 +++++
 lib/librte_eal/common/include/rte_dev.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index fc3bf1a0b..2d8f03472 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -79,6 +79,11 @@ Deprecation Notices
   Function ``rte_eth_dev_get_port_by_name()`` may be used to find
   identifier of the added port.
 
+* eal: In v18.11 ``rte_eal_dev_attach()`` and ``rte_eal_dev_detach()``
+  will be removed.
+  Hotplug functions ``rte_eal_hotplug_add()`` and ``rte_eal_hotplug_remove()``
+  should be used directly.
+
 * pdump: As we changed to use generic IPC, some changes in APIs and structure
   are expected in subsequent release.
 
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index ba6e445fc..a51f296e6 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -175,6 +175,7 @@ struct rte_device {
  * @return
  *   0 on success, negative on error.
  */
+__rte_deprecated
 int rte_eal_dev_attach(const char *name, const char *devargs);
 
 /**
@@ -185,6 +186,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs);
  * @return
  *   0 on success, negative on error.
  */
+__rte_deprecated
 int rte_eal_dev_detach(struct rte_device *dev);
 
 /**
-- 
2.17.1

Re: [dpdk-dev] [PATCH 0/3] deprecate attach and detach functions

[Thomas Monjalon]

11/07/2018 16:14, Andrew Rybchenko:
> As discussed in [1] EAL hotplug should be used directly to add/remove devices.
> 
> app/pdump changes are build-tested only.
> 
> [1] http://mails.dpdk.org/archives/dev/2018-July/107507.html
> 
> Andrew Rybchenko (3):
>   app/pdump: use hotplug add instead of attach
>   ethdev: deprecate attach and detach functions
>   eal: deprecate device attach and detach functions

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

Re: [dpdk-dev] [PATCH 0/3] deprecate attach and detach functions

[Stephen Hemminger]

On Wed, 11 Jul 2018 15:14:07 +0100
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> As discussed in [1] EAL hotplug should be used directly to add/remove devices.
> 
> app/pdump changes are build-tested only.
> 
> [1] http://mails.dpdk.org/archives/dev/2018-July/107507.html
> 
> Andrew Rybchenko (3):
>   app/pdump: use hotplug add instead of attach
>   ethdev: deprecate attach and detach functions
>   eal: deprecate device attach and detach functions
> 
>  app/pdump/main.c                        | 94 ++++++++++++++++++-------
>  app/test-pmd/Makefile                   |  1 +
>  app/test-pmd/meson.build                |  1 +
>  doc/guides/rel_notes/deprecation.rst    | 12 ++++
>  lib/librte_eal/common/include/rte_dev.h |  2 +
>  lib/librte_ethdev/rte_ethdev.h          |  2 +
>  6 files changed, 88 insertions(+), 24 deletions(-)
> 

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [dpdk-dev] [PATCH 0/3] deprecate attach and detach functions

[Thomas Monjalon]

01/08/2018 18:26, Stephen Hemminger:
> On Wed, 11 Jul 2018 15:14:07 +0100
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> > As discussed in [1] EAL hotplug should be used directly to add/remove devices.
> > 
> > app/pdump changes are build-tested only.
> > 
> > [1] http://mails.dpdk.org/archives/dev/2018-July/107507.html
> > 
> > Andrew Rybchenko (3):
> >   app/pdump: use hotplug add instead of attach
> >   ethdev: deprecate attach and detach functions
> >   eal: deprecate device attach and detach functions
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

There are only 2 acks, probably because everybody is on holidays,
but there is no comment after more than 3 weeks.
I don't want to apply the code change in pdump app after -rc3,
so it is applied in 18.08-rc3.
Thanks