[dpdk-dev] [RFC] avoid testpmd only working in XEN

[dpdk-dev] [RFC] avoid testpmd only working in XEN

[Christian Ehrhardt]

With LIBRTE_PMD_XENVIRT enabled testpmd is built in a way to ONLY work
in XEN environments.
It will surface as:
   PMD: gntalloc: ioctl error
   EAL: Error - exiting with code: 1
     Cause: Creation of mbuf pool for socket 0 failed

There could be a complex solution checking if this is a Xen env via
something like:
   #include <xenstore.h>
   struct xs_handle *xsh = NULL;
   xsh = xs_open(XS_OPEN_READONLY);
   check for xsh being NULL ...

But I see no reason to go so complex and instead the patch goes the
simple and more readable way of just falling back to the normal
allocation if the xen based allocation failed.
---
 app/test-pmd/testpmd.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1319917..b008df3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -410,7 +410,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
 	char pool_name[RTE_MEMPOOL_NAMESIZE];
-	struct rte_mempool *rte_mp;
+	struct rte_mempool *rte_mp = NULL;
 	uint32_t mb_size;
 
 	mb_size = sizeof(struct rte_mbuf) + mbuf_seg_size;
@@ -423,24 +423,23 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		rte_pktmbuf_pool_init, NULL,
 		rte_pktmbuf_init, NULL,
 		socket_id, 0);
-
-
-
-#else
-	if (mp_anon != 0)
-		rte_mp = mempool_anon_create(pool_name, nb_mbuf, mb_size,
-				    (unsigned) mb_mempool_cache,
-				    sizeof(struct rte_pktmbuf_pool_private),
-				    rte_pktmbuf_pool_init, NULL,
-				    rte_pktmbuf_init, NULL,
-				    socket_id, 0);
-	else
-		/* wrapper to rte_mempool_create() */
-		rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
-			mb_mempool_cache, 0, mbuf_seg_size, socket_id);
-
 #endif
 
+	/* if the former XEN allocation failed fall back to normal allocation */
+	if (rte_mp == NULL) {
+		if (mp_anon != 0)
+			rte_mp = mempool_anon_create(pool_name, nb_mbuf,
+					mb_size, (unsigned) mb_mempool_cache,
+					sizeof(struct rte_pktmbuf_pool_private),
+					rte_pktmbuf_pool_init, NULL,
+					rte_pktmbuf_init, NULL,
+					socket_id, 0);
+		else
+			/* wrapper to rte_mempool_create() */
+			rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
+				mb_mempool_cache, 0, mbuf_seg_size, socket_id);
+	}
+
 	if (rte_mp == NULL) {
 		rte_exit(EXIT_FAILURE, "Creation of mbuf pool for socket %u "
 						"failed\n", socket_id);
-- 
2.7.0

Re: [dpdk-dev] [RFC] avoid testpmd only working in XEN

[Christian Ehrhardt]

Hi,
I expected more of a discussion fallout as there would be so different
approaches to solve it (with the explicit xs_open stuff I mentioned, but
that I consider overengineering).
I had this running a while now and I can only say for me it works.
A few internal people working with Xen acked it as sufficient - but only by
looking at the code.

Since I got neither objections nor any other negative feedback so far I'll
just rewrite the comment and submit it as patch.

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Mar 1, 2016 at 5:32 PM, Christian Ehrhardt <
christian.ehrhardt@canonical.com> wrote:

> With LIBRTE_PMD_XENVIRT enabled testpmd is built in a way to ONLY work
> in XEN environments.
> It will surface as:
>    PMD: gntalloc: ioctl error
>    EAL: Error - exiting with code: 1
>      Cause: Creation of mbuf pool for socket 0 failed
>
> There could be a complex solution checking if this is a Xen env via
> something like:
>    #include <xenstore.h>
>    struct xs_handle *xsh = NULL;
>    xsh = xs_open(XS_OPEN_READONLY);
>    check for xsh being NULL ...
>
> But I see no reason to go so complex and instead the patch goes the
> simple and more readable way of just falling back to the normal
> allocation if the xen based allocation failed.
> ---
>  app/test-pmd/testpmd.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 1319917..b008df3 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -410,7 +410,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned
> nb_mbuf,
>                  unsigned int socket_id)
>  {
>         char pool_name[RTE_MEMPOOL_NAMESIZE];
> -       struct rte_mempool *rte_mp;
> +       struct rte_mempool *rte_mp = NULL;
>         uint32_t mb_size;
>
>         mb_size = sizeof(struct rte_mbuf) + mbuf_seg_size;
> @@ -423,24 +423,23 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned
> nb_mbuf,
>                 rte_pktmbuf_pool_init, NULL,
>                 rte_pktmbuf_init, NULL,
>                 socket_id, 0);
> -
> -
> -
> -#else
> -       if (mp_anon != 0)
> -               rte_mp = mempool_anon_create(pool_name, nb_mbuf, mb_size,
> -                                   (unsigned) mb_mempool_cache,
> -                                   sizeof(struct
> rte_pktmbuf_pool_private),
> -                                   rte_pktmbuf_pool_init, NULL,
> -                                   rte_pktmbuf_init, NULL,
> -                                   socket_id, 0);
> -       else
> -               /* wrapper to rte_mempool_create() */
> -               rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
> -                       mb_mempool_cache, 0, mbuf_seg_size, socket_id);
> -
>  #endif
>
> +       /* if the former XEN allocation failed fall back to normal
> allocation */
> +       if (rte_mp == NULL) {
> +               if (mp_anon != 0)
> +                       rte_mp = mempool_anon_create(pool_name, nb_mbuf,
> +                                       mb_size, (unsigned)
> mb_mempool_cache,
> +                                       sizeof(struct
> rte_pktmbuf_pool_private),
> +                                       rte_pktmbuf_pool_init, NULL,
> +                                       rte_pktmbuf_init, NULL,
> +                                       socket_id, 0);
> +               else
> +                       /* wrapper to rte_mempool_create() */
> +                       rte_mp = rte_pktmbuf_pool_create(pool_name,
> nb_mbuf,
> +                               mb_mempool_cache, 0, mbuf_seg_size,
> socket_id);
> +       }
> +
>         if (rte_mp == NULL) {
>                 rte_exit(EXIT_FAILURE, "Creation of mbuf pool for socket
> %u "
>                                                 "failed\n", socket_id);
> --
> 2.7.0
>
>

[dpdk-dev] [PATCH v2] testpmd: avoid only working in XEN when LIBRTE_PMD_XENVIRT is configured

[Christian Ehrhardt]

With LIBRTE_PMD_XENVIRT enabled testpmd is built in a way to ONLY work
in XEN environments.
It will surface as:
   PMD: gntalloc: ioctl error
   EAL: Error - exiting with code: 1
     Cause: Creation of mbuf pool for socket 0 failed

With LIBRTE_PMD_XENVIRT enabled this now tries the xen style grant
table allocation, but falls back gracefully for the normal allocation.

The only thing left in the log will be the
   PMD: gntalloc: ioctl error

Updates in v2
- adding missing Signed-off-by and set Pablo on --to with the patch directly

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 app/test-pmd/testpmd.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1319917..b008df3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -410,7 +410,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
 	char pool_name[RTE_MEMPOOL_NAMESIZE];
-	struct rte_mempool *rte_mp;
+	struct rte_mempool *rte_mp = NULL;
 	uint32_t mb_size;
 
 	mb_size = sizeof(struct rte_mbuf) + mbuf_seg_size;
@@ -423,24 +423,23 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		rte_pktmbuf_pool_init, NULL,
 		rte_pktmbuf_init, NULL,
 		socket_id, 0);
-
-
-
-#else
-	if (mp_anon != 0)
-		rte_mp = mempool_anon_create(pool_name, nb_mbuf, mb_size,
-				    (unsigned) mb_mempool_cache,
-				    sizeof(struct rte_pktmbuf_pool_private),
-				    rte_pktmbuf_pool_init, NULL,
-				    rte_pktmbuf_init, NULL,
-				    socket_id, 0);
-	else
-		/* wrapper to rte_mempool_create() */
-		rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
-			mb_mempool_cache, 0, mbuf_seg_size, socket_id);
-
 #endif
 
+	/* if the former XEN allocation failed fall back to normal allocation */
+	if (rte_mp == NULL) {
+		if (mp_anon != 0)
+			rte_mp = mempool_anon_create(pool_name, nb_mbuf,
+					mb_size, (unsigned) mb_mempool_cache,
+					sizeof(struct rte_pktmbuf_pool_private),
+					rte_pktmbuf_pool_init, NULL,
+					rte_pktmbuf_init, NULL,
+					socket_id, 0);
+		else
+			/* wrapper to rte_mempool_create() */
+			rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
+				mb_mempool_cache, 0, mbuf_seg_size, socket_id);
+	}
+
 	if (rte_mp == NULL) {
 		rte_exit(EXIT_FAILURE, "Creation of mbuf pool for socket %u "
 						"failed\n", socket_id);
-- 
2.7.3

Re: [dpdk-dev] [PATCH v2] testpmd: avoid only working in XEN when LIBRTE_PMD_XENVIRT is configured

[Thomas Monjalon]

2016-03-17 15:47, Christian Ehrhardt:
> With LIBRTE_PMD_XENVIRT enabled testpmd is built in a way to ONLY work
> in XEN environments.
> It will surface as:
>    PMD: gntalloc: ioctl error
>    EAL: Error - exiting with code: 1
>      Cause: Creation of mbuf pool for socket 0 failed
> 
> With LIBRTE_PMD_XENVIRT enabled this now tries the xen style grant
> table allocation, but falls back gracefully for the normal allocation.
> 
> The only thing left in the log will be the
>    PMD: gntalloc: ioctl error
> 
> Updates in v2
> - adding missing Signed-off-by and set Pablo on --to with the patch directly
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

Sorry this patch has been forgotten because (wrongly?) superseded in patchwork.
Please do not hesitate to ping when your patch has no follow-up.
Thanks Olivier for reminding this patch.

Applied, thanks