[v4,5/6] ethdev: remove deprecated attach/detach functions

Message ID 20181009223338.18307-6-thomas@monjalon.net
State Superseded
Delegated to: Ferruh Yigit
Headers show
Series
  • replace attach/detach functions
Related show

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon Oct. 9, 2018, 10:33 p.m.
The hotplug attach/detach features are implemented in EAL layer.
There is a new ethdev iterator to retrieve ports from ethdev layer.

As announced earlier, the (buggy) ethdev functions are now removed.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 app/test-pmd/testpmd.c                        |  19 +++-
 doc/guides/prog_guide/index.rst               |   1 -
 .../prog_guide/port_hotplug_framework.rst     | 106 ------------------
 doc/guides/rel_notes/deprecation.rst          |   7 --
 doc/guides/rel_notes/release_18_11.rst        |   8 +-
 drivers/net/virtio/virtio_user_ethdev.c       |   1 -
 lib/librte_ethdev/Makefile                    |   2 +-
 lib/librte_ethdev/meson.build                 |   2 +-
 lib/librte_ethdev/rte_ethdev.c                |  81 -------------
 lib/librte_ethdev/rte_ethdev.h                |  31 -----
 lib/librte_ethdev/rte_ethdev_version.map      |   2 -
 11 files changed, 24 insertions(+), 236 deletions(-)
 delete mode 100644 doc/guides/prog_guide/port_hotplug_framework.rst

Comments

Ferruh Yigit Oct. 16, 2018, 11:03 a.m. | #1
On 10/9/2018 11:33 PM, Thomas Monjalon wrote:
> The hotplug attach/detach features are implemented in EAL layer.
> There is a new ethdev iterator to retrieve ports from ethdev layer.
> 
> As announced earlier, the (buggy) ethdev functions are now removed.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>

> @@ -53,7 +53,6 @@ Programmer's Guide
>      packet_framework
>      vhost_lib
>      metrics_lib
> -    port_hotplug_framework

Any replacement documentation for hotplug?

<...>

> @@ -130,6 +130,12 @@ API Changes
>    functions were deprecated since 17.05 and are replaced by
>    ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
>  
> +* ethdev: The deprecated functions attach/detach were removed in 18.11.
> +  ``rte_eth_dev_attach`` can be replaced by ``RTE_ETH_FOREACH_MATCHING_DEV``
> +  and ``rte_dev_probe`` or ``rte_eal_hotplug_add``.
> +  ``rte_eth_dev_detach`` can be replaced by
> +  ``rte_dev_remove`` or ``rte_eal_hotplug_remove``.

What is the difference between ``rte_dev_remove`` or ``rte_eal_hotplug_remove``,
which one is good for which usage, is there any documentation explaining this?
Thomas Monjalon Oct. 16, 2018, 12:12 p.m. | #2
16/10/2018 13:03, Ferruh Yigit:
> On 10/9/2018 11:33 PM, Thomas Monjalon wrote:
> > The hotplug attach/detach features are implemented in EAL layer.
> > There is a new ethdev iterator to retrieve ports from ethdev layer.
> > 
> > As announced earlier, the (buggy) ethdev functions are now removed.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> <...>
> 
> > @@ -53,7 +53,6 @@ Programmer's Guide
> >      packet_framework
> >      vhost_lib
> >      metrics_lib
> > -    port_hotplug_framework
> 
> Any replacement documentation for hotplug?

No.
We should improve the documentation about device management in general.
I will try to improve it after -rc2.

> > +* ethdev: The deprecated functions attach/detach were removed in 18.11.
> > +  ``rte_eth_dev_attach`` can be replaced by ``RTE_ETH_FOREACH_MATCHING_DEV``
> > +  and ``rte_dev_probe`` or ``rte_eal_hotplug_add``.
> > +  ``rte_eth_dev_detach`` can be replaced by
> > +  ``rte_dev_remove`` or ``rte_eal_hotplug_remove``.
> 
> What is the difference between ``rte_dev_remove`` or ``rte_eal_hotplug_remove``,
> which one is good for which usage, is there any documentation explaining this?

In doxygen, you can see that the parameters are differents, and new ones
are experimental.
That's why we'll keep old ones, at least for this release.

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 001f0e552..a975594ad 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -425,6 +425,7 @@  struct nvgre_encap_conf nvgre_encap_conf = {
 };
 
 /* Forward function declarations */
+static void setup_attached_port(portid_t pi);
 static void map_port_queue_stats_mapping_registers(portid_t pi,
 						   struct rte_port *port);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -1991,7 +1992,7 @@  void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
-	unsigned int socket_id;
+	struct rte_dev_iterator iterator;
 
 	printf("Attaching a new port...\n");
 
@@ -2000,8 +2001,19 @@  attach_port(char *identifier)
 		return;
 	}
 
-	if (rte_eth_dev_attach(identifier, &pi))
+	if (rte_dev_probe(identifier) != 0) {
+		TESTPMD_LOG(ERR, "Failed to attach port %s\n", identifier);
 		return;
+	}
+
+	RTE_ETH_FOREACH_MATCHING_DEV(pi, identifier, &iterator)
+		setup_attached_port(pi);
+}
+
+static void
+setup_attached_port(portid_t pi)
+{
+	unsigned int socket_id;
 
 	socket_id = (unsigned)rte_eth_dev_socket_id(pi);
 	/* if socket_id is invalid, set to 0 */
@@ -2024,7 +2036,6 @@  attach_port(char *identifier)
 void
 detach_port(portid_t port_id)
 {
-	char name[RTE_ETH_NAME_MAX_LEN];
 	uint16_t i;
 
 	printf("Detaching a port...\n");
@@ -2037,7 +2048,7 @@  detach_port(portid_t port_id)
 	if (ports[port_id].flow_list)
 		port_flow_flush(port_id);
 
-	if (rte_eth_dev_detach(port_id, name)) {
+	if (rte_dev_remove(rte_eth_devices[port_id].device)) {
 		TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
 		return;
 	}
diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index c81d9c54f..2086e2442 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -53,7 +53,6 @@  Programmer's Guide
     packet_framework
     vhost_lib
     metrics_lib
-    port_hotplug_framework
     bpf_lib
     source_org
     dev_kit_build_system
diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst b/doc/guides/prog_guide/port_hotplug_framework.rst
deleted file mode 100644
index fb0efc18f..000000000
--- a/doc/guides/prog_guide/port_hotplug_framework.rst
+++ /dev/null
@@ -1,106 +0,0 @@ 
-..  BSD LICENSE
-    Copyright(c) 2015 IGEL Co.,Ltd. All rights reserved.
-    All rights reserved.
-
-    Redistribution and use in source and binary forms, with or without
-    modification, are permitted provided that the following conditions
-    are met:
-
-    * Redistributions of source code must retain the above copyright
-    notice, this list of conditions and the following disclaimer.
-    * Redistributions in binary form must reproduce the above copyright
-    notice, this list of conditions and the following disclaimer in
-    the documentation and/or other materials provided with the
-    distribution.
-    * Neither the name of IGEL Co.,Ltd. nor the names of its
-    contributors may be used to endorse or promote products derived
-    from this software without specific prior written permission.
-
-    THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-    "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-    LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-    A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-    OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-    SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-    LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-    DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-    THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-    (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-    OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-Port Hotplug Framework
-======================
-
-The Port Hotplug Framework provides DPDK applications with the ability to
-attach and detach ports at runtime. Because the framework depends on PMD
-implementation, the ports that PMDs cannot handle are out of scope of this
-framework. Furthermore, after detaching a port from a DPDK application, the
-framework doesn't provide a way for removing the devices from the system.
-For the ports backed by a physical NIC, the kernel will need to support PCI
-Hotplug feature.
-
-Overview
---------
-
-The basic requirements of the Port Hotplug Framework are:
-
-*       DPDK applications that use the Port Hotplug Framework must manage their
-        own ports.
-
-        The Port Hotplug Framework is implemented to allow DPDK applications to
-        manage ports. For example, when DPDK applications call the port attach
-        function, the attached port number is returned. DPDK applications can
-        also detach the port by port number.
-
-*       Kernel support is needed for attaching or detaching physical device
-        ports.
-
-        To attach new physical device ports, the device will be recognized by
-        userspace driver I/O framework in kernel at first. Then DPDK
-        applications can call the Port Hotplug functions to attach the ports.
-        For detaching, steps are vice versa.
-
-*       Before detaching, they must be stopped and closed.
-
-        DPDK applications must call "rte_eth_dev_stop()" and
-        "rte_eth_dev_close()" APIs before detaching ports. These functions will
-        start finalization sequence of the PMDs.
-
-*       The framework doesn't affect legacy DPDK applications behavior.
-
-        If the Port Hotplug functions aren't called, all legacy DPDK apps can
-        still work without modifications.
-
-Port Hotplug API overview
--------------------------
-
-*       Attaching a port
-
-        "rte_eth_dev_attach()" API attaches a port to DPDK application, and
-        returns the attached port number. Before calling the API, the device
-        should be recognized by an userspace driver I/O framework. The API
-        receives a pci address like "0000:01:00.0" or a virtual device name
-        like "net_pcap0,iface=eth0". In the case of virtual device name, the
-        format is the same as the general "--vdev" option of DPDK.
-
-*       Detaching a port
-
-        "rte_eth_dev_detach()" API detaches a port from DPDK application, and
-        returns a pci address of the detached device or a virtual device name
-        of the device.
-
-Reference
----------
-
-        "testpmd" supports the Port Hotplug Framework.
-
-Limitations
------------
-
-*       The Port Hotplug APIs are not thread safe.
-
-*       The framework can only be enabled with Linux. BSD is not supported.
-
-*       Not all PMDs support detaching feature.
-        The underlying bus must support hot-unplug. If not supported,
-        the function ``rte_eth_dev_detach()`` will return negative ENOTSUP.
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 138335dfb..c24506dc1 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -64,13 +64,6 @@  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.
-
 * 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()``
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index c87522f27..1f6ddcb6e 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -130,6 +130,12 @@  API Changes
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
 
+* ethdev: The deprecated functions attach/detach were removed in 18.11.
+  ``rte_eth_dev_attach`` can be replaced by ``RTE_ETH_FOREACH_MATCHING_DEV``
+  and ``rte_dev_probe`` or ``rte_eal_hotplug_add``.
+  ``rte_eth_dev_detach`` can be replaced by
+  ``rte_dev_remove`` or ``rte_eal_hotplug_remove``.
+
 * A new device flag, RTE_ETH_DEV_NOLIVE_MAC_ADDR, changes the order of
   actions inside rte_eth_dev_start regarding MAC set. Some NICs do not
   support MAC changes once the port has started and with this new device
@@ -216,7 +222,7 @@  The libraries prepended with a plus sign were incremented in this version.
      librte_cryptodev.so.5
      librte_distributor.so.1
    + librte_eal.so.9
-     librte_ethdev.so.10
+   + librte_ethdev.so.11
    + librte_eventdev.so.6
      librte_flow_classify.so.1
      librte_gro.so.1
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 525d16cab..d06d47e4b 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -637,7 +637,6 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	return ret;
 }
 
-/** Called by rte_eth_dev_detach() */
 static int
 virtio_user_pmd_remove(struct rte_vdev_device *vdev)
 {
diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index d720dd207..e27bcd5ac 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -16,7 +16,7 @@  LDLIBS += -lrte_mbuf -lrte_kvargs
 
 EXPORT_MAP := rte_ethdev_version.map
 
-LIBABIVER := 10
+LIBABIVER := 11
 
 SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index 172e302f0..6783013fd 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -2,7 +2,7 @@ 
 # Copyright(c) 2017 Intel Corporation
 
 name = 'ethdev'
-version = 10
+version = 11
 allow_experimental_apis = true
 sources = files('ethdev_private.c',
 	'ethdev_profile.c',
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index f195ea917..9ac802f3f 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -792,87 +792,6 @@  eth_err(uint16_t port_id, int ret)
 	return ret;
 }
 
-/* attach the new device, then store port_id of the device */
-int
-rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
-{
-	int current = rte_eth_dev_count_total();
-	struct rte_devargs da;
-	int ret = -1;
-
-	memset(&da, 0, sizeof(da));
-
-	if ((devargs == NULL) || (port_id == NULL)) {
-		ret = -EINVAL;
-		goto err;
-	}
-
-	/* parse devargs */
-	if (rte_devargs_parse(&da, devargs))
-		goto err;
-
-	ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args);
-	if (ret < 0)
-		goto err;
-
-	/* no point looking at the port count if no port exists */
-	if (!rte_eth_dev_count_total()) {
-		RTE_ETHDEV_LOG(ERR, "No port found for device (%s)\n", da.name);
-		ret = -1;
-		goto err;
-	}
-
-	/* if nothing happened, there is a bug here, since some driver told us
-	 * it did attach a device, but did not create a port.
-	 * FIXME: race condition in case of plug-out of another device
-	 */
-	if (current == rte_eth_dev_count_total()) {
-		ret = -1;
-		goto err;
-	}
-
-	*port_id = eth_dev_last_created_port;
-	ret = 0;
-
-err:
-	free(da.args);
-	return ret;
-}
-
-/* detach the device, then store the name of the device */
-int
-rte_eth_dev_detach(uint16_t port_id, char *name __rte_unused)
-{
-	struct rte_device *dev;
-	struct rte_bus *bus;
-	uint32_t dev_flags;
-	int ret = -1;
-
-	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
-
-	dev_flags = rte_eth_devices[port_id].data->dev_flags;
-	if (dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
-		RTE_ETHDEV_LOG(ERR,
-			"Port %"PRIu16" is bonded, cannot detach\n", port_id);
-		return -ENOTSUP;
-	}
-
-	dev = rte_eth_devices[port_id].device;
-	if (dev == NULL)
-		return -EINVAL;
-
-	bus = rte_bus_find_by_device(dev);
-	if (bus == NULL)
-		return -ENOENT;
-
-	ret = rte_eal_hotplug_remove(bus->name, dev->name);
-	if (ret < 0)
-		return ret;
-
-	rte_eth_dev_release_port(&rte_eth_devices[port_id]);
-	return 0;
-}
-
 static int
 rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 1cb2b553b..378c01afa 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1495,37 +1495,6 @@  uint16_t rte_eth_dev_count_avail(void);
  */
 uint16_t __rte_experimental rte_eth_dev_count_total(void);
 
-/**
- * Attach a new Ethernet device specified by arguments.
- *
- * @param devargs
- *  A pointer to a strings array describing the new device
- *  to be attached. The strings should be a pci address like
- *  '0000:01:00.0' or virtual device name like 'net_pcap0'.
- * @param port_id
- *  A pointer to a port identifier actually attached.
- * @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);
-
-/**
- * Detach a Ethernet device specified by port identifier.
- * This function must be called when the device is in the
- * closed state.
- *
- * @param port_id
- *   The port identifier of the device to detach.
- * @param devname
- *   A pointer to a buffer that will be filled with the device name.
- *   This buffer must be at least RTE_DEV_NAME_MAX_LEN long.
- * @return
- *  0 on success and devname is filled, negative on error
- */
-__rte_deprecated
-int rte_eth_dev_detach(uint16_t port_id, char *devname);
-
 /**
  * Convert a numerical speed in Mbps to a bitmap flag that can be used in
  * the bitmap link_speeds of the struct rte_eth_conf
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index b4042e398..351de72d3 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -8,14 +8,12 @@  DPDK_2.2 {
 	rte_eth_allmulticast_get;
 	rte_eth_dev_allocate;
 	rte_eth_dev_allocated;
-	rte_eth_dev_attach;
 	rte_eth_dev_callback_register;
 	rte_eth_dev_callback_unregister;
 	rte_eth_dev_close;
 	rte_eth_dev_configure;
 	rte_eth_dev_count;
 	rte_eth_dev_default_mac_addr_set;
-	rte_eth_dev_detach;
 	rte_eth_dev_filter_supported;
 	rte_eth_dev_flow_ctrl_get;
 	rte_eth_dev_flow_ctrl_set;