[dpdk-dev] [PATCH 00/10] eal cleanup and new options

[dpdk-dev] [PATCH 00/10] eal cleanup and new options

[Thomas Monjalon]

There are some pending patches which requires to factorize some EAL parts
in order to be correctly implemented.
This patchset do the required clean-up and rework these patches to improve
lcore handling:

Didier Pallard (2):
  eal: add core list input format
  config: support 128 cores

Patrick Lu (1):
  eal: get relative core index

Simon Kuenzer (1):
  eal: add option --master-lcore

Thomas Monjalon (6):
  eal: move internal headers in source directory
  eal: factorize common headers
  eal: fix header guards
  eal: factorize internal config reset
  eal: factorize options sanity check
  eal: factorize configuration adjustment

-- 
2.1.3

[dpdk-dev] [PATCH 01/10] eal: move internal headers in source directory

[Thomas Monjalon]

The directory include/ should be reserved to public headers.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/bsdapp/eal/Makefile                       | 1 +
 lib/librte_eal/common/{include => }/eal_options.h        | 0
 lib/librte_eal/common/{include => }/eal_private.h        | 0
 lib/librte_eal/linuxapp/eal/Makefile                     | 1 +
 lib/librte_eal/linuxapp/eal/{include => }/eal_pci_init.h | 0
 lib/librte_eal/linuxapp/eal/{include => }/eal_vfio.h     | 0
 6 files changed, 2 insertions(+)
 rename lib/librte_eal/common/{include => }/eal_options.h (100%)
 rename lib/librte_eal/common/{include => }/eal_private.h (100%)
 rename lib/librte_eal/linuxapp/eal/{include => }/eal_pci_init.h (100%)
 rename lib/librte_eal/linuxapp/eal/{include => }/eal_vfio.h (100%)

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index ca943c1..4683fc3 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -36,6 +36,7 @@ LIB = librte_eal.a
 VPATH += $(RTE_SDK)/lib/librte_eal/common
 
 CFLAGS += -I$(SRCDIR)/include
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common
 CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
 CFLAGS += -I$(RTE_SDK)/lib/librte_ring
 CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
diff --git a/lib/librte_eal/common/include/eal_options.h b/lib/librte_eal/common/eal_options.h
similarity index 100%
rename from lib/librte_eal/common/include/eal_options.h
rename to lib/librte_eal/common/eal_options.h
diff --git a/lib/librte_eal/common/include/eal_private.h b/lib/librte_eal/common/eal_private.h
similarity index 100%
rename from lib/librte_eal/common/include/eal_private.h
rename to lib/librte_eal/common/eal_private.h
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index c99433e..2480cb0 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -36,6 +36,7 @@ LIB = librte_eal.a
 VPATH += $(RTE_SDK)/lib/librte_eal/common
 
 CFLAGS += -I$(SRCDIR)/include
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common
 CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
 CFLAGS += -I$(RTE_SDK)/lib/librte_ring
 CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
similarity index 100%
rename from lib/librte_eal/linuxapp/eal/include/eal_pci_init.h
rename to lib/librte_eal/linuxapp/eal/eal_pci_init.h
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
similarity index 100%
rename from lib/librte_eal/linuxapp/eal/include/eal_vfio.h
rename to lib/librte_eal/linuxapp/eal/eal_vfio.h
-- 
2.1.3

Re: [dpdk-dev] [PATCH 01/10] eal: move internal headers in source directory

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:33PM +0100, Thomas Monjalon wrote:
> The directory include/ should be reserved to public headers.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/bsdapp/eal/Makefile                       | 1 +
>  lib/librte_eal/common/{include => }/eal_options.h        | 0
>  lib/librte_eal/common/{include => }/eal_private.h        | 0
>  lib/librte_eal/linuxapp/eal/Makefile                     | 1 +
>  lib/librte_eal/linuxapp/eal/{include => }/eal_pci_init.h | 0
>  lib/librte_eal/linuxapp/eal/{include => }/eal_vfio.h     | 0
>  6 files changed, 2 insertions(+)
>  rename lib/librte_eal/common/{include => }/eal_options.h (100%)
>  rename lib/librte_eal/common/{include => }/eal_private.h (100%)
>  rename lib/librte_eal/linuxapp/eal/{include => }/eal_pci_init.h (100%)
>  rename lib/librte_eal/linuxapp/eal/{include => }/eal_vfio.h (100%)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index ca943c1..4683fc3 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -36,6 +36,7 @@ LIB = librte_eal.a
>  VPATH += $(RTE_SDK)/lib/librte_eal/common
>  
>  CFLAGS += -I$(SRCDIR)/include
> +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common
>  CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
>  CFLAGS += -I$(RTE_SDK)/lib/librte_ring
>  CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
> diff --git a/lib/librte_eal/common/include/eal_options.h b/lib/librte_eal/common/eal_options.h
> similarity index 100%
> rename from lib/librte_eal/common/include/eal_options.h
> rename to lib/librte_eal/common/eal_options.h
> diff --git a/lib/librte_eal/common/include/eal_private.h b/lib/librte_eal/common/eal_private.h
> similarity index 100%
> rename from lib/librte_eal/common/include/eal_private.h
> rename to lib/librte_eal/common/eal_private.h
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index c99433e..2480cb0 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -36,6 +36,7 @@ LIB = librte_eal.a
>  VPATH += $(RTE_SDK)/lib/librte_eal/common
>  
>  CFLAGS += -I$(SRCDIR)/include
> +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common
>  CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
>  CFLAGS += -I$(RTE_SDK)/lib/librte_ring
>  CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
> diff --git a/lib/librte_eal/linuxapp/eal/include/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
> similarity index 100%
> rename from lib/librte_eal/linuxapp/eal/include/eal_pci_init.h
> rename to lib/librte_eal/linuxapp/eal/eal_pci_init.h
> diff --git a/lib/librte_eal/linuxapp/eal/include/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
> similarity index 100%
> rename from lib/librte_eal/linuxapp/eal/include/eal_vfio.h
> rename to lib/librte_eal/linuxapp/eal/eal_vfio.h
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 02/10] eal: factorize common headers

[Thomas Monjalon]

No need to have different headers for Linux and BSD.
These files are identicals with exception of internal config which has
uio and vfio fields only useful for Linux.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test/test_eal_fs.c                             |   2 +-
 lib/librte_eal/bsdapp/eal/Makefile                 |   2 +-
 lib/librte_eal/bsdapp/eal/include/eal_filesystem.h | 118 ---------------------
 lib/librte_eal/bsdapp/eal/include/eal_hugepages.h  |  67 ------------
 .../bsdapp/eal/include/eal_internal_cfg.h          |  87 ---------------
 lib/librte_eal/bsdapp/eal/include/eal_thread.h     |  53 ---------
 .../bsdapp/eal/include/exec-env/rte_lcore.h        |  67 ------------
 .../bsdapp/eal/include/exec-env/rte_per_lcore.h    |  67 ------------
 .../eal/include => common}/eal_filesystem.h        |   0
 .../eal/include => common}/eal_hugepages.h         |   0
 .../eal/include => common}/eal_internal_cfg.h      |   0
 .../{linuxapp/eal/include => common}/eal_thread.h  |   0
 lib/librte_eal/common/include/rte_lcore.h          |  26 ++++-
 lib/librte_eal/common/include/rte_per_lcore.h      |  12 +--
 lib/librte_eal/linuxapp/eal/Makefile               |   2 +-
 .../linuxapp/eal/include/exec-env/rte_lcore.h      |  67 ------------
 .../linuxapp/eal/include/exec-env/rte_per_lcore.h  |  67 ------------
 17 files changed, 31 insertions(+), 606 deletions(-)
 delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_filesystem.h
 delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_hugepages.h
 delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
 delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_thread.h
 delete mode 100644 lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h
 delete mode 100644 lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h
 rename lib/librte_eal/{linuxapp/eal/include => common}/eal_filesystem.h (100%)
 rename lib/librte_eal/{linuxapp/eal/include => common}/eal_hugepages.h (100%)
 rename lib/librte_eal/{linuxapp/eal/include => common}/eal_internal_cfg.h (100%)
 rename lib/librte_eal/{linuxapp/eal/include => common}/eal_thread.h (100%)
 delete mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h
 delete mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h

diff --git a/app/test/test_eal_fs.c b/app/test/test_eal_fs.c
index cd41b3e..1cbcb9d 100644
--- a/app/test/test_eal_fs.c
+++ b/app/test/test_eal_fs.c
@@ -38,7 +38,7 @@
 #include <errno.h>
 
 /* eal_filesystem.h is not a public header file, so use relative path */
-#include "../../lib/librte_eal/linuxapp/eal/include/eal_filesystem.h"
+#include "../../lib/librte_eal/common/eal_filesystem.h"
 
 static int
 test_parse_sysfs_value(void)
diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index 4683fc3..d434882 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -86,7 +86,7 @@ CFLAGS_eal_thread.o += -Wno-return-type
 CFLAGS_eal_hpet.o += -Wno-return-type
 endif
 
-INC := rte_per_lcore.h rte_lcore.h rte_interrupts.h
+INC := rte_interrupts.h
 
 SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP)-include/exec-env := \
 	$(addprefix include/exec-env/,$(INC))
diff --git a/lib/librte_eal/bsdapp/eal/include/eal_filesystem.h b/lib/librte_eal/bsdapp/eal/include/eal_filesystem.h
deleted file mode 100644
index ce442c9..0000000
--- a/lib/librte_eal/bsdapp/eal/include/eal_filesystem.h
+++ /dev/null
@@ -1,118 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/**
- * @file
- * Stores functions and path defines for files and directories
- * on the filesystem for Linux, that are used by the Linux EAL.
- */
-
-#ifndef _EAL_LINUXAPP_FILESYSTEM_H
-#define _EAL_LINUXAPP_FILESYSTEM_H
-
-/** Path of rte config file. */
-#define RUNTIME_CONFIG_FMT "%s/.%s_config"
-
-#include <stdint.h>
-#include <limits.h>
-#include <unistd.h>
-#include <stdlib.h>
-
-#include <rte_string_fns.h>
-#include "eal_internal_cfg.h"
-
-static const char *default_config_dir = "/var/run";
-
-static inline const char *
-eal_runtime_config_path(void)
-{
-	static char buffer[PATH_MAX]; /* static so auto-zeroed */
-	const char *directory = default_config_dir;
-	const char *home_dir = getenv("HOME");
-
-	if (getuid() != 0 && home_dir != NULL)
-		directory = home_dir;
-	snprintf(buffer, sizeof(buffer) - 1, RUNTIME_CONFIG_FMT, directory,
-			internal_config.hugefile_prefix);
-	return buffer;
-}
-
-/** Path of hugepage info file. */
-#define HUGEPAGE_INFO_FMT "%s/.%s_hugepage_info"
-
-static inline const char *
-eal_hugepage_info_path(void)
-{
-	static char buffer[PATH_MAX]; /* static so auto-zeroed */
-	const char *directory = default_config_dir;
-	const char *home_dir = getenv("HOME");
-
-	if (getuid() != 0 && home_dir != NULL)
-		directory = home_dir;
-	snprintf(buffer, sizeof(buffer) - 1, HUGEPAGE_INFO_FMT, directory,
-			internal_config.hugefile_prefix);
-	return buffer;
-}
-
-/** String format for hugepage map files. */
-#define HUGEFILE_FMT "%s/%smap_%d"
-#define TEMP_HUGEFILE_FMT "%s/%smap_temp_%d"
-
-static inline const char *
-eal_get_hugefile_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
-{
-	snprintf(buffer, buflen, HUGEFILE_FMT, hugedir,
-			internal_config.hugefile_prefix, f_id);
-	buffer[buflen - 1] = '\0';
-	return buffer;
-}
-
-#ifdef RTE_EAL_SINGLE_FILE_SEGMENTS
-static inline const char *
-eal_get_hugefile_temp_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
-{
-	snprintf(buffer, buflen, TEMP_HUGEFILE_FMT, hugedir,
-			internal_config.hugefile_prefix, f_id);
-	buffer[buflen - 1] = '\0';
-	return buffer;
-}
-#endif
-
-/** define the default filename prefix for the %s values above */
-#define HUGEFILE_PREFIX_DEFAULT "rte"
-
-/** Function to read a single numeric value from a file on the filesystem.
- * Used to read information from files on /sys */
-int eal_parse_sysfs_value(const char *filename, unsigned long *val);
-
-#endif /* _EAL_LINUXAPP_FILESYSTEM_H */
diff --git a/lib/librte_eal/bsdapp/eal/include/eal_hugepages.h b/lib/librte_eal/bsdapp/eal/include/eal_hugepages.h
deleted file mode 100644
index 51e090b..0000000
--- a/lib/librte_eal/bsdapp/eal/include/eal_hugepages.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef RTE_LINUXAPP_HUGEPAGES_H_
-#define RTE_LINUXAPP_HUGEPAGES_H_
-
-#include <stddef.h>
-#include <stdint.h>
-#include <limits.h>
-
-#define MAX_HUGEPAGE_PATH PATH_MAX
-
-/**
- * Structure used to store informations about hugepages that we mapped
- * through the files in hugetlbfs.
- */
-struct hugepage_file {
-	void *orig_va;      /**< virtual addr of first mmap() */
-	void *final_va;     /**< virtual addr of 2nd mmap() */
-	uint64_t physaddr;  /**< physical addr */
-	size_t size;        /**< the page size */
-	int socket_id;      /**< NUMA socket ID */
-	int file_id;        /**< the '%d' in HUGEFILE_FMT */
-	int memseg_id;      /**< the memory segment to which page belongs */
-#ifdef RTE_EAL_SINGLE_FILE_SEGMENTS
-	int repeated;		/**< number of times the page size is repeated */
-#endif
-	char filepath[MAX_HUGEPAGE_PATH]; /**< path to backing file on filesystem */
-};
-
-/**
- * Read the information from linux on what hugepages are available
- * for the EAL to use
- */
-int eal_hugepage_info_init(void);
-
-#endif /* EAL_HUGEPAGES_H_ */
diff --git a/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h b/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
deleted file mode 100644
index 24cefc2..0000000
--- a/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
+++ /dev/null
@@ -1,87 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/**
- * @file
- * Holds the structures for the eal internal configuration
- */
-
-#ifndef _EAL_LINUXAPP_INTERNAL_CFG
-#define _EAL_LINUXAPP_INTERNAL_CFG
-
-#include <rte_eal.h>
-
-#define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
-
-/*
- * internal configuration structure for the number, size and
- * mount points of hugepages
- */
-struct hugepage_info {
-	size_t hugepage_sz;   /**< size of a huge page */
-	const char *hugedir;    /**< dir where hugetlbfs is mounted */
-	uint32_t num_pages[RTE_MAX_NUMA_NODES];
-				/**< number of hugepages of that size on each socket */
-	int lock_descriptor;    /**< file descriptor for hugepage dir */
-};
-
-/**
- * internal configuration
- */
-struct internal_config {
-	volatile size_t memory;           /**< amount of asked memory */
-	volatile unsigned force_nchannel; /**< force number of channels */
-	volatile unsigned force_nrank;    /**< force number of ranks */
-	volatile unsigned no_hugetlbfs;   /**< true to disable hugetlbfs */
-	volatile unsigned xen_dom0_support; /**< support app running on Xen Dom0*/
-	volatile unsigned no_pci;         /**< true to disable PCI */
-	volatile unsigned no_hpet;        /**< true to disable HPET */
-	volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
-										* instead of native TSC */
-	volatile unsigned no_shconf;      /**< true if there is no shared config */
-	volatile enum rte_proc_type_t process_type; /* multi-process proc type */
-	/* true to try allocating memory on specific sockets */
-	volatile unsigned force_sockets;
-	volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
-	uintptr_t base_virtaddr;          /**< base address to try and reserve memory from */
-	volatile int syslog_facility;	  /**< facility passed to openlog() */
-	volatile uint32_t log_level;	  /**< default log level */
-	const char *hugefile_prefix;      /**< the base filename of hugetlbfs files */
-	const char *hugepage_dir;         /**< specific hugetlbfs directory to use */
-
-	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
-	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
-};
-extern struct internal_config internal_config; /**< Global EAL configuration. */
-
-#endif
diff --git a/lib/librte_eal/bsdapp/eal/include/eal_thread.h b/lib/librte_eal/bsdapp/eal/include/eal_thread.h
deleted file mode 100644
index d029ad3..0000000
--- a/lib/librte_eal/bsdapp/eal/include/eal_thread.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _EAL_LINUXAPP_THREAD_H_
-#define _EAL_LINUXAPP_THREAD_H_
-
-/**
- * basic loop of thread, called for each thread by eal_init().
- *
- * @param arg
- *   opaque pointer
- */
-__attribute__((noreturn)) void *eal_thread_loop(void *arg);
-
-/**
- * Init per-lcore info for master thread
- *
- * @param lcore_id
- *   identifier of master lcore
- */
-void eal_thread_init_master(unsigned lcore_id);
-
-#endif /* _EAL_LINUXAPP_PRIVATE_H_ */
diff --git a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h b/lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h
deleted file mode 100644
index e19ab54..0000000
--- a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _RTE_LCORE_H_
-#error "don't include this file directly, please include generic <rte_lcore.h>"
-#endif
-
-#ifndef _RTE_LINUXAPP_LCORE_H_
-#define _RTE_LINUXAPP_LCORE_H_
-
-/**
- * @file
- * API for lcore and socket manipulation in linuxapp environment
- */
-
-/**
- * structure storing internal configuration (per-lcore)
- */
-struct lcore_config {
-	unsigned detected;         /**< true if lcore was detected */
-	pthread_t thread_id;       /**< pthread identifier */
-	int pipe_master2slave[2];  /**< communication pipe with master */
-	int pipe_slave2master[2];  /**< communication pipe with master */
-	lcore_function_t * volatile f;         /**< function to call */
-	void * volatile arg;       /**< argument of function */
-	volatile int ret;          /**< return value of function */
-	volatile enum rte_lcore_state_t state; /**< lcore state */
-	unsigned socket_id;        /**< physical socket id for this lcore */
-	unsigned core_id;          /**< core number on socket for this lcore */
-};
-
-/**
- * internal configuration (per-lcore)
- */
-extern struct lcore_config lcore_config[RTE_MAX_LCORE];
-
-#endif /* _RTE_LINUXAPP_LCORE_H_ */
diff --git a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h b/lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h
deleted file mode 100644
index db8f274..0000000
--- a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _RTE_PER_LCORE_H_
-#error "don't include this file directly, please include generic <rte_per_lcore.h>"
-#endif
-
-#ifndef _RTE_LINUXAPP_PER_LCORE_H_
-#define _RTE_LINUXAPP_PER_LCORE_H_
-
-/**
- * @file
- * Per-lcore variables in RTE on linuxapp environment
- */
-
-#include <pthread.h>
-
-/**
- * Macro to define a per lcore variable "var" of type "type", don't
- * use keywords like "static" or "volatile" in type, just prefix the
- * whole macro.
- */
-#define RTE_DEFINE_PER_LCORE(type, name)			\
-	__thread __typeof__(type) per_lcore_##name
-
-/**
- * Macro to declare an extern per lcore variable "var" of type "type"
- */
-#define RTE_DECLARE_PER_LCORE(type, name)			\
-	extern __thread __typeof__(type) per_lcore_##name
-
-/**
- * Read/write the per-lcore variable value
- */
-#define RTE_PER_LCORE(name) (per_lcore_##name)
-
-#endif /* _RTE_LINUXAPP_PER_LCORE_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h
similarity index 100%
rename from lib/librte_eal/linuxapp/eal/include/eal_filesystem.h
rename to lib/librte_eal/common/eal_filesystem.h
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_hugepages.h b/lib/librte_eal/common/eal_hugepages.h
similarity index 100%
rename from lib/librte_eal/linuxapp/eal/include/eal_hugepages.h
rename to lib/librte_eal/common/eal_hugepages.h
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
similarity index 100%
rename from lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h
rename to lib/librte_eal/common/eal_internal_cfg.h
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_thread.h b/lib/librte_eal/common/eal_thread.h
similarity index 100%
rename from lib/librte_eal/linuxapp/eal/include/eal_thread.h
rename to lib/librte_eal/common/eal_thread.h
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 3802a28..a0b4356 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -37,8 +37,7 @@
 /**
  * @file
  *
- * API for lcore and Socket Manipulation. Parts of this are execution
- * environment specific.
+ * API for lcore and socket manipulation
  *
  */
 #include <rte_per_lcore.h>
@@ -51,6 +50,27 @@ extern "C" {
 
 #define LCORE_ID_ANY -1    /**< Any lcore. */
 
+/**
+ * Structure storing internal configuration (per-lcore)
+ */
+struct lcore_config {
+	unsigned detected;         /**< true if lcore was detected */
+	pthread_t thread_id;       /**< pthread identifier */
+	int pipe_master2slave[2];  /**< communication pipe with master */
+	int pipe_slave2master[2];  /**< communication pipe with master */
+	lcore_function_t * volatile f;         /**< function to call */
+	void * volatile arg;       /**< argument of function */
+	volatile int ret;          /**< return value of function */
+	volatile enum rte_lcore_state_t state; /**< lcore state */
+	unsigned socket_id;        /**< physical socket id for this lcore */
+	unsigned core_id;          /**< core number on socket for this lcore */
+};
+
+/**
+ * Internal configuration (per-lcore)
+ */
+extern struct lcore_config lcore_config[RTE_MAX_LCORE];
+
 RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per core "core id". */
 
 /**
@@ -89,8 +109,6 @@ rte_lcore_count(void)
 	return cfg->lcore_count;
 }
 
-#include <exec-env/rte_lcore.h>
-
 /**
  * Return the ID of the physical socket of the logical core we are
  * running on.
diff --git a/lib/librte_eal/common/include/rte_per_lcore.h b/lib/librte_eal/common/include/rte_per_lcore.h
index 14d3521..5434729 100644
--- a/lib/librte_eal/common/include/rte_per_lcore.h
+++ b/lib/librte_eal/common/include/rte_per_lcore.h
@@ -51,26 +51,26 @@
 extern "C" {
 #endif
 
-#include <exec-env/rte_per_lcore.h>
+#include <pthread.h>
 
-#ifdef __DOXYGEN__
 /**
  * Macro to define a per lcore variable "var" of type "type", don't
  * use keywords like "static" or "volatile" in type, just prefix the
  * whole macro.
  */
-#define RTE_DEFINE_PER_LCORE(type, name)
+#define RTE_DEFINE_PER_LCORE(type, name)			\
+	__thread __typeof__(type) per_lcore_##name
 
 /**
  * Macro to declare an extern per lcore variable "var" of type "type"
  */
-#define RTE_DECLARE_PER_LCORE(type, name)
+#define RTE_DECLARE_PER_LCORE(type, name)			\
+	extern __thread __typeof__(type) per_lcore_##name
 
 /**
  * Read/write the per-lcore variable value
  */
-#define RTE_PER_LCORE(name)
-#endif
+#define RTE_PER_LCORE(name) (per_lcore_##name)
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 2480cb0..702273f 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -100,7 +100,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 CFLAGS_eal_thread.o += -Wno-return-type
 endif
 
-INC := rte_per_lcore.h rte_lcore.h rte_interrupts.h rte_kni_common.h rte_dom0_common.h
+INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h
 
 SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP)-include/exec-env := \
 	$(addprefix include/exec-env/,$(INC))
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h
deleted file mode 100644
index e19ab54..0000000
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _RTE_LCORE_H_
-#error "don't include this file directly, please include generic <rte_lcore.h>"
-#endif
-
-#ifndef _RTE_LINUXAPP_LCORE_H_
-#define _RTE_LINUXAPP_LCORE_H_
-
-/**
- * @file
- * API for lcore and socket manipulation in linuxapp environment
- */
-
-/**
- * structure storing internal configuration (per-lcore)
- */
-struct lcore_config {
-	unsigned detected;         /**< true if lcore was detected */
-	pthread_t thread_id;       /**< pthread identifier */
-	int pipe_master2slave[2];  /**< communication pipe with master */
-	int pipe_slave2master[2];  /**< communication pipe with master */
-	lcore_function_t * volatile f;         /**< function to call */
-	void * volatile arg;       /**< argument of function */
-	volatile int ret;          /**< return value of function */
-	volatile enum rte_lcore_state_t state; /**< lcore state */
-	unsigned socket_id;        /**< physical socket id for this lcore */
-	unsigned core_id;          /**< core number on socket for this lcore */
-};
-
-/**
- * internal configuration (per-lcore)
- */
-extern struct lcore_config lcore_config[RTE_MAX_LCORE];
-
-#endif /* _RTE_LINUXAPP_LCORE_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h
deleted file mode 100644
index db8f274..0000000
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _RTE_PER_LCORE_H_
-#error "don't include this file directly, please include generic <rte_per_lcore.h>"
-#endif
-
-#ifndef _RTE_LINUXAPP_PER_LCORE_H_
-#define _RTE_LINUXAPP_PER_LCORE_H_
-
-/**
- * @file
- * Per-lcore variables in RTE on linuxapp environment
- */
-
-#include <pthread.h>
-
-/**
- * Macro to define a per lcore variable "var" of type "type", don't
- * use keywords like "static" or "volatile" in type, just prefix the
- * whole macro.
- */
-#define RTE_DEFINE_PER_LCORE(type, name)			\
-	__thread __typeof__(type) per_lcore_##name
-
-/**
- * Macro to declare an extern per lcore variable "var" of type "type"
- */
-#define RTE_DECLARE_PER_LCORE(type, name)			\
-	extern __thread __typeof__(type) per_lcore_##name
-
-/**
- * Read/write the per-lcore variable value
- */
-#define RTE_PER_LCORE(name) (per_lcore_##name)
-
-#endif /* _RTE_LINUXAPP_PER_LCORE_H_ */
-- 
2.1.3

Re: [dpdk-dev] [PATCH 02/10] eal: factorize common headers

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:34PM +0100, Thomas Monjalon wrote:
> No need to have different headers for Linux and BSD.
> These files are identicals with exception of internal config which has
> uio and vfio fields only useful for Linux.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  app/test/test_eal_fs.c                             |   2 +-
>  lib/librte_eal/bsdapp/eal/Makefile                 |   2 +-
>  lib/librte_eal/bsdapp/eal/include/eal_filesystem.h | 118 ---------------------
>  lib/librte_eal/bsdapp/eal/include/eal_hugepages.h  |  67 ------------
>  .../bsdapp/eal/include/eal_internal_cfg.h          |  87 ---------------
>  lib/librte_eal/bsdapp/eal/include/eal_thread.h     |  53 ---------
>  .../bsdapp/eal/include/exec-env/rte_lcore.h        |  67 ------------
>  .../bsdapp/eal/include/exec-env/rte_per_lcore.h    |  67 ------------
>  .../eal/include => common}/eal_filesystem.h        |   0
>  .../eal/include => common}/eal_hugepages.h         |   0
>  .../eal/include => common}/eal_internal_cfg.h      |   0
>  .../{linuxapp/eal/include => common}/eal_thread.h  |   0
>  lib/librte_eal/common/include/rte_lcore.h          |  26 ++++-
>  lib/librte_eal/common/include/rte_per_lcore.h      |  12 +--
>  lib/librte_eal/linuxapp/eal/Makefile               |   2 +-
>  .../linuxapp/eal/include/exec-env/rte_lcore.h      |  67 ------------
>  .../linuxapp/eal/include/exec-env/rte_per_lcore.h  |  67 ------------
>  17 files changed, 31 insertions(+), 606 deletions(-)
>  delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_filesystem.h
>  delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_hugepages.h
>  delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
>  delete mode 100644 lib/librte_eal/bsdapp/eal/include/eal_thread.h
>  delete mode 100644 lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h
>  delete mode 100644 lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h
>  rename lib/librte_eal/{linuxapp/eal/include => common}/eal_filesystem.h (100%)
>  rename lib/librte_eal/{linuxapp/eal/include => common}/eal_hugepages.h (100%)
>  rename lib/librte_eal/{linuxapp/eal/include => common}/eal_internal_cfg.h (100%)
>  rename lib/librte_eal/{linuxapp/eal/include => common}/eal_thread.h (100%)
>  delete mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h
>  delete mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h
> 
> diff --git a/app/test/test_eal_fs.c b/app/test/test_eal_fs.c
> index cd41b3e..1cbcb9d 100644
> --- a/app/test/test_eal_fs.c
> +++ b/app/test/test_eal_fs.c
> @@ -38,7 +38,7 @@
>  #include <errno.h>
>  
>  /* eal_filesystem.h is not a public header file, so use relative path */
> -#include "../../lib/librte_eal/linuxapp/eal/include/eal_filesystem.h"
> +#include "../../lib/librte_eal/common/eal_filesystem.h"
>  
>  static int
>  test_parse_sysfs_value(void)
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index 4683fc3..d434882 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -86,7 +86,7 @@ CFLAGS_eal_thread.o += -Wno-return-type
>  CFLAGS_eal_hpet.o += -Wno-return-type
>  endif
>  
> -INC := rte_per_lcore.h rte_lcore.h rte_interrupts.h
> +INC := rte_interrupts.h
>  
>  SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP)-include/exec-env := \
>  	$(addprefix include/exec-env/,$(INC))
> diff --git a/lib/librte_eal/bsdapp/eal/include/eal_filesystem.h b/lib/librte_eal/bsdapp/eal/include/eal_filesystem.h
> deleted file mode 100644
> index ce442c9..0000000
> --- a/lib/librte_eal/bsdapp/eal/include/eal_filesystem.h
> +++ /dev/null
> @@ -1,118 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -/**
> - * @file
> - * Stores functions and path defines for files and directories
> - * on the filesystem for Linux, that are used by the Linux EAL.
> - */
> -
> -#ifndef _EAL_LINUXAPP_FILESYSTEM_H
> -#define _EAL_LINUXAPP_FILESYSTEM_H
> -
> -/** Path of rte config file. */
> -#define RUNTIME_CONFIG_FMT "%s/.%s_config"
> -
> -#include <stdint.h>
> -#include <limits.h>
> -#include <unistd.h>
> -#include <stdlib.h>
> -
> -#include <rte_string_fns.h>
> -#include "eal_internal_cfg.h"
> -
> -static const char *default_config_dir = "/var/run";
> -
> -static inline const char *
> -eal_runtime_config_path(void)
> -{
> -	static char buffer[PATH_MAX]; /* static so auto-zeroed */
> -	const char *directory = default_config_dir;
> -	const char *home_dir = getenv("HOME");
> -
> -	if (getuid() != 0 && home_dir != NULL)
> -		directory = home_dir;
> -	snprintf(buffer, sizeof(buffer) - 1, RUNTIME_CONFIG_FMT, directory,
> -			internal_config.hugefile_prefix);
> -	return buffer;
> -}
> -
> -/** Path of hugepage info file. */
> -#define HUGEPAGE_INFO_FMT "%s/.%s_hugepage_info"
> -
> -static inline const char *
> -eal_hugepage_info_path(void)
> -{
> -	static char buffer[PATH_MAX]; /* static so auto-zeroed */
> -	const char *directory = default_config_dir;
> -	const char *home_dir = getenv("HOME");
> -
> -	if (getuid() != 0 && home_dir != NULL)
> -		directory = home_dir;
> -	snprintf(buffer, sizeof(buffer) - 1, HUGEPAGE_INFO_FMT, directory,
> -			internal_config.hugefile_prefix);
> -	return buffer;
> -}
> -
> -/** String format for hugepage map files. */
> -#define HUGEFILE_FMT "%s/%smap_%d"
> -#define TEMP_HUGEFILE_FMT "%s/%smap_temp_%d"
> -
> -static inline const char *
> -eal_get_hugefile_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
> -{
> -	snprintf(buffer, buflen, HUGEFILE_FMT, hugedir,
> -			internal_config.hugefile_prefix, f_id);
> -	buffer[buflen - 1] = '\0';
> -	return buffer;
> -}
> -
> -#ifdef RTE_EAL_SINGLE_FILE_SEGMENTS
> -static inline const char *
> -eal_get_hugefile_temp_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
> -{
> -	snprintf(buffer, buflen, TEMP_HUGEFILE_FMT, hugedir,
> -			internal_config.hugefile_prefix, f_id);
> -	buffer[buflen - 1] = '\0';
> -	return buffer;
> -}
> -#endif
> -
> -/** define the default filename prefix for the %s values above */
> -#define HUGEFILE_PREFIX_DEFAULT "rte"
> -
> -/** Function to read a single numeric value from a file on the filesystem.
> - * Used to read information from files on /sys */
> -int eal_parse_sysfs_value(const char *filename, unsigned long *val);
> -
> -#endif /* _EAL_LINUXAPP_FILESYSTEM_H */
> diff --git a/lib/librte_eal/bsdapp/eal/include/eal_hugepages.h b/lib/librte_eal/bsdapp/eal/include/eal_hugepages.h
> deleted file mode 100644
> index 51e090b..0000000
> --- a/lib/librte_eal/bsdapp/eal/include/eal_hugepages.h
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#ifndef RTE_LINUXAPP_HUGEPAGES_H_
> -#define RTE_LINUXAPP_HUGEPAGES_H_
> -
> -#include <stddef.h>
> -#include <stdint.h>
> -#include <limits.h>
> -
> -#define MAX_HUGEPAGE_PATH PATH_MAX
> -
> -/**
> - * Structure used to store informations about hugepages that we mapped
> - * through the files in hugetlbfs.
> - */
> -struct hugepage_file {
> -	void *orig_va;      /**< virtual addr of first mmap() */
> -	void *final_va;     /**< virtual addr of 2nd mmap() */
> -	uint64_t physaddr;  /**< physical addr */
> -	size_t size;        /**< the page size */
> -	int socket_id;      /**< NUMA socket ID */
> -	int file_id;        /**< the '%d' in HUGEFILE_FMT */
> -	int memseg_id;      /**< the memory segment to which page belongs */
> -#ifdef RTE_EAL_SINGLE_FILE_SEGMENTS
> -	int repeated;		/**< number of times the page size is repeated */
> -#endif
> -	char filepath[MAX_HUGEPAGE_PATH]; /**< path to backing file on filesystem */
> -};
> -
> -/**
> - * Read the information from linux on what hugepages are available
> - * for the EAL to use
> - */
> -int eal_hugepage_info_init(void);
> -
> -#endif /* EAL_HUGEPAGES_H_ */
> diff --git a/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h b/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
> deleted file mode 100644
> index 24cefc2..0000000
> --- a/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
> +++ /dev/null
> @@ -1,87 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -/**
> - * @file
> - * Holds the structures for the eal internal configuration
> - */
> -
> -#ifndef _EAL_LINUXAPP_INTERNAL_CFG
> -#define _EAL_LINUXAPP_INTERNAL_CFG
> -
> -#include <rte_eal.h>
> -
> -#define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
> -
> -/*
> - * internal configuration structure for the number, size and
> - * mount points of hugepages
> - */
> -struct hugepage_info {
> -	size_t hugepage_sz;   /**< size of a huge page */
> -	const char *hugedir;    /**< dir where hugetlbfs is mounted */
> -	uint32_t num_pages[RTE_MAX_NUMA_NODES];
> -				/**< number of hugepages of that size on each socket */
> -	int lock_descriptor;    /**< file descriptor for hugepage dir */
> -};
> -
> -/**
> - * internal configuration
> - */
> -struct internal_config {
> -	volatile size_t memory;           /**< amount of asked memory */
> -	volatile unsigned force_nchannel; /**< force number of channels */
> -	volatile unsigned force_nrank;    /**< force number of ranks */
> -	volatile unsigned no_hugetlbfs;   /**< true to disable hugetlbfs */
> -	volatile unsigned xen_dom0_support; /**< support app running on Xen Dom0*/
> -	volatile unsigned no_pci;         /**< true to disable PCI */
> -	volatile unsigned no_hpet;        /**< true to disable HPET */
> -	volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
> -										* instead of native TSC */
> -	volatile unsigned no_shconf;      /**< true if there is no shared config */
> -	volatile enum rte_proc_type_t process_type; /* multi-process proc type */
> -	/* true to try allocating memory on specific sockets */
> -	volatile unsigned force_sockets;
> -	volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
> -	uintptr_t base_virtaddr;          /**< base address to try and reserve memory from */
> -	volatile int syslog_facility;	  /**< facility passed to openlog() */
> -	volatile uint32_t log_level;	  /**< default log level */
> -	const char *hugefile_prefix;      /**< the base filename of hugetlbfs files */
> -	const char *hugepage_dir;         /**< specific hugetlbfs directory to use */
> -
> -	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
> -	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
> -};
> -extern struct internal_config internal_config; /**< Global EAL configuration. */
> -
> -#endif
> diff --git a/lib/librte_eal/bsdapp/eal/include/eal_thread.h b/lib/librte_eal/bsdapp/eal/include/eal_thread.h
> deleted file mode 100644
> index d029ad3..0000000
> --- a/lib/librte_eal/bsdapp/eal/include/eal_thread.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#ifndef _EAL_LINUXAPP_THREAD_H_
> -#define _EAL_LINUXAPP_THREAD_H_
> -
> -/**
> - * basic loop of thread, called for each thread by eal_init().
> - *
> - * @param arg
> - *   opaque pointer
> - */
> -__attribute__((noreturn)) void *eal_thread_loop(void *arg);
> -
> -/**
> - * Init per-lcore info for master thread
> - *
> - * @param lcore_id
> - *   identifier of master lcore
> - */
> -void eal_thread_init_master(unsigned lcore_id);
> -
> -#endif /* _EAL_LINUXAPP_PRIVATE_H_ */
> diff --git a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h b/lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h
> deleted file mode 100644
> index e19ab54..0000000
> --- a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_lcore.h
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#ifndef _RTE_LCORE_H_
> -#error "don't include this file directly, please include generic <rte_lcore.h>"
> -#endif
> -
> -#ifndef _RTE_LINUXAPP_LCORE_H_
> -#define _RTE_LINUXAPP_LCORE_H_
> -
> -/**
> - * @file
> - * API for lcore and socket manipulation in linuxapp environment
> - */
> -
> -/**
> - * structure storing internal configuration (per-lcore)
> - */
> -struct lcore_config {
> -	unsigned detected;         /**< true if lcore was detected */
> -	pthread_t thread_id;       /**< pthread identifier */
> -	int pipe_master2slave[2];  /**< communication pipe with master */
> -	int pipe_slave2master[2];  /**< communication pipe with master */
> -	lcore_function_t * volatile f;         /**< function to call */
> -	void * volatile arg;       /**< argument of function */
> -	volatile int ret;          /**< return value of function */
> -	volatile enum rte_lcore_state_t state; /**< lcore state */
> -	unsigned socket_id;        /**< physical socket id for this lcore */
> -	unsigned core_id;          /**< core number on socket for this lcore */
> -};
> -
> -/**
> - * internal configuration (per-lcore)
> - */
> -extern struct lcore_config lcore_config[RTE_MAX_LCORE];
> -
> -#endif /* _RTE_LINUXAPP_LCORE_H_ */
> diff --git a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h b/lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h
> deleted file mode 100644
> index db8f274..0000000
> --- a/lib/librte_eal/bsdapp/eal/include/exec-env/rte_per_lcore.h
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#ifndef _RTE_PER_LCORE_H_
> -#error "don't include this file directly, please include generic <rte_per_lcore.h>"
> -#endif
> -
> -#ifndef _RTE_LINUXAPP_PER_LCORE_H_
> -#define _RTE_LINUXAPP_PER_LCORE_H_
> -
> -/**
> - * @file
> - * Per-lcore variables in RTE on linuxapp environment
> - */
> -
> -#include <pthread.h>
> -
> -/**
> - * Macro to define a per lcore variable "var" of type "type", don't
> - * use keywords like "static" or "volatile" in type, just prefix the
> - * whole macro.
> - */
> -#define RTE_DEFINE_PER_LCORE(type, name)			\
> -	__thread __typeof__(type) per_lcore_##name
> -
> -/**
> - * Macro to declare an extern per lcore variable "var" of type "type"
> - */
> -#define RTE_DECLARE_PER_LCORE(type, name)			\
> -	extern __thread __typeof__(type) per_lcore_##name
> -
> -/**
> - * Read/write the per-lcore variable value
> - */
> -#define RTE_PER_LCORE(name) (per_lcore_##name)
> -
> -#endif /* _RTE_LINUXAPP_PER_LCORE_H_ */
> diff --git a/lib/librte_eal/linuxapp/eal/include/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h
> similarity index 100%
> rename from lib/librte_eal/linuxapp/eal/include/eal_filesystem.h
> rename to lib/librte_eal/common/eal_filesystem.h
> diff --git a/lib/librte_eal/linuxapp/eal/include/eal_hugepages.h b/lib/librte_eal/common/eal_hugepages.h
> similarity index 100%
> rename from lib/librte_eal/linuxapp/eal/include/eal_hugepages.h
> rename to lib/librte_eal/common/eal_hugepages.h
> diff --git a/lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> similarity index 100%
> rename from lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h
> rename to lib/librte_eal/common/eal_internal_cfg.h
> diff --git a/lib/librte_eal/linuxapp/eal/include/eal_thread.h b/lib/librte_eal/common/eal_thread.h
> similarity index 100%
> rename from lib/librte_eal/linuxapp/eal/include/eal_thread.h
> rename to lib/librte_eal/common/eal_thread.h
> diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
> index 3802a28..a0b4356 100644
> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -37,8 +37,7 @@
>  /**
>   * @file
>   *
> - * API for lcore and Socket Manipulation. Parts of this are execution
> - * environment specific.
> + * API for lcore and socket manipulation
>   *
>   */
>  #include <rte_per_lcore.h>
> @@ -51,6 +50,27 @@ extern "C" {
>  
>  #define LCORE_ID_ANY -1    /**< Any lcore. */
>  
> +/**
> + * Structure storing internal configuration (per-lcore)
> + */
> +struct lcore_config {
> +	unsigned detected;         /**< true if lcore was detected */
> +	pthread_t thread_id;       /**< pthread identifier */
> +	int pipe_master2slave[2];  /**< communication pipe with master */
> +	int pipe_slave2master[2];  /**< communication pipe with master */
> +	lcore_function_t * volatile f;         /**< function to call */
> +	void * volatile arg;       /**< argument of function */
> +	volatile int ret;          /**< return value of function */
> +	volatile enum rte_lcore_state_t state; /**< lcore state */
> +	unsigned socket_id;        /**< physical socket id for this lcore */
> +	unsigned core_id;          /**< core number on socket for this lcore */
> +};
> +
> +/**
> + * Internal configuration (per-lcore)
> + */
> +extern struct lcore_config lcore_config[RTE_MAX_LCORE];
> +
>  RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per core "core id". */
>  
>  /**
> @@ -89,8 +109,6 @@ rte_lcore_count(void)
>  	return cfg->lcore_count;
>  }
>  
> -#include <exec-env/rte_lcore.h>
> -
>  /**
>   * Return the ID of the physical socket of the logical core we are
>   * running on.
> diff --git a/lib/librte_eal/common/include/rte_per_lcore.h b/lib/librte_eal/common/include/rte_per_lcore.h
> index 14d3521..5434729 100644
> --- a/lib/librte_eal/common/include/rte_per_lcore.h
> +++ b/lib/librte_eal/common/include/rte_per_lcore.h
> @@ -51,26 +51,26 @@
>  extern "C" {
>  #endif
>  
> -#include <exec-env/rte_per_lcore.h>
> +#include <pthread.h>
>  
> -#ifdef __DOXYGEN__
>  /**
>   * Macro to define a per lcore variable "var" of type "type", don't
>   * use keywords like "static" or "volatile" in type, just prefix the
>   * whole macro.
>   */
> -#define RTE_DEFINE_PER_LCORE(type, name)
> +#define RTE_DEFINE_PER_LCORE(type, name)			\
> +	__thread __typeof__(type) per_lcore_##name
>  
>  /**
>   * Macro to declare an extern per lcore variable "var" of type "type"
>   */
> -#define RTE_DECLARE_PER_LCORE(type, name)
> +#define RTE_DECLARE_PER_LCORE(type, name)			\
> +	extern __thread __typeof__(type) per_lcore_##name
>  
>  /**
>   * Read/write the per-lcore variable value
>   */
> -#define RTE_PER_LCORE(name)
> -#endif
> +#define RTE_PER_LCORE(name) (per_lcore_##name)
>  
>  #ifdef __cplusplus
>  }
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 2480cb0..702273f 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -100,7 +100,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
>  CFLAGS_eal_thread.o += -Wno-return-type
>  endif
>  
> -INC := rte_per_lcore.h rte_lcore.h rte_interrupts.h rte_kni_common.h rte_dom0_common.h
> +INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h
>  
>  SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP)-include/exec-env := \
>  	$(addprefix include/exec-env/,$(INC))
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h
> deleted file mode 100644
> index e19ab54..0000000
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_lcore.h
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#ifndef _RTE_LCORE_H_
> -#error "don't include this file directly, please include generic <rte_lcore.h>"
> -#endif
> -
> -#ifndef _RTE_LINUXAPP_LCORE_H_
> -#define _RTE_LINUXAPP_LCORE_H_
> -
> -/**
> - * @file
> - * API for lcore and socket manipulation in linuxapp environment
> - */
> -
> -/**
> - * structure storing internal configuration (per-lcore)
> - */
> -struct lcore_config {
> -	unsigned detected;         /**< true if lcore was detected */
> -	pthread_t thread_id;       /**< pthread identifier */
> -	int pipe_master2slave[2];  /**< communication pipe with master */
> -	int pipe_slave2master[2];  /**< communication pipe with master */
> -	lcore_function_t * volatile f;         /**< function to call */
> -	void * volatile arg;       /**< argument of function */
> -	volatile int ret;          /**< return value of function */
> -	volatile enum rte_lcore_state_t state; /**< lcore state */
> -	unsigned socket_id;        /**< physical socket id for this lcore */
> -	unsigned core_id;          /**< core number on socket for this lcore */
> -};
> -
> -/**
> - * internal configuration (per-lcore)
> - */
> -extern struct lcore_config lcore_config[RTE_MAX_LCORE];
> -
> -#endif /* _RTE_LINUXAPP_LCORE_H_ */
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h
> deleted file mode 100644
> index db8f274..0000000
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_per_lcore.h
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#ifndef _RTE_PER_LCORE_H_
> -#error "don't include this file directly, please include generic <rte_per_lcore.h>"
> -#endif
> -
> -#ifndef _RTE_LINUXAPP_PER_LCORE_H_
> -#define _RTE_LINUXAPP_PER_LCORE_H_
> -
> -/**
> - * @file
> - * Per-lcore variables in RTE on linuxapp environment
> - */
> -
> -#include <pthread.h>
> -
> -/**
> - * Macro to define a per lcore variable "var" of type "type", don't
> - * use keywords like "static" or "volatile" in type, just prefix the
> - * whole macro.
> - */
> -#define RTE_DEFINE_PER_LCORE(type, name)			\
> -	__thread __typeof__(type) per_lcore_##name
> -
> -/**
> - * Macro to declare an extern per lcore variable "var" of type "type"
> - */
> -#define RTE_DECLARE_PER_LCORE(type, name)			\
> -	extern __thread __typeof__(type) per_lcore_##name
> -
> -/**
> - * Read/write the per-lcore variable value
> - */
> -#define RTE_PER_LCORE(name) (per_lcore_##name)
> -
> -#endif /* _RTE_LINUXAPP_PER_LCORE_H_ */
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 03/10] eal: fix header guards

[Thomas Monjalon]

Some guards are missing or have a wrong name.
Others have LINUXAPP in their name but are now common.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/common/eal_filesystem.h   | 6 +++---
 lib/librte_eal/common/eal_hugepages.h    | 6 +++---
 lib/librte_eal/common/eal_internal_cfg.h | 4 ++--
 lib/librte_eal/common/eal_options.h      | 4 ++++
 lib/librte_eal/common/eal_thread.h       | 6 +++---
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/lib/librte_eal/common/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h
index ce442c9..fdb4a70 100644
--- a/lib/librte_eal/common/eal_filesystem.h
+++ b/lib/librte_eal/common/eal_filesystem.h
@@ -37,8 +37,8 @@
  * on the filesystem for Linux, that are used by the Linux EAL.
  */
 
-#ifndef _EAL_LINUXAPP_FILESYSTEM_H
-#define _EAL_LINUXAPP_FILESYSTEM_H
+#ifndef EAL_FILESYSTEM_H
+#define EAL_FILESYSTEM_H
 
 /** Path of rte config file. */
 #define RUNTIME_CONFIG_FMT "%s/.%s_config"
@@ -115,4 +115,4 @@ eal_get_hugefile_temp_path(char *buffer, size_t buflen, const char *hugedir, int
  * Used to read information from files on /sys */
 int eal_parse_sysfs_value(const char *filename, unsigned long *val);
 
-#endif /* _EAL_LINUXAPP_FILESYSTEM_H */
+#endif /* EAL_FILESYSTEM_H */
diff --git a/lib/librte_eal/common/eal_hugepages.h b/lib/librte_eal/common/eal_hugepages.h
index 51e090b..38edac0 100644
--- a/lib/librte_eal/common/eal_hugepages.h
+++ b/lib/librte_eal/common/eal_hugepages.h
@@ -31,8 +31,8 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef RTE_LINUXAPP_HUGEPAGES_H_
-#define RTE_LINUXAPP_HUGEPAGES_H_
+#ifndef EAL_HUGEPAGES_H
+#define EAL_HUGEPAGES_H
 
 #include <stddef.h>
 #include <stdint.h>
@@ -64,4 +64,4 @@ struct hugepage_file {
  */
 int eal_hugepage_info_init(void);
 
-#endif /* EAL_HUGEPAGES_H_ */
+#endif /* EAL_HUGEPAGES_H */
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 8749390..19c84e7 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -36,8 +36,8 @@
  * Holds the structures for the eal internal configuration
  */
 
-#ifndef _EAL_LINUXAPP_INTERNAL_CFG
-#define _EAL_LINUXAPP_INTERNAL_CFG
+#ifndef EAL_INTERNAL_CFG_H
+#define EAL_INTERNAL_CFG_H
 
 #include <rte_eal.h>
 #include <rte_pci_dev_feature_defs.h>
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 22819ec..7a08507 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -30,6 +30,8 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef EAL_OPTIONS_H
+#define EAL_OPTIONS_H
 
 enum {
 	/* long options mapped to a short option */
@@ -82,3 +84,5 @@ extern const struct option eal_long_options[];
 int eal_parse_common_option(int opt, const char *argv,
 			    struct internal_config *conf);
 void eal_common_usage(void);
+
+#endif /* EAL_OPTIONS_H */
diff --git a/lib/librte_eal/common/eal_thread.h b/lib/librte_eal/common/eal_thread.h
index d029ad3..b53b84d 100644
--- a/lib/librte_eal/common/eal_thread.h
+++ b/lib/librte_eal/common/eal_thread.h
@@ -31,8 +31,8 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef _EAL_LINUXAPP_THREAD_H_
-#define _EAL_LINUXAPP_THREAD_H_
+#ifndef EAL_THREAD_H
+#define EAL_THREAD_H
 
 /**
  * basic loop of thread, called for each thread by eal_init().
@@ -50,4 +50,4 @@ __attribute__((noreturn)) void *eal_thread_loop(void *arg);
  */
 void eal_thread_init_master(unsigned lcore_id);
 
-#endif /* _EAL_LINUXAPP_PRIVATE_H_ */
+#endif /* EAL_THREAD_H */
-- 
2.1.3

Re: [dpdk-dev] [PATCH 03/10] eal: fix header guards

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:35PM +0100, Thomas Monjalon wrote:
> Some guards are missing or have a wrong name.
> Others have LINUXAPP in their name but are now common.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

One minor comment below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/common/eal_filesystem.h   | 6 +++---
>  lib/librte_eal/common/eal_hugepages.h    | 6 +++---
>  lib/librte_eal/common/eal_internal_cfg.h | 4 ++--
>  lib/librte_eal/common/eal_options.h      | 4 ++++
>  lib/librte_eal/common/eal_thread.h       | 6 +++---
>  5 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h
> index ce442c9..fdb4a70 100644
> --- a/lib/librte_eal/common/eal_filesystem.h
> +++ b/lib/librte_eal/common/eal_filesystem.h
> @@ -37,8 +37,8 @@
>   * on the filesystem for Linux, that are used by the Linux EAL.
>   */
>  
> -#ifndef _EAL_LINUXAPP_FILESYSTEM_H
> -#define _EAL_LINUXAPP_FILESYSTEM_H
> +#ifndef EAL_FILESYSTEM_H
> +#define EAL_FILESYSTEM_H
>  
>  /** Path of rte config file. */
>  #define RUNTIME_CONFIG_FMT "%s/.%s_config"

no ending comment for #endif - should one be added for completeness?

> @@ -115,4 +115,4 @@ eal_get_hugefile_temp_path(char *buffer, size_t buflen, const char *hugedir, int
>   * Used to read information from files on /sys */
>  int eal_parse_sysfs_value(const char *filename, unsigned long *val);
>  
> -#endif /* _EAL_LINUXAPP_FILESYSTEM_H */
> +#endif /* EAL_FILESYSTEM_H */
> diff --git a/lib/librte_eal/common/eal_hugepages.h b/lib/librte_eal/common/eal_hugepages.h
> index 51e090b..38edac0 100644
> --- a/lib/librte_eal/common/eal_hugepages.h
> +++ b/lib/librte_eal/common/eal_hugepages.h
> @@ -31,8 +31,8 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> -#ifndef RTE_LINUXAPP_HUGEPAGES_H_
> -#define RTE_LINUXAPP_HUGEPAGES_H_
> +#ifndef EAL_HUGEPAGES_H
> +#define EAL_HUGEPAGES_H
>  
>  #include <stddef.h>
>  #include <stdint.h>
> @@ -64,4 +64,4 @@ struct hugepage_file {
>   */
>  int eal_hugepage_info_init(void);
>  
> -#endif /* EAL_HUGEPAGES_H_ */
> +#endif /* EAL_HUGEPAGES_H */
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 8749390..19c84e7 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -36,8 +36,8 @@
>   * Holds the structures for the eal internal configuration
>   */
>  
> -#ifndef _EAL_LINUXAPP_INTERNAL_CFG
> -#define _EAL_LINUXAPP_INTERNAL_CFG
> +#ifndef EAL_INTERNAL_CFG_H
> +#define EAL_INTERNAL_CFG_H
>  
>  #include <rte_eal.h>
>  #include <rte_pci_dev_feature_defs.h>
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 22819ec..7a08507 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -30,6 +30,8 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#ifndef EAL_OPTIONS_H
> +#define EAL_OPTIONS_H
>  
>  enum {
>  	/* long options mapped to a short option */
> @@ -82,3 +84,5 @@ extern const struct option eal_long_options[];
>  int eal_parse_common_option(int opt, const char *argv,
>  			    struct internal_config *conf);
>  void eal_common_usage(void);
> +
> +#endif /* EAL_OPTIONS_H */
> diff --git a/lib/librte_eal/common/eal_thread.h b/lib/librte_eal/common/eal_thread.h
> index d029ad3..b53b84d 100644
> --- a/lib/librte_eal/common/eal_thread.h
> +++ b/lib/librte_eal/common/eal_thread.h
> @@ -31,8 +31,8 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> -#ifndef _EAL_LINUXAPP_THREAD_H_
> -#define _EAL_LINUXAPP_THREAD_H_
> +#ifndef EAL_THREAD_H
> +#define EAL_THREAD_H
>  
>  /**
>   * basic loop of thread, called for each thread by eal_init().
> @@ -50,4 +50,4 @@ __attribute__((noreturn)) void *eal_thread_loop(void *arg);
>   */
>  void eal_thread_init_master(unsigned lcore_id);
>  
> -#endif /* _EAL_LINUXAPP_PRIVATE_H_ */
> +#endif /* EAL_THREAD_H */
> -- 
> 2.1.3
> 

Re: [dpdk-dev] [PATCH 03/10] eal: fix header guards

[Thomas Monjalon]

Thanks for your review, Bruce.
More comments below.

2014-11-25 10:28, Bruce Richardson:
> On Sat, Nov 22, 2014 at 10:43:35PM +0100, Thomas Monjalon wrote:
> > Some guards are missing or have a wrong name.
> > Others have LINUXAPP in their name but are now common.
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> One minor comment below.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
[...]
> > -#ifndef _EAL_LINUXAPP_FILESYSTEM_H
> > -#define _EAL_LINUXAPP_FILESYSTEM_H
> > +#ifndef EAL_FILESYSTEM_H
> > +#define EAL_FILESYSTEM_H
> >  
> >  /** Path of rte config file. */
> >  #define RUNTIME_CONFIG_FMT "%s/.%s_config"
> 
> no ending comment for #endif - should one be added for completeness?
> 
> > @@ -115,4 +115,4 @@ eal_get_hugefile_temp_path(char *buffer, size_t buflen, const char *hugedir, int
> >   * Used to read information from files on /sys */
> >  int eal_parse_sysfs_value(const char *filename, unsigned long *val);
> >  
> > -#endif /* _EAL_LINUXAPP_FILESYSTEM_H */
> > +#endif /* EAL_FILESYSTEM_H */

The ending comment is here.

> > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > @@ -36,8 +36,8 @@
> >   * Holds the structures for the eal internal configuration
> >   */
> >  
> > -#ifndef _EAL_LINUXAPP_INTERNAL_CFG
> > -#define _EAL_LINUXAPP_INTERNAL_CFG
> > +#ifndef EAL_INTERNAL_CFG_H
> > +#define EAL_INTERNAL_CFG_H
> >  
> >  #include <rte_eal.h>
> >  #include <rte_pci_dev_feature_defs.h>

I've added an ending comment for this file.
Is it the one you thought about?

-- 
Thomas

Re: [dpdk-dev] [PATCH 03/10] eal: fix header guards

[Bruce Richardson]

On Tue, Nov 25, 2014 at 01:23:55PM +0100, Thomas Monjalon wrote:
> Thanks for your review, Bruce.
> More comments below.
> 
> 2014-11-25 10:28, Bruce Richardson:
> > On Sat, Nov 22, 2014 at 10:43:35PM +0100, Thomas Monjalon wrote:
> > > Some guards are missing or have a wrong name.
> > > Others have LINUXAPP in their name but are now common.
> > > 
> > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > 
> > One minor comment below.
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> [...]
> > > -#ifndef _EAL_LINUXAPP_FILESYSTEM_H
> > > -#define _EAL_LINUXAPP_FILESYSTEM_H
> > > +#ifndef EAL_FILESYSTEM_H
> > > +#define EAL_FILESYSTEM_H
> > >  
> > >  /** Path of rte config file. */
> > >  #define RUNTIME_CONFIG_FMT "%s/.%s_config"
> > 
> > no ending comment for #endif - should one be added for completeness?
> > 
> > > @@ -115,4 +115,4 @@ eal_get_hugefile_temp_path(char *buffer, size_t buflen, const char *hugedir, int
> > >   * Used to read information from files on /sys */
> > >  int eal_parse_sysfs_value(const char *filename, unsigned long *val);
> > >  
> > > -#endif /* _EAL_LINUXAPP_FILESYSTEM_H */
> > > +#endif /* EAL_FILESYSTEM_H */
> 
> The ending comment is here.
> 
> > > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > > @@ -36,8 +36,8 @@
> > >   * Holds the structures for the eal internal configuration
> > >   */
> > >  
> > > -#ifndef _EAL_LINUXAPP_INTERNAL_CFG
> > > -#define _EAL_LINUXAPP_INTERNAL_CFG
> > > +#ifndef EAL_INTERNAL_CFG_H
> > > +#define EAL_INTERNAL_CFG_H
> > >  
> > >  #include <rte_eal.h>
> > >  #include <rte_pci_dev_feature_defs.h>
> 
> I've added an ending comment for this file.
> Is it the one you thought about?
>
Not sure, I just thought I missed seeing an ending comment on one of the pairs,
however, I'm sure you're on top of it (and if one is missed, it's no big deal
anyway).

/Bruce

[dpdk-dev] [PATCH 04/10] eal: factorize internal config reset

[Thomas Monjalon]

Now that internal config structure is common to Linux and BSD,
we can have a common function to initialize it.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            | 24 +------------------
 lib/librte_eal/common/eal_common_options.c | 37 ++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_internal_cfg.h   |  2 ++
 lib/librte_eal/linuxapp/eal/eal.c          | 28 +---------------------
 4 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index ca99cb9..391ce53 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -321,29 +321,7 @@ eal_parse_args(int argc, char **argv)
 
 	argvopt = argv;
 
-	internal_config.memory = 0;
-	internal_config.force_nrank = 0;
-	internal_config.force_nchannel = 0;
-	internal_config.hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
-	internal_config.hugepage_dir = NULL;
-	internal_config.force_sockets = 0;
-	internal_config.syslog_facility = LOG_DAEMON;
-	/* default value from build option */
-	internal_config.log_level = RTE_LOG_LEVEL;
-#ifdef RTE_LIBEAL_USE_HPET
-	internal_config.no_hpet = 0;
-#else
-	internal_config.no_hpet = 1;
-#endif
-	/* zero out the NUMA config */
-	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_config.socket_mem[i] = 0;
-
-	/* zero out hugedir descriptors */
-	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
-		internal_config.hugepage_info[i].lock_descriptor = 0;
-
-	internal_config.vmware_tsc_map = 0;
+	eal_reset_internal_config(&internal_config);
 
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
 				  eal_long_options, &option_index)) != EOF) {
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 7a5d55e..ffc615a 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -47,6 +47,7 @@
 
 #include "eal_internal_cfg.h"
 #include "eal_options.h"
+#include "eal_filesystem.h"
 
 #define BITS_PER_HEX 4
 
@@ -84,6 +85,42 @@ eal_long_options[] = {
 	{0, 0, 0, 0}
 };
 
+void
+eal_reset_internal_config(struct internal_config *internal_cfg)
+{
+	int i;
+
+	internal_cfg->memory = 0;
+	internal_cfg->force_nrank = 0;
+	internal_cfg->force_nchannel = 0;
+	internal_cfg->hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
+	internal_cfg->hugepage_dir = NULL;
+	internal_cfg->force_sockets = 0;
+	/* zero out the NUMA config */
+	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
+		internal_cfg->socket_mem[i] = 0;
+	/* zero out hugedir descriptors */
+	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
+		internal_cfg->hugepage_info[i].lock_descriptor = -1;
+	internal_cfg->base_virtaddr = 0;
+
+	internal_cfg->syslog_facility = LOG_DAEMON;
+	/* default value from build option */
+	internal_cfg->log_level = RTE_LOG_LEVEL;
+
+	internal_cfg->xen_dom0_support = 0;
+
+	/* if set to NONE, interrupt mode is determined automatically */
+	internal_cfg->vfio_intr_mode = RTE_INTR_MODE_NONE;
+
+#ifdef RTE_LIBEAL_USE_HPET
+	internal_cfg->no_hpet = 0;
+#else
+	internal_cfg->no_hpet = 1;
+#endif
+	internal_cfg->vmware_tsc_map = 0;
+}
+
 /*
  * Parse the coremask given as argument (hexadecimal string) and fill
  * the global configuration (core role and core count) with the parsed
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 19c84e7..a80aeab 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -88,4 +88,6 @@ struct internal_config {
 };
 extern struct internal_config internal_config; /**< Global EAL configuration. */
 
+void eal_reset_internal_config(struct internal_config *internal_cfg);
+
 #endif
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 7a1d087..bb35669 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -513,33 +513,7 @@ eal_parse_args(int argc, char **argv)
 
 	argvopt = argv;
 
-	internal_config.memory = 0;
-	internal_config.force_nrank = 0;
-	internal_config.force_nchannel = 0;
-	internal_config.hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
-	internal_config.hugepage_dir = NULL;
-	internal_config.force_sockets = 0;
-	internal_config.syslog_facility = LOG_DAEMON;
-	/* default value from build option */
-	internal_config.log_level = RTE_LOG_LEVEL;
-	internal_config.xen_dom0_support = 0;
-	/* if set to NONE, interrupt mode is determined automatically */
-	internal_config.vfio_intr_mode = RTE_INTR_MODE_NONE;
-#ifdef RTE_LIBEAL_USE_HPET
-	internal_config.no_hpet = 0;
-#else
-	internal_config.no_hpet = 1;
-#endif
-	/* zero out the NUMA config */
-	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_config.socket_mem[i] = 0;
-
-	/* zero out hugedir descriptors */
-	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
-		internal_config.hugepage_info[i].lock_descriptor = -1;
-
-	internal_config.vmware_tsc_map = 0;
-	internal_config.base_virtaddr = 0;
+	eal_reset_internal_config(&internal_config);
 
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
 				  eal_long_options, &option_index)) != EOF) {
-- 
2.1.3

Re: [dpdk-dev] [PATCH 04/10] eal: factorize internal config reset

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:36PM +0100, Thomas Monjalon wrote:
> Now that internal config structure is common to Linux and BSD,
> we can have a common function to initialize it.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/bsdapp/eal/eal.c            | 24 +------------------
>  lib/librte_eal/common/eal_common_options.c | 37 ++++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_internal_cfg.h   |  2 ++
>  lib/librte_eal/linuxapp/eal/eal.c          | 28 +---------------------
>  4 files changed, 41 insertions(+), 50 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index ca99cb9..391ce53 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -321,29 +321,7 @@ eal_parse_args(int argc, char **argv)
>  
>  	argvopt = argv;
>  
> -	internal_config.memory = 0;
> -	internal_config.force_nrank = 0;
> -	internal_config.force_nchannel = 0;
> -	internal_config.hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
> -	internal_config.hugepage_dir = NULL;
> -	internal_config.force_sockets = 0;
> -	internal_config.syslog_facility = LOG_DAEMON;
> -	/* default value from build option */
> -	internal_config.log_level = RTE_LOG_LEVEL;
> -#ifdef RTE_LIBEAL_USE_HPET
> -	internal_config.no_hpet = 0;
> -#else
> -	internal_config.no_hpet = 1;
> -#endif
> -	/* zero out the NUMA config */
> -	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> -		internal_config.socket_mem[i] = 0;
> -
> -	/* zero out hugedir descriptors */
> -	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
> -		internal_config.hugepage_info[i].lock_descriptor = 0;
> -
> -	internal_config.vmware_tsc_map = 0;
> +	eal_reset_internal_config(&internal_config);
>  
>  	while ((opt = getopt_long(argc, argvopt, eal_short_options,
>  				  eal_long_options, &option_index)) != EOF) {
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 7a5d55e..ffc615a 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -47,6 +47,7 @@
>  
>  #include "eal_internal_cfg.h"
>  #include "eal_options.h"
> +#include "eal_filesystem.h"
>  
>  #define BITS_PER_HEX 4
>  
> @@ -84,6 +85,42 @@ eal_long_options[] = {
>  	{0, 0, 0, 0}
>  };
>  
> +void
> +eal_reset_internal_config(struct internal_config *internal_cfg)
> +{
> +	int i;
> +
> +	internal_cfg->memory = 0;
> +	internal_cfg->force_nrank = 0;
> +	internal_cfg->force_nchannel = 0;
> +	internal_cfg->hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
> +	internal_cfg->hugepage_dir = NULL;
> +	internal_cfg->force_sockets = 0;
> +	/* zero out the NUMA config */
> +	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> +		internal_cfg->socket_mem[i] = 0;
> +	/* zero out hugedir descriptors */
> +	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
> +		internal_cfg->hugepage_info[i].lock_descriptor = -1;
> +	internal_cfg->base_virtaddr = 0;
> +
> +	internal_cfg->syslog_facility = LOG_DAEMON;
> +	/* default value from build option */
> +	internal_cfg->log_level = RTE_LOG_LEVEL;
> +
> +	internal_cfg->xen_dom0_support = 0;
> +
> +	/* if set to NONE, interrupt mode is determined automatically */
> +	internal_cfg->vfio_intr_mode = RTE_INTR_MODE_NONE;
> +
> +#ifdef RTE_LIBEAL_USE_HPET
> +	internal_cfg->no_hpet = 0;
> +#else
> +	internal_cfg->no_hpet = 1;
> +#endif
> +	internal_cfg->vmware_tsc_map = 0;
> +}
> +
>  /*
>   * Parse the coremask given as argument (hexadecimal string) and fill
>   * the global configuration (core role and core count) with the parsed
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 19c84e7..a80aeab 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -88,4 +88,6 @@ struct internal_config {
>  };
>  extern struct internal_config internal_config; /**< Global EAL configuration. */
>  
> +void eal_reset_internal_config(struct internal_config *internal_cfg);
> +
>  #endif
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 7a1d087..bb35669 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -513,33 +513,7 @@ eal_parse_args(int argc, char **argv)
>  
>  	argvopt = argv;
>  
> -	internal_config.memory = 0;
> -	internal_config.force_nrank = 0;
> -	internal_config.force_nchannel = 0;
> -	internal_config.hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
> -	internal_config.hugepage_dir = NULL;
> -	internal_config.force_sockets = 0;
> -	internal_config.syslog_facility = LOG_DAEMON;
> -	/* default value from build option */
> -	internal_config.log_level = RTE_LOG_LEVEL;
> -	internal_config.xen_dom0_support = 0;
> -	/* if set to NONE, interrupt mode is determined automatically */
> -	internal_config.vfio_intr_mode = RTE_INTR_MODE_NONE;
> -#ifdef RTE_LIBEAL_USE_HPET
> -	internal_config.no_hpet = 0;
> -#else
> -	internal_config.no_hpet = 1;
> -#endif
> -	/* zero out the NUMA config */
> -	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> -		internal_config.socket_mem[i] = 0;
> -
> -	/* zero out hugedir descriptors */
> -	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++)
> -		internal_config.hugepage_info[i].lock_descriptor = -1;
> -
> -	internal_config.vmware_tsc_map = 0;
> -	internal_config.base_virtaddr = 0;
> +	eal_reset_internal_config(&internal_config);
>  
>  	while ((opt = getopt_long(argc, argvopt, eal_short_options,
>  				  eal_long_options, &option_index)) != EOF) {
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 05/10] eal: factorize options sanity check

[Thomas Monjalon]

No need to have duplicated check for common options.

Some flags are set for options -c and -m in order to simplify the
checks.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            | 54 ++------------------------
 lib/librte_eal/common/eal_common_options.c | 53 ++++++++++++++++++++++++++
 lib/librte_eal/common/eal_options.h        |  1 +
 lib/librte_eal/linuxapp/eal/eal.c          | 61 ++++--------------------------
 4 files changed, 66 insertions(+), 103 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 391ce53..20a9c5f 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -316,7 +316,6 @@ eal_parse_args(int argc, char **argv)
 	int opt, ret, i;
 	char **argvopt;
 	int option_index;
-	int coremask_ok = 0;
 	char *prgname = argv[0];
 
 	argvopt = argv;
@@ -339,13 +338,8 @@ eal_parse_args(int argc, char **argv)
 			return -1;
 		}
 		/* common parser handled this option */
-		if (ret == 0) {
-			/* special case, note that the common parser accepted
-			 * the coremask option */
-			if (opt == 'c')
-				coremask_ok = 1;
+		if (ret == 0)
 			continue;
-		}
 
 		switch (opt) {
 		default:
@@ -366,51 +360,11 @@ eal_parse_args(int argc, char **argv)
 		}
 	}
 
-	/* sanity checks */
-	if (!coremask_ok) {
-		RTE_LOG(ERR, EAL, "coremask not specified\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (internal_config.process_type == RTE_PROC_AUTO){
+	if (internal_config.process_type == RTE_PROC_AUTO)
 		internal_config.process_type = eal_proc_type_detect();
-	}
-	if (internal_config.process_type == RTE_PROC_INVALID){
-		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (internal_config.process_type == RTE_PROC_PRIMARY &&
-			internal_config.force_nchannel == 0) {
-		RTE_LOG(ERR, EAL, "Number of memory channels (-n) not specified\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (index(internal_config.hugefile_prefix,'%') != NULL){
-		RTE_LOG(ERR, EAL, "Invalid char, '%%', in '"OPT_FILE_PREFIX"' option\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (internal_config.memory > 0 && internal_config.force_sockets == 1) {
-		RTE_LOG(ERR, EAL, "Options -m and --socket-mem cannot be specified "
-				"at the same time\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	/* --no-huge doesn't make sense with either -m or --socket-mem */
-	if (internal_config.no_hugetlbfs &&
-			(internal_config.memory > 0 ||
-					internal_config.force_sockets == 1)) {
-		RTE_LOG(ERR, EAL, "Options -m or --socket-mem cannot be specified "
-				"together with --no-huge!\n");
-		eal_usage(prgname);
-		return -1;
-	}
 
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
-		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
-		RTE_LOG(ERR, EAL, "Error: blacklist [-b] and whitelist "
-			"[-w] options cannot be used at the same time\n");
+	/* sanity checks */
+	if (eal_check_common_options(&internal_config) != 0) {
 		eal_usage(prgname);
 		return -1;
 	}
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index ffc615a..630dfe0 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -85,6 +85,9 @@ eal_long_options[] = {
 	{0, 0, 0, 0}
 };
 
+static int lcores_parsed;
+static int mem_parsed;
+
 void
 eal_reset_internal_config(struct internal_config *internal_cfg)
 {
@@ -197,6 +200,7 @@ eal_parse_coremask(const char *coremask)
 		return -1;
 	/* Update the count of enabled logical cores of the EAL configuration */
 	cfg->lcore_count = count;
+	lcores_parsed = 1;
 	return 0;
 }
 
@@ -305,6 +309,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		conf->memory = atoi(optarg);
 		conf->memory *= 1024ULL;
 		conf->memory *= 1024ULL;
+		mem_parsed = 1;
 		break;
 	/* force number of channels */
 	case 'n':
@@ -394,6 +399,54 @@ eal_parse_common_option(int opt, const char *optarg,
 	return 0;
 }
 
+int
+eal_check_common_options(struct internal_config *internal_cfg)
+{
+	struct rte_config *cfg = rte_eal_get_configuration();
+
+	if (!lcores_parsed) {
+		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
+			"-c\n");
+		return -1;
+	}
+
+	if (internal_cfg->process_type == RTE_PROC_INVALID) {
+		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
+		return -1;
+	}
+	if (internal_cfg->process_type == RTE_PROC_PRIMARY &&
+			internal_cfg->force_nchannel == 0) {
+		RTE_LOG(ERR, EAL, "Number of memory channels (-n) not "
+			"specified\n");
+		return -1;
+	}
+	if (index(internal_cfg->hugefile_prefix, '%') != NULL) {
+		RTE_LOG(ERR, EAL, "Invalid char, '%%', in --"OPT_FILE_PREFIX" "
+			"option\n");
+		return -1;
+	}
+	if (mem_parsed && internal_cfg->force_sockets == 1) {
+		RTE_LOG(ERR, EAL, "Options -m and --"OPT_SOCKET_MEM" cannot "
+			"be specified at the same time\n");
+		return -1;
+	}
+	if (internal_cfg->no_hugetlbfs &&
+			(mem_parsed || internal_cfg->force_sockets == 1)) {
+		RTE_LOG(ERR, EAL, "Options -m or --"OPT_SOCKET_MEM" cannot "
+			"be specified together with --"OPT_NO_HUGE"\n");
+		return -1;
+	}
+
+	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
+		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
+		RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) "
+			"cannot be used at the same time\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 void
 eal_common_usage(void)
 {
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 7a08507..75351c0 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -83,6 +83,7 @@ extern const struct option eal_long_options[];
 
 int eal_parse_common_option(int opt, const char *argv,
 			    struct internal_config *conf);
+int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
 
 #endif /* EAL_OPTIONS_H */
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index bb35669..f5de277 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -507,7 +507,6 @@ eal_parse_args(int argc, char **argv)
 	int opt, ret, i;
 	char **argvopt;
 	int option_index;
-	int coremask_ok = 0;
 	char *prgname = argv[0];
 	struct shared_driver *solib;
 
@@ -531,13 +530,8 @@ eal_parse_args(int argc, char **argv)
 			return -1;
 		}
 		/* common parser handled this option */
-		if (ret == 0) {
-			/* special case, note that the common parser accepted
-			 * the coremask option */
-			if (opt == 'c')
-				coremask_ok = 1;
+		if (ret == 0)
 			continue;
-		}
 
 		switch (opt) {
 		/* force loading of external driver */
@@ -622,58 +616,19 @@ eal_parse_args(int argc, char **argv)
 		}
 	}
 
-	/* sanity checks */
-	if (!coremask_ok) {
-		RTE_LOG(ERR, EAL, "coremask not specified\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (internal_config.process_type == RTE_PROC_AUTO){
+	if (internal_config.process_type == RTE_PROC_AUTO)
 		internal_config.process_type = eal_proc_type_detect();
-	}
-	if (internal_config.process_type == RTE_PROC_INVALID){
-		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (internal_config.process_type == RTE_PROC_PRIMARY &&
-			internal_config.force_nchannel == 0) {
-		RTE_LOG(ERR, EAL, "Number of memory channels (-n) not specified\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (index(internal_config.hugefile_prefix,'%') != NULL){
-		RTE_LOG(ERR, EAL, "Invalid char, '%%', in '"OPT_FILE_PREFIX"' option\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	if (internal_config.memory > 0 && internal_config.force_sockets == 1) {
-		RTE_LOG(ERR, EAL, "Options -m and --socket-mem cannot be specified "
-				"at the same time\n");
-		eal_usage(prgname);
-		return -1;
-	}
-	/* --no-huge doesn't make sense with either -m or --socket-mem */
-	if (internal_config.no_hugetlbfs &&
-			(internal_config.memory > 0 ||
-					internal_config.force_sockets == 1)) {
-		RTE_LOG(ERR, EAL, "Options -m or --socket-mem cannot be specified "
-				"together with --no-huge!\n");
+
+	/* sanity checks */
+	if (eal_check_common_options(&internal_config) != 0) {
 		eal_usage(prgname);
 		return -1;
 	}
+
 	/* --xen-dom0 doesn't make sense with --socket-mem */
 	if (internal_config.xen_dom0_support && internal_config.force_sockets == 1) {
-		RTE_LOG(ERR, EAL, "Options --socket-mem cannot be specified "
-					"together with --xen_dom0!\n");
-		eal_usage(prgname);
-		return -1;
-	}
-
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
-		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
-		RTE_LOG(ERR, EAL, "Error: blacklist [-b] and whitelist "
-			"[-w] options cannot be used at the same time\n");
+		RTE_LOG(ERR, EAL, "Options --"OPT_SOCKET_MEM" cannot be specified "
+			"together with --"OPT_XEN_DOM0"\n");
 		eal_usage(prgname);
 		return -1;
 	}
-- 
2.1.3

Re: [dpdk-dev] [PATCH 05/10] eal: factorize options sanity check

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:37PM +0100, Thomas Monjalon wrote:
> No need to have duplicated check for common options.
> 
> Some flags are set for options -c and -m in order to simplify the
> checks.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/bsdapp/eal/eal.c            | 54 ++------------------------
>  lib/librte_eal/common/eal_common_options.c | 53 ++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_options.h        |  1 +
>  lib/librte_eal/linuxapp/eal/eal.c          | 61 ++++--------------------------
>  4 files changed, 66 insertions(+), 103 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 391ce53..20a9c5f 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -316,7 +316,6 @@ eal_parse_args(int argc, char **argv)
>  	int opt, ret, i;
>  	char **argvopt;
>  	int option_index;
> -	int coremask_ok = 0;
>  	char *prgname = argv[0];
>  
>  	argvopt = argv;
> @@ -339,13 +338,8 @@ eal_parse_args(int argc, char **argv)
>  			return -1;
>  		}
>  		/* common parser handled this option */
> -		if (ret == 0) {
> -			/* special case, note that the common parser accepted
> -			 * the coremask option */
> -			if (opt == 'c')
> -				coremask_ok = 1;
> +		if (ret == 0)
>  			continue;
> -		}
>  
>  		switch (opt) {
>  		default:
> @@ -366,51 +360,11 @@ eal_parse_args(int argc, char **argv)
>  		}
>  	}
>  
> -	/* sanity checks */
> -	if (!coremask_ok) {
> -		RTE_LOG(ERR, EAL, "coremask not specified\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (internal_config.process_type == RTE_PROC_AUTO){
> +	if (internal_config.process_type == RTE_PROC_AUTO)
>  		internal_config.process_type = eal_proc_type_detect();
> -	}
> -	if (internal_config.process_type == RTE_PROC_INVALID){
> -		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (internal_config.process_type == RTE_PROC_PRIMARY &&
> -			internal_config.force_nchannel == 0) {
> -		RTE_LOG(ERR, EAL, "Number of memory channels (-n) not specified\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (index(internal_config.hugefile_prefix,'%') != NULL){
> -		RTE_LOG(ERR, EAL, "Invalid char, '%%', in '"OPT_FILE_PREFIX"' option\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (internal_config.memory > 0 && internal_config.force_sockets == 1) {
> -		RTE_LOG(ERR, EAL, "Options -m and --socket-mem cannot be specified "
> -				"at the same time\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	/* --no-huge doesn't make sense with either -m or --socket-mem */
> -	if (internal_config.no_hugetlbfs &&
> -			(internal_config.memory > 0 ||
> -					internal_config.force_sockets == 1)) {
> -		RTE_LOG(ERR, EAL, "Options -m or --socket-mem cannot be specified "
> -				"together with --no-huge!\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
>  
> -	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
> -		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
> -		RTE_LOG(ERR, EAL, "Error: blacklist [-b] and whitelist "
> -			"[-w] options cannot be used at the same time\n");
> +	/* sanity checks */
> +	if (eal_check_common_options(&internal_config) != 0) {
>  		eal_usage(prgname);
>  		return -1;
>  	}
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index ffc615a..630dfe0 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -85,6 +85,9 @@ eal_long_options[] = {
>  	{0, 0, 0, 0}
>  };
>  
> +static int lcores_parsed;
> +static int mem_parsed;
> +
>  void
>  eal_reset_internal_config(struct internal_config *internal_cfg)
>  {
> @@ -197,6 +200,7 @@ eal_parse_coremask(const char *coremask)
>  		return -1;
>  	/* Update the count of enabled logical cores of the EAL configuration */
>  	cfg->lcore_count = count;
> +	lcores_parsed = 1;
>  	return 0;
>  }
>  
> @@ -305,6 +309,7 @@ eal_parse_common_option(int opt, const char *optarg,
>  		conf->memory = atoi(optarg);
>  		conf->memory *= 1024ULL;
>  		conf->memory *= 1024ULL;
> +		mem_parsed = 1;
>  		break;
>  	/* force number of channels */
>  	case 'n':
> @@ -394,6 +399,54 @@ eal_parse_common_option(int opt, const char *optarg,
>  	return 0;
>  }
>  
> +int
> +eal_check_common_options(struct internal_config *internal_cfg)
> +{
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +
> +	if (!lcores_parsed) {
> +		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
> +			"-c\n");
> +		return -1;
> +	}
> +
> +	if (internal_cfg->process_type == RTE_PROC_INVALID) {
> +		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
> +		return -1;
> +	}
> +	if (internal_cfg->process_type == RTE_PROC_PRIMARY &&
> +			internal_cfg->force_nchannel == 0) {
> +		RTE_LOG(ERR, EAL, "Number of memory channels (-n) not "
> +			"specified\n");
> +		return -1;
> +	}
> +	if (index(internal_cfg->hugefile_prefix, '%') != NULL) {
> +		RTE_LOG(ERR, EAL, "Invalid char, '%%', in --"OPT_FILE_PREFIX" "
> +			"option\n");
> +		return -1;
> +	}
> +	if (mem_parsed && internal_cfg->force_sockets == 1) {
> +		RTE_LOG(ERR, EAL, "Options -m and --"OPT_SOCKET_MEM" cannot "
> +			"be specified at the same time\n");
> +		return -1;
> +	}
> +	if (internal_cfg->no_hugetlbfs &&
> +			(mem_parsed || internal_cfg->force_sockets == 1)) {
> +		RTE_LOG(ERR, EAL, "Options -m or --"OPT_SOCKET_MEM" cannot "
> +			"be specified together with --"OPT_NO_HUGE"\n");
> +		return -1;
> +	}
> +
> +	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
> +		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
> +		RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) "
> +			"cannot be used at the same time\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  void
>  eal_common_usage(void)
>  {
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 7a08507..75351c0 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -83,6 +83,7 @@ extern const struct option eal_long_options[];
>  
>  int eal_parse_common_option(int opt, const char *argv,
>  			    struct internal_config *conf);
> +int eal_check_common_options(struct internal_config *internal_cfg);
>  void eal_common_usage(void);
>  
>  #endif /* EAL_OPTIONS_H */
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index bb35669..f5de277 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -507,7 +507,6 @@ eal_parse_args(int argc, char **argv)
>  	int opt, ret, i;
>  	char **argvopt;
>  	int option_index;
> -	int coremask_ok = 0;
>  	char *prgname = argv[0];
>  	struct shared_driver *solib;
>  
> @@ -531,13 +530,8 @@ eal_parse_args(int argc, char **argv)
>  			return -1;
>  		}
>  		/* common parser handled this option */
> -		if (ret == 0) {
> -			/* special case, note that the common parser accepted
> -			 * the coremask option */
> -			if (opt == 'c')
> -				coremask_ok = 1;
> +		if (ret == 0)
>  			continue;
> -		}
>  
>  		switch (opt) {
>  		/* force loading of external driver */
> @@ -622,58 +616,19 @@ eal_parse_args(int argc, char **argv)
>  		}
>  	}
>  
> -	/* sanity checks */
> -	if (!coremask_ok) {
> -		RTE_LOG(ERR, EAL, "coremask not specified\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (internal_config.process_type == RTE_PROC_AUTO){
> +	if (internal_config.process_type == RTE_PROC_AUTO)
>  		internal_config.process_type = eal_proc_type_detect();
> -	}
> -	if (internal_config.process_type == RTE_PROC_INVALID){
> -		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (internal_config.process_type == RTE_PROC_PRIMARY &&
> -			internal_config.force_nchannel == 0) {
> -		RTE_LOG(ERR, EAL, "Number of memory channels (-n) not specified\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (index(internal_config.hugefile_prefix,'%') != NULL){
> -		RTE_LOG(ERR, EAL, "Invalid char, '%%', in '"OPT_FILE_PREFIX"' option\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	if (internal_config.memory > 0 && internal_config.force_sockets == 1) {
> -		RTE_LOG(ERR, EAL, "Options -m and --socket-mem cannot be specified "
> -				"at the same time\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -	/* --no-huge doesn't make sense with either -m or --socket-mem */
> -	if (internal_config.no_hugetlbfs &&
> -			(internal_config.memory > 0 ||
> -					internal_config.force_sockets == 1)) {
> -		RTE_LOG(ERR, EAL, "Options -m or --socket-mem cannot be specified "
> -				"together with --no-huge!\n");
> +
> +	/* sanity checks */
> +	if (eal_check_common_options(&internal_config) != 0) {
>  		eal_usage(prgname);
>  		return -1;
>  	}
> +
>  	/* --xen-dom0 doesn't make sense with --socket-mem */
>  	if (internal_config.xen_dom0_support && internal_config.force_sockets == 1) {
> -		RTE_LOG(ERR, EAL, "Options --socket-mem cannot be specified "
> -					"together with --xen_dom0!\n");
> -		eal_usage(prgname);
> -		return -1;
> -	}
> -
> -	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
> -		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
> -		RTE_LOG(ERR, EAL, "Error: blacklist [-b] and whitelist "
> -			"[-w] options cannot be used at the same time\n");
> +		RTE_LOG(ERR, EAL, "Options --"OPT_SOCKET_MEM" cannot be specified "
> +			"together with --"OPT_XEN_DOM0"\n");
>  		eal_usage(prgname);
>  		return -1;
>  	}
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 06/10] eal: factorize configuration adjustment

[Thomas Monjalon]

Some adjustments are done after options parsing and are common
to Linux and BSD.

Remove process_type adjustment in rte_config_init() because
it is already done in eal_parse_args().
eal_proc_type_detect() is kept duplicated because it open a
file descriptor which is used later in each eal.c.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            | 18 +++++-------------
 lib/librte_eal/common/eal_common_options.c | 16 ++++++++++++++++
 lib/librte_eal/common/eal_options.h        |  2 ++
 lib/librte_eal/linuxapp/eal/eal.c          | 18 +++++-------------
 4 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 20a9c5f..69f3c03 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -224,7 +224,7 @@ rte_eal_config_attach(void)
 }
 
 /* Detect if we are a primary or a secondary process */
-static enum rte_proc_type_t
+enum rte_proc_type_t
 eal_proc_type_detect(void)
 {
 	enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
@@ -247,9 +247,7 @@ eal_proc_type_detect(void)
 static void
 rte_config_init(void)
 {
-	rte_config.process_type = (internal_config.process_type == RTE_PROC_AUTO) ?
-			eal_proc_type_detect() : /* for auto, detect the type */
-			internal_config.process_type; /* otherwise use what's already set */
+	rte_config.process_type = internal_config.process_type;
 
 	switch (rte_config.process_type){
 	case RTE_PROC_PRIMARY:
@@ -313,7 +311,7 @@ eal_get_hugepage_mem_size(void)
 static int
 eal_parse_args(int argc, char **argv)
 {
-	int opt, ret, i;
+	int opt, ret;
 	char **argvopt;
 	int option_index;
 	char *prgname = argv[0];
@@ -360,8 +358,8 @@ eal_parse_args(int argc, char **argv)
 		}
 	}
 
-	if (internal_config.process_type == RTE_PROC_AUTO)
-		internal_config.process_type = eal_proc_type_detect();
+	if (eal_adjust_config(&internal_config) != 0)
+		return -1;
 
 	/* sanity checks */
 	if (eal_check_common_options(&internal_config) != 0) {
@@ -371,12 +369,6 @@ eal_parse_args(int argc, char **argv)
 
 	if (optind >= 0)
 		argv[optind-1] = prgname;
-
-	/* if no memory amounts were requested, this will result in 0 and
-	 * will be overriden later, right after eal_hugepage_info_init() */
-	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_config.memory += internal_config.socket_mem[i];
-
 	ret = optind-1;
 	optind = 0; /* reset getopt lib */
 	return ret;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 630dfe0..63710b0 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -400,6 +400,22 @@ eal_parse_common_option(int opt, const char *optarg,
 }
 
 int
+eal_adjust_config(struct internal_config *internal_cfg)
+{
+	int i;
+
+	if (internal_config.process_type == RTE_PROC_AUTO)
+		internal_config.process_type = eal_proc_type_detect();
+
+	/* if no memory amounts were requested, this will result in 0 and
+	 * will be overridden later, right after eal_hugepage_info_init() */
+	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
+		internal_cfg->memory += internal_cfg->socket_mem[i];
+
+	return 0;
+}
+
+int
 eal_check_common_options(struct internal_config *internal_cfg)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 75351c0..f58965c 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -83,7 +83,9 @@ extern const struct option eal_long_options[];
 
 int eal_parse_common_option(int opt, const char *argv,
 			    struct internal_config *conf);
+int eal_adjust_config(struct internal_config *internal_cfg);
 int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
+enum rte_proc_type_t eal_proc_type_detect(void);
 
 #endif /* EAL_OPTIONS_H */
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index f5de277..5e5a7a0 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -284,7 +284,7 @@ rte_eal_config_reattach(void)
 }
 
 /* Detect if we are a primary or a secondary process */
-static enum rte_proc_type_t
+enum rte_proc_type_t
 eal_proc_type_detect(void)
 {
 	enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
@@ -307,9 +307,7 @@ eal_proc_type_detect(void)
 static void
 rte_config_init(void)
 {
-	rte_config.process_type = (internal_config.process_type == RTE_PROC_AUTO) ?
-			eal_proc_type_detect() : /* for auto, detect the type */
-			internal_config.process_type; /* otherwise use what's already set */
+	rte_config.process_type = internal_config.process_type;
 
 	switch (rte_config.process_type){
 	case RTE_PROC_PRIMARY:
@@ -504,7 +502,7 @@ eal_get_hugepage_mem_size(void)
 static int
 eal_parse_args(int argc, char **argv)
 {
-	int opt, ret, i;
+	int opt, ret;
 	char **argvopt;
 	int option_index;
 	char *prgname = argv[0];
@@ -616,8 +614,8 @@ eal_parse_args(int argc, char **argv)
 		}
 	}
 
-	if (internal_config.process_type == RTE_PROC_AUTO)
-		internal_config.process_type = eal_proc_type_detect();
+	if (eal_adjust_config(&internal_config) != 0)
+		return -1;
 
 	/* sanity checks */
 	if (eal_check_common_options(&internal_config) != 0) {
@@ -635,12 +633,6 @@ eal_parse_args(int argc, char **argv)
 
 	if (optind >= 0)
 		argv[optind-1] = prgname;
-
-	/* if no memory amounts were requested, this will result in 0 and
-	 * will be overriden later, right after eal_hugepage_info_init() */
-	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_config.memory += internal_config.socket_mem[i];
-
 	ret = optind-1;
 	optind = 0; /* reset getopt lib */
 	return ret;
-- 
2.1.3

Re: [dpdk-dev] [PATCH 06/10] eal: factorize configuration adjustment

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:38PM +0100, Thomas Monjalon wrote:
> Some adjustments are done after options parsing and are common
> to Linux and BSD.
> 
> Remove process_type adjustment in rte_config_init() because
> it is already done in eal_parse_args().
> eal_proc_type_detect() is kept duplicated because it open a
> file descriptor which is used later in each eal.c.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/bsdapp/eal/eal.c            | 18 +++++-------------
>  lib/librte_eal/common/eal_common_options.c | 16 ++++++++++++++++
>  lib/librte_eal/common/eal_options.h        |  2 ++
>  lib/librte_eal/linuxapp/eal/eal.c          | 18 +++++-------------
>  4 files changed, 28 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 20a9c5f..69f3c03 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -224,7 +224,7 @@ rte_eal_config_attach(void)
>  }
>  
>  /* Detect if we are a primary or a secondary process */
> -static enum rte_proc_type_t
> +enum rte_proc_type_t
>  eal_proc_type_detect(void)
>  {
>  	enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
> @@ -247,9 +247,7 @@ eal_proc_type_detect(void)
>  static void
>  rte_config_init(void)
>  {
> -	rte_config.process_type = (internal_config.process_type == RTE_PROC_AUTO) ?
> -			eal_proc_type_detect() : /* for auto, detect the type */
> -			internal_config.process_type; /* otherwise use what's already set */
> +	rte_config.process_type = internal_config.process_type;
>  
>  	switch (rte_config.process_type){
>  	case RTE_PROC_PRIMARY:
> @@ -313,7 +311,7 @@ eal_get_hugepage_mem_size(void)
>  static int
>  eal_parse_args(int argc, char **argv)
>  {
> -	int opt, ret, i;
> +	int opt, ret;
>  	char **argvopt;
>  	int option_index;
>  	char *prgname = argv[0];
> @@ -360,8 +358,8 @@ eal_parse_args(int argc, char **argv)
>  		}
>  	}
>  
> -	if (internal_config.process_type == RTE_PROC_AUTO)
> -		internal_config.process_type = eal_proc_type_detect();
> +	if (eal_adjust_config(&internal_config) != 0)
> +		return -1;
>  
>  	/* sanity checks */
>  	if (eal_check_common_options(&internal_config) != 0) {
> @@ -371,12 +369,6 @@ eal_parse_args(int argc, char **argv)
>  
>  	if (optind >= 0)
>  		argv[optind-1] = prgname;
> -
> -	/* if no memory amounts were requested, this will result in 0 and
> -	 * will be overriden later, right after eal_hugepage_info_init() */
> -	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> -		internal_config.memory += internal_config.socket_mem[i];
> -
>  	ret = optind-1;
>  	optind = 0; /* reset getopt lib */
>  	return ret;
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 630dfe0..63710b0 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -400,6 +400,22 @@ eal_parse_common_option(int opt, const char *optarg,
>  }
>  
>  int
> +eal_adjust_config(struct internal_config *internal_cfg)
> +{
> +	int i;
> +
> +	if (internal_config.process_type == RTE_PROC_AUTO)
> +		internal_config.process_type = eal_proc_type_detect();
> +
> +	/* if no memory amounts were requested, this will result in 0 and
> +	 * will be overridden later, right after eal_hugepage_info_init() */
> +	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> +		internal_cfg->memory += internal_cfg->socket_mem[i];
> +
> +	return 0;
> +}
> +
> +int
>  eal_check_common_options(struct internal_config *internal_cfg)
>  {
>  	struct rte_config *cfg = rte_eal_get_configuration();
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 75351c0..f58965c 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -83,7 +83,9 @@ extern const struct option eal_long_options[];
>  
>  int eal_parse_common_option(int opt, const char *argv,
>  			    struct internal_config *conf);
> +int eal_adjust_config(struct internal_config *internal_cfg);
>  int eal_check_common_options(struct internal_config *internal_cfg);
>  void eal_common_usage(void);
> +enum rte_proc_type_t eal_proc_type_detect(void);
>  
>  #endif /* EAL_OPTIONS_H */
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index f5de277..5e5a7a0 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -284,7 +284,7 @@ rte_eal_config_reattach(void)
>  }
>  
>  /* Detect if we are a primary or a secondary process */
> -static enum rte_proc_type_t
> +enum rte_proc_type_t
>  eal_proc_type_detect(void)
>  {
>  	enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
> @@ -307,9 +307,7 @@ eal_proc_type_detect(void)
>  static void
>  rte_config_init(void)
>  {
> -	rte_config.process_type = (internal_config.process_type == RTE_PROC_AUTO) ?
> -			eal_proc_type_detect() : /* for auto, detect the type */
> -			internal_config.process_type; /* otherwise use what's already set */
> +	rte_config.process_type = internal_config.process_type;
>  
>  	switch (rte_config.process_type){
>  	case RTE_PROC_PRIMARY:
> @@ -504,7 +502,7 @@ eal_get_hugepage_mem_size(void)
>  static int
>  eal_parse_args(int argc, char **argv)
>  {
> -	int opt, ret, i;
> +	int opt, ret;
>  	char **argvopt;
>  	int option_index;
>  	char *prgname = argv[0];
> @@ -616,8 +614,8 @@ eal_parse_args(int argc, char **argv)
>  		}
>  	}
>  
> -	if (internal_config.process_type == RTE_PROC_AUTO)
> -		internal_config.process_type = eal_proc_type_detect();
> +	if (eal_adjust_config(&internal_config) != 0)
> +		return -1;
>  
>  	/* sanity checks */
>  	if (eal_check_common_options(&internal_config) != 0) {
> @@ -635,12 +633,6 @@ eal_parse_args(int argc, char **argv)
>  
>  	if (optind >= 0)
>  		argv[optind-1] = prgname;
> -
> -	/* if no memory amounts were requested, this will result in 0 and
> -	 * will be overriden later, right after eal_hugepage_info_init() */
> -	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> -		internal_config.memory += internal_config.socket_mem[i];
> -
>  	ret = optind-1;
>  	optind = 0; /* reset getopt lib */
>  	return ret;
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 07/10] eal: add core list input format

[Thomas Monjalon]

From: Didier Pallard <didier.pallard@6wind.com>

In current version, used cores can only be specified using a bitmask.
It will now be possible to specify cores in 2 different ways:
- Using a bitmask (-c [0x]nnn): bitmask must be in hex format
- Using a list in following format: -l <c1>[-c2][,c3[-c4],...]

The letter -l can stand for lcore or list.

-l 0-7,16-23,31 being equivalent to -c 0x80FF00FF

Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test/test_eal_flags.c                  | 28 ++++++++++-
 lib/librte_eal/common/eal_common_options.c | 81 ++++++++++++++++++++++++++++--
 2 files changed, 103 insertions(+), 6 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 1f95d7f..5ad89c5 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -484,7 +484,7 @@ test_invalid_r_flag(void)
 }
 
 /*
- * Test that the app doesn't run without the coremask flag. In all cases
+ * Test that the app doesn't run without the coremask/corelist flags. In all cases
  * should give an error and fail to run
  */
 static int
@@ -504,12 +504,22 @@ test_missing_c_flag(void)
 
 	/* -c flag but no coremask value */
 	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
-	/* No -c flag at all */
+	/* No -c or -l flag at all */
 	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
 	/* bad coremask value */
 	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "error" };
 	/* sanity check of tests - valid coremask value */
 	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "1" };
+	/* -l flag but no corelist value */
+	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "3", "-l"};
+	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "3", "-l", " " };
+	/* bad corelist values */
+	const char *argv7[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "error" };
+	const char *argv8[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-" };
+	const char *argv9[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1," };
+	const char *argv10[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1#2" };
+	/* sanity check test - valid corelist value */
+	const char *argv11[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-2,3" };
 
 	if (launch_proc(argv1) == 0
 			|| launch_proc(argv2) == 0
@@ -521,6 +531,20 @@ test_missing_c_flag(void)
 		printf("Error - process did not run ok with valid coremask value\n");
 		return -1;
 	}
+
+	if (launch_proc(argv5) == 0
+			|| launch_proc(argv6) == 0
+			|| launch_proc(argv7) == 0
+			|| launch_proc(argv8) == 0
+			|| launch_proc(argv9) == 0
+			|| launch_proc(argv10) == 0) {
+		printf("Error - process ran without error with invalid -l flag\n");
+		return -1;
+	}
+	if (launch_proc(argv11) != 0) {
+		printf("Error - process did not run ok with valid corelist value\n");
+		return -1;
+	}
 	return 0;
 }
 
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 63710b0..18d03e3 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -32,6 +32,7 @@
  */
 
 #include <stdlib.h>
+#include <unistd.h>
 #include <string.h>
 #include <syslog.h>
 #include <ctype.h>
@@ -55,8 +56,9 @@ const char
 eal_short_options[] =
 	"b:" /* pci-blacklist */
 	"w:" /* pci-whitelist */
-	"c:"
+	"c:" /* coremask */
 	"d:"
+	"l:" /* corelist */
 	"m:"
 	"n:"
 	"r:"
@@ -205,6 +207,67 @@ eal_parse_coremask(const char *coremask)
 }
 
 static int
+eal_parse_corelist(const char *corelist)
+{
+	struct rte_config *cfg = rte_eal_get_configuration();
+	int i, idx = 0;
+	unsigned count = 0;
+	char *end = NULL;
+	int min, max;
+
+	if (corelist == NULL)
+		return -1;
+
+	/* Remove all blank characters ahead and after */
+	while (isblank(*corelist))
+		corelist++;
+	i = strnlen(corelist, sysconf(_SC_ARG_MAX));
+	while ((i > 0) && isblank(corelist[i - 1]))
+		i--;
+
+	/* Reset core roles */
+	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
+		cfg->lcore_role[idx] = ROLE_OFF;
+
+	/* Get list of cores */
+	min = RTE_MAX_LCORE;
+	do {
+		while (isblank(*corelist))
+			corelist++;
+		if (*corelist == '\0')
+			return -1;
+		errno = 0;
+		idx = strtoul(corelist, &end, 10);
+		if (errno || end == NULL)
+			return -1;
+		while (isblank(*end))
+			end++;
+		if (*end == '-') {
+			min = idx;
+		} else if ((*end == ',') || (*end == '\0')) {
+			max = idx;
+			if (min == RTE_MAX_LCORE)
+				min = idx;
+			for (idx = min; idx <= max; idx++) {
+				cfg->lcore_role[idx] = ROLE_RTE;
+				if (count == 0)
+					cfg->master_lcore = idx;
+				count++;
+			}
+			min = RTE_MAX_LCORE;
+		} else
+			return -1;
+		corelist = end + 1;
+	} while (*end != '\0');
+
+	if (count == 0)
+		return -1;
+
+	lcores_parsed = 1;
+	return 0;
+}
+
+static int
 eal_parse_syslog(const char *facility, struct internal_config *conf)
 {
 	int i;
@@ -304,6 +367,13 @@ eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
+	/* corelist */
+	case 'l':
+		if (eal_parse_corelist(optarg) < 0) {
+			RTE_LOG(ERR, EAL, "invalid core list\n");
+			return -1;
+		}
+		break;
 	/* size of memory */
 	case 'm':
 		conf->memory = atoi(optarg);
@@ -421,8 +491,8 @@ eal_check_common_options(struct internal_config *internal_cfg)
 	struct rte_config *cfg = rte_eal_get_configuration();
 
 	if (!lcores_parsed) {
-		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
-			"-c\n");
+		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
+			"-c or -l\n");
 		return -1;
 	}
 
@@ -470,6 +540,9 @@ eal_common_usage(void)
 	       "[--proc-type primary|secondary|auto]\n\n"
 	       "EAL common options:\n"
 	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
+	       "  -l CORELIST  : List of cores to run on\n"
+	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
+	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
 	       "  -n NUM       : Number of memory channels\n"
 	       "  -v           : Display version information on startup\n"
 	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
@@ -494,5 +567,5 @@ eal_common_usage(void)
 	       "  --"OPT_NO_PCI"   : disable pci\n"
 	       "  --"OPT_NO_HPET"  : disable hpet\n"
 	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
-	       "\n");
+	       "\n", RTE_MAX_LCORE);
 }
-- 
2.1.3

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Neil Horman]

On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> From: Didier Pallard <didier.pallard@6wind.com>
> 
> In current version, used cores can only be specified using a bitmask.
> It will now be possible to specify cores in 2 different ways:
> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> 
> The letter -l can stand for lcore or list.
> 
> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> 
Do you want to burn an option letter on that?  It seems like it might be better
to search the string for 0x and base the selection of bitmap of list parsing
based on its presence or absence.


> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  app/test/test_eal_flags.c                  | 28 ++++++++++-
>  lib/librte_eal/common/eal_common_options.c | 81 ++++++++++++++++++++++++++++--
>  2 files changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> index 1f95d7f..5ad89c5 100644
> --- a/app/test/test_eal_flags.c
> +++ b/app/test/test_eal_flags.c
> @@ -484,7 +484,7 @@ test_invalid_r_flag(void)
>  }
>  
>  /*
> - * Test that the app doesn't run without the coremask flag. In all cases
> + * Test that the app doesn't run without the coremask/corelist flags. In all cases
>   * should give an error and fail to run
>   */
>  static int
> @@ -504,12 +504,22 @@ test_missing_c_flag(void)
>  
>  	/* -c flag but no coremask value */
>  	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
> -	/* No -c flag at all */
> +	/* No -c or -l flag at all */
>  	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
>  	/* bad coremask value */
>  	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "error" };
>  	/* sanity check of tests - valid coremask value */
>  	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "1" };
> +	/* -l flag but no corelist value */
> +	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "3", "-l"};
> +	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "3", "-l", " " };
> +	/* bad corelist values */
> +	const char *argv7[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "error" };
> +	const char *argv8[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-" };
> +	const char *argv9[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1," };
> +	const char *argv10[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1#2" };
> +	/* sanity check test - valid corelist value */
> +	const char *argv11[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-2,3" };
>  
>  	if (launch_proc(argv1) == 0
>  			|| launch_proc(argv2) == 0
> @@ -521,6 +531,20 @@ test_missing_c_flag(void)
>  		printf("Error - process did not run ok with valid coremask value\n");
>  		return -1;
>  	}
> +
> +	if (launch_proc(argv5) == 0
> +			|| launch_proc(argv6) == 0
> +			|| launch_proc(argv7) == 0
> +			|| launch_proc(argv8) == 0
> +			|| launch_proc(argv9) == 0
> +			|| launch_proc(argv10) == 0) {
> +		printf("Error - process ran without error with invalid -l flag\n");
> +		return -1;
> +	}
> +	if (launch_proc(argv11) != 0) {
> +		printf("Error - process did not run ok with valid corelist value\n");
> +		return -1;
> +	}
>  	return 0;
>  }
>  
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 63710b0..18d03e3 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -32,6 +32,7 @@
>   */
>  
>  #include <stdlib.h>
> +#include <unistd.h>
>  #include <string.h>
>  #include <syslog.h>
>  #include <ctype.h>
> @@ -55,8 +56,9 @@ const char
>  eal_short_options[] =
>  	"b:" /* pci-blacklist */
>  	"w:" /* pci-whitelist */
> -	"c:"
> +	"c:" /* coremask */
>  	"d:"
> +	"l:" /* corelist */
>  	"m:"
>  	"n:"
>  	"r:"
> @@ -205,6 +207,67 @@ eal_parse_coremask(const char *coremask)
>  }
>  
>  static int
> +eal_parse_corelist(const char *corelist)
> +{
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +	int i, idx = 0;
> +	unsigned count = 0;
> +	char *end = NULL;
> +	int min, max;
> +
> +	if (corelist == NULL)
> +		return -1;
> +
> +	/* Remove all blank characters ahead and after */
> +	while (isblank(*corelist))
> +		corelist++;
> +	i = strnlen(corelist, sysconf(_SC_ARG_MAX));
> +	while ((i > 0) && isblank(corelist[i - 1]))
> +		i--;
> +
> +	/* Reset core roles */
> +	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
> +		cfg->lcore_role[idx] = ROLE_OFF;
> +
> +	/* Get list of cores */
> +	min = RTE_MAX_LCORE;
> +	do {
> +		while (isblank(*corelist))
> +			corelist++;
> +		if (*corelist == '\0')
> +			return -1;
> +		errno = 0;
> +		idx = strtoul(corelist, &end, 10);
> +		if (errno || end == NULL)
> +			return -1;
> +		while (isblank(*end))
> +			end++;
> +		if (*end == '-') {
> +			min = idx;
> +		} else if ((*end == ',') || (*end == '\0')) {
> +			max = idx;
> +			if (min == RTE_MAX_LCORE)
> +				min = idx;
> +			for (idx = min; idx <= max; idx++) {
> +				cfg->lcore_role[idx] = ROLE_RTE;
> +				if (count == 0)
> +					cfg->master_lcore = idx;
> +				count++;
> +			}
> +			min = RTE_MAX_LCORE;
> +		} else
> +			return -1;
> +		corelist = end + 1;
> +	} while (*end != '\0');
> +
> +	if (count == 0)
> +		return -1;
> +
> +	lcores_parsed = 1;
> +	return 0;
> +}
> +
> +static int
>  eal_parse_syslog(const char *facility, struct internal_config *conf)
>  {
>  	int i;
> @@ -304,6 +367,13 @@ eal_parse_common_option(int opt, const char *optarg,
>  			return -1;
>  		}
>  		break;
> +	/* corelist */
> +	case 'l':
> +		if (eal_parse_corelist(optarg) < 0) {
> +			RTE_LOG(ERR, EAL, "invalid core list\n");
> +			return -1;
> +		}
> +		break;
>  	/* size of memory */
>  	case 'm':
>  		conf->memory = atoi(optarg);
> @@ -421,8 +491,8 @@ eal_check_common_options(struct internal_config *internal_cfg)
>  	struct rte_config *cfg = rte_eal_get_configuration();
>  
>  	if (!lcores_parsed) {
> -		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
> -			"-c\n");
> +		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> +			"-c or -l\n");
>  		return -1;
>  	}
>  
> @@ -470,6 +540,9 @@ eal_common_usage(void)
>  	       "[--proc-type primary|secondary|auto]\n\n"
>  	       "EAL common options:\n"
>  	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
> +	       "  -l CORELIST  : List of cores to run on\n"
> +	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> +	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
>  	       "  -n NUM       : Number of memory channels\n"
>  	       "  -v           : Display version information on startup\n"
>  	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
> @@ -494,5 +567,5 @@ eal_common_usage(void)
>  	       "  --"OPT_NO_PCI"   : disable pci\n"
>  	       "  --"OPT_NO_HPET"  : disable hpet\n"
>  	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
> -	       "\n");
> +	       "\n", RTE_MAX_LCORE);
>  }
> -- 
> 2.1.3
> 
> 

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Bruce Richardson]

On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
> On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> > From: Didier Pallard <didier.pallard@6wind.com>
> > 
> > In current version, used cores can only be specified using a bitmask.
> > It will now be possible to specify cores in 2 different ways:
> > - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> > - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> > 
> > The letter -l can stand for lcore or list.
> > 
> > -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> > 
> Do you want to burn an option letter on that?  It seems like it might be better
> to search the string for 0x and base the selection of bitmap of list parsing
> based on its presence or absence.
> 
>

The existing coremask parsing always assumes a hex coremask, so just looking
for a 0x will not work. I prefer this scheme of using a new flag for this method
of specifying the cores to use. 

If you don't want to use up a single-letter option, two alternatives:
1) use a long option instead.
2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
otherwise treat as old. The only abiguity here would be for specifying a single
core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
[0 is obviously a named core as it's an invalid mask, and A-F are obviously
masks.] If we did want this scheme, I would suggest that we allow trailing
commas in the list specifier, so we can force users to clear ambiguity by
either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
However, this is probably more work that it's worth to avoid using up a letter
option.

I'd prefer any of these options to breaking backward compatibility in this case.

/Bruce


> > Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > ---
> >  app/test/test_eal_flags.c                  | 28 ++++++++++-
> >  lib/librte_eal/common/eal_common_options.c | 81 ++++++++++++++++++++++++++++--
> >  2 files changed, 103 insertions(+), 6 deletions(-)
> > 
> > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> > index 1f95d7f..5ad89c5 100644
> > --- a/app/test/test_eal_flags.c
> > +++ b/app/test/test_eal_flags.c
> > @@ -484,7 +484,7 @@ test_invalid_r_flag(void)
> >  }
> >  
> >  /*
> > - * Test that the app doesn't run without the coremask flag. In all cases
> > + * Test that the app doesn't run without the coremask/corelist flags. In all cases
> >   * should give an error and fail to run
> >   */
> >  static int
> > @@ -504,12 +504,22 @@ test_missing_c_flag(void)
> >  
> >  	/* -c flag but no coremask value */
> >  	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
> > -	/* No -c flag at all */
> > +	/* No -c or -l flag at all */
> >  	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
> >  	/* bad coremask value */
> >  	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "error" };
> >  	/* sanity check of tests - valid coremask value */
> >  	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "1" };
> > +	/* -l flag but no corelist value */
> > +	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "3", "-l"};
> > +	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "3", "-l", " " };
> > +	/* bad corelist values */
> > +	const char *argv7[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "error" };
> > +	const char *argv8[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-" };
> > +	const char *argv9[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1," };
> > +	const char *argv10[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1#2" };
> > +	/* sanity check test - valid corelist value */
> > +	const char *argv11[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-2,3" };
> >  
> >  	if (launch_proc(argv1) == 0
> >  			|| launch_proc(argv2) == 0
> > @@ -521,6 +531,20 @@ test_missing_c_flag(void)
> >  		printf("Error - process did not run ok with valid coremask value\n");
> >  		return -1;
> >  	}
> > +
> > +	if (launch_proc(argv5) == 0
> > +			|| launch_proc(argv6) == 0
> > +			|| launch_proc(argv7) == 0
> > +			|| launch_proc(argv8) == 0
> > +			|| launch_proc(argv9) == 0
> > +			|| launch_proc(argv10) == 0) {
> > +		printf("Error - process ran without error with invalid -l flag\n");
> > +		return -1;
> > +	}
> > +	if (launch_proc(argv11) != 0) {
> > +		printf("Error - process did not run ok with valid corelist value\n");
> > +		return -1;
> > +	}
> >  	return 0;
> >  }
> >  
> > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > index 63710b0..18d03e3 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -32,6 +32,7 @@
> >   */
> >  
> >  #include <stdlib.h>
> > +#include <unistd.h>
> >  #include <string.h>
> >  #include <syslog.h>
> >  #include <ctype.h>
> > @@ -55,8 +56,9 @@ const char
> >  eal_short_options[] =
> >  	"b:" /* pci-blacklist */
> >  	"w:" /* pci-whitelist */
> > -	"c:"
> > +	"c:" /* coremask */
> >  	"d:"
> > +	"l:" /* corelist */
> >  	"m:"
> >  	"n:"
> >  	"r:"
> > @@ -205,6 +207,67 @@ eal_parse_coremask(const char *coremask)
> >  }
> >  
> >  static int
> > +eal_parse_corelist(const char *corelist)
> > +{
> > +	struct rte_config *cfg = rte_eal_get_configuration();
> > +	int i, idx = 0;
> > +	unsigned count = 0;
> > +	char *end = NULL;
> > +	int min, max;
> > +
> > +	if (corelist == NULL)
> > +		return -1;
> > +
> > +	/* Remove all blank characters ahead and after */
> > +	while (isblank(*corelist))
> > +		corelist++;
> > +	i = strnlen(corelist, sysconf(_SC_ARG_MAX));
> > +	while ((i > 0) && isblank(corelist[i - 1]))
> > +		i--;
> > +
> > +	/* Reset core roles */
> > +	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
> > +		cfg->lcore_role[idx] = ROLE_OFF;
> > +
> > +	/* Get list of cores */
> > +	min = RTE_MAX_LCORE;
> > +	do {
> > +		while (isblank(*corelist))
> > +			corelist++;
> > +		if (*corelist == '\0')
> > +			return -1;
> > +		errno = 0;
> > +		idx = strtoul(corelist, &end, 10);
> > +		if (errno || end == NULL)
> > +			return -1;
> > +		while (isblank(*end))
> > +			end++;
> > +		if (*end == '-') {
> > +			min = idx;
> > +		} else if ((*end == ',') || (*end == '\0')) {
> > +			max = idx;
> > +			if (min == RTE_MAX_LCORE)
> > +				min = idx;
> > +			for (idx = min; idx <= max; idx++) {
> > +				cfg->lcore_role[idx] = ROLE_RTE;
> > +				if (count == 0)
> > +					cfg->master_lcore = idx;
> > +				count++;
> > +			}
> > +			min = RTE_MAX_LCORE;
> > +		} else
> > +			return -1;
> > +		corelist = end + 1;
> > +	} while (*end != '\0');
> > +
> > +	if (count == 0)
> > +		return -1;
> > +
> > +	lcores_parsed = 1;
> > +	return 0;
> > +}
> > +
> > +static int
> >  eal_parse_syslog(const char *facility, struct internal_config *conf)
> >  {
> >  	int i;
> > @@ -304,6 +367,13 @@ eal_parse_common_option(int opt, const char *optarg,
> >  			return -1;
> >  		}
> >  		break;
> > +	/* corelist */
> > +	case 'l':
> > +		if (eal_parse_corelist(optarg) < 0) {
> > +			RTE_LOG(ERR, EAL, "invalid core list\n");
> > +			return -1;
> > +		}
> > +		break;
> >  	/* size of memory */
> >  	case 'm':
> >  		conf->memory = atoi(optarg);
> > @@ -421,8 +491,8 @@ eal_check_common_options(struct internal_config *internal_cfg)
> >  	struct rte_config *cfg = rte_eal_get_configuration();
> >  
> >  	if (!lcores_parsed) {
> > -		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
> > -			"-c\n");
> > +		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> > +			"-c or -l\n");
> >  		return -1;
> >  	}
> >  
> > @@ -470,6 +540,9 @@ eal_common_usage(void)
> >  	       "[--proc-type primary|secondary|auto]\n\n"
> >  	       "EAL common options:\n"
> >  	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
> > +	       "  -l CORELIST  : List of cores to run on\n"
> > +	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> > +	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
> >  	       "  -n NUM       : Number of memory channels\n"
> >  	       "  -v           : Display version information on startup\n"
> >  	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
> > @@ -494,5 +567,5 @@ eal_common_usage(void)
> >  	       "  --"OPT_NO_PCI"   : disable pci\n"
> >  	       "  --"OPT_NO_HPET"  : disable hpet\n"
> >  	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
> > -	       "\n");
> > +	       "\n", RTE_MAX_LCORE);
> >  }
> > -- 
> > 2.1.3
> > 
> > 

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Thomas Monjalon]

Hi Bruce and Neil,

2014-11-24 11:28, Bruce Richardson:
> On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
> > On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> > > From: Didier Pallard <didier.pallard@6wind.com>
> > > 
> > > In current version, used cores can only be specified using a bitmask.
> > > It will now be possible to specify cores in 2 different ways:
> > > - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> > > - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> > > 
> > > The letter -l can stand for lcore or list.
> > > 
> > > -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> > 
> > Do you want to burn an option letter on that?  It seems like it might be better
> > to search the string for 0x and base the selection of bitmap of list parsing
> > based on its presence or absence.

It was the initial proposal (in April):
	http://dpdk.org/ml/archives/dev/2014-April/002173.html
And I liked keeping only 1 option;
	http://dpdk.org/ml/archives/dev/2014-May/002722.html
But Anatoly raised the compatibility problem:
	http://dpdk.org/ml/archives/dev/2014-May/002723.html
Then there was no other comment so Didier and I reworked a separate option. 

> The existing coremask parsing always assumes a hex coremask, so just looking
> for a 0x will not work. I prefer this scheme of using a new flag for this method
> of specifying the cores to use. 
> 
> If you don't want to use up a single-letter option, two alternatives:
> 1) use a long option instead.
> 2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
> otherwise treat as old. The only abiguity here would be for specifying a single
> core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
> [0 is obviously a named core as it's an invalid mask, and A-F are obviously
> masks.] If we did want this scheme, I would suggest that we allow trailing
> commas in the list specifier, so we can force users to clear ambiguity by
> either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
> However, this is probably more work that it's worth to avoid using up a letter
> option.
> 
> I'd prefer any of these options to breaking backward compatibility in this case.

We need a consensus here.
Who is supporting a "burn" of an one-letter option with clear usage?
Who is supporting a "re-merge" of the 2 syntaxes with more complicated rules
(list syntax is triggered by presence of "-" or ",")? 

Please vote quickly.
Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Bruce Richardson]

On Mon, Nov 24, 2014 at 02:19:16PM +0100, Thomas Monjalon wrote:
> Hi Bruce and Neil,
> 
> 2014-11-24 11:28, Bruce Richardson:
> > On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
> > > On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> > > > From: Didier Pallard <didier.pallard@6wind.com>
> > > > 
> > > > In current version, used cores can only be specified using a bitmask.
> > > > It will now be possible to specify cores in 2 different ways:
> > > > - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> > > > - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> > > > 
> > > > The letter -l can stand for lcore or list.
> > > > 
> > > > -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> > > 
> > > Do you want to burn an option letter on that?  It seems like it might be better
> > > to search the string for 0x and base the selection of bitmap of list parsing
> > > based on its presence or absence.
> 
> It was the initial proposal (in April):
> 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
> And I liked keeping only 1 option;
> 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
> But Anatoly raised the compatibility problem:
> 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
> Then there was no other comment so Didier and I reworked a separate option. 
> 
> > The existing coremask parsing always assumes a hex coremask, so just looking
> > for a 0x will not work. I prefer this scheme of using a new flag for this method
> > of specifying the cores to use. 
> > 
> > If you don't want to use up a single-letter option, two alternatives:
> > 1) use a long option instead.
> > 2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
> > otherwise treat as old. The only abiguity here would be for specifying a single
> > core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
> > [0 is obviously a named core as it's an invalid mask, and A-F are obviously
> > masks.] If we did want this scheme, I would suggest that we allow trailing
> > commas in the list specifier, so we can force users to clear ambiguity by
> > either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
> > However, this is probably more work that it's worth to avoid using up a letter
> > option.
> > 
> > I'd prefer any of these options to breaking backward compatibility in this case.
> 
> We need a consensus here.
> Who is supporting a "burn" of an one-letter option with clear usage?
> Who is supporting a "re-merge" of the 2 syntaxes with more complicated rules
> (list syntax is triggered by presence of "-" or ",")? 
>

Burn!

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Burakov, Anatoly]

> > > > Do you want to burn an option letter on that?  It seems like it
> > > > might be better to search the string for 0x and base the selection
> > > > of bitmap of list parsing based on its presence or absence.
> >
> > It was the initial proposal (in April):
> > 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
> > And I liked keeping only 1 option;
> > 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
> > But Anatoly raised the compatibility problem:
> > 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
> > Then there was no other comment so Didier and I reworked a separate
> option.
> >
> > > The existing coremask parsing always assumes a hex coremask, so just
> > > looking for a 0x will not work. I prefer this scheme of using a new
> > > flag for this method of specifying the cores to use.
> > >
> > > If you don't want to use up a single-letter option, two alternatives:
> > > 1) use a long option instead.
> > > 2) if the -c parameter includes a "-" or a ",", treat it as a
> > > new-style option, otherwise treat as old. The only abiguity here
> > > would be for specifying a single core value 1-9 e.g. is "-c 6" a mask with
> two bits, or a single-core to run on.
> > > [0 is obviously a named core as it's an invalid mask, and A-F are
> > > obviously masks.] If we did want this scheme, I would suggest that
> > > we allow trailing commas in the list specifier, so we can force
> > > users to clear ambiguity by either writing "0x6" or "6," i.e. disallow
> ambiguous values to avoid problems.
> > > However, this is probably more work that it's worth to avoid using
> > > up a letter option.
> > >
> > > I'd prefer any of these options to breaking backward compatibility in this
> case.
> >
> > We need a consensus here.
> > Who is supporting a "burn" of an one-letter option with clear usage?
> > Who is supporting a "re-merge" of the 2 syntaxes with more complicated
> > rules (list syntax is triggered by presence of "-" or ",")?
> >
> 
> Burn!

I would still prefer a long option (we already have a coremask parameter, so another one is kind-of non-essential and IMO shouldn't belong in a scarce resource of one-letter parameters), but if everyone else agrees, the "burn" option is much more preferable to me than complicating syntax of an already existing parameter.

Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Neil Horman]

On Mon, Nov 24, 2014 at 01:37:03PM +0000, Burakov, Anatoly wrote:
> > > > > Do you want to burn an option letter on that?  It seems like it
> > > > > might be better to search the string for 0x and base the selection
> > > > > of bitmap of list parsing based on its presence or absence.
> > >
> > > It was the initial proposal (in April):
> > > 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
> > > And I liked keeping only 1 option;
> > > 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
> > > But Anatoly raised the compatibility problem:
> > > 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
> > > Then there was no other comment so Didier and I reworked a separate
> > option.
> > >
> > > > The existing coremask parsing always assumes a hex coremask, so just
> > > > looking for a 0x will not work. I prefer this scheme of using a new
> > > > flag for this method of specifying the cores to use.
> > > >
> > > > If you don't want to use up a single-letter option, two alternatives:
> > > > 1) use a long option instead.
> > > > 2) if the -c parameter includes a "-" or a ",", treat it as a
> > > > new-style option, otherwise treat as old. The only abiguity here
> > > > would be for specifying a single core value 1-9 e.g. is "-c 6" a mask with
> > two bits, or a single-core to run on.
> > > > [0 is obviously a named core as it's an invalid mask, and A-F are
> > > > obviously masks.] If we did want this scheme, I would suggest that
> > > > we allow trailing commas in the list specifier, so we can force
> > > > users to clear ambiguity by either writing "0x6" or "6," i.e. disallow
> > ambiguous values to avoid problems.
> > > > However, this is probably more work that it's worth to avoid using
> > > > up a letter option.
> > > >
> > > > I'd prefer any of these options to breaking backward compatibility in this
> > case.
> > >
> > > We need a consensus here.
> > > Who is supporting a "burn" of an one-letter option with clear usage?
> > > Who is supporting a "re-merge" of the 2 syntaxes with more complicated
> > > rules (list syntax is triggered by presence of "-" or ",")?
> > >
> > 
> > Burn!
> 
> I would still prefer a long option (we already have a coremask parameter, so another one is kind-of non-essential and IMO shouldn't belong in a scarce resource of one-letter parameters), but if everyone else agrees, the "burn" option is much more preferable to me than complicating syntax of an already existing parameter.
> 
If we can't support two syntaxes with a single option (which still seems
reasonable to me, but I digress), then I would support the additional long
option.  Short options are not only scarse on their own, but they also have to
potentially comingle with options parsed by an application building on the DPDK.
At least with a long option the chances of a conflict are lessened.
Neil

> Thanks,
> Anatoly
> 

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Venkatesan, Venky]

On 11/24/2014 5:28 AM, Bruce Richardson wrote:
> On Mon, Nov 24, 2014 at 02:19:16PM +0100, Thomas Monjalon wrote:
>> Hi Bruce and Neil,
>>
>> 2014-11-24 11:28, Bruce Richardson:
>>> On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
>>>> On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
>>>>> From: Didier Pallard <didier.pallard@6wind.com>
>>>>>
>>>>> In current version, used cores can only be specified using a bitmask.
>>>>> It will now be possible to specify cores in 2 different ways:
>>>>> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
>>>>> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
>>>>>
>>>>> The letter -l can stand for lcore or list.
>>>>>
>>>>> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
>>>> Do you want to burn an option letter on that?  It seems like it might be better
>>>> to search the string for 0x and base the selection of bitmap of list parsing
>>>> based on its presence or absence.
>> It was the initial proposal (in April):
>> 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
>> And I liked keeping only 1 option;
>> 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
>> But Anatoly raised the compatibility problem:
>> 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
>> Then there was no other comment so Didier and I reworked a separate option.
>>
>>> The existing coremask parsing always assumes a hex coremask, so just looking
>>> for a 0x will not work. I prefer this scheme of using a new flag for this method
>>> of specifying the cores to use.
>>>
>>> If you don't want to use up a single-letter option, two alternatives:
>>> 1) use a long option instead.
>>> 2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
>>> otherwise treat as old. The only abiguity here would be for specifying a single
>>> core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
>>> [0 is obviously a named core as it's an invalid mask, and A-F are obviously
>>> masks.] If we did want this scheme, I would suggest that we allow trailing
>>> commas in the list specifier, so we can force users to clear ambiguity by
>>> either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
>>> However, this is probably more work that it's worth to avoid using up a letter
>>> option.
>>>
>>> I'd prefer any of these options to breaking backward compatibility in this case.
>> We need a consensus here.
>> Who is supporting a "burn" of an one-letter option with clear usage?
>> Who is supporting a "re-merge" of the 2 syntaxes with more complicated rules
>> (list syntax is triggered by presence of "-" or ",")?
>>
> Burn!
Burn ^ 2 ;)

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Roger Keith Wiles]

Burn, it is not like we are going to add a huge number of new options in the future and run out of letters.

> On Nov 24, 2014, at 8:52 AM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
> 
> 
> On 11/24/2014 5:28 AM, Bruce Richardson wrote:
>> On Mon, Nov 24, 2014 at 02:19:16PM +0100, Thomas Monjalon wrote:
>>> Hi Bruce and Neil,
>>> 
>>> 2014-11-24 11:28, Bruce Richardson:
>>>> On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
>>>>> On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
>>>>>> From: Didier Pallard <didier.pallard@6wind.com>
>>>>>> 
>>>>>> In current version, used cores can only be specified using a bitmask.
>>>>>> It will now be possible to specify cores in 2 different ways:
>>>>>> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
>>>>>> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
>>>>>> 
>>>>>> The letter -l can stand for lcore or list.
>>>>>> 
>>>>>> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
>>>>> Do you want to burn an option letter on that?  It seems like it might be better
>>>>> to search the string for 0x and base the selection of bitmap of list parsing
>>>>> based on its presence or absence.
>>> It was the initial proposal (in April):
>>> 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
>>> And I liked keeping only 1 option;
>>> 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
>>> But Anatoly raised the compatibility problem:
>>> 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
>>> Then there was no other comment so Didier and I reworked a separate option.
>>> 
>>>> The existing coremask parsing always assumes a hex coremask, so just looking
>>>> for a 0x will not work. I prefer this scheme of using a new flag for this method
>>>> of specifying the cores to use.
>>>> 
>>>> If you don't want to use up a single-letter option, two alternatives:
>>>> 1) use a long option instead.
>>>> 2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
>>>> otherwise treat as old. The only abiguity here would be for specifying a single
>>>> core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
>>>> [0 is obviously a named core as it's an invalid mask, and A-F are obviously
>>>> masks.] If we did want this scheme, I would suggest that we allow trailing
>>>> commas in the list specifier, so we can force users to clear ambiguity by
>>>> either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
>>>> However, this is probably more work that it's worth to avoid using up a letter
>>>> option.
>>>> 
>>>> I'd prefer any of these options to breaking backward compatibility in this case.
>>> We need a consensus here.
>>> Who is supporting a "burn" of an one-letter option with clear usage?
>>> Who is supporting a "re-merge" of the 2 syntaxes with more complicated rules
>>> (list syntax is triggered by presence of "-" or ",")?
>>> 
>> Burn!
> Burn ^ 2 ;)

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Neil Horman]

On Mon, Nov 24, 2014 at 10:12:33AM -0600, Roger Keith Wiles wrote:
> Burn, it is not like we are going to add a huge number of new options in the future and run out of letters.
> 
No, but what about the application authors that need to accomodate all of the
dpdk command line options as well?
Neil

> > On Nov 24, 2014, at 8:52 AM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
> > 
> > 
> > On 11/24/2014 5:28 AM, Bruce Richardson wrote:
> >> On Mon, Nov 24, 2014 at 02:19:16PM +0100, Thomas Monjalon wrote:
> >>> Hi Bruce and Neil,
> >>> 
> >>> 2014-11-24 11:28, Bruce Richardson:
> >>>> On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
> >>>>> On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> >>>>>> From: Didier Pallard <didier.pallard@6wind.com>
> >>>>>> 
> >>>>>> In current version, used cores can only be specified using a bitmask.
> >>>>>> It will now be possible to specify cores in 2 different ways:
> >>>>>> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> >>>>>> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> >>>>>> 
> >>>>>> The letter -l can stand for lcore or list.
> >>>>>> 
> >>>>>> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> >>>>> Do you want to burn an option letter on that?  It seems like it might be better
> >>>>> to search the string for 0x and base the selection of bitmap of list parsing
> >>>>> based on its presence or absence.
> >>> It was the initial proposal (in April):
> >>> 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
> >>> And I liked keeping only 1 option;
> >>> 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
> >>> But Anatoly raised the compatibility problem:
> >>> 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
> >>> Then there was no other comment so Didier and I reworked a separate option.
> >>> 
> >>>> The existing coremask parsing always assumes a hex coremask, so just looking
> >>>> for a 0x will not work. I prefer this scheme of using a new flag for this method
> >>>> of specifying the cores to use.
> >>>> 
> >>>> If you don't want to use up a single-letter option, two alternatives:
> >>>> 1) use a long option instead.
> >>>> 2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
> >>>> otherwise treat as old. The only abiguity here would be for specifying a single
> >>>> core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
> >>>> [0 is obviously a named core as it's an invalid mask, and A-F are obviously
> >>>> masks.] If we did want this scheme, I would suggest that we allow trailing
> >>>> commas in the list specifier, so we can force users to clear ambiguity by
> >>>> either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
> >>>> However, this is probably more work that it's worth to avoid using up a letter
> >>>> option.
> >>>> 
> >>>> I'd prefer any of these options to breaking backward compatibility in this case.
> >>> We need a consensus here.
> >>> Who is supporting a "burn" of an one-letter option with clear usage?
> >>> Who is supporting a "re-merge" of the 2 syntaxes with more complicated rules
> >>> (list syntax is triggered by presence of "-" or ",")?
> >>> 
> >> Burn!
> > Burn ^ 2 ;)
> 
> 

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Roger Keith Wiles]

> On Nov 24, 2014, at 11:04 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Mon, Nov 24, 2014 at 10:12:33AM -0600, Roger Keith Wiles wrote:
>> Burn, it is not like we are going to add a huge number of new options in the future and run out of letters.
>> 
> No, but what about the application authors that need to accomodate all of the
> dpdk command line options as well?

The application authors are not effected. The application authors can use any options after the ‘--‘ as DPDK does not define these options correct except in the example applications.

> Neil
> 
>>> On Nov 24, 2014, at 8:52 AM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
>>> 
>>> 
>>> On 11/24/2014 5:28 AM, Bruce Richardson wrote:
>>>> On Mon, Nov 24, 2014 at 02:19:16PM +0100, Thomas Monjalon wrote:
>>>>> Hi Bruce and Neil,
>>>>> 
>>>>> 2014-11-24 11:28, Bruce Richardson:
>>>>>> On Sat, Nov 22, 2014 at 08:35:17PM -0500, Neil Horman wrote:
>>>>>>> On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
>>>>>>>> From: Didier Pallard <didier.pallard@6wind.com>
>>>>>>>> 
>>>>>>>> In current version, used cores can only be specified using a bitmask.
>>>>>>>> It will now be possible to specify cores in 2 different ways:
>>>>>>>> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
>>>>>>>> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
>>>>>>>> 
>>>>>>>> The letter -l can stand for lcore or list.
>>>>>>>> 
>>>>>>>> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
>>>>>>> Do you want to burn an option letter on that?  It seems like it might be better
>>>>>>> to search the string for 0x and base the selection of bitmap of list parsing
>>>>>>> based on its presence or absence.
>>>>> It was the initial proposal (in April):
>>>>> 	http://dpdk.org/ml/archives/dev/2014-April/002173.html
>>>>> And I liked keeping only 1 option;
>>>>> 	http://dpdk.org/ml/archives/dev/2014-May/002722.html
>>>>> But Anatoly raised the compatibility problem:
>>>>> 	http://dpdk.org/ml/archives/dev/2014-May/002723.html
>>>>> Then there was no other comment so Didier and I reworked a separate option.
>>>>> 
>>>>>> The existing coremask parsing always assumes a hex coremask, so just looking
>>>>>> for a 0x will not work. I prefer this scheme of using a new flag for this method
>>>>>> of specifying the cores to use.
>>>>>> 
>>>>>> If you don't want to use up a single-letter option, two alternatives:
>>>>>> 1) use a long option instead.
>>>>>> 2) if the -c parameter includes a "-" or a ",", treat it as a new-style option,
>>>>>> otherwise treat as old. The only abiguity here would be for specifying a single
>>>>>> core value 1-9 e.g. is "-c 6" a mask with two bits, or a single-core to run on.
>>>>>> [0 is obviously a named core as it's an invalid mask, and A-F are obviously
>>>>>> masks.] If we did want this scheme, I would suggest that we allow trailing
>>>>>> commas in the list specifier, so we can force users to clear ambiguity by
>>>>>> either writing "0x6" or "6," i.e. disallow ambiguous values to avoid problems.
>>>>>> However, this is probably more work that it's worth to avoid using up a letter
>>>>>> option.
>>>>>> 
>>>>>> I'd prefer any of these options to breaking backward compatibility in this case.
>>>>> We need a consensus here.
>>>>> Who is supporting a "burn" of an one-letter option with clear usage?
>>>>> Who is supporting a "re-merge" of the 2 syntaxes with more complicated rules
>>>>> (list syntax is triggered by presence of "-" or ",")?
>>>>> 
>>>> Burn!
>>> Burn ^ 2 ;)
>> 
>> 

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Burakov, Anatoly]

> On Mon, Nov 24, 2014 at 10:12:33AM -0600, Roger Keith Wiles wrote:
> > Burn, it is not like we are going to add a huge number of new options in the
> future and run out of letters.
> >
> No, but what about the application authors that need to accomodate all of
> the dpdk command line options as well?
> Neil

Hi Neil

I don't think that's a problem as all those parameters are separated with a --. I.e. ./testpmd -c f -n 2 -- <testpmd parameters>. Is that not the case?

Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Neil Horman]

On Mon, Nov 24, 2014 at 05:11:16PM +0000, Burakov, Anatoly wrote:
> > On Mon, Nov 24, 2014 at 10:12:33AM -0600, Roger Keith Wiles wrote:
> > > Burn, it is not like we are going to add a huge number of new options in the
> > future and run out of letters.
> > >
> > No, but what about the application authors that need to accomodate all of
> > the dpdk command line options as well?
> > Neil
> 
> Hi Neil
> 
> I don't think that's a problem as all those parameters are separated with a --. I.e. ./testpmd -c f -n 2 -- <testpmd parameters>. Is that not the case?
> 
I don't know, I've not tried, though I'm still not a fan of having a command
line that takes the form:

<application> -c 0xnnnn -- -c ...

That just looks confusing to me.
Neil

> Thanks,
> Anatoly
> 

Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> From: Didier Pallard <didier.pallard@6wind.com>
> 
> In current version, used cores can only be specified using a bitmask.
> It will now be possible to specify cores in 2 different ways:
> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> 
> The letter -l can stand for lcore or list.
> 
> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> 
> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Majority of people seem happy enough using the -l flag, so

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  app/test/test_eal_flags.c                  | 28 ++++++++++-
>  lib/librte_eal/common/eal_common_options.c | 81 ++++++++++++++++++++++++++++--
>  2 files changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> index 1f95d7f..5ad89c5 100644
> --- a/app/test/test_eal_flags.c
> +++ b/app/test/test_eal_flags.c
> @@ -484,7 +484,7 @@ test_invalid_r_flag(void)
>  }
>  
>  /*
> - * Test that the app doesn't run without the coremask flag. In all cases
> + * Test that the app doesn't run without the coremask/corelist flags. In all cases
>   * should give an error and fail to run
>   */
>  static int
> @@ -504,12 +504,22 @@ test_missing_c_flag(void)
>  
>  	/* -c flag but no coremask value */
>  	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
> -	/* No -c flag at all */
> +	/* No -c or -l flag at all */
>  	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
>  	/* bad coremask value */
>  	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "error" };
>  	/* sanity check of tests - valid coremask value */
>  	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "1" };
> +	/* -l flag but no corelist value */
> +	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "3", "-l"};
> +	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "3", "-l", " " };
> +	/* bad corelist values */
> +	const char *argv7[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "error" };
> +	const char *argv8[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-" };
> +	const char *argv9[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1," };
> +	const char *argv10[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1#2" };
> +	/* sanity check test - valid corelist value */
> +	const char *argv11[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-2,3" };
>  
>  	if (launch_proc(argv1) == 0
>  			|| launch_proc(argv2) == 0
> @@ -521,6 +531,20 @@ test_missing_c_flag(void)
>  		printf("Error - process did not run ok with valid coremask value\n");
>  		return -1;
>  	}
> +
> +	if (launch_proc(argv5) == 0
> +			|| launch_proc(argv6) == 0
> +			|| launch_proc(argv7) == 0
> +			|| launch_proc(argv8) == 0
> +			|| launch_proc(argv9) == 0
> +			|| launch_proc(argv10) == 0) {
> +		printf("Error - process ran without error with invalid -l flag\n");
> +		return -1;
> +	}
> +	if (launch_proc(argv11) != 0) {
> +		printf("Error - process did not run ok with valid corelist value\n");
> +		return -1;
> +	}
>  	return 0;
>  }
>  
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 63710b0..18d03e3 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -32,6 +32,7 @@
>   */
>  
>  #include <stdlib.h>
> +#include <unistd.h>
>  #include <string.h>
>  #include <syslog.h>
>  #include <ctype.h>
> @@ -55,8 +56,9 @@ const char
>  eal_short_options[] =
>  	"b:" /* pci-blacklist */
>  	"w:" /* pci-whitelist */
> -	"c:"
> +	"c:" /* coremask */
>  	"d:"
> +	"l:" /* corelist */
>  	"m:"
>  	"n:"
>  	"r:"
> @@ -205,6 +207,67 @@ eal_parse_coremask(const char *coremask)
>  }
>  
>  static int
> +eal_parse_corelist(const char *corelist)
> +{
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +	int i, idx = 0;
> +	unsigned count = 0;
> +	char *end = NULL;
> +	int min, max;
> +
> +	if (corelist == NULL)
> +		return -1;
> +
> +	/* Remove all blank characters ahead and after */
> +	while (isblank(*corelist))
> +		corelist++;
> +	i = strnlen(corelist, sysconf(_SC_ARG_MAX));
> +	while ((i > 0) && isblank(corelist[i - 1]))
> +		i--;
> +
> +	/* Reset core roles */
> +	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
> +		cfg->lcore_role[idx] = ROLE_OFF;
> +
> +	/* Get list of cores */
> +	min = RTE_MAX_LCORE;
> +	do {
> +		while (isblank(*corelist))
> +			corelist++;
> +		if (*corelist == '\0')
> +			return -1;
> +		errno = 0;
> +		idx = strtoul(corelist, &end, 10);
> +		if (errno || end == NULL)
> +			return -1;
> +		while (isblank(*end))
> +			end++;
> +		if (*end == '-') {
> +			min = idx;
> +		} else if ((*end == ',') || (*end == '\0')) {
> +			max = idx;
> +			if (min == RTE_MAX_LCORE)
> +				min = idx;
> +			for (idx = min; idx <= max; idx++) {
> +				cfg->lcore_role[idx] = ROLE_RTE;
> +				if (count == 0)
> +					cfg->master_lcore = idx;
> +				count++;
> +			}
> +			min = RTE_MAX_LCORE;
> +		} else
> +			return -1;
> +		corelist = end + 1;
> +	} while (*end != '\0');
> +
> +	if (count == 0)
> +		return -1;
> +
> +	lcores_parsed = 1;
> +	return 0;
> +}
> +
> +static int
>  eal_parse_syslog(const char *facility, struct internal_config *conf)
>  {
>  	int i;
> @@ -304,6 +367,13 @@ eal_parse_common_option(int opt, const char *optarg,
>  			return -1;
>  		}
>  		break;
> +	/* corelist */
> +	case 'l':
> +		if (eal_parse_corelist(optarg) < 0) {
> +			RTE_LOG(ERR, EAL, "invalid core list\n");
> +			return -1;
> +		}
> +		break;
>  	/* size of memory */
>  	case 'm':
>  		conf->memory = atoi(optarg);
> @@ -421,8 +491,8 @@ eal_check_common_options(struct internal_config *internal_cfg)
>  	struct rte_config *cfg = rte_eal_get_configuration();
>  
>  	if (!lcores_parsed) {
> -		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
> -			"-c\n");
> +		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> +			"-c or -l\n");
>  		return -1;
>  	}
>  
> @@ -470,6 +540,9 @@ eal_common_usage(void)
>  	       "[--proc-type primary|secondary|auto]\n\n"
>  	       "EAL common options:\n"
>  	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
> +	       "  -l CORELIST  : List of cores to run on\n"
> +	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> +	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
>  	       "  -n NUM       : Number of memory channels\n"
>  	       "  -v           : Display version information on startup\n"
>  	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
> @@ -494,5 +567,5 @@ eal_common_usage(void)
>  	       "  --"OPT_NO_PCI"   : disable pci\n"
>  	       "  --"OPT_NO_HPET"  : disable hpet\n"
>  	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
> -	       "\n");
> +	       "\n", RTE_MAX_LCORE);
>  }
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 08/10] config: support 128 cores

[Thomas Monjalon]

From: Didier Pallard <didier.pallard@6wind.com>

New platforms have more than 64 cores.
Now that cores can be specified with a list, we are not limited anymore to 64.
Set default max cores number to 128.

Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
Acked-by: David Marchand <david.marchand@6wind.com>
---
 config/common_bsdapp   | 2 +-
 config/common_linuxapp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/config/common_bsdapp b/config/common_bsdapp
index 57cad76..c4cab7b 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -88,7 +88,7 @@ CONFIG_RTE_LIBNAME=intel_dpdk
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
-CONFIG_RTE_MAX_LCORE=64
+CONFIG_RTE_MAX_LCORE=128
 CONFIG_RTE_MAX_NUMA_NODES=8
 CONFIG_RTE_MAX_MEMSEG=256
 CONFIG_RTE_MAX_MEMZONE=2560
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 57b61c9..63510ec 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -113,7 +113,7 @@ CONFIG_RTE_LIBGLOSS=n
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
-CONFIG_RTE_MAX_LCORE=64
+CONFIG_RTE_MAX_LCORE=128
 CONFIG_RTE_MAX_NUMA_NODES=8
 CONFIG_RTE_MAX_MEMSEG=256
 CONFIG_RTE_MAX_MEMZONE=2560
-- 
2.1.3

Re: [dpdk-dev] [PATCH 08/10] config: support 128 cores

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:40PM +0100, Thomas Monjalon wrote:
> From: Didier Pallard <didier.pallard@6wind.com>
> 
> New platforms have more than 64 cores.
> Now that cores can be specified with a list, we are not limited anymore to 64.
> Set default max cores number to 128.
> 
> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> Acked-by: David Marchand <david.marchand@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  config/common_bsdapp   | 2 +-
>  config/common_linuxapp | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/config/common_bsdapp b/config/common_bsdapp
> index 57cad76..c4cab7b 100644
> --- a/config/common_bsdapp
> +++ b/config/common_bsdapp
> @@ -88,7 +88,7 @@ CONFIG_RTE_LIBNAME=intel_dpdk
>  # Compile Environment Abstraction Layer
>  #
>  CONFIG_RTE_LIBRTE_EAL=y
> -CONFIG_RTE_MAX_LCORE=64
> +CONFIG_RTE_MAX_LCORE=128
>  CONFIG_RTE_MAX_NUMA_NODES=8
>  CONFIG_RTE_MAX_MEMSEG=256
>  CONFIG_RTE_MAX_MEMZONE=2560
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 57b61c9..63510ec 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -113,7 +113,7 @@ CONFIG_RTE_LIBGLOSS=n
>  # Compile Environment Abstraction Layer
>  #
>  CONFIG_RTE_LIBRTE_EAL=y
> -CONFIG_RTE_MAX_LCORE=64
> +CONFIG_RTE_MAX_LCORE=128
>  CONFIG_RTE_MAX_NUMA_NODES=8
>  CONFIG_RTE_MAX_MEMSEG=256
>  CONFIG_RTE_MAX_MEMZONE=2560
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 09/10] eal: get relative core index

[Thomas Monjalon]

From: Patrick Lu <Patrick.Lu@intel.com>

EAL -c option allows the user to enable any lcore in the system.
Often times, the user app wants to know 1st enabled core, 2nd
enabled core, etc, rather than phyical core ID (rte_lcore_id().)

The new API rte_lcore_index() will return an index from enabled lcores
starting from zero.

Signed-off-by: Patrick Lu <patrick.lu@intel.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/common/eal_common_options.c | 13 ++++++++++---
 lib/librte_eal/common/include/rte_lcore.h  | 20 ++++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 18d03e3..c9df8f5 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -185,19 +185,23 @@ eal_parse_coremask(const char *coremask)
 					return -1;
 				}
 				cfg->lcore_role[idx] = ROLE_RTE;
+				lcore_config[idx].core_index = count;
 				if (count == 0)
 					cfg->master_lcore = idx;
 				count++;
 			} else {
 				cfg->lcore_role[idx] = ROLE_OFF;
+				lcore_config[idx].core_index = -1;
 			}
 		}
 	}
 	for (; i >= 0; i--)
 		if (coremask[i] != '0')
 			return -1;
-	for (; idx < RTE_MAX_LCORE; idx++)
+	for (; idx < RTE_MAX_LCORE; idx++) {
 		cfg->lcore_role[idx] = ROLE_OFF;
+		lcore_config[idx].core_index = -1;
+	}
 	if (count == 0)
 		return -1;
 	/* Update the count of enabled logical cores of the EAL configuration */
@@ -225,9 +229,11 @@ eal_parse_corelist(const char *corelist)
 	while ((i > 0) && isblank(corelist[i - 1]))
 		i--;
 
-	/* Reset core roles */
-	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
+	/* Reset config */
+	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
 		cfg->lcore_role[idx] = ROLE_OFF;
+		lcore_config[idx].core_index = -1;
+	}
 
 	/* Get list of cores */
 	min = RTE_MAX_LCORE;
@@ -250,6 +256,7 @@ eal_parse_corelist(const char *corelist)
 				min = idx;
 			for (idx = min; idx <= max; idx++) {
 				cfg->lcore_role[idx] = ROLE_RTE;
+				lcore_config[idx].core_index = count;
 				if (count == 0)
 					cfg->master_lcore = idx;
 				count++;
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index a0b4356..49b2c03 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -64,6 +64,7 @@ struct lcore_config {
 	volatile enum rte_lcore_state_t state; /**< lcore state */
 	unsigned socket_id;        /**< physical socket id for this lcore */
 	unsigned core_id;          /**< core number on socket for this lcore */
+	int core_index;            /**< relative index, starting from 0 */
 };
 
 /**
@@ -110,6 +111,25 @@ rte_lcore_count(void)
 }
 
 /**
+ * Return the index of the lcore starting from zero.
+ * The order is physical or given by command line (-l option).
+ *
+ * @param lcore_id
+ *   The targeted lcore, or -1 for the current one.
+ * @return
+ *   The relative index, or -1 if not enabled.
+ */
+static inline int
+rte_lcore_index(int lcore_id)
+{
+	if (lcore_id >= RTE_MAX_LCORE)
+		return -1;
+	if (lcore_id < 0)
+		lcore_id = rte_lcore_id();
+	return lcore_config[lcore_id].core_index;
+}
+
+/**
  * Return the ID of the physical socket of the logical core we are
  * running on.
  * @return
-- 
2.1.3

Re: [dpdk-dev] [PATCH 09/10] eal: get relative core index

[Bruce Richardson]

On Sat, Nov 22, 2014 at 10:43:41PM +0100, Thomas Monjalon wrote:
> From: Patrick Lu <Patrick.Lu@intel.com>
> 
> EAL -c option allows the user to enable any lcore in the system.
> Often times, the user app wants to know 1st enabled core, 2nd
> enabled core, etc, rather than phyical core ID (rte_lcore_id().)
> 
> The new API rte_lcore_index() will return an index from enabled lcores
> starting from zero.
> 
> Signed-off-by: Patrick Lu <patrick.lu@intel.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

I think this is good as far as it goes, but I think an extra API to get the
lcore with the given index would be a great extension to this. That would
allow you to do:

rte_eal_remote_launch(func0, arg0, rte_lcore_with_index(0));
rte_eal_remote_launch(func1, arg1, rte_lcore_with_index(1));
...

However, as a good start, I think this should be applied for now, so...

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/common/eal_common_options.c | 13 ++++++++++---
>  lib/librte_eal/common/include/rte_lcore.h  | 20 ++++++++++++++++++++
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 18d03e3..c9df8f5 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -185,19 +185,23 @@ eal_parse_coremask(const char *coremask)
>  					return -1;
>  				}
>  				cfg->lcore_role[idx] = ROLE_RTE;
> +				lcore_config[idx].core_index = count;
>  				if (count == 0)
>  					cfg->master_lcore = idx;
>  				count++;
>  			} else {
>  				cfg->lcore_role[idx] = ROLE_OFF;
> +				lcore_config[idx].core_index = -1;
>  			}
>  		}
>  	}
>  	for (; i >= 0; i--)
>  		if (coremask[i] != '0')
>  			return -1;
> -	for (; idx < RTE_MAX_LCORE; idx++)
> +	for (; idx < RTE_MAX_LCORE; idx++) {
>  		cfg->lcore_role[idx] = ROLE_OFF;
> +		lcore_config[idx].core_index = -1;
> +	}
>  	if (count == 0)
>  		return -1;
>  	/* Update the count of enabled logical cores of the EAL configuration */
> @@ -225,9 +229,11 @@ eal_parse_corelist(const char *corelist)
>  	while ((i > 0) && isblank(corelist[i - 1]))
>  		i--;
>  
> -	/* Reset core roles */
> -	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
> +	/* Reset config */
> +	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
>  		cfg->lcore_role[idx] = ROLE_OFF;
> +		lcore_config[idx].core_index = -1;
> +	}
>  
>  	/* Get list of cores */
>  	min = RTE_MAX_LCORE;
> @@ -250,6 +256,7 @@ eal_parse_corelist(const char *corelist)
>  				min = idx;
>  			for (idx = min; idx <= max; idx++) {
>  				cfg->lcore_role[idx] = ROLE_RTE;
> +				lcore_config[idx].core_index = count;
>  				if (count == 0)
>  					cfg->master_lcore = idx;
>  				count++;
> diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
> index a0b4356..49b2c03 100644
> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -64,6 +64,7 @@ struct lcore_config {
>  	volatile enum rte_lcore_state_t state; /**< lcore state */
>  	unsigned socket_id;        /**< physical socket id for this lcore */
>  	unsigned core_id;          /**< core number on socket for this lcore */
> +	int core_index;            /**< relative index, starting from 0 */
>  };
>  
>  /**
> @@ -110,6 +111,25 @@ rte_lcore_count(void)
>  }
>  
>  /**
> + * Return the index of the lcore starting from zero.
> + * The order is physical or given by command line (-l option).
> + *
> + * @param lcore_id
> + *   The targeted lcore, or -1 for the current one.
> + * @return
> + *   The relative index, or -1 if not enabled.
> + */
> +static inline int
> +rte_lcore_index(int lcore_id)
> +{
> +	if (lcore_id >= RTE_MAX_LCORE)
> +		return -1;
> +	if (lcore_id < 0)
> +		lcore_id = rte_lcore_id();
> +	return lcore_config[lcore_id].core_index;
> +}
> +
> +/**
>   * Return the ID of the physical socket of the logical core we are
>   * running on.
>   * @return
> -- 
> 2.1.3
> 

[dpdk-dev] [PATCH 10/10] eal: add option --master-lcore

[Thomas Monjalon]

From: Simon Kuenzer <simon.kuenzer@neclab.eu>

Enable users to specify the lcore id that is used as master lcore.

Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test/test.c                            |  1 +
 app/test/test_eal_flags.c                  | 51 ++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_options.c | 39 ++++++++++++++++++++---
 lib/librte_eal/common/eal_options.h        |  2 ++
 4 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/app/test/test.c b/app/test/test.c
index 9bee6bb..2fecff5 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -82,6 +82,7 @@ do_recursive_call(void)
 	} actions[] =  {
 			{ "run_secondary_instances", test_mp_secondary },
 			{ "test_missing_c_flag", no_action },
+			{ "test_master_lcore_flag", no_action },
 			{ "test_missing_n_flag", no_action },
 			{ "test_no_hpet_flag", no_action },
 			{ "test_whitelist_flag", no_action },
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 5ad89c5..6a6ef08 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -549,6 +549,51 @@ test_missing_c_flag(void)
 }
 
 /*
+ * Test --master-lcore option with matching coremask
+ */
+static int
+test_master_lcore_flag(void)
+{
+#ifdef RTE_EXEC_ENV_BSDAPP
+	/* BSD target doesn't support prefixes at this point */
+	const char *prefix = "";
+#else
+	char prefix[PATH_MAX], tmp[PATH_MAX];
+	if (get_current_prefix(tmp, sizeof(tmp)) == NULL) {
+		printf("Error - unable to get current prefix!\n");
+		return -1;
+	}
+	snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
+#endif
+
+	/* --master-lcore flag but no value */
+	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore"};
+	/* --master-lcore flag with invalid value */
+	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "-1"};
+	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "X"};
+	/* master lcore not in coremask */
+	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "2"};
+	/* valid value */
+	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "1"};
+	/* valid value set before coremask */
+	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "1", "--master-lcore", "1", "-c", "3"};
+
+	if (launch_proc(argv1) == 0
+			|| launch_proc(argv2) == 0
+			|| launch_proc(argv3) == 0
+			|| launch_proc(argv4) == 0) {
+		printf("Error - process ran without error with wrong --master-lcore\n");
+		return -1;
+	}
+	if (launch_proc(argv5) != 0
+			|| launch_proc(argv6) != 0) {
+		printf("Error - process did not run ok with valid --master-lcore\n");
+		return -1;
+	}
+	return 0;
+}
+
+/*
  * Test that the app doesn't run without the -n flag. In all cases
  * should give an error and fail to run.
  * Since -n is not compulsory for MP, we instead use --no-huge and --no-shconf
@@ -1237,6 +1282,12 @@ test_eal_flags(void)
 		return ret;
 	}
 
+	ret = test_master_lcore_flag();
+	if (ret < 0) {
+		printf("Error in test_master_lcore_flag()\n");
+		return ret;
+	}
+
 	ret = test_missing_n_flag();
 	if (ret < 0) {
 		printf("Error in test_missing_n_flag()\n");
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index c9df8f5..54bd31d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -67,6 +67,7 @@ eal_short_options[] =
 const struct option
 eal_long_options[] = {
 	{OPT_HUGE_DIR, 1, 0, OPT_HUGE_DIR_NUM},
+	{OPT_MASTER_LCORE, 1, 0, OPT_MASTER_LCORE_NUM},
 	{OPT_PROC_TYPE, 1, 0, OPT_PROC_TYPE_NUM},
 	{OPT_NO_SHCONF, 0, 0, OPT_NO_SHCONF_NUM},
 	{OPT_NO_HPET, 0, 0, OPT_NO_HPET_NUM},
@@ -186,8 +187,6 @@ eal_parse_coremask(const char *coremask)
 				}
 				cfg->lcore_role[idx] = ROLE_RTE;
 				lcore_config[idx].core_index = count;
-				if (count == 0)
-					cfg->master_lcore = idx;
 				count++;
 			} else {
 				cfg->lcore_role[idx] = ROLE_OFF;
@@ -257,8 +256,6 @@ eal_parse_corelist(const char *corelist)
 			for (idx = min; idx <= max; idx++) {
 				cfg->lcore_role[idx] = ROLE_RTE;
 				lcore_config[idx].core_index = count;
-				if (count == 0)
-					cfg->master_lcore = idx;
 				count++;
 			}
 			min = RTE_MAX_LCORE;
@@ -274,6 +271,22 @@ eal_parse_corelist(const char *corelist)
 	return 0;
 }
 
+/* Changes the lcore id of the master thread */
+static int
+eal_parse_master_lcore(const char *arg)
+{
+	char *parsing_end;
+	struct rte_config *cfg = rte_eal_get_configuration();
+
+	errno = 0;
+	cfg->master_lcore = (uint32_t) strtol(arg, &parsing_end, 0);
+	if (errno || parsing_end[0] != 0)
+		return -1;
+	if (cfg->master_lcore >= RTE_MAX_LCORE)
+		return -1;
+	return 0;
+}
+
 static int
 eal_parse_syslog(const char *facility, struct internal_config *conf)
 {
@@ -439,6 +452,14 @@ eal_parse_common_option(int opt, const char *optarg,
 		conf->process_type = eal_parse_proc_type(optarg);
 		break;
 
+	case OPT_MASTER_LCORE_NUM:
+		if (eal_parse_master_lcore(optarg) < 0) {
+			RTE_LOG(ERR, EAL, "invalid parameter for --"
+					OPT_MASTER_LCORE "\n");
+			return -1;
+		}
+		break;
+
 	case OPT_VDEV_NUM:
 		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
 				optarg) < 0) {
@@ -480,10 +501,15 @@ int
 eal_adjust_config(struct internal_config *internal_cfg)
 {
 	int i;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	if (internal_config.process_type == RTE_PROC_AUTO)
 		internal_config.process_type = eal_proc_type_detect();
 
+	/* default master lcore is the first one */
+	if (cfg->master_lcore == 0)
+		cfg->master_lcore = rte_get_next_lcore(-1, 0, 0);
+
 	/* if no memory amounts were requested, this will result in 0 and
 	 * will be overridden later, right after eal_hugepage_info_init() */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
@@ -502,6 +528,10 @@ eal_check_common_options(struct internal_config *internal_cfg)
 			"-c or -l\n");
 		return -1;
 	}
+	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
+		RTE_LOG(ERR, EAL, "Master lcore is not enabled for DPDK\n");
+		return -1;
+	}
 
 	if (internal_cfg->process_type == RTE_PROC_INVALID) {
 		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
@@ -550,6 +580,7 @@ eal_common_usage(void)
 	       "  -l CORELIST  : List of cores to run on\n"
 	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
 	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
+	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
 	       "  -n NUM       : Number of memory channels\n"
 	       "  -v           : Display version information on startup\n"
 	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index f58965c..e476f8d 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -45,6 +45,8 @@ enum {
 	OPT_LONG_MIN_NUM = 256,
 #define OPT_HUGE_DIR    "huge-dir"
 	OPT_HUGE_DIR_NUM = OPT_LONG_MIN_NUM,
+#define OPT_MASTER_LCORE "master-lcore"
+	OPT_MASTER_LCORE_NUM,
 #define OPT_PROC_TYPE   "proc-type"
 	OPT_PROC_TYPE_NUM,
 #define OPT_NO_SHCONF   "no-shconf"
-- 
2.1.3

Re: [dpdk-dev] [PATCH 10/10] eal: add option --master-lcore

[Simon Kuenzer]

Hi Thomas,

thanks for your work. I have one (minor) comment for this patch that 
should be fixed in a later version.

Acknowledged.


Thanks,

Simon

On 22.11.2014 22:43, Thomas Monjalon wrote:
> From: Simon Kuenzer <simon.kuenzer@neclab.eu>
>
> Enable users to specify the lcore id that is used as master lcore.
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>   app/test/test.c                            |  1 +
>   app/test/test_eal_flags.c                  | 51 ++++++++++++++++++++++++++++++
>   lib/librte_eal/common/eal_common_options.c | 39 ++++++++++++++++++++---
>   lib/librte_eal/common/eal_options.h        |  2 ++
>   4 files changed, 89 insertions(+), 4 deletions(-)
>
> diff --git a/app/test/test.c b/app/test/test.c
> index 9bee6bb..2fecff5 100644
> --- a/app/test/test.c
> +++ b/app/test/test.c
> @@ -82,6 +82,7 @@ do_recursive_call(void)
>   	} actions[] =  {
>   			{ "run_secondary_instances", test_mp_secondary },
>   			{ "test_missing_c_flag", no_action },
> +			{ "test_master_lcore_flag", no_action },
>   			{ "test_missing_n_flag", no_action },
>   			{ "test_no_hpet_flag", no_action },
>   			{ "test_whitelist_flag", no_action },
> diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> index 5ad89c5..6a6ef08 100644
> --- a/app/test/test_eal_flags.c
> +++ b/app/test/test_eal_flags.c
> @@ -549,6 +549,51 @@ test_missing_c_flag(void)
>   }
>
>   /*
> + * Test --master-lcore option with matching coremask
> + */
> +static int
> +test_master_lcore_flag(void)
> +{
> +#ifdef RTE_EXEC_ENV_BSDAPP
> +	/* BSD target doesn't support prefixes at this point */
> +	const char *prefix = "";
> +#else
> +	char prefix[PATH_MAX], tmp[PATH_MAX];
> +	if (get_current_prefix(tmp, sizeof(tmp)) == NULL) {
> +		printf("Error - unable to get current prefix!\n");
> +		return -1;
> +	}
> +	snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
> +#endif
> +
> +	/* --master-lcore flag but no value */
> +	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore"};
> +	/* --master-lcore flag with invalid value */
> +	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "-1"};
> +	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "X"};
> +	/* master lcore not in coremask */
> +	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "2"};
> +	/* valid value */
> +	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "1", "-c", "3", "--master-lcore", "1"};
> +	/* valid value set before coremask */
> +	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "1", "--master-lcore", "1", "-c", "3"};
> +
> +	if (launch_proc(argv1) == 0
> +			|| launch_proc(argv2) == 0
> +			|| launch_proc(argv3) == 0
> +			|| launch_proc(argv4) == 0) {
> +		printf("Error - process ran without error with wrong --master-lcore\n");
> +		return -1;
> +	}
> +	if (launch_proc(argv5) != 0
> +			|| launch_proc(argv6) != 0) {
> +		printf("Error - process did not run ok with valid --master-lcore\n");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +/*
>    * Test that the app doesn't run without the -n flag. In all cases
>    * should give an error and fail to run.
>    * Since -n is not compulsory for MP, we instead use --no-huge and --no-shconf
> @@ -1237,6 +1282,12 @@ test_eal_flags(void)
>   		return ret;
>   	}
>
> +	ret = test_master_lcore_flag();
> +	if (ret < 0) {
> +		printf("Error in test_master_lcore_flag()\n");
> +		return ret;
> +	}
> +
>   	ret = test_missing_n_flag();
>   	if (ret < 0) {
>   		printf("Error in test_missing_n_flag()\n");
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index c9df8f5..54bd31d 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -67,6 +67,7 @@ eal_short_options[] =
>   const struct option
>   eal_long_options[] = {
>   	{OPT_HUGE_DIR, 1, 0, OPT_HUGE_DIR_NUM},
> +	{OPT_MASTER_LCORE, 1, 0, OPT_MASTER_LCORE_NUM},
>   	{OPT_PROC_TYPE, 1, 0, OPT_PROC_TYPE_NUM},
>   	{OPT_NO_SHCONF, 0, 0, OPT_NO_SHCONF_NUM},
>   	{OPT_NO_HPET, 0, 0, OPT_NO_HPET_NUM},
> @@ -186,8 +187,6 @@ eal_parse_coremask(const char *coremask)
>   				}
>   				cfg->lcore_role[idx] = ROLE_RTE;
>   				lcore_config[idx].core_index = count;
> -				if (count == 0)
> -					cfg->master_lcore = idx;
>   				count++;
>   			} else {
>   				cfg->lcore_role[idx] = ROLE_OFF;
> @@ -257,8 +256,6 @@ eal_parse_corelist(const char *corelist)
>   			for (idx = min; idx <= max; idx++) {
>   				cfg->lcore_role[idx] = ROLE_RTE;
>   				lcore_config[idx].core_index = count;
> -				if (count == 0)
> -					cfg->master_lcore = idx;
>   				count++;
>   			}
>   			min = RTE_MAX_LCORE;
> @@ -274,6 +271,22 @@ eal_parse_corelist(const char *corelist)
>   	return 0;
>   }
>
> +/* Changes the lcore id of the master thread */
> +static int
> +eal_parse_master_lcore(const char *arg)
> +{
> +	char *parsing_end;
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +
> +	errno = 0;
> +	cfg->master_lcore = (uint32_t) strtol(arg, &parsing_end, 0);
> +	if (errno || parsing_end[0] != 0)
> +		return -1;
> +	if (cfg->master_lcore >= RTE_MAX_LCORE)
> +		return -1;
> +	return 0;
> +}
> +
>   static int
>   eal_parse_syslog(const char *facility, struct internal_config *conf)
>   {
> @@ -439,6 +452,14 @@ eal_parse_common_option(int opt, const char *optarg,
>   		conf->process_type = eal_parse_proc_type(optarg);
>   		break;
>
> +	case OPT_MASTER_LCORE_NUM:
> +		if (eal_parse_master_lcore(optarg) < 0) {
> +			RTE_LOG(ERR, EAL, "invalid parameter for --"
> +					OPT_MASTER_LCORE "\n");
> +			return -1;
> +		}
> +		break;
> +
>   	case OPT_VDEV_NUM:
>   		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
>   				optarg) < 0) {
> @@ -480,10 +501,15 @@ int
>   eal_adjust_config(struct internal_config *internal_cfg)
>   {
>   	int i;
> +	struct rte_config *cfg = rte_eal_get_configuration();
>
>   	if (internal_config.process_type == RTE_PROC_AUTO)
>   		internal_config.process_type = eal_proc_type_detect();
>
> +	/* default master lcore is the first one */
> +	if (cfg->master_lcore == 0)
> +		cfg->master_lcore = rte_get_next_lcore(-1, 0, 0);
> +

Might be confusing if a user specifies --master-lcore 0 and uses a 
coremask/corelist where core id 0 is not specified.
What about setting cfg->master_lcore to (RTE_MAX_LCORE + 1) on 
initialization in order to distinguish if a master_lcore got specified 
by the user or not?

>   	/* if no memory amounts were requested, this will result in 0 and
>   	 * will be overridden later, right after eal_hugepage_info_init() */
>   	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
> @@ -502,6 +528,10 @@ eal_check_common_options(struct internal_config *internal_cfg)
>   			"-c or -l\n");
>   		return -1;
>   	}
> +	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> +		RTE_LOG(ERR, EAL, "Master lcore is not enabled for DPDK\n");
> +		return -1;
> +	}
>
>   	if (internal_cfg->process_type == RTE_PROC_INVALID) {
>   		RTE_LOG(ERR, EAL, "Invalid process type specified\n");
> @@ -550,6 +580,7 @@ eal_common_usage(void)
>   	       "  -l CORELIST  : List of cores to run on\n"
>   	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
>   	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
> +	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
>   	       "  -n NUM       : Number of memory channels\n"
>   	       "  -v           : Display version information on startup\n"
>   	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index f58965c..e476f8d 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -45,6 +45,8 @@ enum {
>   	OPT_LONG_MIN_NUM = 256,
>   #define OPT_HUGE_DIR    "huge-dir"
>   	OPT_HUGE_DIR_NUM = OPT_LONG_MIN_NUM,
> +#define OPT_MASTER_LCORE "master-lcore"
> +	OPT_MASTER_LCORE_NUM,
>   #define OPT_PROC_TYPE   "proc-type"
>   	OPT_PROC_TYPE_NUM,
>   #define OPT_NO_SHCONF   "no-shconf"
>

Re: [dpdk-dev] [PATCH 10/10] eal: add option --master-lcore

[Thomas Monjalon]

Hi Simon,

2014-11-25 10:09, Simon Kuenzer:
> thanks for your work. I have one (minor) comment for this patch that 
> should be fixed in a later version.

> > +	/* default master lcore is the first one */
> > +	if (cfg->master_lcore == 0)
> > +		cfg->master_lcore = rte_get_next_lcore(-1, 0, 0);
> > +
> 
> Might be confusing if a user specifies --master-lcore 0 and uses a 
> coremask/corelist where core id 0 is not specified.

Yes, in this corner case, master-lcore will be adjusted instead of having
an error.

> What about setting cfg->master_lcore to (RTE_MAX_LCORE + 1) on 
> initialization in order to distinguish if a master_lcore got specified 
> by the user or not?

Even simpler, I can fix it by introducing a flag master_lcore_parsed and
do the adjustment only if the option is not parsed.

-- 
Thomas

Re: [dpdk-dev] [PATCH 10/10] eal: add option --master-lcore

[Bruce Richardson]

On Tue, Nov 25, 2014 at 01:45:22PM +0100, Thomas Monjalon wrote:
> Hi Simon,
> 
> 2014-11-25 10:09, Simon Kuenzer:
> > thanks for your work. I have one (minor) comment for this patch that 
> > should be fixed in a later version.
> 
> > > +	/* default master lcore is the first one */
> > > +	if (cfg->master_lcore == 0)
> > > +		cfg->master_lcore = rte_get_next_lcore(-1, 0, 0);
> > > +
> > 
> > Might be confusing if a user specifies --master-lcore 0 and uses a 
> > coremask/corelist where core id 0 is not specified.
> 
> Yes, in this corner case, master-lcore will be adjusted instead of having
> an error.
> 
> > What about setting cfg->master_lcore to (RTE_MAX_LCORE + 1) on 
> > initialization in order to distinguish if a master_lcore got specified 
> > by the user or not?
> 
> Even simpler, I can fix it by introducing a flag master_lcore_parsed and
> do the adjustment only if the option is not parsed.
>
I agree that that sounds like a simpler approach, since we already have flags
for what args are parsed or not.

/Bruce

Re: [dpdk-dev] [PATCH 10/10] eal: add option --master-lcore

[Simon Kuenzer]

On 25.11.2014 14:39, Bruce Richardson wrote:
> On Tue, Nov 25, 2014 at 01:45:22PM +0100, Thomas Monjalon wrote:
>> Hi Simon,
>>
>> 2014-11-25 10:09, Simon Kuenzer:
>>> thanks for your work. I have one (minor) comment for this patch that
>>> should be fixed in a later version.
>>
>>>> +	/* default master lcore is the first one */
>>>> +	if (cfg->master_lcore == 0)
>>>> +		cfg->master_lcore = rte_get_next_lcore(-1, 0, 0);
>>>> +
>>>
>>> Might be confusing if a user specifies --master-lcore 0 and uses a
>>> coremask/corelist where core id 0 is not specified.
>>
>> Yes, in this corner case, master-lcore will be adjusted instead of having
>> an error.
>>
>>> What about setting cfg->master_lcore to (RTE_MAX_LCORE + 1) on
>>> initialization in order to distinguish if a master_lcore got specified
>>> by the user or not?
>>
>> Even simpler, I can fix it by introducing a flag master_lcore_parsed and
>> do the adjustment only if the option is not parsed.
>>
> I agree that that sounds like a simpler approach, since we already have flags
> for what args are parsed or not.
>
> /Bruce
>

Fine with me :-). I also agree that having the flag is even a cleaner 
solution.

Thanks,

Simon

Re: [dpdk-dev] [PATCH 00/10] eal cleanup and new options

[Thomas Monjalon]

> There are some pending patches which requires to factorize some EAL parts
> in order to be correctly implemented.
> This patchset do the required clean-up and rework these patches to improve
> lcore handling:
> 
> Didier Pallard (2):
>   eal: add core list input format
>   config: support 128 cores
> 
> Patrick Lu (1):
>   eal: get relative core index
> 
> Simon Kuenzer (1):
>   eal: add option --master-lcore
> 
> Thomas Monjalon (6):
>   eal: move internal headers in source directory
>   eal: factorize common headers
>   eal: fix header guards
>   eal: factorize internal config reset
>   eal: factorize options sanity check
>   eal: factorize configuration adjustment

Applied with last comments integrated.

The conclusions to the vote about the -c/-l options are:
- people don't vote (don't care or don't read)
- a few votes give the majority to creating the -l option.

Neil, I'm really not sure what is the best solution. Maybe that applying
this patch will make more voices raising. We'll see, all can be changed.

-- 
Thomas

Re: [dpdk-dev] [PATCH 00/10] eal cleanup and new options

[Bruce Richardson]

On Tue, Nov 25, 2014 at 03:55:19PM +0100, Thomas Monjalon wrote:
> > There are some pending patches which requires to factorize some EAL parts
> > in order to be correctly implemented.
> > This patchset do the required clean-up and rework these patches to improve
> > lcore handling:
> > 
> > Didier Pallard (2):
> >   eal: add core list input format
> >   config: support 128 cores
> > 
> > Patrick Lu (1):
> >   eal: get relative core index
> > 
> > Simon Kuenzer (1):
> >   eal: add option --master-lcore
> > 
> > Thomas Monjalon (6):
> >   eal: move internal headers in source directory
> >   eal: factorize common headers
> >   eal: fix header guards
> >   eal: factorize internal config reset
> >   eal: factorize options sanity check
> >   eal: factorize configuration adjustment
> 
> Applied with last comments integrated.
> 
> The conclusions to the vote about the -c/-l options are:
> - people don't vote (don't care or don't read)
> - a few votes give the majority to creating the -l option.
> 
> Neil, I'm really not sure what is the best solution. Maybe that applying
> this patch will make more voices raising. We'll see, all can be changed.
> 
> -- 
My additional 2c here:
While I don't see much wrong with having core lists passed using a long option,
more and more cores are being added to the latest CPUs, so it's likely that DPDK
users are going to have systems with >64 cores and will want to move from the
coremask specifier to the core list option. In that case, having it as a short
option makes a lot of sense. 

/Bruce