From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 61DF96832 for ; Fri, 13 Jun 2014 15:28:25 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 13 Jun 2014 06:28:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,471,1400050800"; d="scan'208";a="555069749" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga002.fm.intel.com with ESMTP; 13 Jun 2014 06:28:37 -0700 Received: from irsmsx107.ger.corp.intel.com (163.33.3.99) by IRSMSX104.ger.corp.intel.com (163.33.3.159) with Microsoft SMTP Server (TLS) id 14.3.123.3; Fri, 13 Jun 2014 14:28:37 +0100 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.58]) by IRSMSX107.ger.corp.intel.com ([169.254.10.208]) with mapi id 14.03.0123.003; Fri, 13 Jun 2014 14:28:37 +0100 From: "De Lara Guarch, Pablo" To: Neil Horman , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 05/15 v3] ring: Convert to use of PMD_REGISTER_DRIVER and fix linking Thread-Index: AQHPWlCGNgrdq16Knk2O8OjJt4/dk5tvYPNA Date: Fri, 13 Jun 2014 13:28:36 +0000 Message-ID: References: <2601191342CEEE43887BDE71AB9772580EF973E4@IRSMSX105.ger.corp.intel.com> <1397747816-23245-1-git-send-email-nhorman@tuxdriver.com> In-Reply-To: <1397747816-23245-1-git-send-email-nhorman@tuxdriver.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 05/15 v3] ring: Convert to use of PMD_REGISTER_DRIVER and fix linking X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Jun 2014 13:28:26 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman > Sent: Thursday, April 17, 2014 4:17 PM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH 05/15 v3] ring: Convert to use of > PMD_REGISTER_DRIVER and fix linking >=20 > 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 libra= ries. > 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. >=20 > 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 calle= d > internally to the dpdk core library. To correct this I've removed those > functions, and instead allowed them to be called indirectly at initalizat= ion > time using the vdev command line argument key > nodeaction=3D:: > where action is one of ATTACH or CREATE. I've tested out the functionali= ty 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 ut= ility > 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 b= reak > existing applications, but added deprecation warnings to them so that app= s > can > migrate off them. >=20 > Signed-off-by: Neil Horman >=20 > --- > Change notes >=20 > v2) fixed DEPDIR specifcation, should depend on RING not PCAP >=20 > v3) Cleaned up strcmp error checking, printfs, and other parsing issues a= s > pointed out by Konstantin Ananyev > --- > 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(-) >=20 > 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; >=20 > -static struct rte_ring *nullring =3D NULL; > static struct rte_mempool *mp; > static uint8_t start_idx; /* will store the port id of the first of our = new ports > */ >=20 > @@ -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 >=20 > - > -static int > -test_ring_ethdev_create(void) > -{ > - int retval; > - printf("Testing ring pmd create\n"); > - > - retval =3D 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 =3D rte_eth_from_rings(NULL, 0, NULL, 0, > RTE_MAX_NUMA_NODES); > - if (retval >=3D 0) { > - printf("Failure, can create ring pmd on socket %d\n", > RTE_MAX_NUMA_NODES); > - return -1; > - } > - > - retval =3D rte_eth_from_rings(NULL, 1, &r2, 1, SOCKET0); > - if (retval >=3D 0) { > - printf("Failure, can create pmd with null rx rings\n"); > - return -1; > - } > - > - retval =3D rte_eth_from_rings(r1, 1, NULL, 1, SOCKET0); > - if (retval >=3D 0) { > - printf("Failure, can create pmd with null tx rings\n"); > - return -1; > - } > - > - retval =3D 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 =3D 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 =3D 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 =3D "R3"; > - const char * name2 =3D "R4"; > - const char * params_null =3D NULL; > - const char * params =3D "PARAMS"; > struct rte_eth_stats stats; > struct rte_mbuf buf, *pbuf =3D &buf; > struct rte_eth_conf null_conf; >=20 > printf("Testing ring pmd init\n"); >=20 > - 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 >=3D RTE_MAX_ETHPORTS) { > printf(" TX/RX port exceed max eth ports\n"); > return -1; > @@ -371,24 +304,16 @@ test_pmd_ring_init(void) >=20 > rte_eth_dev_stop(RXTX_PORT2); >=20 > - /* Test init same name pmd ring */ > - rte_pmd_ring_devinit(name1, params_null); > return 0; > } >=20 > static int > test_pmd_ring_pair_create(void) > { > - const char * name1 =3D "_RNG_P0"; > struct rte_eth_stats stats, stats2; > struct rte_mbuf buf, *pbuf =3D &buf; > struct rte_eth_conf null_conf; >=20 > - if (rte_eth_ring_pair_create(name1, SOCKET0) < 0) { > - printf("Create ring pair failed\n"); > - return -1; > - } > - > if ((RXTX_PORT4 >=3D RTE_MAX_ETHPORTS) || (RXTX_PORT5 >=3D > 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); >=20 > - /* Test create same name ring pair */ > - if (rte_eth_ring_pair_create(name1, SOCKET0) =3D=3D 0) { > - printf("Create same name ring pair error\n"); > - return -1; > - } > return 0; > } >=20 > static int > test_pmd_ring_pair_attach(void) > { > - const char * name1 =3D "_RNG_P0"; > - const char * name2 =3D "_RNG_P1"; > struct rte_eth_stats stats, stats2; > struct rte_mbuf buf, *pbuf =3D &buf; > struct rte_eth_conf null_conf; >=20 > - if (rte_eth_ring_pair_attach(name1, SOCKET0) < 0) { > - printf("Attach ring pair failed\n"); > - return -1; > - } > - > if ((RXTX_PORT4 >=3D RTE_MAX_ETHPORTS) || (RXTX_PORT5 >=3D > 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); >=20 > - /* Test attach non-existing ring pair */ > - if (rte_eth_ring_pair_attach(name2, SOCKET0) =3D=3D 0) { > - printf("Attach non-existing ring pair error\n"); > - return -1; > - } > return 0; > } >=20 > @@ -568,9 +476,6 @@ test_pmd_ring(void) > return -1; > } >=20 > - if (test_ring_ethdev_create() < 0) > - return -1; > - > if (test_ethdev_configure() < 0) > return -1; >=20 > 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 +=3D rte_eth_ring.h > # this lib depends upon: > DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) +=3D lib/librte_eal lib/librte_rin= g > DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) +=3D lib/librte_mbuf > lib/librte_ether > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) +=3D lib/librte_kvargs >=20 > 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 > #include > #include > +#include > +#include > + > +#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[] =3D { > + ETH_RING_NUMA_NODE_ACTION_ARG, > + NULL > +}; >=20 > 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); > } >=20 > 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); > } >=20 > +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 =3D data; > + int ret; > + char *name; > + char *action; > + char *node; > + char *end; > + > + name =3D strdup(value); > + > + ret =3D -EINVAL; > + > + if (!name) { > + RTE_LOG(WARNING, PMD, "command line paramter is empty > for ring pmd!\n"); > + goto out; > + } > + > + node =3D strchr(name, ':'); > + if (!node) { > + RTE_LOG(WARNING, PMD, "could not parse node value from > %s", name); > + goto out; > + } > + > + *node =3D '\0'; > + node++; > + > + action =3D strchr(node, ':'); > + if (!action) { > + RTE_LOG(WARNING, PMD, "could not action value from %s", > node); > + goto out; > + } > + > + *action =3D '\0'; > + action++; > + > + /* > + * Need to do some sanity checking here > + */ > + > + if (strcmp(action, ETH_RING_ACTION_ATTACH) =3D=3D 0) > + info->list[info->count].action =3D DEV_ATTACH; > + else if (strcmp(action, ETH_RING_ACTION_CREATE) =3D=3D 0) > + info->list[info->count].action =3D DEV_CREATE; > + else > + goto out; > + > + errno =3D 0; > + info->list[info->count].node =3D strtol(node, &end, 10); > + > + if ((errno !=3D 0) || (*end !=3D '\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 =3D 0; > +out: > + free(name); > + return ret; > +} > + > int > rte_pmd_ring_devinit(const char *name, const char *params) > { > + struct rte_kvargs *kvlist; > + int ret =3D 0; > + struct node_action_list *info =3D NULL; > + > RTE_LOG(INFO, PMD, "Initializing pmd_ring for %s\n", name); >=20 > + > if (params =3D=3D NULL || params[0] =3D=3D '\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 =3D 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 =3D rte_kvargs_count(kvlist, > ETH_RING_NUMA_NODE_ACTION_ARG); > + info =3D rte_zmalloc("struct node_action_list", > sizeof(struct node_action_list) + > + (sizeof(struct node_action_pair) * > ret), 0); > + if (!info) > + goto out; > + > + info->total =3D ret; > + info->list =3D (struct node_action_pair*)(info + 1); > + > + ret =3D rte_kvargs_process(kvlist, > ETH_RING_NUMA_NODE_ACTION_ARG, > + parse_kvlist, info); > + > + if (ret < 0) > + goto out_free; > + > + for (info->count =3D 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; > } >=20 > static struct rte_vdev_driver pmd_ring_drv =3D { > @@ -402,9 +528,4 @@ static struct rte_vdev_driver pmd_ring_drv =3D { > .init =3D rte_pmd_ring_devinit, > }; >=20 > -__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 +=3D -lethdev > endif >=20 > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) > -LDLIBS +=3D -lrte_pmd_ring > -endif > - > ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) > LDLIBS +=3D -lrte_malloc > endif > @@ -173,9 +169,19 @@ LDLIBS +=3D -lrte_cmdline > endif >=20 > ifeq ($(RTE_BUILD_SHARED_LIB),n) > + > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) > +LDLIBS +=3D -lrte_pmd_ring > +endif > + > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) > +LDLIBS +=3D -lrte_pmd_ring > +endif > + > ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) > LDLIBS +=3D -lrte_pmd_pcap -lpcap > endif > + > endif >=20 > LDLIBS +=3D $(EXECENV_LDLIBS) > -- > 1.8.3.1 Hi Neil, Sorry for opening this thread again, but I have encountered that the ring P= MD 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, pleas= e? Thanks, Pablo