From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
[66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 9064E5B12
for <dev@dpdk.org>; Wed, 17 Oct 2018 03:54:53 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
by mailout.nyi.internal (Postfix) with ESMTP id 243D322037;
Tue, 16 Oct 2018 21:54:53 -0400 (EDT)
Received: from mailfrontend2 ([10.202.2.163])
by compute1.internal (MEProxy); Tue, 16 Oct 2018 21:54:53 -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=JdagEX+hxz
l7uHCUTX5aGYnFYdFmUmciYqQwkb6UO4M=; b=KYNc513hhqoVKR/YsGUVmkZhkk
aMegMXqAxCVVDb0HGb+fQBwmPkGWktr4oWhZJRj4mH/Ijv+X60kR/HEhB/ICAiCU
mYtlV2O6Tp4Lcud/1kfbEOCskPHdNORgyBKks7saigQb2vB+/x3V+5E3h0GrhsAC
mJOBkfo9/QwTg4Qnc=
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=JdagEX+hxzl7uHCUTX5aGYnFYdFmUmciYqQwkb6UO4M=; b=u2GuuhII
JHn417JX8VIik4R0eL7GlcG96dU2c/gD/dd7Z8WKOIGr1dqDUVvhd8g540UdIf/k
YlaYiu7DiVAnAe0T8hlC0ivxM+AbREsjJ3/BMuISngJeQ/VfsxI8igBpZ7t0KDYW
Gddh4X6OkvVMMUcZy0tH5BMLGhafLJvCGC0+RGNH8iFunCNWoj+Ygkfy1RLWSTn3
fx1irYs87l8pRXAbYracJ8QVEMG/nPnz/CZDZNXngnFjkx9cELP2zl8VjHFkSXSN
Jn4NKBxUlMn/Yl/IvcapD9b9h27qpDtCM5RUNeJer/gee6uruCLgLUi5PbQv8Vl7
+wnShcrJdSDEGg==
X-ME-Sender: <xms:bJbGW8HrJnukss6v-WUve72VfYBKQNRjTSuf4AQqeoXAYUpflohEuQ>
X-ME-Proxy: <xmx:bJbGW5fldLShg_wN4IrXQKr1MCiSf-bYRYWRn3TrSA-yaNFZw5G6sg>
<xmx:bJbGW353h1p0oLsPvuCWm1FWe5pzE8PFFxKDdVPcC6iDP60jQsF_8A>
<xmx:bJbGW9l7KNwnjXBLhHPt7vgAZ7NpD8Flwws6nVO6Em7EFBMiTJXbng>
<xmx:bJbGWy-DXtlnMzKVMfEpXVF_2SXlvS73xwAicKEaiczNA1USgsIqLg>
<xmx:bJbGW4qewDrcgAiK5Vjo6LfXjE4sewZiyduj2lFXGCuGl2IzVO0PGw>
<xmx:bZbGW5fCip9wqSd8PgqmbWx40DnG6s4yXsq0TaXakHw1pifnmUE7LA>
Received: from xps.monjalon.net (184.203.134.77.rev.sfr.net [77.134.203.184])
by mail.messagingengine.com (Postfix) with ESMTPA id 35579102DD;
Tue, 16 Oct 2018 21:54:52 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: ferruh.yigit@intel.com,
arybchenko@solarflare.com
Cc: dev@dpdk.org,
ophirmu@mellanox.com
Date: Wed, 17 Oct 2018 03:54:50 +0200
Message-Id: <20181017015450.15783-5-thomas@monjalon.net>
X-Mailer: git-send-email 2.19.0
In-Reply-To: <20181017015450.15783-1-thomas@monjalon.net>
References: <20180907233929.21950-1-thomas@monjalon.net>
<20181017015450.15783-1-thomas@monjalon.net>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [dpdk-dev] [PATCH v3 4/4] ethdev: complete closing of port
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 17 Oct 2018 01:54:54 -0000
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().
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().
If the driver is trying to free the port again, the function
rte_eth_dev_release_port() will abort with -ENODEV error.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
drivers/net/af_packet/rte_eth_af_packet.c | 4 +++-
drivers/net/bnxt/bnxt_ethdev.c | 4 ----
drivers/net/failsafe/failsafe_ops.c | 2 ++
drivers/net/mlx4/mlx4.c | 2 ++
drivers/net/mlx5/mlx5.c | 2 ++
drivers/net/netvsc/hn_ethdev.c | 5 ++++-
drivers/net/pcap/rte_eth_pcap.c | 5 +++++
drivers/net/softnic/rte_eth_softnic.c | 3 ++-
drivers/net/tap/rte_eth_tap.c | 3 +++
drivers/net/vhost/rte_eth_vhost.c | 4 ----
lib/librte_ethdev/rte_ethdev.c | 9 +++------
lib/librte_ethdev/rte_ethdev.h | 3 +--
12 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 95a98c6b8..2678335f6 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -362,8 +362,10 @@ eth_stats_reset(struct rte_eth_dev *dev)
}
static void
-eth_dev_close(struct rte_eth_dev *dev __rte_unused)
+eth_dev_close(struct rte_eth_dev *dev)
{
+ /* mac_addrs must not be freed alone because part of dev_private */
+ dev->data->mac_addrs = NULL;
}
static void
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 801c6ffad..141bd6376 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -712,10 +712,6 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
if (bp->dev_stopped == 0)
bnxt_dev_stop_op(eth_dev);
- if (eth_dev->data->mac_addrs != NULL) {
- rte_free(eth_dev->data->mac_addrs);
- eth_dev->data->mac_addrs = NULL;
- }
if (bp->grp_info != NULL) {
rte_free(bp->grp_info);
bp->grp_info = NULL;
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 7f8bcd4c6..f10433b30 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -331,6 +331,8 @@ fs_dev_close(struct rte_eth_dev *dev)
sdev->state = DEV_ACTIVE - 1;
}
fs_dev_free_queues(dev);
+ /* mac_addrs must not be freed alone because part of dev_private */
+ dev->data->mac_addrs = NULL;
fs_unlock(dev, 0);
}
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 81a719126..084a456e3 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -218,6 +218,8 @@ mlx4_dev_close(struct rte_eth_dev *dev)
assert(priv->ctx == NULL);
mlx4_intr_uninstall(priv);
memset(priv, 0, sizeof(*priv));
+ /* mac_addrs must not be freed because part of dev_private */
+ dev->data->mac_addrs = NULL;
}
static const struct eth_dev_ops mlx4_dev_ops = {
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index aad82e4d1..9870da51d 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -337,6 +337,8 @@ mlx5_dev_close(struct rte_eth_dev *dev)
}
memset(priv, 0, sizeof(*priv));
priv->domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
+ /* mac_addrs must not be freed alone because part of dev_private */
+ dev->data->mac_addrs = NULL;
}
const struct eth_dev_ops mlx5_dev_ops = {
diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index aa38ee7a3..5a7463628 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -630,11 +630,14 @@ hn_dev_stop(struct rte_eth_dev *dev)
}
static void
-hn_dev_close(struct rte_eth_dev *dev __rte_unused)
+hn_dev_close(struct rte_eth_dev *dev)
{
PMD_INIT_LOG(DEBUG, "close");
hn_vf_close(dev);
+
+ /* mac_addrs must not be freed alone because part of dev_private */
+ dev->data->mac_addrs = NULL;
}
static const struct eth_dev_ops hn_eth_dev_ops = {
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index bfb5d710d..261b09c4f 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -623,6 +623,11 @@ eth_stats_reset(struct rte_eth_dev *dev)
static void
eth_dev_close(struct rte_eth_dev *dev __rte_unused)
{
+ struct pmd_internals *internals = dev->data->dev_private;
+
+ if (internals != NULL && internals->phy_mac == 0)
+ /* not dynamically allocated, must not be freed */
+ dev->data->mac_addrs = NULL;
}
static void
diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
index 9a2418438..083519b38 100644
--- a/drivers/net/softnic/rte_eth_softnic.c
+++ b/drivers/net/softnic/rte_eth_softnic.c
@@ -194,8 +194,9 @@ pmd_dev_stop(struct rte_eth_dev *dev)
}
static void
-pmd_dev_close(struct rte_eth_dev *dev __rte_unused)
+pmd_dev_close(struct rte_eth_dev *dev)
{
+ dev->data->mac_addrs = NULL; /* statically allocated */
return;
}
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 53d37b3cb..32f2f888d 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1000,6 +1000,9 @@ tap_dev_close(struct rte_eth_dev *dev)
* Since TUN device has no more opened file descriptors
* it will be removed from kernel
*/
+
+ /* mac_addrs must not be freed alone because part of dev_private */
+ dev->data->mac_addrs = NULL;
}
static void
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 0cd1a4642..d3e78503b 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -998,12 +998,8 @@ eth_dev_close(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_tx_queues; i++)
rte_free(dev->data->tx_queues[i]);
- rte_free(dev->data->mac_addrs);
free(internal->dev_name);
free(internal->iface_name);
- rte_free(internal);
-
- dev->data->dev_private = NULL;
}
static int
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 178800a5b..987ba5ab1 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -367,6 +367,8 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
{
if (eth_dev == NULL)
return -EINVAL;
+ if (eth_dev->state == RTE_ETH_DEV_UNUSED)
+ return -ENODEV;
rte_eth_dev_shared_data_prepare();
@@ -1384,12 +1386,7 @@ rte_eth_dev_close(uint16_t port_id)
dev->data->dev_started = 0;
(*dev->dev_ops->dev_close)(dev);
- dev->data->nb_rx_queues = 0;
- rte_free(dev->data->rx_queues);
- dev->data->rx_queues = NULL;
- dev->data->nb_tx_queues = 0;
- rte_free(dev->data->tx_queues);
- dev->data->tx_queues = NULL;
+ rte_eth_dev_release_port(dev);
}
int
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index fb40c89e0..dcdeb184b 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1802,8 +1802,7 @@ 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.
*
* @param port_id
* The port identifier of the Ethernet device.
--
2.19.0