[dpdk-dev,v6,6/8] ethdev: add common devargs parser

Message ID 20180328135433.20203-7-declan.doherty@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Doherty, Declan March 28, 2018, 1:54 p.m. UTC
  From: Remy Horton <remy.horton@intel.com>

Introduces a new structure, rte_eth_devargs, to support generic
ethdev arguments common across NET PMDs, with a new API
rte_eth_devargs_parse API to support PMD parsing these arguments.

Signed-off-by: Remy Horton <remy.horton@intel.com>
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 lib/Makefile                            |   1 +
 lib/librte_ether/rte_ethdev.c           | 191 ++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev_driver.h    |  30 +++++
 lib/librte_ether/rte_ethdev_version.map |   1 +
 4 files changed, 223 insertions(+)
  

Comments

Gaëtan Rivet March 29, 2018, 12:12 p.m. UTC | #1
Hi,

On Wed, Mar 28, 2018 at 02:54:31PM +0100, Declan Doherty wrote:
> From: Remy Horton <remy.horton@intel.com>
> 
> Introduces a new structure, rte_eth_devargs, to support generic
> ethdev arguments common across NET PMDs, with a new API
> rte_eth_devargs_parse API to support PMD parsing these arguments.
> 

Here is the future layout of rte_devargs:

   1. The rte_class introduced by [1], should be expanded with a "find_device"
      function, equivalent to that of the rte_bus interface.

   2. Class and Bus should export a match function as well as a parse
      function. The match function would be used for device querying,
      parsing would serve for device declaration.

   3. The match function is already implemented by [1].
      The parse function for bus already exists and should now
      be expanded to classes as well. Its expected input should
      change to be a list of kvargs.

   4. Accompanying those changes, the rte_devargs lib would
      now divide the device string in the three layers identified,
      use the class parse function to identify the intended class, and
      be able to feed each layers with its proper input.

This way, this API should be generic to all layers.

Now, this work in underway but takes time.
The current patch I think is an attempt to go in the right direction,
but in the end is only a compromise between the simple way and the
generic way.

Instead of having rte_eth_devargs_parse, you could have had a simple
rte_eth_representor_set(uint16_t *pid_list, size_t len);

That would have set the proper info within the rte_eth_dev_data. The port_id
list would have been parsed by your PMD by reading the representor
option.

The current version, that feeds directly the devargs to the eth layer,
makes conflicts inevitable (with PMDs having potential representor as
their parameter, or for future ether parameters such as "mac" that will
conflicts with current existing PMD parameters).

I would say that this implementation should be simple at first, for
the current work on representor. If the generic API is ready for this
release, then we might integrate afterward.

[1]: https://dpdk.org/ml/archives/dev/2018-March/092891.html

> Signed-off-by: Remy Horton <remy.horton@intel.com>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---

Regards,
  

Patch

diff --git a/lib/Makefile b/lib/Makefile
index ec965a606..4144d99f9 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -21,6 +21,7 @@  DEPDIRS-librte_cmdline := librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
 DEPDIRS-librte_ether := librte_net librte_eal librte_mempool librte_ring
 DEPDIRS-librte_ether += librte_mbuf
+DEPDIRS-librte_ether += librte_kvargs
 DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev
 DEPDIRS-librte_bbdev := librte_eal librte_mempool librte_mbuf
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 163246433..cdab23feb 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -35,6 +35,7 @@ 
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
 #include <rte_compat.h>
+#include <rte_kvargs.h>
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
@@ -4262,3 +4263,193 @@  rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
 
 	return (*dev->dev_ops->pool_ops_supported)(dev, pool);
 }
+
+typedef int (*rte_eth_devargs_callback_t)(char *str, void *data);
+
+static int
+rte_eth_devargs_tokenise(struct rte_kvargs *arglist, const char *str_in)
+{
+	int state;
+	struct rte_kvargs_pair *pair;
+	char *letter;
+
+	arglist->str = strdup(str_in);
+	if (arglist->str == NULL)
+		return -ENOMEM;
+
+	letter = arglist->str;
+	state = 0;
+	arglist->count = 0;
+	pair = &arglist->pairs[0];
+	while (1) {
+		switch (state) {
+		case 0: /* Initial */
+			if (*letter == '=')
+				return -EINVAL;
+			else if (*letter == '\0')
+				return 0;
+			state = 1;
+			pair->key = letter;
+			/* fall-thru */
+
+		case 1: /* Parsing key */
+			if (*letter == '=') {
+				*letter = '\0';
+				pair->value = letter + 1;
+				state = 2;
+			} else if (*letter == ',' || *letter == '\0')
+				return -EINVAL;
+			break;
+
+
+		case 2: /* Parsing value */
+			if (*letter == '[')
+				state = 3;
+			else if (*letter == ',' || *letter == '\0') {
+				*letter = '\0';
+				arglist->count++;
+				pair = &arglist->pairs[arglist->count];
+				state = 0;
+			}
+			break;
+
+		case 3: /* Parsing list */
+			if (*letter == ']')
+				state = 2;
+			else if (*letter == '\0')
+				return -EINVAL;
+			break;
+		}
+		letter++;
+	}
+}
+
+
+static int
+rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
+	void *data)
+{
+	char *str_start;
+	int state;
+	int result;
+
+	if (*str != '[')
+		/* Single element, not a list */
+		return callback(str, data);
+
+	/* Sanity check, then strip the brackets */
+	str_start = &str[strlen(str) - 1];
+	if (*str_start != ']') {
+		RTE_LOG(ERR, EAL, "(%s): List does not end with ']'", str);
+		return -EINVAL;
+	}
+	str++;
+	*str_start = '\0';
+
+	/* Process list elements */
+	state = 0;
+	while (1) {
+		if (state == 0) {
+			if (*str == '\0')
+				break;
+			if (*str != ',') {
+				str_start = str;
+				state = 1;
+			}
+		} else if (state == 1) {
+			if (*str == ',' || *str == '\0') {
+				if (str > str_start) {
+					/* Non-empty string fragment */
+					*str = '\0';
+					result = callback(str_start, data);
+					if (result < 0)
+						return result;
+				}
+				state = 0;
+			}
+		}
+		str++;
+	}
+	return 0;
+}
+
+static int
+rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
+	const uint16_t max_list)
+{
+	unsigned int lo;
+	unsigned int hi;
+	unsigned int value;
+	int result;
+
+	result = sscanf(str, "%u-%u", &lo, &hi);
+	if (result == 1) {
+		if (*len_list >= max_list)
+			return -ENOMEM;
+		list[(*len_list)++] = lo;
+	} else if (result == 2) {
+		if (lo >= hi)
+			return -EINVAL;
+		for (value = lo; value <= hi; value++) {
+			if (*len_list >= max_list)
+				return -ENOMEM;
+			list[(*len_list)++] = value;
+		}
+	} else
+		return -EINVAL;
+	return 0;
+}
+
+static int
+rte_eth_devargs_parse_ports(char *str, void *data)
+{
+	struct rte_eth_devargs *eth_da = data;
+
+	return rte_eth_devargs_process_range(str, eth_da->ports,
+		&eth_da->nb_ports, RTE_MAX_ETHPORTS);
+}
+
+
+static int
+rte_eth_devargs_parse_representor_ports(char *str, void *data)
+{
+	struct rte_eth_devargs *eth_da = data;
+
+	return rte_eth_devargs_process_range(str, eth_da->representor_ports,
+		&eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
+}
+
+int __rte_experimental
+rte_eth_devargs_parse(const struct rte_devargs * const da,
+	struct rte_eth_devargs *eth_da)
+{
+	struct rte_kvargs args;
+	struct rte_kvargs_pair *pair;
+	unsigned int i;
+	int result;
+
+	memset(eth_da, 0, sizeof(*eth_da));
+
+	result = rte_eth_devargs_tokenise(&args, da->args);
+	if (result < 0)
+		return result;
+
+	for (i = 0; i < args.count; i++) {
+		pair = &args.pairs[i];
+
+		if (strcmp("port", pair->key) == 0) {
+			result = rte_eth_devargs_parse_list(pair->value,
+				rte_eth_devargs_parse_ports, eth_da);
+			if (result < 0)
+				return result;
+		} else if (strcmp("representor", pair->key) == 0) {
+			result = rte_eth_devargs_parse_list(pair->value,
+				rte_eth_devargs_parse_representor_ports,
+				eth_da);
+			if (result < 0)
+				return result;
+		}
+	}
+
+	return 0;
+}
diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
index 4896cea93..2e9ce96a6 100644
--- a/lib/librte_ether/rte_ethdev_driver.h
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -126,6 +126,36 @@  rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
 			 unsigned align, int socket_id);
 
 
+/** Generic Ethernet device arguments  */
+struct rte_eth_devargs {
+	uint16_t ports[RTE_MAX_ETHPORTS];
+	/** port/s number to enable on a multi-port single function */
+	uint16_t nb_ports;
+	/** number of ports in ports field */
+	uint16_t representor_ports[RTE_MAX_ETHPORTS];
+	/** representor port/s identifier to enable on device */
+	uint16_t nb_representor_ports;
+	/** number of ports in representor port field */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function to parse ethdev arguments
+ *
+ * @param devargs
+ *  device arguments
+ * @param eth_devargs
+ *  parsed ethdev specific arguments.
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_devargs_parse(const struct rte_devargs * const devargs,
+	struct rte_eth_devargs *eth_devargs);
+
 typedef int (*ethdev_init_t)(struct rte_eth_dev *ethdev, void *init_params);
 typedef int (*ethdev_bus_specific_init)(struct rte_eth_dev *ethdev,
 	void *bus_specific_init_params);
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index 48b08bc36..99d396c5f 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -234,6 +234,7 @@  EXPERIMENTAL {
 EXPERIMENTAL {
 	global:
 
+	rt_eth_devargs_parse;
 	rte_eth_dev_create;
 	rte_eth_dev_destroy;