Hi Thomas,

On Wed, Jul 11, 2018 at 10:19:07AM +0200, Thomas Monjalon wrote:

05/07/2018 13:48, Gaetan Rivet:

quoted

+/**
+ * @internal
+ * Parse a device string and store its information in an
+ * rte_devargs structure.

Please, explain what is a layer.

quoted

+ *
+ * Note: if the "data" field of da points to devstr,

Better to use "devargs" as variable name, instead of "da".

quoted

+ * then no dynamic allocation is performed and the rte_devargs
+ * can be safely discarded.
+ *
+ * Otherwise ``data`` will hold a workable copy of devstr, that will be
+ * used by layers descriptors within rte_devargs. In this case,
+ * any rte_devargs should be cleaned-up before being freed.
+ *
+ * @param da
+ *   rte_devargs structure to fill.
+ *
+ * @param devstr
+ *   Device string.
+ *
+ * @return
+ *   0 on success.
+ *   Negative errno values on error (rte_errno is set).
+ */
+int
+rte_devargs_layers_parse(struct rte_devargs *da,
+			 const char *devstr);
+
 #endif /* _EAL_PRIVATE_H_ */

diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 6c3b6326b..148600258 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h

@@ -51,12 +51,19 @@ struct rte_devargs {
 	enum rte_devtype type;
 	/** Device policy. */
 	enum rte_dev_policy policy;
-	/** Bus handle for the device. */
-	struct rte_bus *bus;
 	/** Name of the device. */
 	char name[RTE_DEV_NAME_MAX_LEN];
+	RTE_STD_C11
+	union {
 	/** Arguments string as given by user or "" for no argument. */
-	char *args;
+		char *args;
+		const char *drvstr;
+	};
+	struct rte_bus *bus; /**< bus handle. */
+	struct rte_class *cls; /**< class handle. */

"class" is more readable than "cls"

I was thinking that maybe this could be a problem when trying to build
with C++. The EAL headers in DPDK are meant to be included in C++ apps,
I think ``class`` would be an issue then.

If I'm mistaken, then sure, class is a better name.

quoted

+	const char *busstr; /**< bus-related part of device string. */

bus_str ?

quoted

+	const char *clsstr; /**< bus-related part of device string. */

class_str ?
+ there is a typo in the comment (copy/pasted "bus")

quoted

+	const char *data; /**< Device string storage. */
 };

Otherwise, ok with the rest of the comments, will fix (as well as the
previous emails). Thanks for reading.

-- 
Gaëtan Rivet
6WIND