DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devargs: do not replace already inserted devargs
@ 2018-11-07 23:21 Thomas Monjalon
  2018-11-08 11:25 ` Stojaczyk, Dariusz
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2018-11-07 23:21 UTC (permalink / raw)
  To: dev; +Cc: gaetan.rivet, qi.z.zhang, jia.guo, dariusz.stojaczyk

The devargs of a device can be replaced by a newly allocated one
when trying to probe again the same device (multi-process or
multi-ports scenarios). This is breaking some pointer references.

It can be avoided by copying the new content, freeing the new devargs,
and returning the already inserted pointer.

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

This patch is not tested, I want to share the idea as soon as possible.
I hope it can fix some of the issues recently raised.

I think there are some mess in:
	- scan functions and devargs
	- vdev hotplug
We can revisit these areas in 19.02,
and consider this patch as a simple fix for 18.11.

---
 drivers/bus/vdev/vdev.c                     |  6 ++--
 lib/librte_eal/common/eal_common_dev.c      |  3 +-
 lib/librte_eal/common/eal_common_devargs.c  | 37 +++++++++++++++++----
 lib/librte_eal/common/include/rte_devargs.h |  4 ++-
 4 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 9c66bdc78..2c03ca418 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -224,7 +224,6 @@ insert_vdev(const char *name, const char *args,
 	}
 
 	dev->device.bus = &rte_vdev_bus;
-	dev->device.devargs = devargs;
 	dev->device.numa_node = SOCKET_ID_ANY;
 	dev->device.name = devargs->name;
 
@@ -238,9 +237,10 @@ insert_vdev(const char *name, const char *args,
 		goto fail;
 	}
 
-	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
 	if (init)
-		rte_devargs_insert(devargs);
+		rte_devargs_insert(&devargs);
+	dev->device.devargs = devargs;
+	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
 
 	if (p_dev)
 		*p_dev = dev;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 5759ec2d8..1fdc9ab17 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -150,10 +150,11 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 		goto err_devarg;
 	}
 
-	ret = rte_devargs_insert(da);
+	ret = rte_devargs_insert(&da);
 	if (ret)
 		goto err_devarg;
 
+	/* the rte_devargs will be referenced in the matching rte_device */
 	ret = da->bus->scan();
 	if (ret)
 		goto err_devarg;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index b7b9cb69e..0f8d997c8 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -263,14 +263,39 @@ rte_devargs_parsef(struct rte_devargs *da, const char *format, ...)
 }
 
 int __rte_experimental
-rte_devargs_insert(struct rte_devargs *da)
+rte_devargs_insert(struct rte_devargs **da)
 {
-	int ret;
+	struct rte_devargs *listed_da;
+	void *tmp;
+
+	if (*da == NULL || (*da)->bus == NULL)
+		return -1;
+
+	TAILQ_FOREACH_SAFE(listed_da, &devargs_list, next, tmp) {
+		if (listed_da == *da)
+			/* devargs already in the list */
+			return 0;
+		if (strcmp(listed_da->bus->name, (*da)->bus->name) == 0 &&
+				strcmp(listed_da->name, (*da)->name) == 0) {
+			/* device already in devargs list, must be updated */
+			listed_da->type = (*da)->type;
+			listed_da->policy = (*da)->policy;
+			free(listed_da->args);
+			listed_da->args = (*da)->args;
+			listed_da->bus = (*da)->bus;
+			listed_da->cls = (*da)->cls;
+			listed_da->bus_str = (*da)->bus_str;
+			listed_da->cls_str = (*da)->cls_str;
+			listed_da->data = (*da)->data;
+			/* replace provided devargs with found one */
+			free(*da);
+			*da = listed_da;
+			return 0;
+		}
+	}
 
-	ret = rte_devargs_remove(da);
-	if (ret < 0)
-		return ret;
-	TAILQ_INSERT_TAIL(&devargs_list, da, next);
+	/* new devargs in the list */
+	TAILQ_INSERT_TAIL(&devargs_list, *da, next);
 	return 0;
 }
 
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index b1f121f83..29b3fb7c8 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -146,6 +146,8 @@ __attribute__((format(printf, 2, 0)));
  *
  * @param da
  *  The devargs structure to insert.
+ *  If a devargs for the same device is already inserted,
+ *  it will be updated and returned. It means *da pointer can change.
  *
  * @return
  *   - 0 on success
@@ -153,7 +155,7 @@ __attribute__((format(printf, 2, 0)));
  */
 __rte_experimental
 int
-rte_devargs_insert(struct rte_devargs *da);
+rte_devargs_insert(struct rte_devargs **da);
 
 /**
  * Add a device to the user device list
-- 
2.19.0

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-11 23:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-07 23:21 [dpdk-dev] [PATCH] devargs: do not replace already inserted devargs Thomas Monjalon
2018-11-08 11:25 ` Stojaczyk, Dariusz
2018-11-08 12:26   ` Thomas Monjalon
2018-11-08 12:35     ` Stojaczyk, Dariusz
2018-11-08 12:53       ` Thomas Monjalon
2018-11-08 17:12     ` Zhang, Qi Z
2018-11-11 23:12       ` Thomas Monjalon
2018-11-08 14:02   ` Slava Ovsiienko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).