DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH 05/15 v2] ring: Convert to use of PMD_REGISTER_DRIVER and fix linking
Date: Wed, 16 Apr 2014 09:53:54 -0400	[thread overview]
Message-ID: <1397656434-516-1-git-send-email-nhorman@tuxdriver.com> (raw)
In-Reply-To: <1397585169-14537-6-git-send-email-nhorman@tuxdriver.com>

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@tuxdriver.com>

---
Change notes

v2) fixed DEPDIR specifcation, should depend on RING not PCAP
---
 app/test/test_pmd_ring.c           |  95 ----------------------------
 lib/librte_pmd_ring/Makefile       |   1 +
 lib/librte_pmd_ring/rte_eth_ring.c | 123 ++++++++++++++++++++++++++++++++++---
 mk/rte.app.mk                      |  14 +++--
 4 files changed, 124 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..266cfc0 100644
--- a/lib/librte_pmd_ring/rte_eth_ring.c
+++ b/lib/librte_pmd_ring/rte_eth_ring.c
@@ -38,6 +38,15 @@
 #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"
+
+static const char *valid_arguments[] = {
+	ETH_RING_NUMA_NODE_ACTION_ARG,
+	NULL
+};
 
 struct ring_queue {
 	struct rte_ring *rng;
@@ -373,28 +382,127 @@ 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;
+
+	name = strdup(value);
+
+	ret = -1;
+
+	if (!name)
+		goto out;
+
+	node = strchr(name, ':');
+	*node = '\0';
+	node++;
+
+	action = strchr(node, ':');
+	*action = '\0';
+	action++;
+
+	/*
+	 * Need to do some sanity checking here
+	 */
+
+	if (strcmp(action, "ATTACH"))
+		if (strcmp(action, "CREATE"))
+			goto out;
+
+	info->list[info->count].node = strtol(node, NULL, 10);
+
+	info->list[info->count].action = strcmp(action, "ATTACH") ? DEV_CREATE : DEV_ATTACH;
+
+	strncpy(info->list[info->count].name, name, PATH_MAX);
+
+	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);
+		printf("Parsing kvargs\n");
+		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);
+			printf("counting kvargs\n");
+			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);
+
+			printf("We have %d nodeaction pairs\n", info->total);
+			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++) {
+				printf("Doing something with a ring\n");
+				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 +510,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

  reply	other threads:[~2014-04-16 13:54 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-15 18:05 [dpdk-dev] [PATCH 0/15] dpdk: Separate compile time linkage between eal lib and pmd's Neil Horman
2014-04-15 18:05 ` [dpdk-dev] [PATCH 01/15] makefiles: Fixed -share command line option error Neil Horman
2014-04-16  9:22   ` Thomas Monjalon
2014-04-16 11:00     ` Neil Horman
2014-04-16 11:37       ` Thomas Monjalon
2014-04-16 13:51   ` [dpdk-dev] [PATCH 01/15 v2] " Neil Horman
2014-04-18 11:23     ` Thomas Monjalon
2014-04-18 13:18       ` Neil Horman
2014-04-18 13:29         ` Thomas Monjalon
2014-04-18 17:36           ` Neil Horman
2014-04-21 14:41           ` Neil Horman
2014-04-29 23:42     ` Thomas Monjalon
2014-05-02 11:09       ` Neil Horman
2014-05-02 12:22         ` Thomas Monjalon
2014-05-02 13:01           ` Neil Horman
2014-05-02 13:18             ` Thomas Monjalon
2014-04-15 18:05 ` [dpdk-dev] [PATCH 02/15] make: include whole archive on static link Neil Horman
2014-04-16  9:26   ` Thomas Monjalon
2014-04-16 11:02     ` Neil Horman
2014-04-16 11:40       ` Thomas Monjalon
2014-04-16 13:02         ` Neil Horman
2014-04-16 13:33           ` Neil Horman
2014-04-15 18:05 ` [dpdk-dev] [PATCH 03/15] pmd: Add PMD_REGISTER_DRIVER macro Neil Horman
2014-04-16 11:52   ` Thomas Monjalon
2014-04-16 12:59     ` John W. Linville
2014-04-16 13:08     ` Neil Horman
2014-04-16 16:11       ` Olivier MATZ
2014-04-16 17:15         ` John W. Linville
2014-04-16 17:29         ` Neil Horman
2014-04-17  8:08           ` Olivier MATZ
2014-04-17 10:59             ` Neil Horman
2014-04-18 11:42   ` Thomas Monjalon
2014-04-18 12:04     ` Neil Horman
2014-04-18 12:08       ` Thomas Monjalon
2014-04-18 13:20         ` Neil Horman
2014-04-18 13:32           ` Thomas Monjalon
2014-04-18 17:42             ` Neil Horman
2014-04-15 18:05 ` [dpdk-dev] [PATCH 04/15] pcap: Convert to use of PMD_REGISTER_DRIVER and fix linking Neil Horman
2014-04-15 18:05 ` [dpdk-dev] [PATCH 05/15] ring: " Neil Horman
2014-04-16 13:53   ` Neil Horman [this message]
2014-04-17  9:50   ` Ananyev, Konstantin
2014-04-17 11:06     ` Neil Horman
2014-04-17 15:16     ` [dpdk-dev] [PATCH 05/15 v3] " Neil Horman
2014-06-13 13:28       ` De Lara Guarch, Pablo
2014-04-15 18:06 ` [dpdk-dev] [PATCH 06/15] xenvirt: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 07/15] eal: Make vdev init path generic for both virtual and physcial devices Neil Horman
2014-04-18 12:02   ` Thomas Monjalon
2014-04-15 18:06 ` [dpdk-dev] [PATCH 08/15] igb: Convert to use of PMD_REGISTER_DRIVER and fix linking Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 09/15] igbvf: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 10/15] e1000: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 11/15] ixgbe: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 12/15] ixgbevf: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 13/15] virtio: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 14/15] vmxnet3: " Neil Horman
2014-04-15 18:06 ` [dpdk-dev] [PATCH 15/15] pmd: Remove rte_pmd_init_all Neil Horman
2014-04-21 14:59 ` [dpdk-dev] [PATCH v5 00/14] dpdk: Separate compile time linkage between eal lib and pmd's Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 01/14] makefiles: Fixed -share command line option error Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 02/14] pmd: Add PMD_REGISTER_DRIVER macro Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 03/14] pcap: Convert to use of PMD_REGISTER_DRIVER and fix linking Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 04/14] ring: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 05/14] xenvirt: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 06/14] eal: Make vdev init path generic for both virtual and physcial devices Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 07/14] igb: Convert to use of PMD_REGISTER_DRIVER and fix linking Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 08/14] igbvf: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 09/14] e1000: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 10/14] ixgbe: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 11/14] ixgbevf: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 12/14] virtio: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 13/14] vmxnet3: " Neil Horman
2014-04-21 14:59   ` [dpdk-dev] [PATCH 0/X v5 14/14] pmd: Remove rte_pmd_init_all Neil Horman
2014-04-21 17:05   ` [dpdk-dev] [PATCH v5 00/14] dpdk: Separate compile time linkage between eal lib and pmd's Neil Horman
2014-04-21 20:10   ` Stephen Hemminger
2014-04-21 20:36     ` Neil Horman
2014-05-16 15:28   ` Neil Horman
2014-05-16 15:39     ` Thomas Monjalon
2014-05-20 12:45   ` Thomas Monjalon
2014-05-20 14:13     ` Neil Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1397656434-516-1-git-send-email-nhorman@tuxdriver.com \
    --to=nhorman@tuxdriver.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).