All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] net/mlx4: fix minor resource leak during init
@ 2018-05-22 15:36 Adrien Mazarguil
  2018-05-22 15:37 ` [PATCH 2/2] net/mlx5: remove limitation on number of instances Adrien Mazarguil
  2018-06-17  8:05 ` [PATCH 1/2] net/mlx4: fix minor resource leak during init Shahaf Shuler
  0 siblings, 2 replies; 4+ messages in thread
From: Adrien Mazarguil @ 2018-05-22 15:36 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: Ferruh Yigit, dev, stable

Temporary IB device context and list are not freed in case of a successful
initialization of the device.

This issue is caused by the two following commits, the first of which
causes initialization to return early, while the second one goes a bit
overboard while switching to negative errno values; an internal variable
(err) is needed to tell success from failure at the end of the function
since rte_errno is not reliable enough.

Fixes: f2318196c71a ("net/mlx4: remove limitation on number of instances")
Fixes: 9d14b27308a0 ("net/mlx4: standardize on negative errno values")
Cc: stable@dpdk.org

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 drivers/net/mlx4/mlx4.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 9f8ecd072..e604c5b8c 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -589,14 +589,14 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 	ibv_dev = list[i];
 	DEBUG("device opened");
 	if (mlx4_glue->query_device(attr_ctx, &device_attr)) {
-		rte_errno = ENODEV;
+		err = ENODEV;
 		goto error;
 	}
 	INFO("%u port(s) detected", device_attr.phys_port_cnt);
 	conf.ports.present |= (UINT64_C(1) << device_attr.phys_port_cnt) - 1;
 	if (mlx4_args(pci_dev->device.devargs, &conf)) {
 		ERROR("failed to process device arguments");
-		rte_errno = EINVAL;
+		err = EINVAL;
 		goto error;
 	}
 	/* Use all ports when none are defined */
@@ -604,7 +604,7 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		conf.ports.enabled = conf.ports.present;
 	/* Retrieve extended device attributes. */
 	if (mlx4_glue->query_device_ex(attr_ctx, NULL, &device_attr_ex)) {
-		rte_errno = ENODEV;
+		err = ENODEV;
 		goto error;
 	}
 	assert(device_attr.max_sge >= MLX4_MAX_SGE);
@@ -623,18 +623,18 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		DEBUG("using port %u", port);
 		ctx = mlx4_glue->open_device(ibv_dev);
 		if (ctx == NULL) {
-			rte_errno = ENODEV;
+			err = ENODEV;
 			goto port_error;
 		}
 		/* Check port status. */
 		err = mlx4_glue->query_port(ctx, port, &port_attr);
 		if (err) {
-			rte_errno = err;
-			ERROR("port query failed: %s", strerror(rte_errno));
+			err = ENODEV;
+			ERROR("port query failed: %s", strerror(err));
 			goto port_error;
 		}
 		if (port_attr.link_layer != IBV_LINK_LAYER_ETHERNET) {
-			rte_errno = ENOTSUP;
+			err = ENOTSUP;
 			ERROR("port %d is not configured in Ethernet mode",
 			      port);
 			goto port_error;
@@ -644,15 +644,16 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 			      port, mlx4_glue->port_state_str(port_attr.state),
 			      port_attr.state);
 		/* Make asynchronous FD non-blocking to handle interrupts. */
-		if (mlx4_fd_set_non_blocking(ctx->async_fd) < 0) {
+		err = mlx4_fd_set_non_blocking(ctx->async_fd);
+		if (err) {
 			ERROR("cannot make asynchronous FD non-blocking: %s",
-			      strerror(rte_errno));
+			      strerror(err));
 			goto port_error;
 		}
 		/* Allocate protection domain. */
 		pd = mlx4_glue->alloc_pd(ctx);
 		if (pd == NULL) {
-			rte_errno = ENOMEM;
+			err = ENOMEM;
 			ERROR("PD allocation failure");
 			goto port_error;
 		}
@@ -661,7 +662,7 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 				   sizeof(*priv),
 				   RTE_CACHE_LINE_SIZE);
 		if (priv == NULL) {
-			rte_errno = ENOMEM;
+			err = ENOMEM;
 			ERROR("priv allocation failure");
 			goto port_error;
 		}
@@ -691,9 +692,10 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		DEBUG("FCS stripping toggling is %ssupported",
 		      priv->hw_fcs_strip ? "" : "not ");
 		/* Configure the first MAC address by default. */
-		if (mlx4_get_mac(priv, &mac.addr_bytes)) {
+		err = mlx4_get_mac(priv, &mac.addr_bytes);
+		if (err) {
 			ERROR("cannot get MAC address, is mlx4_en loaded?"
-			      " (rte_errno: %s)", strerror(rte_errno));
+			      " (error: %s)", strerror(err));
 			goto port_error;
 		}
 		INFO("port %u MAC address is %02x:%02x:%02x:%02x:%02x:%02x",
@@ -726,8 +728,8 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 			eth_dev = rte_eth_dev_allocate(name);
 		}
 		if (eth_dev == NULL) {
+			err = ENOMEM;
 			ERROR("can not allocate rte ethdev");
-			rte_errno = ENOMEM;
 			goto port_error;
 		}
 		eth_dev->data->dev_private = priv;
@@ -773,8 +775,6 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 			rte_eth_dev_release_port(eth_dev);
 		break;
 	}
-	if (i == device_attr.phys_port_cnt)
-		return 0;
 	/*
 	 * XXX if something went wrong in the loop above, there is a resource
 	 * leak (ctx, pd, priv, dpdk ethdev) but we can do nothing about it as
@@ -786,8 +786,9 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		claim_zero(mlx4_glue->close_device(attr_ctx));
 	if (list)
 		mlx4_glue->free_device_list(list);
-	assert(rte_errno >= 0);
-	return -rte_errno;
+	if (err)
+		rte_errno = err;
+	return -err;
 }
 
 static const struct rte_pci_id mlx4_pci_id_map[] = {
-- 
2.11.0

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

* [PATCH 2/2] net/mlx5: remove limitation on number of instances
  2018-05-22 15:36 [PATCH 1/2] net/mlx4: fix minor resource leak during init Adrien Mazarguil
@ 2018-05-22 15:37 ` Adrien Mazarguil
  2018-05-23  0:31   ` Yongseok Koh
  2018-06-17  8:05 ` [PATCH 1/2] net/mlx4: fix minor resource leak during init Shahaf Shuler
  1 sibling, 1 reply; 4+ messages in thread
From: Adrien Mazarguil @ 2018-05-22 15:37 UTC (permalink / raw)
  To: Shahaf Shuler; +Cc: Ferruh Yigit, dev, Nelio Laranjeiro, Yongseok Koh

This artificial limitation was inherited from the mlx4 code base and has no
purpose other than adding unnecessary noise.

This patch is a port of commit f2318196c71a ("net/mlx4: remove limitation
on number of instances").

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 54 +-------------------------------------------
 1 file changed, 1 insertion(+), 53 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 006665600..8d1c2347e 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -396,39 +396,6 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
 	.is_removed = mlx5_is_removed,
 };
 
-static struct {
-	struct rte_pci_addr pci_addr; /* associated PCI address */
-	uint32_t ports; /* physical ports bitfield. */
-} mlx5_dev[32];
-
-/**
- * Get device index in mlx5_dev[] from PCI bus address.
- *
- * @param[in] pci_addr
- *   PCI bus address to look for.
- *
- * @return
- *   mlx5_dev[] index on success, -1 on failure.
- */
-static int
-mlx5_dev_idx(struct rte_pci_addr *pci_addr)
-{
-	unsigned int i;
-	int ret = -1;
-
-	assert(pci_addr != NULL);
-	for (i = 0; (i != RTE_DIM(mlx5_dev)); ++i) {
-		if ((mlx5_dev[i].pci_addr.domain == pci_addr->domain) &&
-		    (mlx5_dev[i].pci_addr.bus == pci_addr->bus) &&
-		    (mlx5_dev[i].pci_addr.devid == pci_addr->devid) &&
-		    (mlx5_dev[i].pci_addr.function == pci_addr->function))
-			return i;
-		if ((mlx5_dev[i].ports == 0) && (ret == -1))
-			ret = i;
-	}
-	return ret;
-}
-
 /**
  * Verify and store value for device argument.
  *
@@ -698,7 +665,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	unsigned int mprq_max_stride_size_n = 0;
 	unsigned int mprq_min_stride_num_n = 0;
 	unsigned int mprq_max_stride_num_n = 0;
-	int idx;
 	int i;
 	struct mlx5dv_context attrs_out = {0};
 #ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
@@ -708,16 +674,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	/* Prepare shared data between primary and secondary process. */
 	mlx5_prepare_shared_data();
 	assert(pci_drv == &mlx5_driver);
-	/* Get mlx5_dev[] index. */
-	idx = mlx5_dev_idx(&pci_dev->addr);
-	if (idx == -1) {
-		DRV_LOG(ERR, "this driver cannot support any more adapters");
-		err = ENOMEM;
-		goto error;
-	}
-	DRV_LOG(DEBUG, "using driver device index %d", idx);
-	/* Save PCI address. */
-	mlx5_dev[idx].pci_addr = pci_dev->addr;
 	list = mlx5_glue->get_device_list(&i);
 	if (list == NULL) {
 		assert(errno);
@@ -873,7 +829,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		char name[RTE_ETH_NAME_MAX_LEN];
 		int len;
 		uint32_t port = i + 1; /* ports are indexed from one */
-		uint32_t test = (1 << i);
 		struct ibv_context *ctx = NULL;
 		struct ibv_port_attr port_attr;
 		struct ibv_pd *pd = NULL;
@@ -908,7 +863,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			 pci_dev->addr.devid, pci_dev->addr.function);
 		if (device_attr.orig_attr.phys_port_cnt > 1)
 			snprintf(name + len, sizeof(name), " port %u", i);
-		mlx5_dev[idx].ports |= test;
 		if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
 			eth_dev = rte_eth_dev_attach_secondary(name);
 			if (eth_dev == NULL) {
@@ -948,7 +902,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			rte_eth_dev_probing_finish(eth_dev);
 			continue;
 		}
-		DRV_LOG(DEBUG, "using port %u (%08" PRIx32 ")", port, test);
+		DRV_LOG(DEBUG, "using port %u", port);
 		ctx = mlx5_glue->open_device(ibv_dev);
 		if (ctx == NULL) {
 			err = ENODEV;
@@ -979,7 +933,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			err = ENOMEM;
 			goto port_error;
 		}
-		mlx5_dev[idx].ports |= test;
 		/* from rte_ethdev.c */
 		priv = rte_zmalloc("ethdev private structure",
 				   sizeof(*priv),
@@ -1210,11 +1163,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	 * long as the dpdk does not provide a way to deallocate a ethdev and a
 	 * way to enumerate the registered ethdevs to free the previous ones.
 	 */
-	/* no port found, complain */
-	if (!mlx5_dev[idx].ports) {
-		rte_errno = ENODEV;
-		err = rte_errno;
-	}
 error:
 	if (attr_ctx)
 		claim_zero(mlx5_glue->close_device(attr_ctx));
-- 
2.11.0

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

* Re: [PATCH 2/2] net/mlx5: remove limitation on number of instances
  2018-05-22 15:37 ` [PATCH 2/2] net/mlx5: remove limitation on number of instances Adrien Mazarguil
@ 2018-05-23  0:31   ` Yongseok Koh
  0 siblings, 0 replies; 4+ messages in thread
From: Yongseok Koh @ 2018-05-23  0:31 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: Shahaf Shuler, Ferruh Yigit, dev, Nélio Laranjeiro


> On May 22, 2018, at 8:37 AM, Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
> 
> This artificial limitation was inherited from the mlx4 code base and has no
> purpose other than adding unnecessary noise.
> 
> This patch is a port of commit f2318196c71a ("net/mlx4: remove limitation
> on number of instances").
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: Yongseok Koh <yskoh@mellanox.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

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

* Re: [PATCH 1/2] net/mlx4: fix minor resource leak during init
  2018-05-22 15:36 [PATCH 1/2] net/mlx4: fix minor resource leak during init Adrien Mazarguil
  2018-05-22 15:37 ` [PATCH 2/2] net/mlx5: remove limitation on number of instances Adrien Mazarguil
@ 2018-06-17  8:05 ` Shahaf Shuler
  1 sibling, 0 replies; 4+ messages in thread
From: Shahaf Shuler @ 2018-06-17  8:05 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: Ferruh Yigit, dev, stable

Tuesday, May 22, 2018 6:37 PM, Adrien Mazarguil:
> Subject: [PATCH 1/2] net/mlx4: fix minor resource leak during init
> 
> Temporary IB device context and list are not freed in case of a successful
> initialization of the device.
> 
> This issue is caused by the two following commits, the first of which causes
> initialization to return early, while the second one goes a bit overboard while
> switching to negative errno values; an internal variable
> (err) is needed to tell success from failure at the end of the function since
> rte_errno is not reliable enough.
> 
> Fixes: f2318196c71a ("net/mlx4: remove limitation on number of instances")
> Fixes: 9d14b27308a0 ("net/mlx4: standardize on negative errno values")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Series applied to next-net-mlx, thanks. 

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

end of thread, other threads:[~2018-06-17  8:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-22 15:36 [PATCH 1/2] net/mlx4: fix minor resource leak during init Adrien Mazarguil
2018-05-22 15:37 ` [PATCH 2/2] net/mlx5: remove limitation on number of instances Adrien Mazarguil
2018-05-23  0:31   ` Yongseok Koh
2018-06-17  8:05 ` [PATCH 1/2] net/mlx4: fix minor resource leak during init Shahaf Shuler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.