[v7,2/8] eal: add header files to support os specifics

Message ID 20190328232451.16988-3-anand.rawat@intel.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • HelloWorld example for windows
Related show

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues
ci/checkpatch success coding style OK

Commit Message

Anand Rawat March 28, 2019, 11:24 p.m.
Added rte_os.h files to support os specific functionality.
Updated rte_common.h to include rte_os.h. Updated lib/meson.build to
inject rte_os.h in every library.

Signed-off-by: Anand Rawat <anand.rawat@intel.com>
Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
Reviewed-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
Reviewed-by: Ranjit Menon <ranjit.menon@intel.com>
---
 lib/librte_eal/common/include/rte_common.h    |  5 +++-
 .../common/include/rte_string_fns.h           |  4 ++-
 .../freebsd/eal/include/exec-env/rte_os.h     | 10 +++++++
 .../linux/eal/include/exec-env/rte_os.h       |  8 +++++
 .../windows/eal/include/exec-env/rte_os.h     | 30 +++++++++++++++++++
 meson.build                                   |  6 ++--
 6 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 lib/librte_eal/freebsd/eal/include/exec-env/rte_os.h
 create mode 100644 lib/librte_eal/linux/eal/include/exec-env/rte_os.h
 create mode 100644 lib/librte_eal/windows/eal/include/exec-env/rte_os.h

Comments

Thomas Monjalon April 1, 2019, 11:09 p.m. | #1
Hi,

29/03/2019 00:24, Anand Rawat:
> Added rte_os.h files to support os specific functionality.
> Updated rte_common.h to include rte_os.h. Updated lib/meson.build to
> inject rte_os.h in every library.
> 
> Signed-off-by: Anand Rawat <anand.rawat@intel.com>
> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Reviewed-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Reviewed-by: Ranjit Menon <ranjit.menon@intel.com>
> ---
>  lib/librte_eal/common/include/rte_common.h    |  5 +++-
>  .../common/include/rte_string_fns.h           |  4 ++-
>  .../freebsd/eal/include/exec-env/rte_os.h     | 10 +++++++
>  .../linux/eal/include/exec-env/rte_os.h       |  8 +++++
>  .../windows/eal/include/exec-env/rte_os.h     | 30 +++++++++++++++++++
>  meson.build                                   |  6 ++--

An update of the legacy Makefiles is missing.
It should be something like that:

--- a/lib/librte_eal/freebsd/eal/Makefile
+++ b/lib/librte_eal/freebsd/eal/Makefile
@@ -86,7 +86,7 @@ CFLAGS_eal_thread.o += -Wno-return-type
 CFLAGS_eal_hpet.o += -Wno-return-type
 endif
 
-INC :=  # no bsd specific headers
+INC := rte_os.h
 
 SYMLINK-$(CONFIG_RTE_EXEC_ENV_FREEBSD)-include := $(addprefix include/,$(INC))
 
--- a/lib/librte_eal/linux/eal/Makefile
+++ b/lib/librte_eal/linux/eal/Makefile
@@ -93,7 +93,8 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 CFLAGS_eal_thread.o += -Wno-return-type
 endif
 
-INC := rte_kni_common.h
+INC := rte_os.h
+INC += rte_kni_common.h
 
 SYMLINK-$(CONFIG_RTE_EXEC_ENV_LINUX)-include := $(addprefix include/,$(INC))


>  create mode 100644 lib/librte_eal/freebsd/eal/include/exec-env/rte_os.h
>  create mode 100644 lib/librte_eal/linux/eal/include/exec-env/rte_os.h
>  create mode 100644 lib/librte_eal/windows/eal/include/exec-env/rte_os.h
[...]
> +/* os specific include */

Please write OS (uppercase) in this comment and others,
so we can understand it is an acronym.

> +#include <rte_os.h>

You don't prefix with exec-env/ sub-directory, unlike what is done
for rte_kni_common.h, and I think it is a good idea.
However it will require one more patch to make it work with
the make-based system which currently installs the include in exec-env/.
I have just submitted such a patch to remove the exec-env/ directory
both from installed and source hierarchy:
	https://patches.dpdk.org/patch/52031/
Please rebase on top of my patch and move rte_os.h one level upper.
Thanks


One more nit:
> --- /dev/null
> +++ b/lib/librte_eal/windows/eal/include/exec-env/rte_os.h
> +/* macro substitution for windows supported strerror_r */
> +#define strerror_r(a, b, c) strerror_s(b, c, a)
> +
> +/* macro substitution for windows supported strdup */
> +#define strdup(str)	_strdup(str)
> +
> +/* macro substitution for windows supported ssize_t type */
> +typedef SSIZE_T ssize_t;
> +
> +/* macro substitution for windows supported strtok_r */
> +#define strtok_r(str, delim, saveptr) strtok_s(str, delim, saveptr)

Please write Windows with first letter uppercase.
In this file, the comments looks superfluous and can be removed.
Or you can replace by something like "There is no strdup in Microsoft libc."
Anand Rawat April 2, 2019, 3:52 a.m. | #2
On 4/1/2019 4:09 PM, Thomas Monjalon wrote:
> Hi,
> 
> An update of the legacy Makefiles is missing.
> It should be something like that:
> 
> --- a/lib/librte_eal/freebsd/eal/Makefile
> +++ b/lib/librte_eal/freebsd/eal/Makefile
> @@ -86,7 +86,7 @@ CFLAGS_eal_thread.o += -Wno-return-type
>   CFLAGS_eal_hpet.o += -Wno-return-type
>   endif
>   
> -INC :=  # no bsd specific headers
> +INC := rte_os.h
>   
>   SYMLINK-$(CONFIG_RTE_EXEC_ENV_FREEBSD)-include := $(addprefix include/,$(INC))
>   
> --- a/lib/librte_eal/linux/eal/Makefile
> +++ b/lib/librte_eal/linux/eal/Makefile
> @@ -93,7 +93,8 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
>   CFLAGS_eal_thread.o += -Wno-return-type
>   endif
>   
> -INC := rte_kni_common.h
> +INC := rte_os.h
> +INC += rte_kni_common.h
>   
>   SYMLINK-$(CONFIG_RTE_EXEC_ENV_LINUX)-include := $(addprefix include/,$(INC))
> 

Adding the change to v8 along with a change in 
mk/target/<os>/rte.vars.mk to include path 'librte_eal/<OS>/eal/include'
for rte_os.h. This is done to make it available during the build process.

Patch

diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index bcf8afd39..3e4768f4a 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2014 Intel Corporation
+ * Copyright(c) 2010-2019 Intel Corporation
  */
 
 #ifndef _RTE_COMMON_H_
@@ -24,6 +24,9 @@  extern "C" {
 
 #include <rte_config.h>
 
+/* os specific include */
+#include <rte_os.h>
+
 #ifndef typeof
 #define typeof __typeof__
 #endif
diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
index 85bfe6c9a..8bac8243c 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2014 Intel Corporation
+ * Copyright(c) 2010-2019 Intel Corporation
  */
 
 /**
@@ -18,6 +18,8 @@  extern "C" {
 #include <stdio.h>
 #include <string.h>
 
+#include <rte_common.h>
+
 /**
  * Takes string "string" parameter and splits it at character "delim"
  * up to maxtokens-1 times - to give "maxtokens" resulting tokens. Like
diff --git a/lib/librte_eal/freebsd/eal/include/exec-env/rte_os.h b/lib/librte_eal/freebsd/eal/include/exec-env/rte_os.h
new file mode 100644
index 000000000..bda0c2d92
--- /dev/null
+++ b/lib/librte_eal/freebsd/eal/include/exec-env/rte_os.h
@@ -0,0 +1,10 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Intel Corporation
+ */
+
+#ifndef _RTE_OS_H_
+#define _RTE_OS_H_
+
+/* stub file for os specific logic */
+
+#endif /* _RTE_OS_H_ */
diff --git a/lib/librte_eal/linux/eal/include/exec-env/rte_os.h b/lib/librte_eal/linux/eal/include/exec-env/rte_os.h
new file mode 100644
index 000000000..c43ec6da5
--- /dev/null
+++ b/lib/librte_eal/linux/eal/include/exec-env/rte_os.h
@@ -0,0 +1,8 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Intel Corporation
+ */
+
+#ifndef _RTE_OS_H_
+#define _RTE_OS_H_
+
+#endif /* _RTE_OS_H_ */
diff --git a/lib/librte_eal/windows/eal/include/exec-env/rte_os.h b/lib/librte_eal/windows/eal/include/exec-env/rte_os.h
new file mode 100644
index 000000000..65230cae9
--- /dev/null
+++ b/lib/librte_eal/windows/eal/include/exec-env/rte_os.h
@@ -0,0 +1,30 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Intel Corporation
+ */
+
+#ifndef _RTE_OS_H_
+#define _RTE_OS_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <BaseTsd.h>
+
+/* macro substitution for windows supported strerror_r */
+#define strerror_r(a, b, c) strerror_s(b, c, a)
+
+/* macro substitution for windows supported strdup */
+#define strdup(str)	_strdup(str)
+
+/* macro substitution for windows supported ssize_t type */
+typedef SSIZE_T ssize_t;
+
+/* macro substitution for windows supported strtok_r */
+#define strtok_r(str, delim, saveptr) strtok_s(str, delim, saveptr)
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_OS_H_ */
diff --git a/meson.build b/meson.build
index fa6bf3d07..62eb6b8cf 100644
--- a/meson.build
+++ b/meson.build
@@ -1,5 +1,5 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2017 Intel Corporation
+# Copyright(c) 2017-2019 Intel Corporation
 
 project('DPDK', 'C',
 	# Get version number from file.
@@ -23,7 +23,9 @@  dpdk_app_link_libraries = []
 # configure the build, and make sure configs here and in config folder are
 # able to be included in any file. We also store a global array of include dirs
 # for passing to pmdinfogen scripts
-global_inc = include_directories('.', 'config', 'lib/librte_eal/common/include')
+global_inc = include_directories('.', 'config',
+			'lib/librte_eal/common/include',
+			'lib/librte_eal/@0@/eal/include/exec-env'.format(host_machine.system()))
 subdir('config')
 
 # build libs and drivers