[dpdk-dev] [PATCH 05/15 v3] ring: Convert to use of PMD_REGISTER_DRIVER and fix linking

De Lara Guarch, Pablo pablo.de.lara.guarch at intel.com
Fri Jun 13 15:28:36 CEST 2014


> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Neil Horman
> Sent: Thursday, April 17, 2014 4:17 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 05/15 v3] ring: Convert to use of
> PMD_REGISTER_DRIVER and fix linking
> 
> convert the ring driver to use the PMD_REGISTER_DRIVER macro and fix up
> the
> Makefile so that its linkage is only done if we are building static libraries.
> This means that the test applications now have no reference to the ring
> library
> when building DSO's and must specify its use on the command line with the -
> d
> option.  Static linking will still initalize the driver automatically.
> 
> Note that the ring driver was also written in such a way that it violated some
> general layering principles, several functions were contained in the pmd
> which
> were being called by example from the test application in the app/test
> directory.  Specifically it was calling eth_ring_pair_attach,
> eth_ring_pair_create and rte_eth_ring_devinit, which should only be called
> internally to the dpdk core library.  To correct this I've removed those
> functions, and instead allowed them to be called indirectly at initalization
> time using the vdev command line argument key
> nodeaction=<name>:<node>:<action>
> where action is one of ATTACH or CREATE.  I've tested out the functionality of
> the command line with the testpmd utility, with success, and have removed
> the
> called functions from the test utility.  This will affect how the test utility
> is invoked (the -d and --vdev option will need to be specified on the
> command
> line now), but honestly, given the way it was coded, I think the testing of the
> ring pmd was not the best example of how to code with dpdk to begin with.  I
> have also left the two layer violating functions in place, so as not to break
> existing applications, but added deprecation warnings to them so that apps
> can
> migrate off them.
> 
> Signed-off-by: Neil Horman <nhorman at tuxdriver.com>
> 
> ---
> Change notes
> 
> v2) fixed DEPDIR specifcation, should depend on RING not PCAP
> 
> v3) Cleaned up strcmp error checking, printfs, and other parsing issues as
> pointed out by Konstantin Ananyev <konstantin.ananyev at intel.com>
> ---
>  app/test/test_pmd_ring.c           |  95 -------------------------
>  lib/librte_pmd_ring/Makefile       |   1 +
>  lib/librte_pmd_ring/rte_eth_ring.c | 141
> ++++++++++++++++++++++++++++++++++---
>  mk/rte.app.mk                      |  14 ++--
>  4 files changed, 142 insertions(+), 109 deletions(-)
> 
> diff --git a/app/test/test_pmd_ring.c b/app/test/test_pmd_ring.c
> index 4d9c2ba..1fe38fa 100644
> --- a/app/test/test_pmd_ring.c
> +++ b/app/test/test_pmd_ring.c
> @@ -42,7 +42,6 @@
>  /* two test rings, r1 is used by two ports, r2 just by one */
>  static struct rte_ring *r1[2], *r2;
> 
> -static struct rte_ring *nullring = NULL;
>  static struct rte_mempool *mp;
>  static uint8_t start_idx; /* will store the port id of the first of our new ports
> */
> 
> @@ -59,58 +58,6 @@ static uint8_t start_idx; /* will store the port id of the
> first of our new port
>  #define MBUF_SIZE (2048 + sizeof(struct rte_mbuf) +
> RTE_PKTMBUF_HEADROOM)
>  #define NB_MBUF   512
> 
> -
> -static int
> -test_ring_ethdev_create(void)
> -{
> -	int retval;
> -	printf("Testing ring pmd create\n");
> -
> -	retval = rte_eth_from_rings(NULL, 0, NULL, 0, SOCKET0);
> -	if (retval < 0) {
> -		printf("Failure, failed to create zero-sized RXTX ring pmd\n");
> -		return -1;
> -	}
> -
> -	retval = rte_eth_from_rings(NULL, 0, NULL, 0,
> RTE_MAX_NUMA_NODES);
> -	if (retval >= 0) {
> -		printf("Failure, can create ring pmd on socket %d\n",
> RTE_MAX_NUMA_NODES);
> -		return -1;
> -	}
> -
> -	retval = rte_eth_from_rings(NULL, 1, &r2, 1, SOCKET0);
> -	if (retval >= 0) {
> -		printf("Failure, can create pmd with null rx rings\n");
> -		return -1;
> -	}
> -
> -	retval = rte_eth_from_rings(r1, 1, NULL, 1, SOCKET0);
> -	if (retval >= 0) {
> -		printf("Failure, can create pmd with null tx rings\n");
> -		return -1;
> -	}
> -
> -	retval = rte_eth_from_rings(&nullring, 1, r1, 2, SOCKET0);
> -	if (retval < 0) {
> -		printf("Failure, failed to create TX-only ring pmd\n");
> -		return -1;
> -	}
> -
> -	retval = rte_eth_from_rings(r1, 1, &nullring, 1, SOCKET0);
> -	if (retval < 0) {
> -		printf("Failure, failed to create RX-only ring pmd\n");
> -		return -1;
> -	}
> -
> -	retval = rte_eth_from_rings(&r2, 1, &r2, 1, SOCKET0);
> -	if (retval < 0) {
> -		printf("Failure, failed to create RXTX ring pmd\n");
> -		return -1;
> -	}
> -
> -	return 0;
> -}
> -
>  static int
>  test_ethdev_configure(void)
>  {
> @@ -305,26 +252,12 @@ test_stats_reset(void)
>  static int
>  test_pmd_ring_init(void)
>  {
> -	const char * name1 = "R3";
> -	const char * name2 = "R4";
> -	const char * params_null = NULL;
> -	const char * params = "PARAMS";
>  	struct rte_eth_stats stats;
>  	struct rte_mbuf buf, *pbuf = &buf;
>  	struct rte_eth_conf null_conf;
> 
>  	printf("Testing ring pmd init\n");
> 
> -	if (rte_pmd_ring_devinit(name1, params_null) < 0) {
> -		printf("Testing ring pmd init fail\n");
> -		return -1;
> -	}
> -
> -	if (rte_pmd_ring_devinit(name2, params) < 0) {
> -		printf("Testing ring pmd init fail\n");
> -		return -1;
> -	}
> -
>  	if (RXTX_PORT2 >= RTE_MAX_ETHPORTS) {
>  		printf(" TX/RX port exceed max eth ports\n");
>  		return -1;
> @@ -371,24 +304,16 @@ test_pmd_ring_init(void)
> 
>  	rte_eth_dev_stop(RXTX_PORT2);
> 
> -	/* Test init same name pmd ring */
> -	rte_pmd_ring_devinit(name1, params_null);
>  	return 0;
>  }
> 
>  static int
>  test_pmd_ring_pair_create(void)
>  {
> -	const char * name1 = "_RNG_P0";
>  	struct rte_eth_stats stats, stats2;
>  	struct rte_mbuf buf, *pbuf = &buf;
>  	struct rte_eth_conf null_conf;
> 
> -	if (rte_eth_ring_pair_create(name1, SOCKET0) < 0) {
> -		printf("Create ring pair failed\n");
> -		return -1;
> -	}
> -
>  	if ((RXTX_PORT4 >= RTE_MAX_ETHPORTS) || (RXTX_PORT5 >=
> RTE_MAX_ETHPORTS)) {
>  		printf(" TX/RX port exceed max eth ports\n");
>  		return -1;
> @@ -447,28 +372,16 @@ test_pmd_ring_pair_create(void)
>  	rte_eth_dev_stop(RXTX_PORT4);
>  	rte_eth_dev_stop(RXTX_PORT5);
> 
> -	/* Test create same name ring pair */
> -	if (rte_eth_ring_pair_create(name1, SOCKET0) == 0) {
> -		printf("Create same name ring pair error\n");
> -		return -1;
> -	}
>  	return 0;
>  }
> 
>  static int
>  test_pmd_ring_pair_attach(void)
>  {
> -	const char * name1 = "_RNG_P0";
> -	const char * name2 = "_RNG_P1";
>  	struct rte_eth_stats stats, stats2;
>  	struct rte_mbuf buf, *pbuf = &buf;
>  	struct rte_eth_conf null_conf;
> 
> -	if (rte_eth_ring_pair_attach(name1, SOCKET0) < 0) {
> -		printf("Attach ring pair failed\n");
> -		return -1;
> -	}
> -
>  	if ((RXTX_PORT4 >= RTE_MAX_ETHPORTS) || (RXTX_PORT5 >=
> RTE_MAX_ETHPORTS)) {
>  		printf(" TX/RX port exceed max eth ports\n");
>  		return -1;
> @@ -529,11 +442,6 @@ test_pmd_ring_pair_attach(void)
>  	rte_eth_dev_stop(RXTX_PORT4);
>  	rte_eth_dev_stop(RXTX_PORT5);
> 
> -	/* Test attach non-existing ring pair */
> -	if (rte_eth_ring_pair_attach(name2, SOCKET0) == 0) {
> -		printf("Attach non-existing ring pair error\n");
> -		return -1;
> -	}
>  	return 0;
>  }
> 
> @@ -568,9 +476,6 @@ test_pmd_ring(void)
>  		return -1;
>  	}
> 
> -	if (test_ring_ethdev_create() < 0)
> -		return -1;
> -
>  	if (test_ethdev_configure() < 0)
>  		return -1;
> 
> diff --git a/lib/librte_pmd_ring/Makefile b/lib/librte_pmd_ring/Makefile
> index 73b2d38..35d40cb 100644
> --- a/lib/librte_pmd_ring/Makefile
> +++ b/lib/librte_pmd_ring/Makefile
> @@ -52,5 +52,6 @@ SYMLINK-y-include += rte_eth_ring.h
>  # this lib depends upon:
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += lib/librte_eal lib/librte_ring
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += lib/librte_mbuf
> lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += lib/librte_kvargs
> 
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_pmd_ring/rte_eth_ring.c
> b/lib/librte_pmd_ring/rte_eth_ring.c
> index cee3fff..3a129b6 100644
> --- a/lib/librte_pmd_ring/rte_eth_ring.c
> +++ b/lib/librte_pmd_ring/rte_eth_ring.c
> @@ -38,6 +38,17 @@
>  #include <rte_memcpy.h>
>  #include <rte_string_fns.h>
>  #include <rte_vdev.h>
> +#include <rte_pmd.h>
> +#include <rte_kvargs.h>
> +
> +#define ETH_RING_NUMA_NODE_ACTION_ARG	"nodeaction"
> +#define ETH_RING_ACTION_CREATE		"CREATE"
> +#define ETH_RING_ACTION_ATTACH		"ATTACH"
> +
> +static const char *valid_arguments[] = {
> +	ETH_RING_NUMA_NODE_ACTION_ARG,
> +	NULL
> +};
> 
>  struct ring_queue {
>  	struct rte_ring *rng;
> @@ -373,28 +384,143 @@ eth_dev_ring_pair_create(const char *name,
> const unsigned numa_node,
>  int
>  rte_eth_ring_pair_create(const char *name, const unsigned numa_node)
>  {
> +	RTE_LOG(WARNING, PMD, "rte_eth_ring_pair_create is
> deprecated\n");
>  	return eth_dev_ring_pair_create(name, numa_node, DEV_CREATE);
>  }
> 
>  int
>  rte_eth_ring_pair_attach(const char *name, const unsigned numa_node)
>  {
> +	RTE_LOG(WARNING, PMD, "rte_eth_ring_pair_attach is
> deprecated\n");
>  	return eth_dev_ring_pair_create(name, numa_node, DEV_ATTACH);
>  }
> 
> +struct node_action_pair {
> +	char name[PATH_MAX];
> +	unsigned node;
> +	enum dev_action action;
> +};
> +
> +struct node_action_list {
> +	unsigned total;
> +	unsigned count;
> +	struct node_action_pair *list;
> +};
> +
> +static int parse_kvlist (const char *key __rte_unused, const char *value,
> void *data)
> +{
> +	struct node_action_list *info = data;
> +	int ret;
> +	char *name;
> +	char *action;
> +	char *node;
> +	char *end;
> +
> +	name = strdup(value);
> +
> +	ret = -EINVAL;
> +
> +	if (!name) {
> +		RTE_LOG(WARNING, PMD, "command line paramter is empty
> for ring pmd!\n");
> +		goto out;
> +	}
> +
> +	node = strchr(name, ':');
> +	if (!node) {
> +		RTE_LOG(WARNING, PMD, "could not parse node value from
> %s", name);
> +		goto out;
> +	}
> +
> +	*node = '\0';
> +	node++;
> +
> +	action = strchr(node, ':');
> +	if (!action) {
> +		RTE_LOG(WARNING, PMD, "could not action value from %s",
> node);
> +		goto out;
> +	}
> +
> +	*action = '\0';
> +	action++;
> +
> +	/*
> +	 * Need to do some sanity checking here
> +	 */
> +
> +	if (strcmp(action, ETH_RING_ACTION_ATTACH) == 0)
> +		info->list[info->count].action = DEV_ATTACH;
> +	else if (strcmp(action, ETH_RING_ACTION_CREATE) == 0)
> +		info->list[info->count].action = DEV_CREATE;
> +	else
> +		goto out;
> +
> +	errno = 0;
> +	info->list[info->count].node = strtol(node, &end, 10);
> +
> +	if ((errno != 0) || (*end != '\0')) {
> +		RTE_LOG(WARNING, PMD, "node value %s is unparseable as
> a number\n", node);
> +		goto out;
> +	}
> +
> +	rte_snprintf(info->list[info->count].name, sizeof(info->list[info-
> >count].name), "%s", name);
> +
> +	info->count++;
> +
> +	ret = 0;
> +out:
> +	free(name);
> +	return ret;
> +}
> +
>  int
>  rte_pmd_ring_devinit(const char *name, const char *params)
>  {
> +	struct rte_kvargs *kvlist;
> +	int ret = 0;
> +	struct node_action_list *info = NULL;
> +
>  	RTE_LOG(INFO, PMD, "Initializing pmd_ring for %s\n", name);
> 
> +
>  	if (params == NULL || params[0] == '\0')
>  		eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE);
>  	else {
> -		RTE_LOG(INFO, PMD, "Ignoring unsupported parameters
> when creating"
> -				" rings-backed ethernet device\n");
> -		eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE);
> +		kvlist = rte_kvargs_parse(params, valid_arguments);
> +
> +		if (!kvlist) {
> +			RTE_LOG(INFO, PMD, "Ignoring unsupported
> parameters when creating"
> +					" rings-backed ethernet device\n");
> +			eth_dev_ring_create(name, rte_socket_id(),
> DEV_CREATE);
> +			return 0;
> +		} else {
> +			eth_dev_ring_create(name, rte_socket_id(),
> DEV_CREATE);
> +			ret = rte_kvargs_count(kvlist,
> ETH_RING_NUMA_NODE_ACTION_ARG);
> +			info = rte_zmalloc("struct node_action_list",
> sizeof(struct node_action_list) +
> +					   (sizeof(struct node_action_pair) *
> ret), 0);
> +			if (!info)
> +				goto out;
> +
> +			info->total = ret;
> +			info->list = (struct node_action_pair*)(info + 1);
> +
> +			ret = rte_kvargs_process(kvlist,
> ETH_RING_NUMA_NODE_ACTION_ARG,
> +						 parse_kvlist, info);
> +
> +			if (ret < 0)
> +				goto out_free;
> +
> +			for (info->count = 0; info->count < info->total; info-
> >count++) {
> +				eth_dev_ring_pair_create(name, info-
> >list[info->count].node,
> +						    info->list[info-
> >count].action);
> +			}
> +
> +		}
>  	}
> -	return 0;
> +
> +out_free:
> +	rte_free(info);
> +out:
> +	return ret;
>  }
> 
>  static struct rte_vdev_driver pmd_ring_drv = {
> @@ -402,9 +528,4 @@ static struct rte_vdev_driver pmd_ring_drv = {
>  	.init = rte_pmd_ring_devinit,
>  };
> 
> -__attribute__((constructor))
> -static void
> -rte_pmd_ring_init(void)
> -{
> -	rte_eal_vdev_driver_register(&pmd_ring_drv);
> -}
> +PMD_REGISTER_DRIVER(pmd_ring_drv, PMD_VDEV);
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index e6d09b8..4f167aa 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -133,10 +133,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y)
>  LDLIBS += -lethdev
>  endif
> 
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
> -LDLIBS += -lrte_pmd_ring
> -endif
> -
>  ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y)
>  LDLIBS += -lrte_malloc
>  endif
> @@ -173,9 +169,19 @@ LDLIBS += -lrte_cmdline
>  endif
> 
>  ifeq ($(RTE_BUILD_SHARED_LIB),n)
> +
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
> +LDLIBS += -lrte_pmd_ring
> +endif
> +
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
> +LDLIBS += -lrte_pmd_ring
> +endif
> +
>  ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>  LDLIBS += -lrte_pmd_pcap -lpcap
>  endif
> +
>  endif
> 
>  LDLIBS += $(EXECENV_LDLIBS)
> --
> 1.8.3.1

Hi Neil,

Sorry for opening this thread again, but I have encountered that the ring PMD unit test (ring_pmd_autotest in the test application) fails, due to this patch, as several functions calls were removed (such as rte_eth_ring_pair_create) . According to the explanation of this patch, you said that using -d/-vdev is required for the test app. Have you checked if unit test worked for you? In that case, could you tell me which parameters are needed, please?

Thanks,
Pablo


More information about the dev mailing list