[dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option

[dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option

[Anatoly Burakov]

According to our docs, only Linuxapp supports base-virtaddr option.
That is, strictly speaking, not true because most of the things
that are attempting to respect base-virtaddr are in common files,
so FreeBSD already *mostly* supports this option in practice.

This commit fixes the remaining bits to explicitly support
base-virtaddr option, and moves the arg parsing from EAL to common
options parsing code. Documentation is also updated to reflect
that all platforms now support base-virtaddr.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
 doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
 doc/guides/rel_notes/release_19_08.rst        |  5 +++
 lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
 lib/librte_eal/freebsd/eal/eal.c              | 13 ++++++-
 lib/librte_eal/linux/eal/eal.c                | 39 -------------------
 6 files changed, 60 insertions(+), 47 deletions(-)

diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
index cf421a56e..ed8b0e35b 100644
--- a/doc/guides/linux_gsg/eal_args.include.rst
+++ b/doc/guides/linux_gsg/eal_args.include.rst
@@ -86,6 +86,12 @@ Multiprocessing-related options
 
     Set the type of the current process.
 
+*   ``--base-virtaddr <address>``
+
+    Attempt to use a different starting address for all memory maps of the
+    primary DPDK process. This can be helpful if secondary processes cannot
+    start due to conflicts in address map.
+
 Memory-related options
 ~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
index c63f0f49a..b2cc60e44 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -49,12 +49,6 @@ Multiprocessing-related options
     allows running multiple independent DPDK primary/secondary processes under
     different prefixes.
 
-*   ``--base-virtaddr <address>``
-
-    Attempt to use a different starting address for all memory maps of the
-    primary DPDK process. This can be helpful if secondary processes cannot
-    start due to conflicts in address map.
-
 Memory-related options
 ~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
index 4a1fd8dd8..1b58d9282 100644
--- a/doc/guides/rel_notes/release_19_08.rst
+++ b/doc/guides/rel_notes/release_19_08.rst
@@ -56,6 +56,11 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **FreeBSD now supports `--base-virtaddr` EAL option.**
+
+  FreeBSD version now also supports setting base virtual address for mapping
+  pages and resources into its address space.
+
 * **Added MCS lock.**
 
   MCS lock provides scalability by spinning on a CPU/thread local variable
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 512d5088e..156e48e19 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -20,6 +20,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_lcore.h>
+#include <rte_memory.h>
 #include <rte_tailq.h>
 #include <rte_version.h>
 #include <rte_devargs.h>
@@ -1095,6 +1096,36 @@ eal_parse_iova_mode(const char *name)
 	return 0;
 }
 
+static int
+eal_parse_base_virtaddr(const char *arg)
+{
+	char *end;
+	uint64_t addr;
+
+	errno = 0;
+	addr = strtoull(arg, &end, 16);
+
+	/* check for errors */
+	if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
+		return -1;
+
+	/* make sure we don't exceed 32-bit boundary on 32-bit target */
+#ifndef RTE_ARCH_64
+	if (addr >= UINTPTR_MAX)
+		return -1;
+#endif
+
+	/* align the addr on 16M boundary, 16MB is the minimum huge page
+	 * size on IBM Power architecture. If the addr is aligned to 16MB,
+	 * it can align to 2MB for x86. So this alignment can also be used
+	 * on x86 and other architectures.
+	 */
+	internal_config.base_virtaddr =
+		RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
+
+	return 0;
+}
+
 /* caller is responsible for freeing the returned string */
 static char *
 available_cores(void)
@@ -1408,6 +1439,13 @@ eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
+	case OPT_BASE_VIRTADDR_NUM:
+		if (eal_parse_base_virtaddr(optarg) < 0) {
+			RTE_LOG(ERR, EAL, "invalid parameter for --"
+					OPT_BASE_VIRTADDR "\n");
+			return -1;
+		}
+		break;
 
 	/* don't know what to do, leave this to caller */
 	default:
diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index d53f0fe69..7ebe11db6 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -227,6 +227,14 @@ rte_eal_config_create(void)
 	if (internal_config.no_shconf)
 		return 0;
 
+	/* map the config before base address so that we don't waste a page */
+	if (internal_config.base_virtaddr != 0)
+		rte_mem_cfg_addr = (void *)
+			RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
+			sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
+	else
+		rte_mem_cfg_addr = NULL;
+
 	if (mem_cfg_fd < 0){
 		mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600);
 		if (mem_cfg_fd < 0) {
@@ -254,8 +262,9 @@ rte_eal_config_create(void)
 		return -1;
 	}
 
-	rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
-				PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
+	rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
+			sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
+			MAP_SHARED, mem_cfg_fd, 0);
 
 	if (rte_mem_cfg_addr == MAP_FAILED){
 		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 2e5499f9b..79f5d70c3 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -609,35 +609,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
 	return 0;
 }
 
-static int
-eal_parse_base_virtaddr(const char *arg)
-{
-	char *end;
-	uint64_t addr;
-
-	errno = 0;
-	addr = strtoull(arg, &end, 16);
-
-	/* check for errors */
-	if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
-		return -1;
-
-	/* make sure we don't exceed 32-bit boundary on 32-bit target */
-#ifndef RTE_ARCH_64
-	if (addr >= UINTPTR_MAX)
-		return -1;
-#endif
-
-	/* align the addr on 16M boundary, 16MB is the minimum huge page
-	 * size on IBM Power architecture. If the addr is aligned to 16MB,
-	 * it can align to 2MB for x86. So this alignment can also be used
-	 * on x86 */
-	internal_config.base_virtaddr =
-		RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
-
-	return 0;
-}
-
 static int
 eal_parse_vfio_intr(const char *mode)
 {
@@ -796,16 +767,6 @@ eal_parse_args(int argc, char **argv)
 			internal_config.force_socket_limits = 1;
 			break;
 
-		case OPT_BASE_VIRTADDR_NUM:
-			if (eal_parse_base_virtaddr(optarg) < 0) {
-				RTE_LOG(ERR, EAL, "invalid parameter for --"
-						OPT_BASE_VIRTADDR "\n");
-				eal_usage(prgname);
-				ret = -1;
-				goto out;
-			}
-			break;
-
 		case OPT_VFIO_INTR_NUM:
 			if (eal_parse_vfio_intr(optarg) < 0) {
 				RTE_LOG(ERR, EAL, "invalid parameters for --"
-- 
2.17.1

Re: [dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option

[Thomas Monjalon]

16/07/2019 13:25, Anatoly Burakov:
> According to our docs, only Linuxapp supports base-virtaddr option.
> That is, strictly speaking, not true because most of the things
> that are attempting to respect base-virtaddr are in common files,
> so FreeBSD already *mostly* supports this option in practice.
> 
> This commit fixes the remaining bits to explicitly support
> base-virtaddr option, and moves the arg parsing from EAL to common
> options parsing code. Documentation is also updated to reflect
> that all platforms now support base-virtaddr.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

This patch is deferred to 19.11.

Re: [dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option

[David Marchand]

On Tue, Jul 16, 2019 at 1:25 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> According to our docs, only Linuxapp supports base-virtaddr option.
> That is, strictly speaking, not true because most of the things
> that are attempting to respect base-virtaddr are in common files,
> so FreeBSD already *mostly* supports this option in practice.
>
> This commit fixes the remaining bits to explicitly support
> base-virtaddr option, and moves the arg parsing from EAL to common
> options parsing code. Documentation is also updated to reflect
> that all platforms now support base-virtaddr.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
>  doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
>  doc/guides/rel_notes/release_19_08.rst        |  5 +++
>  lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
>  lib/librte_eal/freebsd/eal/eal.c              | 13 ++++++-
>  lib/librte_eal/linux/eal/eal.c                | 39 -------------------
>  6 files changed, 60 insertions(+), 47 deletions(-)
>
> diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
> index cf421a56e..ed8b0e35b 100644
> --- a/doc/guides/linux_gsg/eal_args.include.rst
> +++ b/doc/guides/linux_gsg/eal_args.include.rst
> @@ -86,6 +86,12 @@ Multiprocessing-related options
>
>      Set the type of the current process.
>
> +*   ``--base-virtaddr <address>``
> +
> +    Attempt to use a different starting address for all memory maps of the
> +    primary DPDK process. This can be helpful if secondary processes cannot
> +    start due to conflicts in address map.
> +

doc/guides/freebsd_gsg/freebsd_eal_parameters.rst:.. include::
../linux_gsg/eal_args.include.rst

Ok, a bit misleading to put in linux_gsg/, so writing this here if
someone else looks at this :-)


>  Memory-related options
>  ~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> index c63f0f49a..b2cc60e44 100644
> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> @@ -49,12 +49,6 @@ Multiprocessing-related options
>      allows running multiple independent DPDK primary/secondary processes under
>      different prefixes.
>
> -*   ``--base-virtaddr <address>``
> -
> -    Attempt to use a different starting address for all memory maps of the
> -    primary DPDK process. This can be helpful if secondary processes cannot
> -    start due to conflicts in address map.
> -
>  Memory-related options
>  ~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
> index 4a1fd8dd8..1b58d9282 100644
> --- a/doc/guides/rel_notes/release_19_08.rst
> +++ b/doc/guides/rel_notes/release_19_08.rst
> @@ -56,6 +56,11 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =========================================================
>
> +* **FreeBSD now supports `--base-virtaddr` EAL option.**
> +
> +  FreeBSD version now also supports setting base virtual address for mapping
> +  pages and resources into its address space.
> +
>  * **Added MCS lock.**
>
>    MCS lock provides scalability by spinning on a CPU/thread local variable

Well, obviously, this needs some rebase on 19.11-rc0 :-)

> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 512d5088e..156e48e19 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -20,6 +20,7 @@
>  #include <rte_eal.h>
>  #include <rte_log.h>
>  #include <rte_lcore.h>
> +#include <rte_memory.h>
>  #include <rte_tailq.h>
>  #include <rte_version.h>
>  #include <rte_devargs.h>
> @@ -1095,6 +1096,36 @@ eal_parse_iova_mode(const char *name)
>         return 0;
>  }
>
> +static int
> +eal_parse_base_virtaddr(const char *arg)
> +{
> +       char *end;
> +       uint64_t addr;
> +
> +       errno = 0;
> +       addr = strtoull(arg, &end, 16);
> +
> +       /* check for errors */
> +       if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
> +               return -1;
> +
> +       /* make sure we don't exceed 32-bit boundary on 32-bit target */
> +#ifndef RTE_ARCH_64
> +       if (addr >= UINTPTR_MAX)
> +               return -1;
> +#endif
> +
> +       /* align the addr on 16M boundary, 16MB is the minimum huge page
> +        * size on IBM Power architecture. If the addr is aligned to 16MB,
> +        * it can align to 2MB for x86. So this alignment can also be used
> +        * on x86 and other architectures.
> +        */
> +       internal_config.base_virtaddr =
> +               RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
> +
> +       return 0;
> +}
> +
>  /* caller is responsible for freeing the returned string */
>  static char *
>  available_cores(void)
> @@ -1408,6 +1439,13 @@ eal_parse_common_option(int opt, const char *optarg,
>                         return -1;
>                 }
>                 break;
> +       case OPT_BASE_VIRTADDR_NUM:
> +               if (eal_parse_base_virtaddr(optarg) < 0) {
> +                       RTE_LOG(ERR, EAL, "invalid parameter for --"
> +                                       OPT_BASE_VIRTADDR "\n");
> +                       return -1;
> +               }
> +               break;
>
>         /* don't know what to do, leave this to caller */
>         default:
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index d53f0fe69..7ebe11db6 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -227,6 +227,14 @@ rte_eal_config_create(void)
>         if (internal_config.no_shconf)
>                 return 0;
>
> +       /* map the config before base address so that we don't waste a page */
> +       if (internal_config.base_virtaddr != 0)
> +               rte_mem_cfg_addr = (void *)
> +                       RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
> +                       sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
> +       else
> +               rte_mem_cfg_addr = NULL;
> +
>         if (mem_cfg_fd < 0){
>                 mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600);
>                 if (mem_cfg_fd < 0) {
> @@ -254,8 +262,9 @@ rte_eal_config_create(void)
>                 return -1;
>         }
>
> -       rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> -                               PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
> +       rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
> +                       sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
> +                       MAP_SHARED, mem_cfg_fd, 0);
>
>         if (rte_mem_cfg_addr == MAP_FAILED){
>                 RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");

Nit: when compared to Linux implementation, the reattach step does not
recommend using the --base-virtaddr in case the remmapping failed.


> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 2e5499f9b..79f5d70c3 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -609,35 +609,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
>         return 0;
>  }
>
> -static int
> -eal_parse_base_virtaddr(const char *arg)
> -{
> -       char *end;
> -       uint64_t addr;
> -
> -       errno = 0;
> -       addr = strtoull(arg, &end, 16);
> -
> -       /* check for errors */
> -       if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
> -               return -1;
> -
> -       /* make sure we don't exceed 32-bit boundary on 32-bit target */
> -#ifndef RTE_ARCH_64
> -       if (addr >= UINTPTR_MAX)
> -               return -1;
> -#endif
> -
> -       /* align the addr on 16M boundary, 16MB is the minimum huge page
> -        * size on IBM Power architecture. If the addr is aligned to 16MB,
> -        * it can align to 2MB for x86. So this alignment can also be used
> -        * on x86 */
> -       internal_config.base_virtaddr =
> -               RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
> -
> -       return 0;
> -}
> -
>  static int
>  eal_parse_vfio_intr(const char *mode)
>  {
> @@ -796,16 +767,6 @@ eal_parse_args(int argc, char **argv)
>                         internal_config.force_socket_limits = 1;
>                         break;
>
> -               case OPT_BASE_VIRTADDR_NUM:
> -                       if (eal_parse_base_virtaddr(optarg) < 0) {
> -                               RTE_LOG(ERR, EAL, "invalid parameter for --"
> -                                               OPT_BASE_VIRTADDR "\n");
> -                               eal_usage(prgname);
> -                               ret = -1;
> -                               goto out;
> -                       }
> -                       break;
> -
>                 case OPT_VFIO_INTR_NUM:
>                         if (eal_parse_vfio_intr(optarg) < 0) {
>                                 RTE_LOG(ERR, EAL, "invalid parameters for --"
> --
> 2.17.1


Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option

[Burakov, Anatoly]

On 12-Aug-19 11:19 AM, David Marchand wrote:
> On Tue, Jul 16, 2019 at 1:25 PM Anatoly Burakov
> <anatoly.burakov@intel.com> wrote:
>>
>> According to our docs, only Linuxapp supports base-virtaddr option.
>> That is, strictly speaking, not true because most of the things
>> that are attempting to respect base-virtaddr are in common files,
>> so FreeBSD already *mostly* supports this option in practice.
>>
>> This commit fixes the remaining bits to explicitly support
>> base-virtaddr option, and moves the arg parsing from EAL to common
>> options parsing code. Documentation is also updated to reflect
>> that all platforms now support base-virtaddr.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
>>   doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
>>   doc/guides/rel_notes/release_19_08.rst        |  5 +++
>>   lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
>>   lib/librte_eal/freebsd/eal/eal.c              | 13 ++++++-
>>   lib/librte_eal/linux/eal/eal.c                | 39 -------------------
>>   6 files changed, 60 insertions(+), 47 deletions(-)
>>
>> diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
>> index cf421a56e..ed8b0e35b 100644
>> --- a/doc/guides/linux_gsg/eal_args.include.rst
>> +++ b/doc/guides/linux_gsg/eal_args.include.rst
>> @@ -86,6 +86,12 @@ Multiprocessing-related options
>>
>>       Set the type of the current process.
>>
>> +*   ``--base-virtaddr <address>``
>> +
>> +    Attempt to use a different starting address for all memory maps of the
>> +    primary DPDK process. This can be helpful if secondary processes cannot
>> +    start due to conflicts in address map.
>> +
> 
> doc/guides/freebsd_gsg/freebsd_eal_parameters.rst:.. include::
> ../linux_gsg/eal_args.include.rst
> 
> Ok, a bit misleading to put in linux_gsg/, so writing this here if
> someone else looks at this :-)

This was agreed upon when this file was first introduced. We don't have 
a "common" section and there's no real way to create it without 
triggering a bunch of errors in doxygen, so it was decided that putting 
this in a Linux GSG is the best way to do this.

> 
> 
>>   Memory-related options
>>   ~~~~~~~~~~~~~~~~~~~~~~
>>
>> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
>> index c63f0f49a..b2cc60e44 100644
>> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
>> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
>> @@ -49,12 +49,6 @@ Multiprocessing-related options
>>       allows running multiple independent DPDK primary/secondary processes under
>>       different prefixes.
>>
>> -*   ``--base-virtaddr <address>``
>> -
>> -    Attempt to use a different starting address for all memory maps of the
>> -    primary DPDK process. This can be helpful if secondary processes cannot
>> -    start due to conflicts in address map.
>> -
>>   Memory-related options
>>   ~~~~~~~~~~~~~~~~~~~~~~
>>
>> diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
>> index 4a1fd8dd8..1b58d9282 100644
>> --- a/doc/guides/rel_notes/release_19_08.rst
>> +++ b/doc/guides/rel_notes/release_19_08.rst
>> @@ -56,6 +56,11 @@ New Features
>>        Also, make sure to start the actual text at the margin.
>>        =========================================================
>>
>> +* **FreeBSD now supports `--base-virtaddr` EAL option.**
>> +
>> +  FreeBSD version now also supports setting base virtual address for mapping
>> +  pages and resources into its address space.
>> +
>>   * **Added MCS lock.**
>>
>>     MCS lock provides scalability by spinning on a CPU/thread local variable
> 
> Well, obviously, this needs some rebase on 19.11-rc0 :-)

Yes, will do.

> 
>> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
>> index 512d5088e..156e48e19 100644
>> --- a/lib/librte_eal/common/eal_common_options.c
>> +++ b/lib/librte_eal/common/eal_common_options.c
>> @@ -20,6 +20,7 @@
>>   #include <rte_eal.h>
>>   #include <rte_log.h>

<snip>

>> -       rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
>> -                               PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
>> +       rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
>> +                       sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
>> +                       MAP_SHARED, mem_cfg_fd, 0);
>>
>>          if (rte_mem_cfg_addr == MAP_FAILED){
>>                  RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
> 
> Nit: when compared to Linux implementation, the reattach step does not
> recommend using the --base-virtaddr in case the remmapping failed.
> 

Good catch, will fix.

> 
>> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
>> index 2e5499f9b..79f5d70c3 100644
>> --- a/lib/librte_eal/linux/eal/eal.c
>> +++ b/lib/librte_eal/linux/eal/eal.c
>> @@ -609,35 +609,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
>>          return 0;
>>   }

> 
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option

[David Marchand]

Hello Anatoly,

On Mon, Aug 12, 2019 at 3:38 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 12-Aug-19 11:19 AM, David Marchand wrote:
> > On Tue, Jul 16, 2019 at 1:25 PM Anatoly Burakov
> > <anatoly.burakov@intel.com> wrote:
> >>
> >> According to our docs, only Linuxapp supports base-virtaddr option.
> >> That is, strictly speaking, not true because most of the things
> >> that are attempting to respect base-virtaddr are in common files,
> >> so FreeBSD already *mostly* supports this option in practice.
> >>
> >> This commit fixes the remaining bits to explicitly support
> >> base-virtaddr option, and moves the arg parsing from EAL to common
> >> options parsing code. Documentation is also updated to reflect
> >> that all platforms now support base-virtaddr.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---
> >>   doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
> >>   doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
> >>   doc/guides/rel_notes/release_19_08.rst        |  5 +++
> >>   lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
> >>   lib/librte_eal/freebsd/eal/eal.c              | 13 ++++++-
> >>   lib/librte_eal/linux/eal/eal.c                | 39 -------------------
> >>   6 files changed, 60 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
> >> index cf421a56e..ed8b0e35b 100644
> >> --- a/doc/guides/linux_gsg/eal_args.include.rst
> >> +++ b/doc/guides/linux_gsg/eal_args.include.rst
> >> @@ -86,6 +86,12 @@ Multiprocessing-related options
> >>
> >>       Set the type of the current process.
> >>
> >> +*   ``--base-virtaddr <address>``
> >> +
> >> +    Attempt to use a different starting address for all memory maps of the
> >> +    primary DPDK process. This can be helpful if secondary processes cannot
> >> +    start due to conflicts in address map.
> >> +
> >
> > doc/guides/freebsd_gsg/freebsd_eal_parameters.rst:.. include::
> > ../linux_gsg/eal_args.include.rst
> >
> > Ok, a bit misleading to put in linux_gsg/, so writing this here if
> > someone else looks at this :-)
>
> This was agreed upon when this file was first introduced. We don't have
> a "common" section and there's no real way to create it without
> triggering a bunch of errors in doxygen, so it was decided that putting
> this in a Linux GSG is the best way to do this.
>
> >
> >
> >>   Memory-related options
> >>   ~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> >> index c63f0f49a..b2cc60e44 100644
> >> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> >> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> >> @@ -49,12 +49,6 @@ Multiprocessing-related options
> >>       allows running multiple independent DPDK primary/secondary processes under
> >>       different prefixes.
> >>
> >> -*   ``--base-virtaddr <address>``
> >> -
> >> -    Attempt to use a different starting address for all memory maps of the
> >> -    primary DPDK process. This can be helpful if secondary processes cannot
> >> -    start due to conflicts in address map.
> >> -
> >>   Memory-related options
> >>   ~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
> >> index 4a1fd8dd8..1b58d9282 100644
> >> --- a/doc/guides/rel_notes/release_19_08.rst
> >> +++ b/doc/guides/rel_notes/release_19_08.rst
> >> @@ -56,6 +56,11 @@ New Features
> >>        Also, make sure to start the actual text at the margin.
> >>        =========================================================
> >>
> >> +* **FreeBSD now supports `--base-virtaddr` EAL option.**
> >> +
> >> +  FreeBSD version now also supports setting base virtual address for mapping
> >> +  pages and resources into its address space.
> >> +
> >>   * **Added MCS lock.**
> >>
> >>     MCS lock provides scalability by spinning on a CPU/thread local variable
> >
> > Well, obviously, this needs some rebase on 19.11-rc0 :-)
>
> Yes, will do.
>
> >
> >> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> >> index 512d5088e..156e48e19 100644
> >> --- a/lib/librte_eal/common/eal_common_options.c
> >> +++ b/lib/librte_eal/common/eal_common_options.c
> >> @@ -20,6 +20,7 @@
> >>   #include <rte_eal.h>
> >>   #include <rte_log.h>
>
> <snip>
>
> >> -       rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> >> -                               PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
> >> +       rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
> >> +                       sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
> >> +                       MAP_SHARED, mem_cfg_fd, 0);
> >>
> >>          if (rte_mem_cfg_addr == MAP_FAILED){
> >>                  RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
> >
> > Nit: when compared to Linux implementation, the reattach step does not
> > recommend using the --base-virtaddr in case the remmapping failed.
> >
>
> Good catch, will fix.

Can you prepare a v2?
Thanks.


-- 
David Marchand

[dpdk-dev] [PATCH v2 1/2] eal/freebsd: add support for base virtaddr option

[Anatoly Burakov]

According to our docs, only Linuxapp supports base-virtaddr option.
That is, strictly speaking, not true because most of the things
that are attempting to respect base-virtaddr are in common files,
so FreeBSD already *mostly* supports this option in practice.

This commit fixes the remaining bits to explicitly support
base-virtaddr option, and moves the arg parsing from EAL to common
options parsing code. Documentation is also updated to reflect
that all platforms now support base-virtaddr.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---

Notes:
    v2:
    - Harmonize FreeBSD reattach implementation with Linux

 doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
 doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
 doc/guides/rel_notes/release_19_11.rst        |  5 +++
 lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
 lib/librte_eal/freebsd/eal/eal.c              | 31 ++++++++++-----
 lib/librte_eal/linux/eal/eal.c                | 39 -------------------
 6 files changed, 70 insertions(+), 55 deletions(-)

diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
index cf421a56eb..ed8b0e35b0 100644
--- a/doc/guides/linux_gsg/eal_args.include.rst
+++ b/doc/guides/linux_gsg/eal_args.include.rst
@@ -86,6 +86,12 @@ Multiprocessing-related options
 
     Set the type of the current process.
 
+*   ``--base-virtaddr <address>``
+
+    Attempt to use a different starting address for all memory maps of the
+    primary DPDK process. This can be helpful if secondary processes cannot
+    start due to conflicts in address map.
+
 Memory-related options
 ~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
index c63f0f49a0..b2cc60e447 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -49,12 +49,6 @@ Multiprocessing-related options
     allows running multiple independent DPDK primary/secondary processes under
     different prefixes.
 
-*   ``--base-virtaddr <address>``
-
-    Attempt to use a different starting address for all memory maps of the
-    primary DPDK process. This can be helpful if secondary processes cannot
-    start due to conflicts in address map.
-
 Memory-related options
 ~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index 856088c5c7..95231d13dd 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -56,6 +56,11 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **FreeBSD now supports `--base-virtaddr` EAL option.**
+
+  FreeBSD version now also supports setting base virtual address for mapping
+  pages and resources into its address space.
+
 * **Added Lock-free Stack for aarch64.**
 
   The lock-free stack implementation is enabled for aarch64 platforms.
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 05cae5f75c..1cdbd35655 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -20,6 +20,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_lcore.h>
+#include <rte_memory.h>
 #include <rte_tailq.h>
 #include <rte_version.h>
 #include <rte_devargs.h>
@@ -1095,6 +1096,36 @@ eal_parse_iova_mode(const char *name)
 	return 0;
 }
 
+static int
+eal_parse_base_virtaddr(const char *arg)
+{
+	char *end;
+	uint64_t addr;
+
+	errno = 0;
+	addr = strtoull(arg, &end, 16);
+
+	/* check for errors */
+	if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
+		return -1;
+
+	/* make sure we don't exceed 32-bit boundary on 32-bit target */
+#ifndef RTE_ARCH_64
+	if (addr >= UINTPTR_MAX)
+		return -1;
+#endif
+
+	/* align the addr on 16M boundary, 16MB is the minimum huge page
+	 * size on IBM Power architecture. If the addr is aligned to 16MB,
+	 * it can align to 2MB for x86. So this alignment can also be used
+	 * on x86 and other architectures.
+	 */
+	internal_config.base_virtaddr =
+		RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
+
+	return 0;
+}
+
 /* caller is responsible for freeing the returned string */
 static char *
 available_cores(void)
@@ -1408,6 +1439,13 @@ eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
+	case OPT_BASE_VIRTADDR_NUM:
+		if (eal_parse_base_virtaddr(optarg) < 0) {
+			RTE_LOG(ERR, EAL, "invalid parameter for --"
+					OPT_BASE_VIRTADDR "\n");
+			return -1;
+		}
+		break;
 
 	/* don't know what to do, leave this to caller */
 	default:
diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index f86e9aa318..c7e1264205 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -226,6 +226,14 @@ rte_eal_config_create(void)
 	if (internal_config.no_shconf)
 		return 0;
 
+	/* map the config before base address so that we don't waste a page */
+	if (internal_config.base_virtaddr != 0)
+		rte_mem_cfg_addr = (void *)
+			RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
+			sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
+	else
+		rte_mem_cfg_addr = NULL;
+
 	if (mem_cfg_fd < 0){
 		mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600);
 		if (mem_cfg_fd < 0) {
@@ -253,8 +261,9 @@ rte_eal_config_create(void)
 		return -1;
 	}
 
-	rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
-				PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
+	rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
+			sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
+			MAP_SHARED, mem_cfg_fd, 0);
 
 	if (rte_mem_cfg_addr == MAP_FAILED){
 		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
@@ -332,15 +341,17 @@ rte_eal_config_reattach(void)
 	close(mem_cfg_fd);
 	mem_cfg_fd = -1;
 
-	if (mem_config == MAP_FAILED) {
+	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
+		if (mem_config != MAP_FAILED) {
+			/* errno is stale, don't use */
+			RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
+					  " - please use '--base-virtaddr' option\n",
+				rte_mem_cfg_addr, mem_config);
+			munmap(mem_config, sizeof(struct rte_mem_config));
+			return -1;
+		}
 		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n",
-				errno, strerror(errno));
-		return -1;
-	} else if (mem_config != rte_mem_cfg_addr) {
-		/* errno is stale, don't use */
-		RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]\n",
-			  rte_mem_cfg_addr, mem_config);
-		munmap(mem_config, sizeof(struct rte_mem_config));
+			errno, strerror(errno));
 		return -1;
 	}
 
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index f39720637f..ef5dafa94f 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -611,35 +611,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
 	return 0;
 }
 
-static int
-eal_parse_base_virtaddr(const char *arg)
-{
-	char *end;
-	uint64_t addr;
-
-	errno = 0;
-	addr = strtoull(arg, &end, 16);
-
-	/* check for errors */
-	if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
-		return -1;
-
-	/* make sure we don't exceed 32-bit boundary on 32-bit target */
-#ifndef RTE_ARCH_64
-	if (addr >= UINTPTR_MAX)
-		return -1;
-#endif
-
-	/* align the addr on 16M boundary, 16MB is the minimum huge page
-	 * size on IBM Power architecture. If the addr is aligned to 16MB,
-	 * it can align to 2MB for x86. So this alignment can also be used
-	 * on x86 */
-	internal_config.base_virtaddr =
-		RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
-
-	return 0;
-}
-
 static int
 eal_parse_vfio_intr(const char *mode)
 {
@@ -798,16 +769,6 @@ eal_parse_args(int argc, char **argv)
 			internal_config.force_socket_limits = 1;
 			break;
 
-		case OPT_BASE_VIRTADDR_NUM:
-			if (eal_parse_base_virtaddr(optarg) < 0) {
-				RTE_LOG(ERR, EAL, "invalid parameter for --"
-						OPT_BASE_VIRTADDR "\n");
-				eal_usage(prgname);
-				ret = -1;
-				goto out;
-			}
-			break;
-
 		case OPT_VFIO_INTR_NUM:
 			if (eal_parse_vfio_intr(optarg) < 0) {
 				RTE_LOG(ERR, EAL, "invalid parameters for --"
-- 
2.17.1

[dpdk-dev] [PATCH v2 2/2] eal: use define instead of raw flag name

[Anatoly Burakov]

We are using '--base-virtaddr' in a few places. We have a define for that,
so use it instead.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/freebsd/eal/eal.c        | 3 ++-
 lib/librte_eal/freebsd/eal/eal_memory.c | 5 ++++-
 lib/librte_eal/linux/eal/eal.c          | 4 ++--
 lib/librte_eal/linux/eal/eal_memory.c   | 3 ++-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index c7e1264205..14e1713294 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -345,7 +345,8 @@ rte_eal_config_reattach(void)
 		if (mem_config != MAP_FAILED) {
 			/* errno is stale, don't use */
 			RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
-					  " - please use '--base-virtaddr' option\n",
+					  " - please use '--" OPT_BASE_VIRTADDR
+					  "' option\n",
 				rte_mem_cfg_addr, mem_config);
 			munmap(mem_config, sizeof(struct rte_mem_config));
 			return -1;
diff --git a/lib/librte_eal/freebsd/eal/eal_memory.c b/lib/librte_eal/freebsd/eal/eal_memory.c
index cd31827c2b..a637127d5e 100644
--- a/lib/librte_eal/freebsd/eal/eal_memory.c
+++ b/lib/librte_eal/freebsd/eal/eal_memory.c
@@ -14,6 +14,8 @@
 #include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
+
+#include "eal_options.h"
 #include "eal_private.h"
 #include "eal_internal_cfg.h"
 #include "eal_filesystem.h"
@@ -367,7 +369,8 @@ alloc_va_space(struct rte_memseg_list *msl)
 	addr = eal_get_virtual_area(msl->base_va, &mem_sz, page_sz, 0, flags);
 	if (addr == NULL) {
 		if (rte_errno == EADDRNOTAVAIL)
-			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - please use '--base-virtaddr' option\n",
+			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - "
+				"please use '--" OPT_BASE_VIRTADDR "' option\n",
 				(unsigned long long)mem_sz, msl->base_va);
 		else
 			RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index ef5dafa94f..13ffabd470 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -435,8 +435,8 @@ rte_eal_config_reattach(void)
 		if (mem_config != MAP_FAILED) {
 			/* errno is stale, don't use */
 			RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
-				" - please use '--base-virtaddr' option\n",
-				rte_mem_cfg_addr, mem_config);
+				" - please use '--" OPT_BASE_VIRTADDR
+				"' option\n", rte_mem_cfg_addr, mem_config);
 			munmap(mem_config, sizeof(struct rte_mem_config));
 			return -1;
 		}
diff --git a/lib/librte_eal/linux/eal/eal_memory.c b/lib/librte_eal/linux/eal/eal_memory.c
index 8f62c343d6..197c67ac4b 100644
--- a/lib/librte_eal/linux/eal/eal_memory.c
+++ b/lib/librte_eal/linux/eal/eal_memory.c
@@ -832,7 +832,8 @@ alloc_va_space(struct rte_memseg_list *msl)
 	addr = eal_get_virtual_area(msl->base_va, &mem_sz, page_sz, 0, flags);
 	if (addr == NULL) {
 		if (rte_errno == EADDRNOTAVAIL)
-			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - please use '--base-virtaddr' option\n",
+			RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - "
+				"please use '--" OPT_BASE_VIRTADDR "' option\n",
 				(unsigned long long)mem_sz, msl->base_va);
 		else
 			RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 1/2] eal/freebsd: add support for base virtaddr option

[David Marchand]

On Thu, Oct 24, 2019 at 5:18 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> According to our docs, only Linuxapp supports base-virtaddr option.
> That is, strictly speaking, not true because most of the things
> that are attempting to respect base-virtaddr are in common files,
> so FreeBSD already *mostly* supports this option in practice.
>
> This commit fixes the remaining bits to explicitly support
> base-virtaddr option, and moves the arg parsing from EAL to common
> options parsing code. Documentation is also updated to reflect
> that all platforms now support base-virtaddr.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> ---
>
> Notes:
>     v2:
>     - Harmonize FreeBSD reattach implementation with Linux
>
>  doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
>  doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
>  doc/guides/rel_notes/release_19_11.rst        |  5 +++
>  lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++

I was about to apply it, and I realised that the usage() in Linux
still references the --base-virtaddr option but it was not moved to
the common code.
What do you think of this hunk?

diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index 1cdbd35..020f36e 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -1692,6 +1692,7 @@ static int xdigit2val(unsigned char c)
               "  --"OPT_NO_PCI"            Disable PCI\n"
               "  --"OPT_NO_HPET"           Disable HPET\n"
               "  --"OPT_NO_SHCONF"         No shared config (mmap'd files)\n"
+              "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
               "\n", RTE_MAX_LCORE);
        rte_option_usage();
 }
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index ef5dafa..c0aac21 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -539,7 +539,6 @@ enum rte_proc_type_t
               "  --"OPT_SOCKET_LIMIT"      Limit memory allocation on
sockets (comma separated values)\n"
               "  --"OPT_HUGE_DIR"          Directory where hugetlbfs
is mounted\n"
               "  --"OPT_FILE_PREFIX"       Prefix for hugepage filenames\n"
-              "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
               "  --"OPT_CREATE_UIO_DEV"    Create /dev/uioX (usually
done by hotplug)\n"
               "  --"OPT_VFIO_INTR"         Interrupt mode for VFIO
(legacy|msi|msix)\n"
               "  --"OPT_LEGACY_MEM"        Legacy memory mode (no
dynamic allocation, contiguous segments)\n"


>  lib/librte_eal/freebsd/eal/eal.c              | 31 ++++++++++-----
>  lib/librte_eal/linux/eal/eal.c                | 39 -------------------
>  6 files changed, 70 insertions(+), 55 deletions(-)
>
> diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
> index cf421a56eb..ed8b0e35b0 100644
> --- a/doc/guides/linux_gsg/eal_args.include.rst
> +++ b/doc/guides/linux_gsg/eal_args.include.rst
> @@ -86,6 +86,12 @@ Multiprocessing-related options
>
>      Set the type of the current process.
>
> +*   ``--base-virtaddr <address>``
> +
> +    Attempt to use a different starting address for all memory maps of the
> +    primary DPDK process. This can be helpful if secondary processes cannot
> +    start due to conflicts in address map.
> +
>  Memory-related options
>  ~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> index c63f0f49a0..b2cc60e447 100644
> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> @@ -49,12 +49,6 @@ Multiprocessing-related options
>      allows running multiple independent DPDK primary/secondary processes under
>      different prefixes.
>
> -*   ``--base-virtaddr <address>``
> -
> -    Attempt to use a different starting address for all memory maps of the
> -    primary DPDK process. This can be helpful if secondary processes cannot
> -    start due to conflicts in address map.
> -
>  Memory-related options
>  ~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index 856088c5c7..95231d13dd 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -56,6 +56,11 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =========================================================
>
> +* **FreeBSD now supports `--base-virtaddr` EAL option.**
> +
> +  FreeBSD version now also supports setting base virtual address for mapping
> +  pages and resources into its address space.
> +
>  * **Added Lock-free Stack for aarch64.**
>
>    The lock-free stack implementation is enabled for aarch64 platforms.
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 05cae5f75c..1cdbd35655 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -20,6 +20,7 @@
>  #include <rte_eal.h>
>  #include <rte_log.h>
>  #include <rte_lcore.h>
> +#include <rte_memory.h>
>  #include <rte_tailq.h>
>  #include <rte_version.h>
>  #include <rte_devargs.h>
> @@ -1095,6 +1096,36 @@ eal_parse_iova_mode(const char *name)
>         return 0;
>  }
>
> +static int
> +eal_parse_base_virtaddr(const char *arg)
> +{
> +       char *end;
> +       uint64_t addr;
> +
> +       errno = 0;
> +       addr = strtoull(arg, &end, 16);
> +
> +       /* check for errors */
> +       if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
> +               return -1;
> +
> +       /* make sure we don't exceed 32-bit boundary on 32-bit target */
> +#ifndef RTE_ARCH_64
> +       if (addr >= UINTPTR_MAX)
> +               return -1;
> +#endif
> +
> +       /* align the addr on 16M boundary, 16MB is the minimum huge page
> +        * size on IBM Power architecture. If the addr is aligned to 16MB,
> +        * it can align to 2MB for x86. So this alignment can also be used
> +        * on x86 and other architectures.
> +        */
> +       internal_config.base_virtaddr =
> +               RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
> +
> +       return 0;
> +}
> +
>  /* caller is responsible for freeing the returned string */
>  static char *
>  available_cores(void)
> @@ -1408,6 +1439,13 @@ eal_parse_common_option(int opt, const char *optarg,
>                         return -1;
>                 }
>                 break;
> +       case OPT_BASE_VIRTADDR_NUM:
> +               if (eal_parse_base_virtaddr(optarg) < 0) {
> +                       RTE_LOG(ERR, EAL, "invalid parameter for --"
> +                                       OPT_BASE_VIRTADDR "\n");
> +                       return -1;
> +               }
> +               break;
>
>         /* don't know what to do, leave this to caller */
>         default:
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index f86e9aa318..c7e1264205 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -226,6 +226,14 @@ rte_eal_config_create(void)
>         if (internal_config.no_shconf)
>                 return 0;
>
> +       /* map the config before base address so that we don't waste a page */
> +       if (internal_config.base_virtaddr != 0)
> +               rte_mem_cfg_addr = (void *)
> +                       RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
> +                       sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
> +       else
> +               rte_mem_cfg_addr = NULL;
> +
>         if (mem_cfg_fd < 0){
>                 mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600);
>                 if (mem_cfg_fd < 0) {
> @@ -253,8 +261,9 @@ rte_eal_config_create(void)
>                 return -1;
>         }
>
> -       rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> -                               PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
> +       rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
> +                       sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
> +                       MAP_SHARED, mem_cfg_fd, 0);
>
>         if (rte_mem_cfg_addr == MAP_FAILED){
>                 RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
> @@ -332,15 +341,17 @@ rte_eal_config_reattach(void)
>         close(mem_cfg_fd);
>         mem_cfg_fd = -1;
>
> -       if (mem_config == MAP_FAILED) {
> +       if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> +               if (mem_config != MAP_FAILED) {
> +                       /* errno is stale, don't use */
> +                       RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
> +                                         " - please use '--base-virtaddr' option\n",
> +                               rte_mem_cfg_addr, mem_config);
> +                       munmap(mem_config, sizeof(struct rte_mem_config));
> +                       return -1;
> +               }
>                 RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n",
> -                               errno, strerror(errno));
> -               return -1;
> -       } else if (mem_config != rte_mem_cfg_addr) {
> -               /* errno is stale, don't use */
> -               RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]\n",
> -                         rte_mem_cfg_addr, mem_config);
> -               munmap(mem_config, sizeof(struct rte_mem_config));
> +                       errno, strerror(errno));
>                 return -1;
>         }
>
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index f39720637f..ef5dafa94f 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -611,35 +611,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
>         return 0;
>  }
>
> -static int
> -eal_parse_base_virtaddr(const char *arg)
> -{
> -       char *end;
> -       uint64_t addr;
> -
> -       errno = 0;
> -       addr = strtoull(arg, &end, 16);
> -
> -       /* check for errors */
> -       if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
> -               return -1;
> -
> -       /* make sure we don't exceed 32-bit boundary on 32-bit target */
> -#ifndef RTE_ARCH_64
> -       if (addr >= UINTPTR_MAX)
> -               return -1;
> -#endif
> -
> -       /* align the addr on 16M boundary, 16MB is the minimum huge page
> -        * size on IBM Power architecture. If the addr is aligned to 16MB,
> -        * it can align to 2MB for x86. So this alignment can also be used
> -        * on x86 */
> -       internal_config.base_virtaddr =
> -               RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
> -
> -       return 0;
> -}
> -
>  static int
>  eal_parse_vfio_intr(const char *mode)
>  {
> @@ -798,16 +769,6 @@ eal_parse_args(int argc, char **argv)
>                         internal_config.force_socket_limits = 1;
>                         break;
>
> -               case OPT_BASE_VIRTADDR_NUM:
> -                       if (eal_parse_base_virtaddr(optarg) < 0) {
> -                               RTE_LOG(ERR, EAL, "invalid parameter for --"
> -                                               OPT_BASE_VIRTADDR "\n");
> -                               eal_usage(prgname);
> -                               ret = -1;
> -                               goto out;
> -                       }
> -                       break;
> -
>                 case OPT_VFIO_INTR_NUM:
>                         if (eal_parse_vfio_intr(optarg) < 0) {
>                                 RTE_LOG(ERR, EAL, "invalid parameters for --"
> --
> 2.17.1

Re: [dpdk-dev] [PATCH v2 2/2] eal: use define instead of raw flag name

[David Marchand]

On Thu, Oct 24, 2019 at 5:18 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> We are using '--base-virtaddr' in a few places. We have a define for that,
> so use it instead.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/freebsd/eal/eal.c        | 3 ++-
>  lib/librte_eal/freebsd/eal/eal_memory.c | 5 ++++-
>  lib/librte_eal/linux/eal/eal.c          | 4 ++--
>  lib/librte_eal/linux/eal/eal_memory.c   | 3 ++-
>  4 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index c7e1264205..14e1713294 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -345,7 +345,8 @@ rte_eal_config_reattach(void)
>                 if (mem_config != MAP_FAILED) {
>                         /* errno is stale, don't use */
>                         RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
> -                                         " - please use '--base-virtaddr' option\n",
> +                                         " - please use '--" OPT_BASE_VIRTADDR
> +                                         "' option\n",
>                                 rte_mem_cfg_addr, mem_config);
>                         munmap(mem_config, sizeof(struct rte_mem_config));
>                         return -1;
> diff --git a/lib/librte_eal/freebsd/eal/eal_memory.c b/lib/librte_eal/freebsd/eal/eal_memory.c
> index cd31827c2b..a637127d5e 100644
> --- a/lib/librte_eal/freebsd/eal/eal_memory.c
> +++ b/lib/librte_eal/freebsd/eal/eal_memory.c
> @@ -14,6 +14,8 @@
>  #include <rte_errno.h>
>  #include <rte_log.h>
>  #include <rte_string_fns.h>
> +
> +#include "eal_options.h"
>  #include "eal_private.h"
>  #include "eal_internal_cfg.h"
>  #include "eal_filesystem.h"
> @@ -367,7 +369,8 @@ alloc_va_space(struct rte_memseg_list *msl)
>         addr = eal_get_virtual_area(msl->base_va, &mem_sz, page_sz, 0, flags);
>         if (addr == NULL) {
>                 if (rte_errno == EADDRNOTAVAIL)
> -                       RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - please use '--base-virtaddr' option\n",
> +                       RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - "
> +                               "please use '--" OPT_BASE_VIRTADDR "' option\n",
>                                 (unsigned long long)mem_sz, msl->base_va);
>                 else
>                         RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index ef5dafa94f..13ffabd470 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -435,8 +435,8 @@ rte_eal_config_reattach(void)
>                 if (mem_config != MAP_FAILED) {
>                         /* errno is stale, don't use */
>                         RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
> -                               " - please use '--base-virtaddr' option\n",
> -                               rte_mem_cfg_addr, mem_config);
> +                               " - please use '--" OPT_BASE_VIRTADDR
> +                               "' option\n", rte_mem_cfg_addr, mem_config);
>                         munmap(mem_config, sizeof(struct rte_mem_config));
>                         return -1;
>                 }
> diff --git a/lib/librte_eal/linux/eal/eal_memory.c b/lib/librte_eal/linux/eal/eal_memory.c
> index 8f62c343d6..197c67ac4b 100644
> --- a/lib/librte_eal/linux/eal/eal_memory.c
> +++ b/lib/librte_eal/linux/eal/eal_memory.c
> @@ -832,7 +832,8 @@ alloc_va_space(struct rte_memseg_list *msl)
>         addr = eal_get_virtual_area(msl->base_va, &mem_sz, page_sz, 0, flags);
>         if (addr == NULL) {
>                 if (rte_errno == EADDRNOTAVAIL)
> -                       RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - please use '--base-virtaddr' option\n",
> +                       RTE_LOG(ERR, EAL, "Could not mmap %llu bytes at [%p] - "
> +                               "please use '--" OPT_BASE_VIRTADDR "' option\n",
>                                 (unsigned long long)mem_sz, msl->base_va);
>                 else
>                         RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
> --
> 2.17.1

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/2] eal/freebsd: add support for base virtaddr option

[Burakov, Anatoly]

On 24-Oct-19 7:56 PM, David Marchand wrote:
> On Thu, Oct 24, 2019 at 5:18 PM Anatoly Burakov
> <anatoly.burakov@intel.com> wrote:
>>
>> According to our docs, only Linuxapp supports base-virtaddr option.
>> That is, strictly speaking, not true because most of the things
>> that are attempting to respect base-virtaddr are in common files,
>> so FreeBSD already *mostly* supports this option in practice.
>>
>> This commit fixes the remaining bits to explicitly support
>> base-virtaddr option, and moves the arg parsing from EAL to common
>> options parsing code. Documentation is also updated to reflect
>> that all platforms now support base-virtaddr.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> Reviewed-by: David Marchand <david.marchand@redhat.com>
>> ---
>>
>> Notes:
>>      v2:
>>      - Harmonize FreeBSD reattach implementation with Linux
>>
>>   doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
>>   doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
>>   doc/guides/rel_notes/release_19_11.rst        |  5 +++
>>   lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
> 
> I was about to apply it, and I realised that the usage() in Linux
> still references the --base-virtaddr option but it was not moved to
> the common code.
> What do you think of this hunk?
> 
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 1cdbd35..020f36e 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -1692,6 +1692,7 @@ static int xdigit2val(unsigned char c)
>                 "  --"OPT_NO_PCI"            Disable PCI\n"
>                 "  --"OPT_NO_HPET"           Disable HPET\n"
>                 "  --"OPT_NO_SHCONF"         No shared config (mmap'd files)\n"
> +              "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
>                 "\n", RTE_MAX_LCORE);
>          rte_option_usage();
>   }
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index ef5dafa..c0aac21 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -539,7 +539,6 @@ enum rte_proc_type_t
>                 "  --"OPT_SOCKET_LIMIT"      Limit memory allocation on
> sockets (comma separated values)\n"
>                 "  --"OPT_HUGE_DIR"          Directory where hugetlbfs
> is mounted\n"
>                 "  --"OPT_FILE_PREFIX"       Prefix for hugepage filenames\n"
> -              "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
>                 "  --"OPT_CREATE_UIO_DEV"    Create /dev/uioX (usually
> done by hotplug)\n"
>                 "  --"OPT_VFIO_INTR"         Interrupt mode for VFIO
> (legacy|msi|msix)\n"
>                 "  --"OPT_LEGACY_MEM"        Legacy memory mode (no
> dynamic allocation, contiguous segments)\n"
> 
> 

Should be OK, i think. The whitespace seems to be slightly off, but 
otherwise, looks good.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v2 1/2] eal/freebsd: add support for base virtaddr option

[David Marchand]

On Fri, Oct 25, 2019 at 11:06 AM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 24-Oct-19 7:56 PM, David Marchand wrote:
> > On Thu, Oct 24, 2019 at 5:18 PM Anatoly Burakov
> > <anatoly.burakov@intel.com> wrote:
> >>
> >> According to our docs, only Linuxapp supports base-virtaddr option.
> >> That is, strictly speaking, not true because most of the things
> >> that are attempting to respect base-virtaddr are in common files,
> >> so FreeBSD already *mostly* supports this option in practice.
> >>
> >> This commit fixes the remaining bits to explicitly support
> >> base-virtaddr option, and moves the arg parsing from EAL to common
> >> options parsing code. Documentation is also updated to reflect
> >> that all platforms now support base-virtaddr.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> Reviewed-by: David Marchand <david.marchand@redhat.com>
> >> ---
> >>
> >> Notes:
> >>      v2:
> >>      - Harmonize FreeBSD reattach implementation with Linux
> >>
> >>   doc/guides/linux_gsg/eal_args.include.rst     |  6 +++
> >>   doc/guides/linux_gsg/linux_eal_parameters.rst |  6 ---
> >>   doc/guides/rel_notes/release_19_11.rst        |  5 +++
> >>   lib/librte_eal/common/eal_common_options.c    | 38 ++++++++++++++++++
> >
> > I was about to apply it, and I realised that the usage() in Linux
> > still references the --base-virtaddr option but it was not moved to
> > the common code.
> > What do you think of this hunk?
> >
> > diff --git a/lib/librte_eal/common/eal_common_options.c
> > b/lib/librte_eal/common/eal_common_options.c
> > index 1cdbd35..020f36e 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -1692,6 +1692,7 @@ static int xdigit2val(unsigned char c)
> >                 "  --"OPT_NO_PCI"            Disable PCI\n"
> >                 "  --"OPT_NO_HPET"           Disable HPET\n"
> >                 "  --"OPT_NO_SHCONF"         No shared config (mmap'd files)\n"
> > +              "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
> >                 "\n", RTE_MAX_LCORE);
> >          rte_option_usage();
> >   }
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index ef5dafa..c0aac21 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -539,7 +539,6 @@ enum rte_proc_type_t
> >                 "  --"OPT_SOCKET_LIMIT"      Limit memory allocation on
> > sockets (comma separated values)\n"
> >                 "  --"OPT_HUGE_DIR"          Directory where hugetlbfs
> > is mounted\n"
> >                 "  --"OPT_FILE_PREFIX"       Prefix for hugepage filenames\n"
> > -              "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
> >                 "  --"OPT_CREATE_UIO_DEV"    Create /dev/uioX (usually
> > done by hotplug)\n"
> >                 "  --"OPT_VFIO_INTR"         Interrupt mode for VFIO
> > (legacy|msi|msix)\n"
> >                 "  --"OPT_LEGACY_MEM"        Legacy memory mode (no
> > dynamic allocation, contiguous segments)\n"
> >
> >
>
> Should be OK, i think. The whitespace seems to be slightly off, but
> otherwise, looks good.

I suppose you mean the inline patch was whitespace damaged :-).
I moved this option a bit earlier in the list.

Please check and send a fix if you think it is wrong.


Applied, thanks.


--
David Marchand

Re: [dpdk-dev] [PATCH v2 2/2] eal: use define instead of raw flag name

[David Marchand]

On Thu, Oct 24, 2019 at 8:59 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Thu, Oct 24, 2019 at 5:18 PM Anatoly Burakov
> <anatoly.burakov@intel.com> wrote:
> >
> > We are using '--base-virtaddr' in a few places. We have a define for that,
> > so use it instead.
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > diff --git a/lib/librte_eal/freebsd/eal/eal_memory.c b/lib/librte_eal/freebsd/eal/eal_memory.c
> > index cd31827c2b..a637127d5e 100644
> > --- a/lib/librte_eal/freebsd/eal/eal_memory.c
> > +++ b/lib/librte_eal/freebsd/eal/eal_memory.c
> > @@ -14,6 +14,8 @@
> >  #include <rte_errno.h>
> >  #include <rte_log.h>
> >  #include <rte_string_fns.h>
> > +
> > +#include "eal_options.h"
> >  #include "eal_private.h"
> >  #include "eal_internal_cfg.h"
> >  #include "eal_filesystem.h"


Hit build issue on FreeBSD:
In file included from ../lib/librte_eal/freebsd/eal/eal_memory.c:18:0:
../lib/librte_eal/common/eal_options.h:79:15: error: 'struct
internal_config' declared inside parameter list will not be visible
outside of this definition or declaration [-Werror]
        struct internal_config *conf);
               ^~~~~~~~~~~~~~~

Fixed order of header inclusion.


Applied, thanks.


--
David Marchand