From patchwork Fri Oct 19 02:07:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Monjalon X-Patchwork-Id: 47067 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6F6831B176; Fri, 19 Oct 2018 04:08:17 +0200 (CEST) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 64C0D1B12E for ; Fri, 19 Oct 2018 04:08:07 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 119A7229F0; Thu, 18 Oct 2018 22:08:07 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 18 Oct 2018 22:08:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=mesmtp; bh=yGIMgctg/o Exi+jf21k4YVZ8LoWRiLvbEg/MgunrpEY=; b=lEu+DA9xfwGtkbFWkigl4GCr/J V9pNtC6+STs1D3nOswj7ovYhPKS1u3mOCNfic2PeUpFkF2VvSxVbK/fObPm+o/uY V3mBjyoan3JZeBh86GCFdJMIhb72r8PBCucxAiRVuGSyybfTPIzecal7d306mrkg hnHYqqNdNxgY5RayE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; bh=yGIMgctg/oExi+jf21k4YVZ8LoWRiLvbEg/MgunrpEY=; b=O72YmKGy SQvtAWQ2l63k7ZNrGUh2wR8CehNH1ENV0RVV4SqFwLJlKl32giGgcb3UBg9wT+W6 yyR8CkddH/jlND/Pl8I0bp3z60S2KkHibg2FwGVSJ3h9mDaIlFZ83WnH4Qk2bMor B5+po13OSXaglCWeS7TRUq66jQQDc6Ryo14oki0xoBmxQANzPfp8SOqJRGFNSo7g 4CLxF3IfSuOdr4ZM0SBKg802hZWABJ3WYzXEFj8xo5nnPyFBFtKL+yf+I1Wwv1t2 9cMY2OK3o/cqoJieUzTYzzUEQxiZMCT8F+0qIntZSXX95wAJjn2/5+afRAsrNdBm iubXI4HBMr7aEw== X-ME-Sender: X-ME-Proxy: Received: from xps.monjalon.net (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id E9957102E4; Thu, 18 Oct 2018 22:08:05 -0400 (EDT) From: Thomas Monjalon To: ferruh.yigit@intel.com, arybchenko@solarflare.com Cc: dev@dpdk.org, ophirmu@mellanox.com, bernard.iremonger@intel.com, rahul.lakkireddy@chelsio.com Date: Fri, 19 Oct 2018 04:07:57 +0200 Message-Id: <20181019020757.27698-7-thomas@monjalon.net> X-Mailer: git-send-email 2.19.0 In-Reply-To: <20181019020757.27698-1-thomas@monjalon.net> References: <20180907233929.21950-1-thomas@monjalon.net> <20181019020757.27698-1-thomas@monjalon.net> MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH v6 6/6] ethdev: complete closing of port X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" After closing a port, it cannot be restarted. So there is no reason to not free all associated resources. The last step was done with rte_eth_dev_detach() which is deprecated. Instead of blindly removing the associated rte_device, the driver should check if no more port (ethdev, cryptodev, etc) is open for the device. The last ethdev freeing which were done by rte_eth_dev_detach(), are now done at the end of rte_eth_dev_close() if the driver supports the flag RTE_ETH_DEV_CLOSE_REMOVE. There will be a transition period for PMDs to enable this new flag and migrate to the new behaviour. When enabling RTE_ETH_DEV_CLOSE_REMOVE, the PMD must free all its private resources for the port, in its dev_close function. It is advised to call the dev_close function in the remove function in order to support removing a device without closing its ports. Some drivers does not allocate MAC addresses dynamically or separately. In those cases, the pointer is set to NULL, in order to avoid wrongly freeing them in rte_eth_dev_release_port(). A closed port will have the state RTE_ETH_DEV_UNUSED which is considered as invalid by rte_eth_dev_is_valid_port(). So validity is not checked anymore for closed ports in testpmd. Signed-off-by: Thomas Monjalon Reviewed-by: Andrew Rybchenko --- app/test-pmd/testpmd.c | 16 +++------------- doc/guides/rel_notes/release_18_11.rst | 5 +++++ lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++ lib/librte_ethdev/rte_ethdev.h | 9 +++++++-- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index d5a8a14af..b0b418424 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -1938,18 +1938,6 @@ port_is_started(portid_t port_id) return 1; } -static int -port_is_closed(portid_t port_id) -{ - if (port_id_is_invalid(port_id, ENABLED_WARN)) - return 0; - - if (ports[port_id].port_status != RTE_PORT_CLOSED) - return 0; - - return 1; -} - int start_port(portid_t pid) { @@ -2253,6 +2241,8 @@ close_port(portid_t pid) port_flow_flush(pi); rte_eth_dev_close(pi); + remove_unused_fwd_ports(); + if (rte_atomic16_cmpset(&(port->port_status), RTE_PORT_HANDLING, RTE_PORT_CLOSED) == 0) printf("Port %d cannot be set to closed\n", pi); @@ -2343,7 +2333,7 @@ detach_port(portid_t port_id) printf("Detaching a port...\n"); - if (!port_is_closed(port_id)) { + if (ports[port_id].port_status != RTE_PORT_CLOSED) { if (ports[port_id].port_status != RTE_PORT_STOPPED) { printf("Port not stopped\n"); return; diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst index 3d5a34ed9..1cbc93d9d 100644 --- a/doc/guides/rel_notes/release_18_11.rst +++ b/doc/guides/rel_notes/release_18_11.rst @@ -235,6 +235,11 @@ API Changes functions were deprecated since 17.05 and are replaced by ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``. +* ethdev: A call to ``rte_eth_dev_release_port()`` has been added in + ``rte_eth_dev_close()``. As a consequence, a closed port is freed + and seen as invalid because of its state ``RTE_ETH_DEV_UNUSED``. + This new behaviour is enabled per driver for a migration period. + * 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 diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 178800a5b..0979c0c7b 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1384,6 +1384,16 @@ rte_eth_dev_close(uint16_t port_id) dev->data->dev_started = 0; (*dev->dev_ops->dev_close)(dev); + /* check behaviour flag - temporary for PMD migration */ + if ((dev->data->dev_flags & RTE_ETH_DEV_CLOSE_REMOVE) != 0) { + /* new behaviour: send event + reset state + free all data */ + rte_eth_dev_release_port(dev); + return; + } + RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n" + "The driver %s should migrate to the new behaviour.\n", + dev->device->driver->name); + /* old behaviour: only free queue arrays */ dev->data->nb_rx_queues = 0; rte_free(dev->data->rx_queues); dev->data->rx_queues = NULL; diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h index fb40c89e0..c4caf0642 100644 --- a/lib/librte_ethdev/rte_ethdev.h +++ b/lib/librte_ethdev/rte_ethdev.h @@ -1259,6 +1259,11 @@ struct rte_eth_dev_owner { char name[RTE_ETH_MAX_OWNER_NAME_LEN]; /**< The owner name. */ }; +/** + * Port is released (i.e. totally freed and data erased) on close. + * Temporary flag for PMD migration to new rte_eth_dev_close() behaviour. + */ +#define RTE_ETH_DEV_CLOSE_REMOVE 0x0001 /** Device supports link state interrupt */ #define RTE_ETH_DEV_INTR_LSC 0x0002 /** Device is a bonded slave */ @@ -1802,8 +1807,8 @@ int rte_eth_dev_set_link_down(uint16_t port_id); /** * Close a stopped Ethernet device. The device cannot be restarted! - * The function frees all resources except for needed by the - * closed state. To free these resources, call rte_eth_dev_detach(). + * The function frees all port resources if the driver supports + * the flag RTE_ETH_DEV_CLOSE_REMOVE. * * @param port_id * The port identifier of the Ethernet device.