[dpdk-dev,v8,6/9] ethdev: add common devargs parser

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

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Doherty, Declan April 26, 2018, 10:41 a.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. The
patch add support for a representor argument  passed with passed with
the EAL -w option. The representor parameter allows the user to specify
which representor ports to initialise on a device.

The argument supports passing a single representor port, a list of
port values or a range of port values.

-w BDF,representor=1  # create representor port 1 on pci device BDF
-w BDF,representor=[1,2,5,6,10] # create representor ports in list
-w BDF,representor=[0-31] # create representor ports in range

Signed-off-by: Remy Horton <remy.horton@intel.com>
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 doc/guides/prog_guide/poll_mode_drv.rst |  19 ++++
 lib/Makefile                            |   1 +
 lib/librte_ether/rte_ethdev.c           | 182 ++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev_driver.h    |  30 ++++++
 lib/librte_ether/rte_ethdev_version.map |   1 +
 5 files changed, 233 insertions(+)
  

Comments

Ananyev, Konstantin April 26, 2018, 12:03 p.m. UTC | #1
> -----Original Message-----
> From: Doherty, Declan
> Sent: Thursday, April 26, 2018 11:41 AM
> To: dev@dpdk.org
> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Shahaf Shuler <shahafs@mellanox.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Horton, Remy <remy.horton@intel.com>;
> Doherty, Declan <declan.doherty@intel.com>
> Subject: [dpdk-dev][PATCH v8 6/9] ethdev: add common devargs parser
> 
> 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. The
> patch add support for a representor argument  passed with passed with
> the EAL -w option. The representor parameter allows the user to specify
> which representor ports to initialise on a device.
> 
> The argument supports passing a single representor port, a list of
> port values or a range of port values.
> 
> -w BDF,representor=1  # create representor port 1 on pci device BDF
> -w BDF,representor=[1,2,5,6,10] # create representor ports in list
> -w BDF,representor=[0-31] # create representor ports in range
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  doc/guides/prog_guide/poll_mode_drv.rst |  19 ++++
>  lib/Makefile                            |   1 +
>  lib/librte_ether/rte_ethdev.c           | 182 ++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev_driver.h    |  30 ++++++
>  lib/librte_ether/rte_ethdev_version.map |   1 +
>  5 files changed, 233 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
> index e5d01874e..09a93baec 100644
> --- a/doc/guides/prog_guide/poll_mode_drv.rst
> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> @@ -345,6 +345,25 @@ Ethernet Device API
> 
>  The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK API Reference*.
> 
> +Ethernet Device Standard Device Arguments
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Standard Ethernet device arguments allow for a set of commonly used arguments/
> +parameters which are applicable to all Ethernet devices to be available to for
> +specification of specific device and for passing common configuration
> +parameters to those ports.
> +
> +* ``representor`` for a device which supports the creation of representor ports
> +  this argument allows user to specify which switch ports to enable port
> +  representors for.::
> +
> +   -w BDBF,representor=0
> +   -w BDBF,representor=[0,4,6,9]
> +   -w BDBF,representor=[0-31]
> +
> +Note: PMDs are not required to support the standard device arguments and users
> +should consult the relevant PMD documentation to see support devargs.
> +
>  Extended Statistics API
>  ~~~~~~~~~~~~~~~~~~~~~~~
> 
> diff --git a/lib/Makefile b/lib/Makefile
> index 965be6c8d..536775e59 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 621f8af7f..cb85d8bb7 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -34,6 +34,7 @@
>  #include <rte_errno.h>
>  #include <rte_spinlock.h>
>  #include <rte_string_fns.h>
> ++#include <rte_kvargs.h>
> 
>  #include "rte_ether.h"
>  #include "rte_ethdev.h"
> @@ -4101,6 +4102,187 @@ 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)
> +{

I still think that if you'd like to extend rte_kvarrgs to be able to parse something like: "key=[val1,val2,...,valn]",
you have to make it generic kvargs ability and put it into librte_kvargs, not try to introduce your own new parser here.
Imagine that in addition to your 'port=[val1,val2, ..valn]' devargs string would contain some extra (let say device specific)
parameters.
What would happen, when PMD will try to use rte_kvargs_parse() on such string?
My understanding - it would fail, correct?

As an alternative - as I remember rte_kvargs allows you to have multiple identical key, i.e: "key=val1,key=val2,...,key=valn".
Why not to use that way, if you don't want to introduce extra code in rte_kvargs?

> +	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';
> +				arglist->count++;
> +				pair = &arglist->pairs[arglist->count];
> +				state = 0;
> +			} else if (*letter == '\0') {
> +				letter--;
> +				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)
> +{
> +	uint16_t lo, hi, val;
> +	int result;
> +
> +	result = sscanf(str, "%hu-%hu", &lo, &hi);
> +	if (result == 1) {
> +		if (*len_list >= max_list)
> +			return -ENOMEM;
> +		list[(*len_list)++] = lo;
> +	} else if (result == 2) {
> +		if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)

lo > RTE_MAX_ETHPORTS is redundant here.

> +			return -EINVAL;
> +		for (val = lo; val <= hi; val++) {
> +			if (*len_list >= max_list)
> +				return -ENOMEM;
> +			list[(*len_list)++] = val;
> +		}
> +	} else
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +
> +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 char *dargs, struct rte_eth_devargs *eth_da)
> +{
> +	struct rte_kvargs args;
> +	struct rte_kvargs_pair *pair;
> +	unsigned int i;
> +	int result = 0;
> +
> +	memset(eth_da, 0, sizeof(*eth_da));
> +
> +	result = rte_eth_devargs_tokenise(&args, dargs);
> +	if (result < 0)
> +		goto parse_cleanup;
> +
> +	for (i = 0; i < args.count; i++) {
> +		pair = &args.pairs[i];
> +		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)
> +				goto parse_cleanup;
> +		}
> +	}
> +
> +parse_cleanup:
> +	if (args.str)
> +		free(args.str);
> +
> +	return result;
> +}
> +
>  RTE_INIT(ethdev_init_log);
>  static void
>  ethdev_init_log(void)
> diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
> index 8c61ab2f4..492da754a 100644
> --- a/lib/librte_ether/rte_ethdev_driver.h
> +++ b/lib/librte_ether/rte_ethdev_driver.h
> @@ -189,6 +189,36 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
>  }
> 
> 
> +/** 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 char *dargs, 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 c4380aa31..41c3d2699 100644
> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> @@ -206,6 +206,7 @@ DPDK_18.02 {
>  EXPERIMENTAL {
>  	global:
> 
> +	rte_eth_devargs_parse;
>  	rte_eth_dev_count_avail;
>  	rte_eth_dev_count_total;
>  	rte_eth_dev_create;
> --
> 2.14.3
  
Ferruh Yigit April 26, 2018, 12:15 p.m. UTC | #2
On 4/26/2018 11:41 AM, 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. The
> patch add support for a representor argument  passed with passed with
> the EAL -w option. The representor parameter allows the user to specify
> which representor ports to initialise on a device.
> 
> The argument supports passing a single representor port, a list of
> port values or a range of port values.
> 
> -w BDF,representor=1  # create representor port 1 on pci device BDF
> -w BDF,representor=[1,2,5,6,10] # create representor ports in list
> -w BDF,representor=[0-31] # create representor ports in range
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

<...>

> +/**
> + * @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 char *dargs, struct rte_eth_devargs *eth_devargs);

API doc build giving warning because of doxygen comments, devargs vs dargs, I
will fix while applying as "devargs"
  
Ferruh Yigit April 26, 2018, 2:21 p.m. UTC | #3
On 4/26/2018 1:03 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Doherty, Declan
>> Sent: Thursday, April 26, 2018 11:41 AM
>> To: dev@dpdk.org
>> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Shahaf Shuler <shahafs@mellanox.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Horton, Remy <remy.horton@intel.com>;
>> Doherty, Declan <declan.doherty@intel.com>
>> Subject: [dpdk-dev][PATCH v8 6/9] ethdev: add common devargs parser
>>
>> 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. The
>> patch add support for a representor argument  passed with passed with
>> the EAL -w option. The representor parameter allows the user to specify
>> which representor ports to initialise on a device.
>>
>> The argument supports passing a single representor port, a list of
>> port values or a range of port values.
>>
>> -w BDF,representor=1  # create representor port 1 on pci device BDF
>> -w BDF,representor=[1,2,5,6,10] # create representor ports in list
>> -w BDF,representor=[0-31] # create representor ports in range
>>
>> Signed-off-by: Remy Horton <remy.horton@intel.com>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>>  doc/guides/prog_guide/poll_mode_drv.rst |  19 ++++
>>  lib/Makefile                            |   1 +
>>  lib/librte_ether/rte_ethdev.c           | 182 ++++++++++++++++++++++++++++++++
>>  lib/librte_ether/rte_ethdev_driver.h    |  30 ++++++
>>  lib/librte_ether/rte_ethdev_version.map |   1 +
>>  5 files changed, 233 insertions(+)
>>
>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
>> index e5d01874e..09a93baec 100644
>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
>> @@ -345,6 +345,25 @@ Ethernet Device API
>>
>>  The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK API Reference*.
>>
>> +Ethernet Device Standard Device Arguments
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Standard Ethernet device arguments allow for a set of commonly used arguments/
>> +parameters which are applicable to all Ethernet devices to be available to for
>> +specification of specific device and for passing common configuration
>> +parameters to those ports.
>> +
>> +* ``representor`` for a device which supports the creation of representor ports
>> +  this argument allows user to specify which switch ports to enable port
>> +  representors for.::
>> +
>> +   -w BDBF,representor=0
>> +   -w BDBF,representor=[0,4,6,9]
>> +   -w BDBF,representor=[0-31]
>> +
>> +Note: PMDs are not required to support the standard device arguments and users
>> +should consult the relevant PMD documentation to see support devargs.
>> +
>>  Extended Statistics API
>>  ~~~~~~~~~~~~~~~~~~~~~~~
>>
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 965be6c8d..536775e59 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 621f8af7f..cb85d8bb7 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -34,6 +34,7 @@
>>  #include <rte_errno.h>
>>  #include <rte_spinlock.h>
>>  #include <rte_string_fns.h>
>> ++#include <rte_kvargs.h>
>>
>>  #include "rte_ether.h"
>>  #include "rte_ethdev.h"
>> @@ -4101,6 +4102,187 @@ 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)
>> +{
> 
> I still think that if you'd like to extend rte_kvarrgs to be able to parse something like: "key=[val1,val2,...,valn]",
> you have to make it generic kvargs ability and put it into librte_kvargs, not try to introduce your own new parser here.
> Imagine that in addition to your 'port=[val1,val2, ..valn]' devargs string would contain some extra (let say device specific)
> parameters.
> What would happen, when PMD will try to use rte_kvargs_parse() on such string?
> My understanding - it would fail, correct?
> 
> As an alternative - as I remember rte_kvargs allows you to have multiple identical key, i.e: "key=val1,key=val2,...,key=valn".
> Why not to use that way, if you don't want to introduce extra code in rte_kvargs?

Hi Declan, Remy,

I will continue with existing patchset, for the sake of the rc1,
can you please address these comments as incremental updates to the set?

>> +static int
>> +rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
>> +	const uint16_t max_list)
>> +{
>> +	uint16_t lo, hi, val;
>> +	int result;
>> +
>> +	result = sscanf(str, "%hu-%hu", &lo, &hi);
>> +	if (result == 1) {
>> +		if (*len_list >= max_list)
>> +			return -ENOMEM;
>> +		list[(*len_list)++] = lo;
>> +	} else if (result == 2) {
>> +		if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
> 
> lo > RTE_MAX_ETHPORTS is redundant here.

Same here, thanks.
  
Doherty, Declan April 26, 2018, 2:28 p.m. UTC | #4
On 26/04/2018 1:03 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Doherty, Declan
>> Sent: Thursday, April 26, 2018 11:41 AM
>> To: dev@dpdk.org
>> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Shahaf Shuler <shahafs@mellanox.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Horton, Remy <remy.horton@intel.com>;
>> Doherty, Declan <declan.doherty@intel.com>
>> Subject: [dpdk-dev][PATCH v8 6/9] ethdev: add common devargs parser
>>
>> 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. The
>> patch add support for a representor argument  passed with passed with
>> the EAL -w option. The representor parameter allows the user to specify
>> which representor ports to initialise on a device.
>>
>> The argument supports passing a single representor port, a list of
>> port values or a range of port values.
>>
>> -w BDF,representor=1  # create representor port 1 on pci device BDF
>> -w BDF,representor=[1,2,5,6,10] # create representor ports in list
>> -w BDF,representor=[0-31] # create representor ports in range
>>
>> Signed-off-by: Remy Horton <remy.horton@intel.com>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>>   doc/guides/prog_guide/poll_mode_drv.rst |  19 ++++
>>   lib/Makefile                            |   1 +
>>   lib/librte_ether/rte_ethdev.c           | 182 ++++++++++++++++++++++++++++++++
>>   lib/librte_ether/rte_ethdev_driver.h    |  30 ++++++
>>   lib/librte_ether/rte_ethdev_version.map |   1 +
>>   5 files changed, 233 insertions(+)
>>
>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
>> index e5d01874e..09a93baec 100644
>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
>> @@ -345,6 +345,25 @@ Ethernet Device API
>>
>>   The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK API Reference*.
>>
>> +Ethernet Device Standard Device Arguments
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Standard Ethernet device arguments allow for a set of commonly used arguments/
>> +parameters which are applicable to all Ethernet devices to be available to for
>> +specification of specific device and for passing common configuration
>> +parameters to those ports.
>> +
>> +* ``representor`` for a device which supports the creation of representor ports
>> +  this argument allows user to specify which switch ports to enable port
>> +  representors for.::
>> +
>> +   -w BDBF,representor=0
>> +   -w BDBF,representor=[0,4,6,9]
>> +   -w BDBF,representor=[0-31]
>> +
>> +Note: PMDs are not required to support the standard device arguments and users
>> +should consult the relevant PMD documentation to see support devargs.
>> +
>>   Extended Statistics API
>>   ~~~~~~~~~~~~~~~~~~~~~~~
>>
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 965be6c8d..536775e59 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 621f8af7f..cb85d8bb7 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -34,6 +34,7 @@
>>   #include <rte_errno.h>
>>   #include <rte_spinlock.h>
>>   #include <rte_string_fns.h>
>> ++#include <rte_kvargs.h>
>>
>>   #include "rte_ether.h"
>>   #include "rte_ethdev.h"
>> @@ -4101,6 +4102,187 @@ 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)
>> +{
> 
> I still think that if you'd like to extend rte_kvarrgs to be able to parse something like: "key=[val1,val2,...,valn]",
> you have to make it generic kvargs ability and put it into librte_kvargs, not try to introduce your own new parser here.
> Imagine that in addition to your 'port=[val1,val2, ..valn]' devargs string would contain some extra (let say device specific)
> parameters.
> What would happen, when PMD will try to use rte_kvargs_parse() on such string?
> My understanding - it would fail, correct?
> 
> As an alternative - as I remember rte_kvargs allows you to have multiple identical key, i.e: "key=val1,key=val2,...,key=valn".
> Why not to use that way, if you don't want to introduce extra code in rte_kvargs?
> 

Hey Konstantin, the rationale for keeping this independent from 
librte_kvargs was that it is likely that the implementation of parsing 
devarfs will change in the next release due the proposed rework on the 
whole devargs infrastructure. I hadn't considered the potential issue 
with rte_kvargs_parse() on string using the proposed syntax here. I'll 
send a patch for alignment with librte_kvargs for the next release 
candidate.

>> +	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';
>> +				arglist->count++;
>> +				pair = &arglist->pairs[arglist->count];
>> +				state = 0;
>> +			} else if (*letter == '\0') {
>> +				letter--;
>> +				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)
>> +{
>> +	uint16_t lo, hi, val;
>> +	int result;
>> +
>> +	result = sscanf(str, "%hu-%hu", &lo, &hi);
>> +	if (result == 1) {
>> +		if (*len_list >= max_list)
>> +			return -ENOMEM;
>> +		list[(*len_list)++] = lo;
>> +	} else if (result == 2) {
>> +		if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
> 
> lo > RTE_MAX_ETHPORTS is redundant here.
> 
>> +			return -EINVAL;
>> +		for (val = lo; val <= hi; val++) {
>> +			if (*len_list >= max_list)
>> +				return -ENOMEM;
>> +			list[(*len_list)++] = val;
>> +		}
>> +	} else
>> +		return -EINVAL;
>> +	return 0;
>> +}
>> +
>> +
>> +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 char *dargs, struct rte_eth_devargs *eth_da)
>> +{
>> +	struct rte_kvargs args;
>> +	struct rte_kvargs_pair *pair;
>> +	unsigned int i;
>> +	int result = 0;
>> +
>> +	memset(eth_da, 0, sizeof(*eth_da));
>> +
>> +	result = rte_eth_devargs_tokenise(&args, dargs);
>> +	if (result < 0)
>> +		goto parse_cleanup;
>> +
>> +	for (i = 0; i < args.count; i++) {
>> +		pair = &args.pairs[i];
>> +		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)
>> +				goto parse_cleanup;
>> +		}
>> +	}
>> +
>> +parse_cleanup:
>> +	if (args.str)
>> +		free(args.str);
>> +
>> +	return result;
>> +}
>> +
>>   RTE_INIT(ethdev_init_log);
>>   static void
>>   ethdev_init_log(void)
>> diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
>> index 8c61ab2f4..492da754a 100644
>> --- a/lib/librte_ether/rte_ethdev_driver.h
>> +++ b/lib/librte_ether/rte_ethdev_driver.h
>> @@ -189,6 +189,36 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
>>   }
>>
>>
>> +/** 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 char *dargs, 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 c4380aa31..41c3d2699 100644
>> --- a/lib/librte_ether/rte_ethdev_version.map
>> +++ b/lib/librte_ether/rte_ethdev_version.map
>> @@ -206,6 +206,7 @@ DPDK_18.02 {
>>   EXPERIMENTAL {
>>   	global:
>>
>> +	rte_eth_devargs_parse;
>>   	rte_eth_dev_count_avail;
>>   	rte_eth_dev_count_total;
>>   	rte_eth_dev_create;
>> --
>> 2.14.3
>
  
Remy Horton April 26, 2018, 2:30 p.m. UTC | #5
On 26/04/2018 13:03, Ananyev, Konstantin wrote:
[..]
> I still think that if you'd like to extend rte_kvarrgs to be able to parse something like: "key=[val1,val2,...,valn]",
> you have to make it generic kvargs ability and put it into librte_kvargs, not try to introduce your own new parser here.
> Imagine that in addition to your 'port=[val1,val2, ..valn]' devargs string would contain some extra (let say device specific)
> parameters.
> What would happen, when PMD will try to use rte_kvargs_parse() on such string?
> My understanding - it would fail, correct?

This is partly dependent on what will (and won't) devargs provide when 
it is finalised. It was insourced in order to unblock the rest of the 
patchset in the meantime.
  
Thomas Monjalon April 26, 2018, 2:44 p.m. UTC | #6
26/04/2018 16:28, Doherty, Declan:
> On 26/04/2018 1:03 PM, Ananyev, Konstantin wrote:
> > From: Doherty, Declan
> >> +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)
> >> +{
> > 
> > I still think that if you'd like to extend rte_kvarrgs to be able to parse something like: "key=[val1,val2,...,valn]",
> > you have to make it generic kvargs ability and put it into librte_kvargs, not try to introduce your own new parser here.
> > Imagine that in addition to your 'port=[val1,val2, ..valn]' devargs string would contain some extra (let say device specific)
> > parameters.
> > What would happen, when PMD will try to use rte_kvargs_parse() on such string?
> > My understanding - it would fail, correct?
> > 
> > As an alternative - as I remember rte_kvargs allows you to have multiple identical key, i.e: "key=val1,key=val2,...,key=valn".
> > Why not to use that way, if you don't want to introduce extra code in rte_kvargs?
> > 
> 
> Hey Konstantin, the rationale for keeping this independent from 
> librte_kvargs was that it is likely that the implementation of parsing 
> devarfs will change in the next release due the proposed rework on the 
> whole devargs infrastructure. I hadn't considered the potential issue 
> with rte_kvargs_parse() on string using the proposed syntax here. I'll 
> send a patch for alignment with librte_kvargs for the next release 
> candidate.

The new devargs infra will rely on librte_kvargs.
So improving kvargs is the right way here.
  
Ananyev, Konstantin April 26, 2018, 2:48 p.m. UTC | #7
> -----Original Message-----

> From: Doherty, Declan

> Sent: Thursday, April 26, 2018 3:29 PM

> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org

> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>;

> Shahaf Shuler <shahafs@mellanox.com>; Horton, Remy <remy.horton@intel.com>

> Subject: Re: [dpdk-dev][PATCH v8 6/9] ethdev: add common devargs parser

> 

> On 26/04/2018 1:03 PM, Ananyev, Konstantin wrote:

> >

> >

> >> -----Original Message-----

> >> From: Doherty, Declan

> >> Sent: Thursday, April 26, 2018 11:41 AM

> >> To: dev@dpdk.org

> >> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon

> <thomas@monjalon.net>;

> >> Shahaf Shuler <shahafs@mellanox.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Horton, Remy

> <remy.horton@intel.com>;

> >> Doherty, Declan <declan.doherty@intel.com>

> >> Subject: [dpdk-dev][PATCH v8 6/9] ethdev: add common devargs parser

> >>

> >> 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. The

> >> patch add support for a representor argument  passed with passed with

> >> the EAL -w option. The representor parameter allows the user to specify

> >> which representor ports to initialise on a device.

> >>

> >> The argument supports passing a single representor port, a list of

> >> port values or a range of port values.

> >>

> >> -w BDF,representor=1  # create representor port 1 on pci device BDF

> >> -w BDF,representor=[1,2,5,6,10] # create representor ports in list

> >> -w BDF,representor=[0-31] # create representor ports in range

> >>

> >> Signed-off-by: Remy Horton <remy.horton@intel.com>

> >> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

> >> ---

> >>   doc/guides/prog_guide/poll_mode_drv.rst |  19 ++++

> >>   lib/Makefile                            |   1 +

> >>   lib/librte_ether/rte_ethdev.c           | 182 ++++++++++++++++++++++++++++++++

> >>   lib/librte_ether/rte_ethdev_driver.h    |  30 ++++++

> >>   lib/librte_ether/rte_ethdev_version.map |   1 +

> >>   5 files changed, 233 insertions(+)

> >>

> >> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst

> >> index e5d01874e..09a93baec 100644

> >> --- a/doc/guides/prog_guide/poll_mode_drv.rst

> >> +++ b/doc/guides/prog_guide/poll_mode_drv.rst

> >> @@ -345,6 +345,25 @@ Ethernet Device API

> >>

> >>   The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK API Reference*.

> >>

> >> +Ethernet Device Standard Device Arguments

> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> >> +

> >> +Standard Ethernet device arguments allow for a set of commonly used arguments/

> >> +parameters which are applicable to all Ethernet devices to be available to for

> >> +specification of specific device and for passing common configuration

> >> +parameters to those ports.

> >> +

> >> +* ``representor`` for a device which supports the creation of representor ports

> >> +  this argument allows user to specify which switch ports to enable port

> >> +  representors for.::

> >> +

> >> +   -w BDBF,representor=0

> >> +   -w BDBF,representor=[0,4,6,9]

> >> +   -w BDBF,representor=[0-31]

> >> +

> >> +Note: PMDs are not required to support the standard device arguments and users

> >> +should consult the relevant PMD documentation to see support devargs.

> >> +

> >>   Extended Statistics API

> >>   ~~~~~~~~~~~~~~~~~~~~~~~

> >>

> >> diff --git a/lib/Makefile b/lib/Makefile

> >> index 965be6c8d..536775e59 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 621f8af7f..cb85d8bb7 100644

> >> --- a/lib/librte_ether/rte_ethdev.c

> >> +++ b/lib/librte_ether/rte_ethdev.c

> >> @@ -34,6 +34,7 @@

> >>   #include <rte_errno.h>

> >>   #include <rte_spinlock.h>

> >>   #include <rte_string_fns.h>

> >> ++#include <rte_kvargs.h>

> >>

> >>   #include "rte_ether.h"

> >>   #include "rte_ethdev.h"

> >> @@ -4101,6 +4102,187 @@ 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)

> >> +{

> >

> > I still think that if you'd like to extend rte_kvarrgs to be able to parse something like: "key=[val1,val2,...,valn]",

> > you have to make it generic kvargs ability and put it into librte_kvargs, not try to introduce your own new parser here.

> > Imagine that in addition to your 'port=[val1,val2, ..valn]' devargs string would contain some extra (let say device specific)

> > parameters.

> > What would happen, when PMD will try to use rte_kvargs_parse() on such string?

> > My understanding - it would fail, correct?

> >

> > As an alternative - as I remember rte_kvargs allows you to have multiple identical key, i.e: "key=val1,key=val2,...,key=valn".

> > Why not to use that way, if you don't want to introduce extra code in rte_kvargs?

> >

> 

> Hey Konstantin, the rationale for keeping this independent from

> librte_kvargs was that it is likely that the implementation of parsing

> devarfs will change in the next release due the proposed rework on the

> whole devargs infrastructure. I hadn't considered the potential issue

> with rte_kvargs_parse() on string using the proposed syntax here. I'll

> send a patch for alignment with librte_kvargs for the next release

> candidate.


Ok, thanks Declan.
Konstantin
  

Patch

diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index e5d01874e..09a93baec 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -345,6 +345,25 @@  Ethernet Device API
 
 The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK API Reference*.
 
+Ethernet Device Standard Device Arguments
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Standard Ethernet device arguments allow for a set of commonly used arguments/
+parameters which are applicable to all Ethernet devices to be available to for
+specification of specific device and for passing common configuration
+parameters to those ports.
+
+* ``representor`` for a device which supports the creation of representor ports
+  this argument allows user to specify which switch ports to enable port
+  representors for.::
+
+   -w BDBF,representor=0
+   -w BDBF,representor=[0,4,6,9]
+   -w BDBF,representor=[0-31]
+
+Note: PMDs are not required to support the standard device arguments and users
+should consult the relevant PMD documentation to see support devargs.
+
 Extended Statistics API
 ~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/Makefile b/lib/Makefile
index 965be6c8d..536775e59 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 621f8af7f..cb85d8bb7 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -34,6 +34,7 @@ 
 #include <rte_errno.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
++#include <rte_kvargs.h>
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
@@ -4101,6 +4102,187 @@  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';
+				arglist->count++;
+				pair = &arglist->pairs[arglist->count];
+				state = 0;
+			} else if (*letter == '\0') {
+				letter--;
+				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)
+{
+	uint16_t lo, hi, val;
+	int result;
+
+	result = sscanf(str, "%hu-%hu", &lo, &hi);
+	if (result == 1) {
+		if (*len_list >= max_list)
+			return -ENOMEM;
+		list[(*len_list)++] = lo;
+	} else if (result == 2) {
+		if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
+			return -EINVAL;
+		for (val = lo; val <= hi; val++) {
+			if (*len_list >= max_list)
+				return -ENOMEM;
+			list[(*len_list)++] = val;
+		}
+	} else
+		return -EINVAL;
+	return 0;
+}
+
+
+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 char *dargs, struct rte_eth_devargs *eth_da)
+{
+	struct rte_kvargs args;
+	struct rte_kvargs_pair *pair;
+	unsigned int i;
+	int result = 0;
+
+	memset(eth_da, 0, sizeof(*eth_da));
+
+	result = rte_eth_devargs_tokenise(&args, dargs);
+	if (result < 0)
+		goto parse_cleanup;
+
+	for (i = 0; i < args.count; i++) {
+		pair = &args.pairs[i];
+		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)
+				goto parse_cleanup;
+		}
+	}
+
+parse_cleanup:
+	if (args.str)
+		free(args.str);
+
+	return result;
+}
+
 RTE_INIT(ethdev_init_log);
 static void
 ethdev_init_log(void)
diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
index 8c61ab2f4..492da754a 100644
--- a/lib/librte_ether/rte_ethdev_driver.h
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -189,6 +189,36 @@  rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
 }
 
 
+/** 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 char *dargs, 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 c4380aa31..41c3d2699 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -206,6 +206,7 @@  DPDK_18.02 {
 EXPERIMENTAL {
 	global:
 
+	rte_eth_devargs_parse;
 	rte_eth_dev_count_avail;
 	rte_eth_dev_count_total;
 	rte_eth_dev_create;