[dpdk-dev] [PATCH v2] eal/windows: add pthread TLS function support

[dpdk-dev] [PATCH v2] eal/windows: add pthread TLS function support

[Tal Shnaiderman]

Add the following functions to the pthread shim implementation
for Windows as they are needed for thread safe rte_flow functions.

pthread_key_create.
pthread_key_delete.
pthread_getspecific.
pthread_setspecific.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v2: fix style issues
---
 lib/librte_eal/windows/include/pthread.h | 43 ++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/lib/librte_eal/windows/include/pthread.h b/lib/librte_eal/windows/include/pthread.h
index fb11a07ce6..8fe18e6f00 100644
--- a/lib/librte_eal/windows/include/pthread.h
+++ b/lib/librte_eal/windows/include/pthread.h
@@ -34,6 +34,8 @@ typedef CRITICAL_SECTION pthread_mutex_t;
 
 typedef SYNCHRONIZATION_BARRIER pthread_barrier_t;
 
+typedef DWORD pthread_key_t;
+
 #define pthread_barrier_init(barrier, attr, count) \
 	InitializeSynchronizationBarrier(barrier, count, -1)
 #define pthread_barrier_wait(barrier) EnterSynchronizationBarrier(barrier, \
@@ -179,6 +181,47 @@ pthread_mutex_destroy(pthread_mutex_t *mutex)
 	return 0;
 }
 
+static inline int
+pthread_key_create(pthread_key_t *key,
+		    __rte_unused void (*destructor)(void *))
+{
+	*key = TlsAlloc();
+	if ((*key) == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		return -ENOMEM;
+	}
+	return 0;
+}
+
+static inline int
+pthread_key_delete(pthread_key_t key)
+{
+	if (!TlsFree(key)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		return -1;
+	}
+	return 0;
+}
+
+static inline void *
+pthread_getspecific(pthread_key_t key)
+{
+	return TlsGetValue(key);
+}
+
+static inline int
+pthread_setspecific(pthread_key_t key, const void *value)
+{
+	/* discard const qualifier */
+	char *p = (char *) (uintptr_t) value;
+
+	if (!TlsSetValue(key, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v2] eal/windows: add pthread TLS function support

[Dmitry Kozlyuk]

On Sun, 13 Dec 2020 22:24:37 +0200, Tal Shnaiderman wrote:
> Add the following functions to the pthread shim implementation
> for Windows as they are needed for thread safe rte_flow functions.
> 
> pthread_key_create.
> pthread_key_delete.
> pthread_getspecific.
> pthread_setspecific.
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> ---
> v2: fix style issues
> ---
>  lib/librte_eal/windows/include/pthread.h | 43 ++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)

It's been planned to remove the shim, not to extend it; and to introduce a
generic threading API in EAL. Why not start with these functions? It's quite
an isolated set.

[dpdk-dev] [PATCH v3] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for tls functionality in EAL.

The following functions are added:
rte_tls_create_key - function to create a tls data key.
rte_tls_delete_key - function to delete a tls data key.
rte_tls_set_thread_value - function to set value bound to the tls key
rte_tls_get_thread_value - function to get value bound to the tls key

tls key will be defied by the new type rte_tls_key_t

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
---
 lib/librte_eal/include/meson.build |  1 +
 lib/librte_eal/include/rte_tls.h   | 88 ++++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def |  5 +++
 lib/librte_eal/unix/eal_unix_tls.c | 79 ++++++++++++++++++++++++++++++++++
 lib/librte_eal/unix/meson.build    |  1 +
 lib/librte_eal/version.map         |  6 +++
 lib/librte_eal/windows/eal_tls.c   | 72 +++++++++++++++++++++++++++++++
 lib/librte_eal/windows/meson.build |  1 +
 8 files changed, 253 insertions(+)
 create mode 100644 lib/librte_eal/include/rte_tls.h
 create mode 100644 lib/librte_eal/unix/eal_unix_tls.c
 create mode 100644 lib/librte_eal/windows/eal_tls.c

diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..a307137242 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -41,6 +41,7 @@ headers += files(
 	'rte_string_fns.h',
 	'rte_tailq.h',
 	'rte_time.h',
+	'rte_tls.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
 	'rte_trace_point_register.h',
diff --git a/lib/librte_eal/include/rte_tls.h b/lib/librte_eal/include/rte_tls.h
new file mode 100644
index 0000000000..3f84c683bc
--- /dev/null
+++ b/lib/librte_eal/include/rte_tls.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Mellanox Technologies, Ltd
+ */
+
+#include <rte_compat.h>
+
+#ifndef _RTE_TLS_H_
+#define _RTE_TLS_H_
+
+/**
+ * @file
+ *
+ * TLS functions
+ *
+ * Simple TLS functionality supplied by eal.
+ */
+
+/**
+ * Opaque pointer for tls key.
+ */
+typedef struct eal_tls_key *rte_tls_key_t;
+
+/**
+ * Function to create a tls data key visible to all threads in the process
+ * function need to be called once to create a key usable by all threads.
+ * rte_tls_key_t is an opaque pointer used to store the allocated key.
+ * and optional destructor can be set to be called when a thread expires.
+ *
+ * @param key
+ *   The rte_tls_key_t will cantain the allocated key
+ * @param destructor
+ *   The function to be called when the thread expires
+ *   Not supported on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative error number -ENOMEM or -ENOEXEC
+ */
+__rte_experimental
+int
+rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *));
+
+/**
+ * Function to delete a tls data key visible to all threads in the process
+ * rte_tls_key_t is an opaque pointer used to allocated the key.
+ *
+ * @param key
+ *   The rte_tls_key_t will cantain the allocated key
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative error number -ENOMEM or -ENOEXEC
+ */
+__rte_experimental
+int
+rte_tls_delete_key(rte_tls_key_t key);
+
+/**
+ * Function to set value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key_t key
+ * @param value
+ *   The value bound to the rte_tls_key_t key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative error number -ENOMEM or -ENOEXEC
+ */
+__rte_experimental
+int
+rte_tls_set_thread_value(rte_tls_key_t key, const void *value);
+
+/**
+ * Function to get value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key_t key
+ *
+ * @return
+ *   On success, value data pointer.
+ *   On failure, a negative error number is set in rte_errno.
+ */
+__rte_experimental
+void *
+rte_tls_get_thread_value(rte_tls_key_t key);
+
+#endif /* _RTE_TLS_H_ */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 6a6be1cfa6..2d5078eb76 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -306,6 +306,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_tls_create_key
+	rte_tls_delete_key
+	rte_tls_set_thread_value
+	rte_tls_get_thread_value
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/unix/eal_unix_tls.c b/lib/librte_eal/unix/eal_unix_tls.c
new file mode 100644
index 0000000000..8b99f8d701
--- /dev/null
+++ b/lib/librte_eal/unix/eal_unix_tls.c
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_tls.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -ENOMEM;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 rte_strerror(err));
+		free(*key);
+		return -ENOEXEC;
+	}
+	return 0;
+}
+
+int
+rte_tls_delete_key(rte_tls_key_t key)
+{
+	int err;
+
+	if (!key)
+		return -EINVAL;
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 rte_strerror(err));
+		free(key);
+		return -ENOEXEC;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_tls_set_thread_value(rte_tls_key_t key, const void *value)
+{
+	int err;
+
+	if (!key)
+		return -EINVAL;
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			 rte_strerror(err));
+		free(key);
+		return -ENOEXEC;
+	}
+	return 0;
+}
+
+void *
+rte_tls_get_thread_value(rte_tls_key_t key)
+{
+	if (!key)
+		rte_errno = EINVAL;
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/unix/meson.build b/lib/librte_eal/unix/meson.build
index d3af6b6fe2..74596a8817 100644
--- a/lib/librte_eal/unix/meson.build
+++ b/lib/librte_eal/unix/meson.build
@@ -5,4 +5,5 @@ sources += files(
 	'eal_file.c',
 	'eal_unix_memory.c',
 	'eal_unix_timer.c',
+	'eal_unix_tls.c',
 )
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..3ec95d4cbb 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_tls_create_key;
+	rte_tls_delete_key;
+	rte_tls_set_thread_value;
+	rte_tls_get_thread_value;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/eal_tls.c b/lib/librte_eal/windows/eal_tls.c
new file mode 100644
index 0000000000..73fb17ef6d
--- /dev/null
+++ b/lib/librte_eal/windows/eal_tls.c
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_tls.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_tls_create_key(rte_tls_key_t *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -ENOMEM;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -ENOEXEC;
+	}
+	return 0;
+}
+
+int
+rte_tls_delete_key(rte_tls_key_t key)
+{
+	if (!key)
+		return -EINVAL;
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -ENOEXEC;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_tls_set_thread_value(rte_tls_key_t key, const void *value)
+{
+	if (!key)
+		return -EINVAL;
+	/* discard const qualifier */
+	char *p = (char *) (uintptr_t) value;
+
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -ENOEXEC;
+	}
+	return 0;
+}
+
+void *
+rte_tls_get_thread_value(rte_tls_key_t key)
+{
+	if (!key)
+		rte_errno = EINVAL;
+	void *output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..38f51f8967 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -17,6 +17,7 @@ sources += files(
 	'eal_mp.c',
 	'eal_thread.c',
 	'eal_timer.c',
+	'eal_tls.c',
 	'fnmatch.c',
 	'getopt.c',
 )
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v3] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Thu, 17 Dec 2020 19:49:13 +0200, Tal Shnaiderman wrote:
> Add support for tls functionality in EAL.
> 
> The following functions are added:
> rte_tls_create_key - function to create a tls data key.
> rte_tls_delete_key - function to delete a tls data key.
> rte_tls_set_thread_value - function to set value bound to the tls key
> rte_tls_get_thread_value - function to get value bound to the tls key
> 
> tls key will be defied by the new type rte_tls_key_t
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> ---
> v3: switch from pthread shim to generic eal implementation [DmitryK]

Hi Tal,

Unix code can be placed in common/ directory, so that it can be eventually
used on Windows with external pthread implementation.

See more comments inline.

> +++ b/lib/librte_eal/include/rte_tls.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Mellanox Technologies, Ltd
> + */
> +
> +#include <rte_compat.h>
> +
> +#ifndef _RTE_TLS_H_
> +#define _RTE_TLS_H_
> +
> +/**
> + * @file
> + *
> + * TLS functions
> + *
> + * Simple TLS functionality supplied by eal.
> + */

These functions are supposed to be the first part of an API that will be
used instead of naked pthread in DPDK. Maybe more generic names are in order,
like rte_thread.h and rte_thread_tls_create/destroy/get/set(). In particular,
rte_tls_*() is confusing compared to rte_lcore_*().

> +
> +/**
> + * Opaque pointer for tls key.
> + */
> +typedef struct eal_tls_key *rte_tls_key_t;

"_t" suffix is reserved by POSIX, "rte_" prefix is sufficient.

> +
> +/**
> + * Function to create a tls data key visible to all threads in the process

Typos: "TLS", "EAL".

> + * function need to be called once to create a key usable by all threads.
> + * rte_tls_key_t is an opaque pointer used to store the allocated key.
> + * and optional destructor can be set to be called when a thread expires.
> + *
> + * @param key
> + *   The rte_tls_key_t will cantain the allocated key

Typo: "cantain". I'd say: "Pointer to store the allocated rte_tls_key".

> + * @param destructor
> + *   The function to be called when the thread expires

Typo: no period (line-break will be removed by Doxygen).

> + *   Not supported on Windows OS.

Some drivers (net/mlx5, bus/dpaa, bus/fsmlc) rely on this feature.
Admittedly, it would be hard to implement. You know net/mlx5 well, how will
it be affected? Do you plan to stop relying on this feature or implement it
in the future? Anyway, LGTM for now.

> + *
> + * @return
> + *   On success, zero.
> + *   On failure, a negative error number -ENOMEM or -ENOEXEC

Let's not make POSIX codes part of the API.

> + */
> +__rte_experimental
> +int
> +rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *));

[...]
> +int
> +rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *))
> +{
> +	int err;
> +
> +	*key = malloc(sizeof(struct eal_tls_key));
> +	if ((*key) == NULL) {
> +		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
> +		return -ENOMEM;
> +	}
> +	err = pthread_key_create(&((*key)->thread_index), destructor);
> +	if (err) {
> +		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
> +			 rte_strerror(err));

Use strerror() for OS error codes here and below.

> +		free(*key);
> +		return -ENOEXEC;
> +	}
> +	return 0;
> +}
> +

[...]
> +int
> +rte_tls_set_thread_value(rte_tls_key_t key, const void *value)
> +{
> +	if (!key)
> +		return -EINVAL;
> +	/* discard const qualifier */
> +	char *p = (char *) (uintptr_t) value;

Please declare all variables at the top of a block.

> +
> +	if (!TlsSetValue(key->thread_index, p)) {
> +		RTE_LOG_WIN32_ERR("TlsSetValue()");
> +		return -ENOEXEC;
> +	}
> +	return 0;
> +}
> +
> +void *
> +rte_tls_get_thread_value(rte_tls_key_t key)
> +{
> +	if (!key)
> +		rte_errno = EINVAL;
> +	void *output = TlsGetValue(key->thread_index);

Same as above.

> +	if (GetLastError() != ERROR_SUCCESS) {
> +		RTE_LOG_WIN32_ERR("TlsGetValue()");
> +		rte_errno = ENOEXEC;
> +		return NULL;
> +	}
> +	return output;
> +}

Re: [dpdk-dev] [PATCH v3] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [PATCH v3] eal: add generic thread-local-storage functions
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 17 Dec 2020 19:49:13 +0200, Tal Shnaiderman wrote:
> > Add support for tls functionality in EAL.
> >
> > The following functions are added:
> > rte_tls_create_key - function to create a tls data key.
> > rte_tls_delete_key - function to delete a tls data key.
> > rte_tls_set_thread_value - function to set value bound to the tls key
> > rte_tls_get_thread_value - function to get value bound to the tls key
> >
> > tls key will be defied by the new type rte_tls_key_t
> >
> > Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > ---
> > v3: switch from pthread shim to generic eal implementation [DmitryK]
> 
> Hi Tal,
> 
> Unix code can be placed in common/ directory, so that it can be eventually
> used on Windows with external pthread implementation.
> 
> See more comments inline.
> 
> > +++ b/lib/librte_eal/include/rte_tls.h
> > @@ -0,0 +1,88 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2020 Mellanox Technologies, Ltd  */
> > +
> > +#include <rte_compat.h>
> > +
> > +#ifndef _RTE_TLS_H_
> > +#define _RTE_TLS_H_
> > +
> > +/**
> > + * @file
> > + *
> > + * TLS functions
> > + *
> > + * Simple TLS functionality supplied by eal.
> > + */
> 
> These functions are supposed to be the first part of an API that will be used
> instead of naked pthread in DPDK. Maybe more generic names are in order,
> like rte_thread.h and rte_thread_tls_create/destroy/get/set(). In particular,
> rte_tls_*() is confusing compared to rte_lcore_*().
> 
> > +
> > +/**
> > + * Opaque pointer for tls key.
> > + */
> > +typedef struct eal_tls_key *rte_tls_key_t;
> 
> "_t" suffix is reserved by POSIX, "rte_" prefix is sufficient.
> 
> > +
> > +/**
> > + * Function to create a tls data key visible to all threads in the
> > +process
> 
> Typos: "TLS", "EAL".
> 

Didn't find the EAL typo.

> > + * function need to be called once to create a key usable by all threads.
> > + * rte_tls_key_t is an opaque pointer used to store the allocated key.
> > + * and optional destructor can be set to be called when a thread expires.
> > + *
> > + * @param key
> > + *   The rte_tls_key_t will cantain the allocated key
> 
> Typo: "cantain". I'd say: "Pointer to store the allocated rte_tls_key".
> 
> > + * @param destructor
> > + *   The function to be called when the thread expires
> 
> Typo: no period (line-break will be removed by Doxygen).
> 
> > + *   Not supported on Windows OS.
> 
> Some drivers (net/mlx5, bus/dpaa, bus/fsmlc) rely on this feature.
> Admittedly, it would be hard to implement. You know net/mlx5 well, how
> will it be affected? Do you plan to stop relying on this feature or implement it
> in the future? Anyway, LGTM for now.

In net/mlx5 we will not relay on the destructor for the Windows flow and plan to do the housekeeping in the PMD, to summarize we save HANDLE for each thread using rte_tls_set_thread_value in the PMD destruction flow we call the needed release function for each terminated thread.

I used the opaque pointer in case future development might want to implement It without breaking the API, I assumed additions to the TLS key struct on Windows side will be needed.

Thanks for the review, I'll send a v4 after fixing your comments.

> 
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative error number -ENOMEM or -ENOEXEC
> 
> Let's not make POSIX codes part of the API.
> 
> > + */
> > +__rte_experimental
> > +int
> > +rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *));
> 
> [...]
> > +int
> > +rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *)) {
> > +     int err;
> > +
> > +     *key = malloc(sizeof(struct eal_tls_key));
> > +     if ((*key) == NULL) {
> > +             RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
> > +             return -ENOMEM;
> > +     }
> > +     err = pthread_key_create(&((*key)->thread_index), destructor);
> > +     if (err) {
> > +             RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
> > +                      rte_strerror(err));
> 
> Use strerror() for OS error codes here and below.
> 
> > +             free(*key);
> > +             return -ENOEXEC;
> > +     }
> > +     return 0;
> > +}
> > +
> 
> [...]
> > +int
> > +rte_tls_set_thread_value(rte_tls_key_t key, const void *value) {
> > +     if (!key)
> > +             return -EINVAL;
> > +     /* discard const qualifier */
> > +     char *p = (char *) (uintptr_t) value;
> 
> Please declare all variables at the top of a block.
> 
> > +
> > +     if (!TlsSetValue(key->thread_index, p)) {
> > +             RTE_LOG_WIN32_ERR("TlsSetValue()");
> > +             return -ENOEXEC;
> > +     }
> > +     return 0;
> > +}
> > +
> > +void *
> > +rte_tls_get_thread_value(rte_tls_key_t key) {
> > +     if (!key)
> > +             rte_errno = EINVAL;
> > +     void *output = TlsGetValue(key->thread_index);
> 
> Same as above.
> 
> > +     if (GetLastError() != ERROR_SUCCESS) {
> > +             RTE_LOG_WIN32_ERR("TlsGetValue()");
> > +             rte_errno = ENOEXEC;
> > +             return NULL;
> > +     }
> > +     return output;
> > +}

[dpdk-dev] [PATCH v4] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for tls functionality in EAL.

The following functions are added:
rte_thread_tls_create_key - function to create a tls data key.
rte_thread_tls_delete_key - function to delete a tls data key.
rte_thread_tls_set_value - function to set value bound to the tls key
rte_thread_tls_get_value - function to get value bound to the tls key

tls key will be defined by the new type rte_tls_key

Windows implementation is under librte_eal/windows and
implemented using WIN32 API for Windows only.

common implementation is under librte_eal/common and
implemented using pthread for UNIX and Windows compilation
using extenral pthread libraries, when supported.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
v4: modify file names, function names, move unix code to common
for future external pthreads support [DmitryK]
---
 lib/librte_eal/common/meson.build   |  1 +
 lib/librte_eal/common/rte_thread.c  | 87 ++++++++++++++++++++++++++++++++++++
 lib/librte_eal/include/meson.build  |  1 +
 lib/librte_eal/include/rte_thread.h | 88 +++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |  5 +++
 lib/librte_eal/version.map          |  6 +++
 lib/librte_eal/windows/meson.build  |  6 +++
 lib/librte_eal/windows/rte_thread.c | 82 ++++++++++++++++++++++++++++++++++
 8 files changed, 276 insertions(+)
 create mode 100644 lib/librte_eal/common/rte_thread.c
 create mode 100644 lib/librte_eal/include/rte_thread.h
 create mode 100644 lib/librte_eal/windows/rte_thread.c

diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 39abf7a0a4..69c88b3349 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -77,6 +77,7 @@ sources += files(
 	'rte_random.c',
 	'rte_reciprocal.c',
 	'rte_service.c',
+	'rte_thread.c',
 )
 
 if is_linux
diff --git a/lib/librte_eal/common/rte_thread.c b/lib/librte_eal/common/rte_thread.c
new file mode 100644
index 0000000000..a2923a4ad1
--- /dev/null
+++ b/lib/librte_eal/common/rte_thread.c
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 strerror(err));
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_delete_key(rte_tls_key key)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 strerror(err));
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			strerror(err));
+		free(key);
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_get_value(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..0dea342e1d 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -40,6 +40,7 @@ headers += files(
 	'rte_service_component.h',
 	'rte_string_fns.h',
 	'rte_tailq.h',
+	'rte_thread.h',
 	'rte_time.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
new file mode 100644
index 0000000000..6686f6183c
--- /dev/null
+++ b/lib/librte_eal/include/rte_thread.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Mellanox Technologies, Ltd
+ */
+
+#include <rte_compat.h>
+
+#ifndef _RTE_THREAD_H_
+#define _RTE_THREAD_H_
+
+/**
+ * @file
+ *
+ * Threading functions
+ *
+ * Simple threads functionality supplied by EAL.
+ */
+
+/**
+ * Opaque pointer for TLS key.
+ */
+typedef struct eal_tls_key *rte_tls_key;
+
+/**
+ * Function to create a TLS data key visible to all threads in the process
+ * function need to be called once to create a key usable by all threads.
+ * rte_tls_key is an opaque pointer used to store the allocated key.
+ * and optional destructor can be set to be called when a thread expires.
+ *
+ * @param key
+ *   Pointer to store the allocated rte_tls_key
+ * @param destructor
+ *   The function to be called when the thread expires.
+ *   Not supported on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *));
+
+/**
+ * Function to delete a TLS data key visible to all threads in the process
+ * rte_tls_key is the opaque pointer allocated by rte_thread_tls_create_key.
+ *
+ * @param key
+ *   The rte_tls_key will cantain the allocated key
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_delete_key(rte_tls_key key);
+
+/**
+ * Function to set value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_create_key.
+ * @param value
+ *   The value bound to the rte_tls_key key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value);
+
+/**
+ * Function to get value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_create_key.
+ *
+ * @return
+ *   On success, value data pointer (can also be NULL).
+ *   On failure, NULL and an error number is set in rte_errno.
+ */
+__rte_experimental
+void *
+rte_thread_tls_get_value(rte_tls_key key);
+
+#endif /* _RTE_THREAD_H_ */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 6a6be1cfa6..a071fe9c2f 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -306,6 +306,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_thread_tls_create_key
+	rte_thread_tls_delete_key
+	rte_thread_tls_set_value
+	rte_thread_tls_get_value
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..8559694bd5 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_thread_tls_create_key;
+	rte_thread_tls_delete_key;
+	rte_thread_tls_set_value;
+	rte_thread_tls_get_value;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..1f1398dfe9 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -21,4 +21,10 @@ sources += files(
 	'getopt.c',
 )
 
+if (dpdk_conf.has('use_external_pthread'))
+	sources += 'librte_eal/common/rte_thread.c'
+else
+	sources += 'librte_eal/windows/rte_thread.c'
+endif
+
 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
new file mode 100644
index 0000000000..645eca7478
--- /dev/null
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_thread.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_thread_tls_create_key(rte_tls_key *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_delete_key(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value)
+{
+	char *p;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	/* discard const qualifier */
+	p = (char *) (uintptr_t) value;
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_get_value(rte_tls_key key)
+{
+	void *output;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v4] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

> [...]
> +int
> +rte_thread_tls_set_value(rte_tls_key key, const void *value)
> +{
> +	int err;
> +
> +	if (!key) {
> +		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
> +		return -1;
> +	}
> +	err = pthread_setspecific(key->thread_index, value);
> +	if (err) {
> +		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
> +			strerror(err));
> +		free(key);

Why free(key) here? Probably a typo.

> [...]
> +__rte_experimental
> +int
> +rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *));
> +
> +/**
> + * Function to delete a TLS data key visible to all threads in the process
> + * rte_tls_key is the opaque pointer allocated by rte_thread_tls_create_key.
> + *
> + * @param key
> + *   The rte_tls_key will cantain the allocated key

cantain -> contain


> diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
> index 3b2faf29eb..1f1398dfe9 100644
> --- a/lib/librte_eal/windows/meson.build
> +++ b/lib/librte_eal/windows/meson.build
> @@ -21,4 +21,10 @@ sources += files(
>  	'getopt.c',
>  )
>  
> +if (dpdk_conf.has('use_external_pthread'))

Please describe the new option in meson_options.txt.
Maybe drop "external" from the name, what do you think?

Re: [dpdk-dev] [PATCH v4] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [PATCH v4] eal: add generic thread-local-storage functions
> 
> External email: Use caution opening links or attachments
> 
> 
> > [...]
> > +int
> > +rte_thread_tls_set_value(rte_tls_key key, const void *value) {
> > +     int err;
> > +
> > +     if (!key) {
> > +             RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
> > +             return -1;
> > +     }
> > +     err = pthread_setspecific(key->thread_index, value);
> > +     if (err) {
> > +             RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
> > +                     strerror(err));
> > +             free(key);
> 
> Why free(key) here? Probably a typo.
> 
> > [...]
> > +__rte_experimental
> > +int
> > +rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void
> > +*));
> > +
> > +/**
> > + * Function to delete a TLS data key visible to all threads in the
> > +process
> > + * rte_tls_key is the opaque pointer allocated by
> rte_thread_tls_create_key.
> > + *
> > + * @param key
> > + *   The rte_tls_key will cantain the allocated key
> 
> cantain -> contain
> 
> 
> > diff --git a/lib/librte_eal/windows/meson.build
> > b/lib/librte_eal/windows/meson.build
> > index 3b2faf29eb..1f1398dfe9 100644
> > --- a/lib/librte_eal/windows/meson.build
> > +++ b/lib/librte_eal/windows/meson.build
> > @@ -21,4 +21,10 @@ sources += files(
> >       'getopt.c',
> >  )
> >
> > +if (dpdk_conf.has('use_external_pthread'))
> 
> Please describe the new option in meson_options.txt.
> Maybe drop "external" from the name, what do you think?

How about use_windows_pthread to be clear that it's unrelated to UNIX pthreads?

Re: [dpdk-dev] [PATCH v4] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Wed, 23 Dec 2020 11:44:31 +0000, Tal Shnaiderman wrote:
> > > diff --git a/lib/librte_eal/windows/meson.build
> > > b/lib/librte_eal/windows/meson.build
> > > index 3b2faf29eb..1f1398dfe9 100644
> > > --- a/lib/librte_eal/windows/meson.build
> > > +++ b/lib/librte_eal/windows/meson.build
> > > @@ -21,4 +21,10 @@ sources += files(
> > >       'getopt.c',
> > >  )
> > >
> > > +if (dpdk_conf.has('use_external_pthread'))  
> > 
> > Please describe the new option in meson_options.txt.
> > Maybe drop "external" from the name, what do you think?  
> 
> How about use_windows_pthread to be clear that it's unrelated to UNIX pthreads?

Sounds good.

Re: [dpdk-dev] [EXTERNAL] Re: [PATCH v4] eal: add generic thread-local-storage functions

[Dmitry Malloy (MESHCHANINOV)]

I wonder if this should be consistent with Nick's meson (to use external pthread library) change?

-----Original Message-----
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> 
Sent: Wednesday, December 23, 2020 3:59 AM
To: Tal Shnaiderman <talshn@nvidia.com>
Cc: dev@dpdk.org; thomas <thomas@monjalon.net>; Kadam, Pallavi <pallavi.kadam@intel.com>; navasile@linux.microsoft.com; Dmitry Malloy (MESHCHANINOV) <dmitrym@microsoft.com>; david.marchand@redhat.com
Subject: [EXTERNAL] Re: [PATCH v4] eal: add generic thread-local-storage functions

On Wed, 23 Dec 2020 11:44:31 +0000, Tal Shnaiderman wrote:
> > > diff --git a/lib/librte_eal/windows/meson.build
> > > b/lib/librte_eal/windows/meson.build
> > > index 3b2faf29eb..1f1398dfe9 100644
> > > --- a/lib/librte_eal/windows/meson.build
> > > +++ b/lib/librte_eal/windows/meson.build
> > > @@ -21,4 +21,10 @@ sources += files(
> > >       'getopt.c',
> > >  )
> > >
> > > +if (dpdk_conf.has('use_external_pthread'))  
> > 
> > Please describe the new option in meson_options.txt.
> > Maybe drop "external" from the name, what do you think?  
> 
> How about use_windows_pthread to be clear that it's unrelated to UNIX pthreads?

Sounds good.

[dpdk-dev] [PATCH v5] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for tls functionality in EAL.

The following functions are added:
rte_thread_tls_create_key - function to create a tls data key.
rte_thread_tls_delete_key - function to delete a tls data key.
rte_thread_tls_set_value - function to set value bound to the tls key
rte_thread_tls_get_value - function to get value bound to the tls key

tls key will be defined by the new type rte_tls_key

Windows implementation is under librte_eal/windows and
implemented using WIN32 API for Windows only.

common implementation is under librte_eal/common and
implemented using pthread for UNIX and Windows compilation
using extenral pthread libraries, when supported.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
v4: modify file names, function names, move unix code to common
for future external pthreads support [DmitryK]
v5: rename var used for extenal pthreads, add description in
meson_options.txt. [DmitryK]
---
 lib/librte_eal/common/meson.build   |  1 +
 lib/librte_eal/common/rte_thread.c  | 86 ++++++++++++++++++++++++++++++++++++
 lib/librte_eal/include/meson.build  |  1 +
 lib/librte_eal/include/rte_thread.h | 88 +++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |  5 +++
 lib/librte_eal/version.map          |  6 +++
 lib/librte_eal/windows/meson.build  |  6 +++
 lib/librte_eal/windows/rte_thread.c | 82 ++++++++++++++++++++++++++++++++++
 meson_options.txt                   |  2 +
 9 files changed, 277 insertions(+)
 create mode 100644 lib/librte_eal/common/rte_thread.c
 create mode 100644 lib/librte_eal/include/rte_thread.h
 create mode 100644 lib/librte_eal/windows/rte_thread.c

diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 39abf7a0a4..69c88b3349 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -77,6 +77,7 @@ sources += files(
 	'rte_random.c',
 	'rte_reciprocal.c',
 	'rte_service.c',
+	'rte_thread.c',
 )
 
 if is_linux
diff --git a/lib/librte_eal/common/rte_thread.c b/lib/librte_eal/common/rte_thread.c
new file mode 100644
index 0000000000..56a182d1f1
--- /dev/null
+++ b/lib/librte_eal/common/rte_thread.c
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 strerror(err));
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_delete_key(rte_tls_key key)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 strerror(err));
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			strerror(err));
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_get_value(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..0dea342e1d 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -40,6 +40,7 @@ headers += files(
 	'rte_service_component.h',
 	'rte_string_fns.h',
 	'rte_tailq.h',
+	'rte_thread.h',
 	'rte_time.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
new file mode 100644
index 0000000000..d1206586a5
--- /dev/null
+++ b/lib/librte_eal/include/rte_thread.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Mellanox Technologies, Ltd
+ */
+
+#include <rte_compat.h>
+
+#ifndef _RTE_THREAD_H_
+#define _RTE_THREAD_H_
+
+/**
+ * @file
+ *
+ * Threading functions
+ *
+ * Simple threads functionality supplied by EAL.
+ */
+
+/**
+ * Opaque pointer for TLS key.
+ */
+typedef struct eal_tls_key *rte_tls_key;
+
+/**
+ * Function to create a TLS data key visible to all threads in the process
+ * function need to be called once to create a key usable by all threads.
+ * rte_tls_key is an opaque pointer used to store the allocated key.
+ * and optional destructor can be set to be called when a thread expires.
+ *
+ * @param key
+ *   Pointer to store the allocated rte_tls_key
+ * @param destructor
+ *   The function to be called when the thread expires.
+ *   Not supported on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *));
+
+/**
+ * Function to delete a TLS data key visible to all threads in the process
+ * rte_tls_key is the opaque pointer allocated by rte_thread_tls_create_key.
+ *
+ * @param key
+ *   The rte_tls_key will contain the allocated key
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_delete_key(rte_tls_key key);
+
+/**
+ * Function to set value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_create_key.
+ * @param value
+ *   The value bound to the rte_tls_key key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value);
+
+/**
+ * Function to get value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_create_key.
+ *
+ * @return
+ *   On success, value data pointer (can also be NULL).
+ *   On failure, NULL and an error number is set in rte_errno.
+ */
+__rte_experimental
+void *
+rte_thread_tls_get_value(rte_tls_key key);
+
+#endif /* _RTE_THREAD_H_ */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 6a6be1cfa6..a071fe9c2f 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -306,6 +306,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_thread_tls_create_key
+	rte_thread_tls_delete_key
+	rte_thread_tls_set_value
+	rte_thread_tls_get_value
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..8559694bd5 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_thread_tls_create_key;
+	rte_thread_tls_delete_key;
+	rte_thread_tls_set_value;
+	rte_thread_tls_get_value;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..f4c3e2f12c 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -21,4 +21,10 @@ sources += files(
 	'getopt.c',
 )
 
+if (dpdk_conf.has('use_windows_pthread'))
+	sources += 'librte_eal/common/rte_thread.c'
+else
+	sources += 'librte_eal/windows/rte_thread.c'
+endif
+
 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
new file mode 100644
index 0000000000..645eca7478
--- /dev/null
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_thread.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_thread_tls_create_key(rte_tls_key *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_delete_key(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value)
+{
+	char *p;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	/* discard const qualifier */
+	p = (char *) (uintptr_t) value;
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_get_value(rte_tls_key key)
+{
+	void *output;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
diff --git a/meson_options.txt b/meson_options.txt
index e384e6dbb2..33d65434b3 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -34,3 +34,5 @@ option('tests', type: 'boolean', value: true,
 	description: 'build unit tests')
 option('use_hpet', type: 'boolean', value: false,
 	description: 'use HPET timer in EAL')
+option('use_windows_pthread', type: 'boolean', value: false,
+	description: 'use external pthread libraries for Windows in EAL')
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v5] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Sat, 26 Dec 2020 18:08:48 +0200, Tal Shnaiderman wrote:
> diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
> index 3b2faf29eb..f4c3e2f12c 100644
> --- a/lib/librte_eal/windows/meson.build
> +++ b/lib/librte_eal/windows/meson.build
> @@ -21,4 +21,10 @@ sources += files(
>  	'getopt.c',
>  )
>  
> +if (dpdk_conf.has('use_windows_pthread'))
> +	sources += 'librte_eal/common/rte_thread.c'
> +else
> +	sources += 'librte_eal/windows/rte_thread.c'
> +endif
> +

You need get_option(), not dpdk_conf (apologies again for not being precise
when I drafted the approach):

--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -21,7 +21,7 @@ sources += files(
        'getopt.c',
 )
 
-if (dpdk_conf.has('use_windows_pthread'))
+if get_option('use_windows_pthread')
        sources += 'librte_eal/common/rte_thread.c'
 else
        sources += 'librte_eal/windows/rte_thread.c'

Worse, with -Duse_windows_pthread=true file in common directory includes
<pthread.h>, but it finds pthread shim from windows subdirectory, not the
file from external library or MinGW toolchain. So the option is not usable
until the shim exists. I suggest removing the option for now, let's
reintroduce it when rte_thread.h grows and the shim goes away.

Re: [dpdk-dev] [PATCH v5] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [PATCH v5] eal: add generic thread-local-storage functions
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sat, 26 Dec 2020 18:08:48 +0200, Tal Shnaiderman wrote:
> > diff --git a/lib/librte_eal/windows/meson.build
> > b/lib/librte_eal/windows/meson.build
> > index 3b2faf29eb..f4c3e2f12c 100644
> > --- a/lib/librte_eal/windows/meson.build
> > +++ b/lib/librte_eal/windows/meson.build
> > @@ -21,4 +21,10 @@ sources += files(
> >       'getopt.c',
> >  )
> >
> > +if (dpdk_conf.has('use_windows_pthread'))
> > +     sources += 'librte_eal/common/rte_thread.c'
> > +else
> > +     sources += 'librte_eal/windows/rte_thread.c'
> > +endif
> > +
> 
> You need get_option(), not dpdk_conf (apologies again for not being precise
> when I drafted the approach):
> 
> --- a/lib/librte_eal/windows/meson.build
> +++ b/lib/librte_eal/windows/meson.build
> @@ -21,7 +21,7 @@ sources += files(
>         'getopt.c',
>  )
> 
> -if (dpdk_conf.has('use_windows_pthread'))
> +if get_option('use_windows_pthread')
>         sources += 'librte_eal/common/rte_thread.c'
>  else
>         sources += 'librte_eal/windows/rte_thread.c'
> 
> Worse, with -Duse_windows_pthread=true file in common directory
> includes <pthread.h>, but it finds pthread shim from windows subdirectory,
> not the file from external library or MinGW toolchain. So the option is not
> usable until the shim exists. I suggest removing the option for now, let's
> reintroduce it when rte_thread.h grows and the shim goes away.
> 

Right, thanks for checking, I'll send a version without it.

I was planning to add support for thread priority control to address the issue of threads with realtime priority denying cpu time from OS threads, I think it will be a good opportunity to move pthread_create to the new API.

[dpdk-dev] [PATCH v6] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for tls functionality in EAL.

The following functions are added:
rte_thread_tls_create_key - function to create a tls data key.
rte_thread_tls_delete_key - function to delete a tls data key.
rte_thread_tls_set_value - function to set value bound to the tls key
rte_thread_tls_get_value - function to get value bound to the tls key

tls key will be defined by the new type rte_tls_key

Windows implementation is under librte_eal/windows and
implemented using WIN32 API for Windows only.

common implementation is under librte_eal/common and
implemented using pthread for UNIX and Windows compilation
using extenral pthread libraries, when supported.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
v4: modify file names, function names, move unix code to common
for future external pthreads support [DmitryK]
v5: rename var used for extenal pthreads, add description in
meson_options.txt. [DmitryK]
v6: remove external_pthread support as it collide with pthread
shim implementation [DmitryK]
---
 lib/librte_eal/common/meson.build   |  1 +
 lib/librte_eal/common/rte_thread.c  | 86 ++++++++++++++++++++++++++++++++++++
 lib/librte_eal/include/meson.build  |  1 +
 lib/librte_eal/include/rte_thread.h | 88 +++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |  5 +++
 lib/librte_eal/version.map          |  6 +++
 lib/librte_eal/windows/meson.build  |  1 +
 lib/librte_eal/windows/rte_thread.c | 82 ++++++++++++++++++++++++++++++++++
 8 files changed, 270 insertions(+)
 create mode 100644 lib/librte_eal/common/rte_thread.c
 create mode 100644 lib/librte_eal/include/rte_thread.h
 create mode 100644 lib/librte_eal/windows/rte_thread.c

diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 39abf7a0a4..69c88b3349 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -77,6 +77,7 @@ sources += files(
 	'rte_random.c',
 	'rte_reciprocal.c',
 	'rte_service.c',
+	'rte_thread.c',
 )
 
 if is_linux
diff --git a/lib/librte_eal/common/rte_thread.c b/lib/librte_eal/common/rte_thread.c
new file mode 100644
index 0000000000..56a182d1f1
--- /dev/null
+++ b/lib/librte_eal/common/rte_thread.c
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 strerror(err));
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_delete_key(rte_tls_key key)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 strerror(err));
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			strerror(err));
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_get_value(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..0dea342e1d 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -40,6 +40,7 @@ headers += files(
 	'rte_service_component.h',
 	'rte_string_fns.h',
 	'rte_tailq.h',
+	'rte_thread.h',
 	'rte_time.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
new file mode 100644
index 0000000000..d1206586a5
--- /dev/null
+++ b/lib/librte_eal/include/rte_thread.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Mellanox Technologies, Ltd
+ */
+
+#include <rte_compat.h>
+
+#ifndef _RTE_THREAD_H_
+#define _RTE_THREAD_H_
+
+/**
+ * @file
+ *
+ * Threading functions
+ *
+ * Simple threads functionality supplied by EAL.
+ */
+
+/**
+ * Opaque pointer for TLS key.
+ */
+typedef struct eal_tls_key *rte_tls_key;
+
+/**
+ * Function to create a TLS data key visible to all threads in the process
+ * function need to be called once to create a key usable by all threads.
+ * rte_tls_key is an opaque pointer used to store the allocated key.
+ * and optional destructor can be set to be called when a thread expires.
+ *
+ * @param key
+ *   Pointer to store the allocated rte_tls_key
+ * @param destructor
+ *   The function to be called when the thread expires.
+ *   Not supported on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *));
+
+/**
+ * Function to delete a TLS data key visible to all threads in the process
+ * rte_tls_key is the opaque pointer allocated by rte_thread_tls_create_key.
+ *
+ * @param key
+ *   The rte_tls_key will contain the allocated key
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_delete_key(rte_tls_key key);
+
+/**
+ * Function to set value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_create_key.
+ * @param value
+ *   The value bound to the rte_tls_key key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value);
+
+/**
+ * Function to get value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_create_key.
+ *
+ * @return
+ *   On success, value data pointer (can also be NULL).
+ *   On failure, NULL and an error number is set in rte_errno.
+ */
+__rte_experimental
+void *
+rte_thread_tls_get_value(rte_tls_key key);
+
+#endif /* _RTE_THREAD_H_ */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index f195d32e57..a66c413366 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -308,6 +308,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_thread_tls_create_key
+	rte_thread_tls_delete_key
+	rte_thread_tls_set_value
+	rte_thread_tls_get_value
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..8559694bd5 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_thread_tls_create_key;
+	rte_thread_tls_delete_key;
+	rte_thread_tls_set_value;
+	rte_thread_tls_get_value;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..42ff5c2d59 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -19,6 +19,7 @@ sources += files(
 	'eal_timer.c',
 	'fnmatch.c',
 	'getopt.c',
+	'rte_thread.c',
 )
 
 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
new file mode 100644
index 0000000000..645eca7478
--- /dev/null
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_thread.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_thread_tls_create_key(rte_tls_key *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_delete_key(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_set_value(rte_tls_key key, const void *value)
+{
+	char *p;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	/* discard const qualifier */
+	p = (char *) (uintptr_t) value;
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_get_value(rte_tls_key key)
+{
+	void *output;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v6] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Wed, 30 Dec 2020 13:12:44 +0200, Tal Shnaiderman wrote:
> Add support for tls functionality in EAL.
> 
> The following functions are added:
> rte_thread_tls_create_key - function to create a tls data key.
> rte_thread_tls_delete_key - function to delete a tls data key.
> rte_thread_tls_set_value - function to set value bound to the tls key
> rte_thread_tls_get_value - function to get value bound to the tls key
> 
> tls key will be defined by the new type rte_tls_key
> 
> Windows implementation is under librte_eal/windows and
> implemented using WIN32 API for Windows only.
> 
> common implementation is under librte_eal/common and
> implemented using pthread for UNIX and Windows compilation
> using extenral pthread libraries, when supported.
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> ---
> v3: switch from pthread shim to generic eal implementation [DmitryK]
> v4: modify file names, function names, move unix code to common
> for future external pthreads support [DmitryK]
> v5: rename var used for extenal pthreads, add description in
> meson_options.txt. [DmitryK]
> v6: remove external_pthread support as it collide with pthread
> shim implementation [DmitryK]
> ---

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Re: [dpdk-dev] [PATCH v6] eal: add generic thread-local-storage functions

[Thomas Monjalon]

30/12/2020 12:12, Tal Shnaiderman:
> Add support for tls functionality in EAL.

You should write TLS uppercase here and below.

There is already a TLS capability in rte_per_lcore.h,
using __thread keyword.
Note: I think the historical wording "lcore" is confusing.
We talk about lcore, assuming the thread is bound to its lcore.

Please explain in the patch why the more complex TLS functions
are required, i.e. can __thread variable be used in Windows?
limitation, etc.

> The following functions are added:
> rte_thread_tls_create_key - function to create a tls data key.
> rte_thread_tls_delete_key - function to delete a tls data key.
> rte_thread_tls_set_value - function to set value bound to the tls key
> rte_thread_tls_get_value - function to get value bound to the tls key

We can assume the TLS key logic is known,
but this patch is the right place to explain it.

> tls key will be defined by the new type rte_tls_key
> 
> Windows implementation is under librte_eal/windows and
> implemented using WIN32 API for Windows only.
> 
> common implementation is under librte_eal/common and
> implemented using pthread for UNIX and Windows compilation
> using extenral pthread libraries, when supported.

typo: external

[...]
> v3: switch from pthread shim to generic eal implementation [DmitryK]
> v4: modify file names, function names, move unix code to common
> for future external pthreads support [DmitryK]

For now, there is no external pthread library.
It did not have been discussed generally in the techboard.
As it is using the Unix pthread API, it sounds more logical
to have this implementation in the Unix sub-directory I think.

> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> +	'rte_thread.c',
[...]
> --- a/lib/librte_eal/include/meson.build
> +++ b/lib/librte_eal/include/meson.build
> +	'rte_thread.h',

There are existing functions in rte_lcore.h and rte_per_lcore.h
which are managing threads.
I think a prior patch should move such functions in rte_thread files.
At least we can start moving the affinity functions,
which are not using pthread types as parameters.

> --- /dev/null
> +++ b/lib/librte_eal/include/rte_thread.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Mellanox Technologies, Ltd
> + */
> +
> +#include <rte_compat.h>

Why do you need rte_compat?

> +/**
> + * Function to create a TLS data key visible to all threads in the process
> + * function need to be called once to create a key usable by all threads.
> + * rte_tls_key is an opaque pointer used to store the allocated key.
> + * and optional destructor can be set to be called when a thread expires.

You may explain that the key is later used to get/set a value.

> + *
> + * @param key
> + *   Pointer to store the allocated rte_tls_key
> + * @param destructor
> + *   The function to be called when the thread expires.
> + *   Not supported on Windows OS.

What happens when specifying a destructor on Windows?

> + *
> + * @return
> + *   On success, zero.
> + *   On failure, a negative number.
> + */
> +__rte_experimental
> +int
> +rte_thread_tls_create_key(rte_tls_key *key, void (*destructor)(void *));

For .h files, we have the return type on the same line (I know it is stupid).

I would suggest moving the verb "create" at the end of the function names.

[...]
> +rte_thread_tls_delete_key(rte_tls_key key);

Could be rte_thread_tls_key_delete?

[...]
> +rte_thread_tls_set_value(rte_tls_key key, const void *value);

rte_thread_tls_value_set?

> +rte_thread_tls_get_value(rte_tls_key key);

rte_thread_tls_value_get?

[dpdk-dev] [PATCH v7 0/2] support generic threading functions

[Tal Shnaiderman]

Add API for generic threading functions in EAL which do not reference pthread API.

Tal Shnaiderman (2):
  eal: move thread affinity functions to new file
  eal: add generic thread-local-storage functions

 lib/librte_eal/include/meson.build  |   1 +
 lib/librte_eal/include/rte_lcore.h  |  22 +-------
 lib/librte_eal/include/rte_thread.h | 107 ++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |   5 ++
 lib/librte_eal/unix/meson.build     |   1 +
 lib/librte_eal/unix/rte_thread.c    |  86 +++++++++++++++++++++++++++++
 lib/librte_eal/version.map          |   6 ++
 lib/librte_eal/windows/meson.build  |   1 +
 lib/librte_eal/windows/rte_thread.c |  83 ++++++++++++++++++++++++++++
 9 files changed, 291 insertions(+), 21 deletions(-)
 create mode 100644 lib/librte_eal/include/rte_thread.h
 create mode 100644 lib/librte_eal/unix/rte_thread.c
 create mode 100644 lib/librte_eal/windows/rte_thread.c

-- 
2.16.1.windows.4

[dpdk-dev] [PATCH v7 1/2] eal: move thread affinity functions to new file

[Tal Shnaiderman]

Move the definition of the functions
rte_thread_set_affinity and rte_thread_get_affinity
to new file, rte_thread.h

The file will implement generic threading functionality
and will only host threading functions which do not reference
pthread API.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
 lib/librte_eal/include/meson.build  |  1 +
 lib/librte_eal/include/rte_lcore.h  | 22 +--------------------
 lib/librte_eal/include/rte_thread.h | 39 +++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 21 deletions(-)
 create mode 100644 lib/librte_eal/include/rte_thread.h

diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..0dea342e1d 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -40,6 +40,7 @@ headers += files(
 	'rte_service_component.h',
 	'rte_string_fns.h',
 	'rte_tailq.h',
+	'rte_thread.h',
 	'rte_time.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
diff --git a/lib/librte_eal/include/rte_lcore.h b/lib/librte_eal/include/rte_lcore.h
index 48b87e253a..0fe0bd839c 100644
--- a/lib/librte_eal/include/rte_lcore.h
+++ b/lib/librte_eal/include/rte_lcore.h
@@ -15,6 +15,7 @@
 #include <rte_per_lcore.h>
 #include <rte_eal.h>
 #include <rte_launch.h>
+#include <rte_thread.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -357,27 +358,6 @@ __rte_experimental
 void
 rte_lcore_dump(FILE *f);
 
-/**
- * Set core affinity of the current thread.
- * Support both EAL and non-EAL thread and update TLS.
- *
- * @param cpusetp
- *   Point to cpu_set_t for setting current thread affinity.
- * @return
- *   On success, return 0; otherwise return -1;
- */
-int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
-
-/**
- * Get core affinity of the current thread.
- *
- * @param cpusetp
- *   Point to cpu_set_t for getting current thread cpu affinity.
- *   It presumes input is not NULL, otherwise it causes panic.
- *
- */
-void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
-
 /**
  * Set thread names.
  *
diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
new file mode 100644
index 0000000000..dbc4b3adf8
--- /dev/null
+++ b/lib/librte_eal/include/rte_thread.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Mellanox Technologies, Ltd
+ */
+
+#include <rte_os.h>
+
+#ifndef _RTE_THREAD_H_
+#define _RTE_THREAD_H_
+
+/**
+ * @file
+ *
+ * Threading functions
+ *
+ * Simple threads functionality supplied by EAL.
+ */
+
+/**
+ * Set core affinity of the current thread.
+ * Support both EAL and non-EAL thread and update TLS.
+ *
+ * @param cpusetp
+ *   Point to cpu_set_t for setting current thread affinity.
+ * @return
+ *   On success, return 0; otherwise return -1;
+ */
+int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
+
+/**
+ * Get core affinity of the current thread.
+ *
+ * @param cpusetp
+ *   Point to cpu_set_t for getting current thread cpu affinity.
+ *   It presumes input is not NULL, otherwise it causes panic.
+ *
+ */
+void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
+
+#endif /* _RTE_THREAD_H_ */
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v7 1/2] eal: move thread affinity functions to new file

[Dmitry Kozlyuk]

On Tue,  5 Jan 2021 19:06:34 +0200, Tal Shnaiderman wrote:
[...]
> diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
> new file mode 100644
> index 0000000000..dbc4b3adf8
> --- /dev/null
> +++ b/lib/librte_eal/include/rte_thread.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Mellanox Technologies, Ltd
> + */
> +
> +#include <rte_os.h>
> +
> +#ifndef _RTE_THREAD_H_
> +#define _RTE_THREAD_H_

#ifdef __cplusplus
extern "C" {
#endif

[...]

#ifdef __cplusplus
}
#endif

> +#endif /* _RTE_THREAD_H_ */

[dpdk-dev] [PATCH v8 0/2] support generic threading functions

[Tal Shnaiderman]

Add API for generic threading functions in EAL which do not reference pthread API.
---
v8:
	* Documentation and minor code changes (DmitryK).
---
Tal Shnaiderman (2):
  eal: move thread affinity functions to new file
  eal: add generic thread-local-storage functions

 lib/librte_eal/include/meson.build  |   1 +
 lib/librte_eal/include/rte_lcore.h  |  22 +------
 lib/librte_eal/include/rte_thread.h | 113 ++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |   5 ++
 lib/librte_eal/unix/meson.build     |   1 +
 lib/librte_eal/unix/rte_thread.c    |  86 +++++++++++++++++++++++++++
 lib/librte_eal/version.map          |   6 ++
 lib/librte_eal/windows/meson.build  |   1 +
 lib/librte_eal/windows/rte_thread.c |  83 ++++++++++++++++++++++++++
 9 files changed, 297 insertions(+), 21 deletions(-)
 create mode 100644 lib/librte_eal/include/rte_thread.h
 create mode 100644 lib/librte_eal/unix/rte_thread.c
 create mode 100644 lib/librte_eal/windows/rte_thread.c

-- 
2.16.1.windows.4

[dpdk-dev] [PATCH v8 1/2] eal: move thread affinity functions to new file

[Tal Shnaiderman]

Move the definition of the functions
rte_thread_set_affinity and rte_thread_get_affinity
to new file, rte_thread.h

The file will implement generic threading functionality
and will only host threading functions which do not reference
pthread API.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
 lib/librte_eal/include/meson.build  |  1 +
 lib/librte_eal/include/rte_lcore.h  | 22 +----------------
 lib/librte_eal/include/rte_thread.h | 47 +++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 21 deletions(-)
 create mode 100644 lib/librte_eal/include/rte_thread.h

diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..0dea342e1d 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -40,6 +40,7 @@ headers += files(
 	'rte_service_component.h',
 	'rte_string_fns.h',
 	'rte_tailq.h',
+	'rte_thread.h',
 	'rte_time.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
diff --git a/lib/librte_eal/include/rte_lcore.h b/lib/librte_eal/include/rte_lcore.h
index 48b87e253a..0fe0bd839c 100644
--- a/lib/librte_eal/include/rte_lcore.h
+++ b/lib/librte_eal/include/rte_lcore.h
@@ -15,6 +15,7 @@
 #include <rte_per_lcore.h>
 #include <rte_eal.h>
 #include <rte_launch.h>
+#include <rte_thread.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -357,27 +358,6 @@ __rte_experimental
 void
 rte_lcore_dump(FILE *f);
 
-/**
- * Set core affinity of the current thread.
- * Support both EAL and non-EAL thread and update TLS.
- *
- * @param cpusetp
- *   Point to cpu_set_t for setting current thread affinity.
- * @return
- *   On success, return 0; otherwise return -1;
- */
-int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
-
-/**
- * Get core affinity of the current thread.
- *
- * @param cpusetp
- *   Point to cpu_set_t for getting current thread cpu affinity.
- *   It presumes input is not NULL, otherwise it causes panic.
- *
- */
-void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
-
 /**
  * Set thread names.
  *
diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
new file mode 100644
index 0000000000..43bf568d59
--- /dev/null
+++ b/lib/librte_eal/include/rte_thread.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Mellanox Technologies, Ltd
+ */
+
+#include <rte_os.h>
+
+#ifndef _RTE_THREAD_H_
+#define _RTE_THREAD_H_
+
+/**
+ * @file
+ *
+ * Threading functions
+ *
+ * Simple threads functionality supplied by EAL.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Set core affinity of the current thread.
+ * Support both EAL and non-EAL thread and update TLS.
+ *
+ * @param cpusetp
+ *   Point to cpu_set_t for setting current thread affinity.
+ * @return
+ *   On success, return 0; otherwise return -1;
+ */
+int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
+
+/**
+ * Get core affinity of the current thread.
+ *
+ * @param cpusetp
+ *   Point to cpu_set_t for getting current thread cpu affinity.
+ *   It presumes input is not NULL, otherwise it causes panic.
+ *
+ */
+void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_THREAD_H_ */
-- 
2.16.1.windows.4

[dpdk-dev] [PATCH v8 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for TLS functionality in EAL.

The following functions are added:
rte_thread_tls_key_create - create a TLS data key.
rte_thread_tls_key_delete - delete a TLS data key.
rte_thread_tls_value_set - set value bound to the TLS key
rte_thread_tls_value_get - get value bound to the TLS key

TLS key is defined by the new type rte_tls_key.

The API allocates the thread local storage (TLS) key.
Any thread of the process can subsequently use this key
to store and retrieve values that are local to the thread.

Those functions are added in addition to TLS capability
in rte_per_lcore.h to allow abstraction of the pthread
layer for all operating systems.

Windows implementation is under librte_eal/windows and
implemented using WIN32 API for Windows only.

Unix implementation is under librte_eal/unix and
implemented using pthread for UNIX compilation.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
v4: modify file names, function names, move unix code to common
for future external pthreads support [DmitryK]
v5: rename var used for external pthreads, add description in
meson_options.txt. [DmitryK]
v6: remove external_pthread support as it collide with pthread
shim implementation [DmitryK]
v7: move unix code to unix instead of common, rename functions
add more documentation [Thomas]
---
 lib/librte_eal/include/rte_thread.h | 66 ++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |  5 +++
 lib/librte_eal/unix/meson.build     |  1 +
 lib/librte_eal/unix/rte_thread.c    | 86 +++++++++++++++++++++++++++++++++++++
 lib/librte_eal/version.map          |  6 +++
 lib/librte_eal/windows/meson.build  |  1 +
 lib/librte_eal/windows/rte_thread.c | 83 +++++++++++++++++++++++++++++++++++
 7 files changed, 248 insertions(+)
 create mode 100644 lib/librte_eal/unix/rte_thread.c
 create mode 100644 lib/librte_eal/windows/rte_thread.c

diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
index 43bf568d59..ea1a1847de 100644
--- a/lib/librte_eal/include/rte_thread.h
+++ b/lib/librte_eal/include/rte_thread.h
@@ -19,6 +19,11 @@
 extern "C" {
 #endif
 
+/**
+ * TLS key type, an opaque pointer.
+ */
+typedef struct eal_tls_key *rte_tls_key;
+
 /**
  * Set core affinity of the current thread.
  * Support both EAL and non-EAL thread and update TLS.
@@ -40,6 +45,67 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
  */
 void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 
+/**
+ * Create a TLS data key visible to all threads in the process.
+ * the created key is later used to get/set a value.
+ * and optional destructor can be set to be called when a thread exits.
+ *
+ * @param key
+ *   Pointer to store the allocated rte_tls_key
+ * @param destructor
+ *   The function to be called when the thread exits.
+ *   Ignored on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+
+__rte_experimental
+int rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *));
+
+/**
+ * Delete a TLS data key visible to all threads in the process
+ * rte_tls_key is the opaque pointer allocated by rte_thread_tls_key_create.
+ *
+ * @param key
+ *   The rte_tls_key allocated by rte_thread_tls_key_create().
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int rte_thread_tls_key_delete(rte_tls_key key);
+
+/**
+ * Set value bound to the TLS key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_key_create.
+ * @param value
+ *   The value bound to the rte_tls_key key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int rte_thread_tls_value_set(rte_tls_key key, const void *value);
+
+/**
+ * Get value bound to the TLS key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_key_create.
+ *
+ * @return
+ *   On success, value data pointer (can also be NULL).
+ *   On failure, NULL and an error number is set in rte_errno.
+ */
+__rte_experimental
+void *rte_thread_tls_value_get(rte_tls_key key);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 6a6be1cfa6..a11b078ac6 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -306,6 +306,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_thread_tls_key_create
+	rte_thread_tls_key_delete
+	rte_thread_tls_value_set
+	rte_thread_tls_value_get
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/unix/meson.build b/lib/librte_eal/unix/meson.build
index d3af6b6fe2..71221b84a4 100644
--- a/lib/librte_eal/unix/meson.build
+++ b/lib/librte_eal/unix/meson.build
@@ -5,4 +5,5 @@ sources += files(
 	'eal_file.c',
 	'eal_unix_memory.c',
 	'eal_unix_timer.c',
+	'rte_thread.c',
 )
diff --git a/lib/librte_eal/unix/rte_thread.c b/lib/librte_eal/unix/rte_thread.c
new file mode 100644
index 0000000000..06c8093d78
--- /dev/null
+++ b/lib/librte_eal/unix/rte_thread.c
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(*key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.");
+		return -1;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 strerror(err));
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_key_delete(rte_tls_key key)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 strerror(err));
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_value_set(rte_tls_key key, const void *value)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			strerror(err));
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_value_get(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..272db0504a 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_thread_tls_key_create;
+	rte_thread_tls_key_delete;
+	rte_thread_tls_value_set;
+	rte_thread_tls_value_get;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..42ff5c2d59 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -19,6 +19,7 @@ sources += files(
 	'eal_timer.c',
 	'fnmatch.c',
 	'getopt.c',
+	'rte_thread.c',
 )
 
 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
new file mode 100644
index 0000000000..4608bfcc5e
--- /dev/null
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ */
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_thread.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_thread_tls_key_create(rte_tls_key *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(*key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.");
+		return -1;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_key_delete(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_value_set(rte_tls_key key, const void *value)
+{
+	char *p;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	/* discard const qualifier */
+	p = (char *) (uintptr_t) value;
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_value_get(rte_tls_key key)
+{
+	void *output;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v8 2/2] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Wed,  6 Jan 2021 21:45:43 +0200, Tal Shnaiderman wrote:
[...]
> +/**
> + * Delete a TLS data key visible to all threads in the process
> + * rte_tls_key is the opaque pointer allocated by rte_thread_tls_key_create.

This line repeats @param key description.

> + *
> + * @param key
> + *   The rte_tls_key allocated by rte_thread_tls_key_create().
> + *
> + * @return
> + *   On success, zero.
> + *   On failure, a negative number.
> + */
> +__rte_experimental
> +int rte_thread_tls_key_delete(rte_tls_key key);

[...]
> +
> +struct eal_tls_key {
> +	pthread_key_t thread_index;
> +};
> +
> +int
> +rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
> +{
> +	int err;
> +
> +	*key = malloc(sizeof(*key));

Should be sizeof(**key), which would be sizeof(struct eal_tls_key), as needed.
Same for Windows file.
I had to double-check my comment on v7.

Re: [dpdk-dev] [PATCH v8 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [PATCH v8 2/2] eal: add generic thread-local-storage functions
> 
> External email: Use caution opening links or attachments
> 
> 
> On Wed,  6 Jan 2021 21:45:43 +0200, Tal Shnaiderman wrote:
> [...]
> > +/**
> > + * Delete a TLS data key visible to all threads in the process
> > + * rte_tls_key is the opaque pointer allocated by
> rte_thread_tls_key_create.
> 
> This line repeats @param key description.
> 
> > + *
> > + * @param key
> > + *   The rte_tls_key allocated by rte_thread_tls_key_create().
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +__rte_experimental
> > +int rte_thread_tls_key_delete(rte_tls_key key);
> 
> [...]
> > +
> > +struct eal_tls_key {
> > +     pthread_key_t thread_index;
> > +};
> > +
> > +int
> > +rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void
> > +*)) {
> > +     int err;
> > +
> > +     *key = malloc(sizeof(*key));
> 
> Should be sizeof(**key), which would be sizeof(struct eal_tls_key), as
> needed.
> Same for Windows file.
> I had to double-check my comment on v7.

Thanks, missed this one as well, sending v9.

[dpdk-dev] [PATCH v9 0/2] support generic threading functions

[Tal Shnaiderman]

Add API for generic threading functions in EAL which do not reference pthread API.
---
v8:
	* Documentation and minor code changes (DmitryK).
v9: 
	* Fix sizeof value in key creation, docu change (DmitryK) 
---

Tal Shnaiderman (2):
  eal: move thread affinity functions to new file
  eal: add generic thread-local-storage functions

 lib/librte_eal/include/meson.build  |   1 +
 lib/librte_eal/include/rte_lcore.h  |  22 +------
 lib/librte_eal/include/rte_thread.h | 112 ++++++++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |   5 ++
 lib/librte_eal/unix/meson.build     |   1 +
 lib/librte_eal/unix/rte_thread.c    |  86 +++++++++++++++++++++++++++
 lib/librte_eal/version.map          |   6 ++
 lib/librte_eal/windows/meson.build  |   1 +
 lib/librte_eal/windows/rte_thread.c |  83 ++++++++++++++++++++++++++
 9 files changed, 296 insertions(+), 21 deletions(-)
 create mode 100644 lib/librte_eal/include/rte_thread.h
 create mode 100644 lib/librte_eal/unix/rte_thread.c
 create mode 100644 lib/librte_eal/windows/rte_thread.c

-- 
2.16.1.windows.4

[dpdk-dev] [PATCH v9 1/2] eal: move thread affinity functions to new file

[Tal Shnaiderman]

Move the definition of the functions
rte_thread_set_affinity and rte_thread_get_affinity
to new file, rte_thread.h

The file will implement generic threading functionality
and will only host threading functions which do not reference
pthread API.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
 lib/librte_eal/include/meson.build  |  1 +
 lib/librte_eal/include/rte_lcore.h  | 22 +----------------
 lib/librte_eal/include/rte_thread.h | 47 +++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 21 deletions(-)
 create mode 100644 lib/librte_eal/include/rte_thread.h

diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index dc007084ff..0dea342e1d 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -40,6 +40,7 @@ headers += files(
 	'rte_service_component.h',
 	'rte_string_fns.h',
 	'rte_tailq.h',
+	'rte_thread.h',
 	'rte_time.h',
 	'rte_trace.h',
 	'rte_trace_point.h',
diff --git a/lib/librte_eal/include/rte_lcore.h b/lib/librte_eal/include/rte_lcore.h
index 48b87e253a..0fe0bd839c 100644
--- a/lib/librte_eal/include/rte_lcore.h
+++ b/lib/librte_eal/include/rte_lcore.h
@@ -15,6 +15,7 @@
 #include <rte_per_lcore.h>
 #include <rte_eal.h>
 #include <rte_launch.h>
+#include <rte_thread.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -357,27 +358,6 @@ __rte_experimental
 void
 rte_lcore_dump(FILE *f);
 
-/**
- * Set core affinity of the current thread.
- * Support both EAL and non-EAL thread and update TLS.
- *
- * @param cpusetp
- *   Point to cpu_set_t for setting current thread affinity.
- * @return
- *   On success, return 0; otherwise return -1;
- */
-int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
-
-/**
- * Get core affinity of the current thread.
- *
- * @param cpusetp
- *   Point to cpu_set_t for getting current thread cpu affinity.
- *   It presumes input is not NULL, otherwise it causes panic.
- *
- */
-void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
-
 /**
  * Set thread names.
  *
diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
new file mode 100644
index 0000000000..43bf568d59
--- /dev/null
+++ b/lib/librte_eal/include/rte_thread.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Mellanox Technologies, Ltd
+ */
+
+#include <rte_os.h>
+
+#ifndef _RTE_THREAD_H_
+#define _RTE_THREAD_H_
+
+/**
+ * @file
+ *
+ * Threading functions
+ *
+ * Simple threads functionality supplied by EAL.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Set core affinity of the current thread.
+ * Support both EAL and non-EAL thread and update TLS.
+ *
+ * @param cpusetp
+ *   Point to cpu_set_t for setting current thread affinity.
+ * @return
+ *   On success, return 0; otherwise return -1;
+ */
+int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
+
+/**
+ * Get core affinity of the current thread.
+ *
+ * @param cpusetp
+ *   Point to cpu_set_t for getting current thread cpu affinity.
+ *   It presumes input is not NULL, otherwise it causes panic.
+ *
+ */
+void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_THREAD_H_ */
-- 
2.16.1.windows.4

[dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for TLS functionality in EAL.

The following functions are added:
rte_thread_tls_key_create - create a TLS data key.
rte_thread_tls_key_delete - delete a TLS data key.
rte_thread_tls_value_set - set value bound to the TLS key
rte_thread_tls_value_get - get value bound to the TLS key

TLS key is defined by the new type rte_tls_key.

The API allocates the thread local storage (TLS) key.
Any thread of the process can subsequently use this key
to store and retrieve values that are local to the thread.

Those functions are added in addition to TLS capability
in rte_per_lcore.h to allow abstraction of the pthread
layer for all operating systems.

Windows implementation is under librte_eal/windows and
implemented using WIN32 API for Windows only.

Unix implementation is under librte_eal/unix and
implemented using pthread for UNIX compilation.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
v4: modify file names, function names, move unix code to common
for future external pthreads support [DmitryK]
v5: rename var used for external pthreads, add description in
meson_options.txt. [DmitryK]
v6: remove external_pthread support as it collide with pthread
shim implementation [DmitryK]
v7: move unix code to unix instead of common, rename functions
add more documentation [Thomas]
---
 lib/librte_eal/include/rte_thread.h | 65 ++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |  5 +++
 lib/librte_eal/unix/meson.build     |  1 +
 lib/librte_eal/unix/rte_thread.c    | 86 +++++++++++++++++++++++++++++++++++++
 lib/librte_eal/version.map          |  6 +++
 lib/librte_eal/windows/meson.build  |  1 +
 lib/librte_eal/windows/rte_thread.c | 83 +++++++++++++++++++++++++++++++++++
 7 files changed, 247 insertions(+)
 create mode 100644 lib/librte_eal/unix/rte_thread.c
 create mode 100644 lib/librte_eal/windows/rte_thread.c

diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
index 43bf568d59..eca62b29ee 100644
--- a/lib/librte_eal/include/rte_thread.h
+++ b/lib/librte_eal/include/rte_thread.h
@@ -19,6 +19,11 @@
 extern "C" {
 #endif
 
+/**
+ * TLS key type, an opaque pointer.
+ */
+typedef struct eal_tls_key *rte_tls_key;
+
 /**
  * Set core affinity of the current thread.
  * Support both EAL and non-EAL thread and update TLS.
@@ -40,6 +45,66 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
  */
 void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 
+/**
+ * Create a TLS data key visible to all threads in the process.
+ * the created key is later used to get/set a value.
+ * and optional destructor can be set to be called when a thread exits.
+ *
+ * @param key
+ *   Pointer to store the allocated rte_tls_key
+ * @param destructor
+ *   The function to be called when the thread exits.
+ *   Ignored on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+
+__rte_experimental
+int rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *));
+
+/**
+ * Delete a TLS data key visible to all threads in the process.
+ *
+ * @param key
+ *   The rte_tls_key allocated by rte_thread_tls_key_create().
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int rte_thread_tls_key_delete(rte_tls_key key);
+
+/**
+ * Set value bound to the TLS key on behalf of the calling thread.
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_key_create.
+ * @param value
+ *   The value bound to the rte_tls_key key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int rte_thread_tls_value_set(rte_tls_key key, const void *value);
+
+/**
+ * Get value bound to the TLS key on behalf of the calling thread.
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_key_create.
+ *
+ * @return
+ *   On success, value data pointer (can also be NULL).
+ *   On failure, NULL and an error number is set in rte_errno.
+ */
+__rte_experimental
+void *rte_thread_tls_value_get(rte_tls_key key);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 6a6be1cfa6..a11b078ac6 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -306,6 +306,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_thread_tls_key_create
+	rte_thread_tls_key_delete
+	rte_thread_tls_value_set
+	rte_thread_tls_value_get
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/unix/meson.build b/lib/librte_eal/unix/meson.build
index d3af6b6fe2..71221b84a4 100644
--- a/lib/librte_eal/unix/meson.build
+++ b/lib/librte_eal/unix/meson.build
@@ -5,4 +5,5 @@ sources += files(
 	'eal_file.c',
 	'eal_unix_memory.c',
 	'eal_unix_timer.c',
+	'rte_thread.c',
 )
diff --git a/lib/librte_eal/unix/rte_thread.c b/lib/librte_eal/unix/rte_thread.c
new file mode 100644
index 0000000000..ae58e1bf31
--- /dev/null
+++ b/lib/librte_eal/unix/rte_thread.c
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(**key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.");
+		return -1;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 strerror(err));
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_key_delete(rte_tls_key key)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 strerror(err));
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_value_set(rte_tls_key key, const void *value)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			strerror(err));
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_value_get(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..272db0504a 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_thread_tls_key_create;
+	rte_thread_tls_key_delete;
+	rte_thread_tls_value_set;
+	rte_thread_tls_value_get;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..42ff5c2d59 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -19,6 +19,7 @@ sources += files(
 	'eal_timer.c',
 	'fnmatch.c',
 	'getopt.c',
+	'rte_thread.c',
 )
 
 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
new file mode 100644
index 0000000000..5cb3a91483
--- /dev/null
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ */
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_thread.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_thread_tls_key_create(rte_tls_key *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(**key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.");
+		return -1;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_key_delete(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_value_set(rte_tls_key key, const void *value)
+{
+	char *p;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		return -1;
+	}
+	/* discard const qualifier */
+	p = (char *) (uintptr_t) value;
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_value_get(rte_tls_key key)
+{
+	void *output;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Wed,  6 Jan 2021 22:35:53 +0200, Tal Shnaiderman wrote:
[...]
> +int
> +rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
> +{
> +	int err;
> +
> +	*key = malloc(sizeof(**key));
> +	if ((*key) == NULL) {
> +		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.");

Missing "\n", same for Windows part.

Aside from this nit, for series,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage functions

[Burakov, Anatoly]

On 06-Jan-21 8:35 PM, Tal Shnaiderman wrote:
> Add support for TLS functionality in EAL.
> 
> The following functions are added:
> rte_thread_tls_key_create - create a TLS data key.
> rte_thread_tls_key_delete - delete a TLS data key.
> rte_thread_tls_value_set - set value bound to the TLS key
> rte_thread_tls_value_get - get value bound to the TLS key
> 
> TLS key is defined by the new type rte_tls_key.
> 
> The API allocates the thread local storage (TLS) key.
> Any thread of the process can subsequently use this key
> to store and retrieve values that are local to the thread.
> 
> Those functions are added in addition to TLS capability
> in rte_per_lcore.h to allow abstraction of the pthread
> layer for all operating systems.
> 
> Windows implementation is under librte_eal/windows and
> implemented using WIN32 API for Windows only.
> 
> Unix implementation is under librte_eal/unix and
> implemented using pthread for UNIX compilation.
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> ---

I'm a bit late to the party as the patch has been merged already, but 
perhaps as a further enhancement, you could add rte_errno setting for 
errors? You seem to have it only for get() API but not for others, and 
you pass pthread_setspecific()'s return value unmodified, even though it 
might return an error. Presumably, these error codes would be different 
on Unix and Windows, so return values would effectively be potentially 
different on different OS's, so perhaps it's better to return -1 with 
rte_errno to indicate which specific error was seen.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage functions

[Medvedkin, Vladimir]

Hi Tal,

On 10/02/2021 13:33, Burakov, Anatoly wrote:
> On 06-Jan-21 8:35 PM, Tal Shnaiderman wrote:
>> Add support for TLS functionality in EAL.
>>
>> The following functions are added:
>> rte_thread_tls_key_create - create a TLS data key.
>> rte_thread_tls_key_delete - delete a TLS data key.
>> rte_thread_tls_value_set - set value bound to the TLS key
>> rte_thread_tls_value_get - get value bound to the TLS key
>>
>> TLS key is defined by the new type rte_tls_key.

I would suggest changing rte_tls_key to rte_thread_tls_key so that it is 
consistent with the API and not confused with transport layer security.

>>
>> The API allocates the thread local storage (TLS) key.
>> Any thread of the process can subsequently use this key
>> to store and retrieve values that are local to the thread.
>>
>> Those functions are added in addition to TLS capability
>> in rte_per_lcore.h to allow abstraction of the pthread
>> layer for all operating systems.
>>
>> Windows implementation is under librte_eal/windows and
>> implemented using WIN32 API for Windows only.
>>
>> Unix implementation is under librte_eal/unix and
>> implemented using pthread for UNIX compilation.
>>
>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
>> ---
> 

-- 
Regards,
Vladimir

Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage
> functions
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Tal,
> 
> On 10/02/2021 13:33, Burakov, Anatoly wrote:
> > On 06-Jan-21 8:35 PM, Tal Shnaiderman wrote:
> >> Add support for TLS functionality in EAL.
> >>
> >> The following functions are added:
> >> rte_thread_tls_key_create - create a TLS data key.
> >> rte_thread_tls_key_delete - delete a TLS data key.
> >> rte_thread_tls_value_set - set value bound to the TLS key
> >> rte_thread_tls_value_get - get value bound to the TLS key
> >>
> >> TLS key is defined by the new type rte_tls_key.
> 
> I would suggest changing rte_tls_key to rte_thread_tls_key so that it is
> consistent with the API and not confused with transport layer security.
> 

Thanks Vladimir, will modify for 21.05.

> >>
> >> The API allocates the thread local storage (TLS) key.
> >> Any thread of the process can subsequently use this key to store and
> >> retrieve values that are local to the thread.
> >>
> >> Those functions are added in addition to TLS capability in
> >> rte_per_lcore.h to allow abstraction of the pthread layer for all
> >> operating systems.
> >>
> >> Windows implementation is under librte_eal/windows and implemented
> >> using WIN32 API for Windows only.
> >>
> >> Unix implementation is under librte_eal/unix and implemented using
> >> pthread for UNIX compilation.
> >>
> >> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> >> ---
> >
> 
> --
> Regards,
> Vladimir

Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [dpdk-dev] [PATCH v9 2/2] eal: add generic thread-local-storage
> functions
> 
> External email: Use caution opening links or attachments
> 
> 
> On 06-Jan-21 8:35 PM, Tal Shnaiderman wrote:
> > Add support for TLS functionality in EAL.
> >
> > The following functions are added:
> > rte_thread_tls_key_create - create a TLS data key.
> > rte_thread_tls_key_delete - delete a TLS data key.
> > rte_thread_tls_value_set - set value bound to the TLS key
> > rte_thread_tls_value_get - get value bound to the TLS key
> >
> > TLS key is defined by the new type rte_tls_key.
> >
> > The API allocates the thread local storage (TLS) key.
> > Any thread of the process can subsequently use this key to store and
> > retrieve values that are local to the thread.
> >
> > Those functions are added in addition to TLS capability in
> > rte_per_lcore.h to allow abstraction of the pthread layer for all
> > operating systems.
> >
> > Windows implementation is under librte_eal/windows and implemented
> > using WIN32 API for Windows only.
> >
> > Unix implementation is under librte_eal/unix and implemented using
> > pthread for UNIX compilation.
> >
> > Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > ---
> 
> I'm a bit late to the party as the patch has been merged already, but perhaps
> as a further enhancement, you could add rte_errno setting for errors? You
> seem to have it only for get() API but not for others, and you pass
> pthread_setspecific()'s return value unmodified, even though it might return
> an error. Presumably, these error codes would be different on Unix and
> Windows, so return values would effectively be potentially different on
> different OS's, so perhaps it's better to return -1 with rte_errno to indicate
> which specific error was seen.
> 

Thanks Anatoly, I'll add the error codes to rte_errno in 21.05.

> --
> Thanks,
> Anatoly

Re: [dpdk-dev] [PATCH v9 0/2] support generic threading functions

[Thomas Monjalon]

06/01/2021 21:35, Tal Shnaiderman:
> Tal Shnaiderman (2):
>   eal: move thread affinity functions to new file
>   eal: add generic thread-local-storage functions

Acked and applied with required minor changes, thanks.

[dpdk-dev] [PATCH v7 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

Add support for TLS functionality in EAL.

The following functions are added:
rte_thread_tls_key_create - function to create a TLS data key.
rte_thread_tls_key_delete - function to delete a TLS data key.
rte_thread_tls_value_set - function to set value bound to the TLS key
rte_thread_tls_value_get - function to get value bound to the TLS key

TLS key will be defined by the new type rte_tls_key.

The API Allocates the thread local storage (TLS) key.
Any thread of the process can subsequently use this key
to store and retrieve values that are local to the thread.

Those functions are added in addition to TLS capability
in rte_per_lcore.h to allow user to use the thread reasouce
destruction capability in rte_thread_tls_key_create.

Windows implementation is under librte_eal/windows and
implemented using WIN32 API for Windows only.

common implementation is under librte_eal/common and
implemented using pthread for UNIX compilation.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
v3: switch from pthread shim to generic eal implementation [DmitryK]
v4: modify file names, function names, move unix code to common
for future external pthreads support [DmitryK]
v5: rename var used for external pthreads, add description in
meson_options.txt. [DmitryK]
v6: remove external_pthread support as it collide with pthread
shim implementation [DmitryK]
v7: move unix code to unix instead of common, rename functions
add more documentation [Thomas]
---
 lib/librte_eal/include/rte_thread.h | 68 +++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_exports.def  |  5 +++
 lib/librte_eal/unix/meson.build     |  1 +
 lib/librte_eal/unix/rte_thread.c    | 86 +++++++++++++++++++++++++++++++++++++
 lib/librte_eal/version.map          |  6 +++
 lib/librte_eal/windows/meson.build  |  1 +
 lib/librte_eal/windows/rte_thread.c | 83 +++++++++++++++++++++++++++++++++++
 7 files changed, 250 insertions(+)
 create mode 100644 lib/librte_eal/unix/rte_thread.c
 create mode 100644 lib/librte_eal/windows/rte_thread.c

diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
index dbc4b3adf8..f5c5290ffc 100644
--- a/lib/librte_eal/include/rte_thread.h
+++ b/lib/librte_eal/include/rte_thread.h
@@ -15,6 +15,11 @@
  * Simple threads functionality supplied by EAL.
  */
 
+/**
+ * Opaque pointer for TLS key.
+ */
+typedef struct eal_tls_key *rte_tls_key;
+
 /**
  * Set core affinity of the current thread.
  * Support both EAL and non-EAL thread and update TLS.
@@ -36,4 +41,67 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
  */
 void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 
+/**
+ * Function to create a TLS data key visible to all threads in the process
+ * function need to be called once to create a key usable by all threads.
+ * rte_tls_key is an opaque pointer used to store the allocated key.
+ * which is later used to get/set a value.
+ * and optional destructor can be set to be called when a thread expires.
+ *
+ * @param key
+ *   Pointer to store the allocated rte_tls_key
+ * @param destructor
+ *   The function to be called when the thread expires.
+ *   Ignored on Windows OS.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+
+__rte_experimental
+int rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *));
+
+/**
+ * Function to delete a TLS data key visible to all threads in the process
+ * rte_tls_key is the opaque pointer allocated by rte_thread_tls_key_create.
+ *
+ * @param key
+ *   The rte_tls_key will contain the allocated key
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int rte_thread_tls_key_delete(rte_tls_key key);
+
+/**
+ * Function to set value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_key_create.
+ * @param value
+ *   The value bound to the rte_tls_key key for the calling thread.
+ *
+ * @return
+ *   On success, zero.
+ *   On failure, a negative number.
+ */
+__rte_experimental
+int rte_thread_tls_value_set(rte_tls_key key, const void *value);
+
+/**
+ * Function to get value bound to the tls key on behalf of the calling thread
+ *
+ * @param key
+ *   The rte_tls_key key allocated by rte_thread_tls_key_create.
+ *
+ * @return
+ *   On success, value data pointer (can also be NULL).
+ *   On failure, NULL and an error number is set in rte_errno.
+ */
+__rte_experimental
+void *rte_thread_tls_value_get(rte_tls_key key);
+
 #endif /* _RTE_THREAD_H_ */
diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 6a6be1cfa6..a11b078ac6 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -306,6 +306,11 @@ EXPORTS
 	rte_vect_get_max_simd_bitwidth
 	rte_vect_set_max_simd_bitwidth
 
+	rte_thread_tls_key_create
+	rte_thread_tls_key_delete
+	rte_thread_tls_value_set
+	rte_thread_tls_value_get
+
 	rte_mem_lock
 	rte_mem_map
 	rte_mem_page_size
diff --git a/lib/librte_eal/unix/meson.build b/lib/librte_eal/unix/meson.build
index d3af6b6fe2..71221b84a4 100644
--- a/lib/librte_eal/unix/meson.build
+++ b/lib/librte_eal/unix/meson.build
@@ -5,4 +5,5 @@ sources += files(
 	'eal_file.c',
 	'eal_unix_memory.c',
 	'eal_unix_timer.c',
+	'rte_thread.c',
 )
diff --git a/lib/librte_eal/unix/rte_thread.c b/lib/librte_eal/unix/rte_thread.c
new file mode 100644
index 0000000000..2a88aea6a6
--- /dev/null
+++ b/lib/librte_eal/unix/rte_thread.c
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+	pthread_key_t thread_index;
+};
+
+int
+rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
+{
+	int err;
+
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	err = pthread_key_create(&((*key)->thread_index), destructor);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+			 strerror(err));
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_key_delete(rte_tls_key key)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_key_delete(key->thread_index);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+			 strerror(err));
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_value_set(rte_tls_key key, const void *value)
+{
+	int err;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	err = pthread_setspecific(key->thread_index, value);
+	if (err) {
+		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+			strerror(err));
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_value_get(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	return pthread_getspecific(key->thread_index);
+}
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 354c068f31..272db0504a 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -403,6 +403,12 @@ EXPERIMENTAL {
 	rte_service_lcore_may_be_active;
 	rte_vect_get_max_simd_bitwidth;
 	rte_vect_set_max_simd_bitwidth;
+
+	# added in 21.02
+	rte_thread_tls_key_create;
+	rte_thread_tls_key_delete;
+	rte_thread_tls_value_set;
+	rte_thread_tls_value_get;
 };
 
 INTERNAL {
diff --git a/lib/librte_eal/windows/meson.build b/lib/librte_eal/windows/meson.build
index 3b2faf29eb..42ff5c2d59 100644
--- a/lib/librte_eal/windows/meson.build
+++ b/lib/librte_eal/windows/meson.build
@@ -19,6 +19,7 @@ sources += files(
 	'eal_timer.c',
 	'fnmatch.c',
 	'getopt.c',
+	'rte_thread.c',
 )
 
 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true)
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
new file mode 100644
index 0000000000..4ced283c62
--- /dev/null
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ */
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_thread.h>
+#include <rte_windows.h>
+
+struct eal_tls_key {
+	DWORD thread_index;
+};
+
+int
+rte_thread_tls_key_create(rte_tls_key *key,
+		__rte_unused void (*destructor)(void *))
+{
+	*key = malloc(sizeof(struct eal_tls_key));
+	if ((*key) == NULL) {
+		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
+		return -1;
+	}
+	(*key)->thread_index = TlsAlloc();
+	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
+		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		free(*key);
+		return -1;
+	}
+	return 0;
+}
+
+int
+rte_thread_tls_key_delete(rte_tls_key key)
+{
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	if (!TlsFree(key->thread_index)) {
+		RTE_LOG_WIN32_ERR("TlsFree()");
+		free(key);
+		return -1;
+	}
+	free(key);
+	return 0;
+}
+
+int
+rte_thread_tls_value_set(rte_tls_key key, const void *value)
+{
+	char *p;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		return -1;
+	}
+	/* discard const qualifier */
+	p = (char *) (uintptr_t) value;
+	if (!TlsSetValue(key->thread_index, p)) {
+		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		return -1;
+	}
+	return 0;
+}
+
+void *
+rte_thread_tls_value_get(rte_tls_key key)
+{
+	void *output;
+
+	if (!key) {
+		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");
+		rte_errno = EINVAL;
+		return NULL;
+	}
+	output = TlsGetValue(key->thread_index);
+	if (GetLastError() != ERROR_SUCCESS) {
+		RTE_LOG_WIN32_ERR("TlsGetValue()");
+		rte_errno = ENOEXEC;
+		return NULL;
+	}
+	return output;
+}
-- 
2.16.1.windows.4

Re: [dpdk-dev] [PATCH v7 2/2] eal: add generic thread-local-storage functions

[Dmitry Kozlyuk]

On Tue,  5 Jan 2021 19:06:35 +0200, Tal Shnaiderman wrote:
> Add support for TLS functionality in EAL.
> 
> The following functions are added:
> rte_thread_tls_key_create - function to create a TLS data key.
> rte_thread_tls_key_delete - function to delete a TLS data key.
> rte_thread_tls_value_set - function to set value bound to the TLS key
> rte_thread_tls_value_get - function to get value bound to the TLS key

"Function to" is redundant both here and in Doxygen comments.

> TLS key will be defined by the new type rte_tls_key.

Please use present tense when describing patch content.

> The API Allocates the thread local storage (TLS) key.

Typo: "allocates".

> Any thread of the process can subsequently use this key
> to store and retrieve values that are local to the thread.
> 
> Those functions are added in addition to TLS capability
> in rte_per_lcore.h to allow user to use the thread reasouce
> destruction capability in rte_thread_tls_key_create.

Err, I assumed we're abstracting pthread, otherwise the capability has
already been there. Also, a typo: "resource".

> Windows implementation is under librte_eal/windows and
> implemented using WIN32 API for Windows only.
> 
> common implementation is under librte_eal/common and
> implemented using pthread for UNIX compilation.

librte_eal/unix

[...]
> +/**
> + * Opaque pointer for TLS key.
> + */
> +typedef struct eal_tls_key *rte_tls_key;

Suggesting "TLS key type, an opaque pointer". Now it's unclear if there's a
TLS key and a pointer to it or TLS key is an opaque pointer itself (which is
the case, API-wise).

> +
>  /**
>   * Set core affinity of the current thread.
>   * Support both EAL and non-EAL thread and update TLS.
> @@ -36,4 +41,67 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
>   */
>  void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
>  
> +/**
> + * Function to create a TLS data key visible to all threads in the process
> + * function need to be called once to create a key usable by all threads.

The sentences repeat each other and are not separated.

> + * rte_tls_key is an opaque pointer used to store the allocated key.
> + * which is later used to get/set a value.
> + * and optional destructor can be set to be called when a thread expires.

"expires" -> "exits", here and below.

No need to repeat parameter description in function description.

> + *
> + * @param key
> + *   Pointer to store the allocated rte_tls_key
> + * @param destructor
> + *   The function to be called when the thread expires.
> + *   Ignored on Windows OS.
> + *
> + * @return
> + *   On success, zero.
> + *   On failure, a negative number.
> + */
> +
> +__rte_experimental
> +int rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *));

> +
> +/**
> + * Function to delete a TLS data key visible to all threads in the process
> + * rte_tls_key is the opaque pointer allocated by rte_thread_tls_key_create.
> + *
> + * @param key
> + *   The rte_tls_key will contain the allocated key

"Will"? It's an rte_tls_key key allocated by rte_thread_tls_key_create().

> + *
> + * @return
> + *   On success, zero.
> + *   On failure, a negative number.
> + */
> +__rte_experimental
> +int rte_thread_tls_key_delete(rte_tls_key key);

> +/**
> + * Function to set value bound to the tls key on behalf of the calling thread

"tls" -> "TLS"

> + *
> + * @param key
> + *   The rte_tls_key key allocated by rte_thread_tls_key_create.
> + * @param value
> + *   The value bound to the rte_tls_key key for the calling thread.
> + *
> + * @return
> + *   On success, zero.
> + *   On failure, a negative number.
> + */
> +__rte_experimental
> +int rte_thread_tls_value_set(rte_tls_key key, const void *value);
> +
> +/**
> + * Function to get value bound to the tls key on behalf of the calling thread

"tls" -> "TLS"

> + *
> + * @param key
> + *   The rte_tls_key key allocated by rte_thread_tls_key_create.
> + *
> + * @return
> + *   On success, value data pointer (can also be NULL).
> + *   On failure, NULL and an error number is set in rte_errno.
> + */
> +__rte_experimental
> +void *rte_thread_tls_value_get(rte_tls_key key);
> +
>  #endif /* _RTE_THREAD_H_ */

> diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
> new file mode 100644
> index 0000000000..4ced283c62
> --- /dev/null
> +++ b/lib/librte_eal/windows/rte_thread.c
> @@ -0,0 +1,83 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2021 Mellanox Technologies, Ltd
> + */
> +
> +#include <rte_common.h>
> +#include <rte_errno.h>
> +#include <rte_thread.h>
> +#include <rte_windows.h>
> +
> +struct eal_tls_key {
> +	DWORD thread_index;
> +};
> +
> +int
> +rte_thread_tls_key_create(rte_tls_key *key,
> +		__rte_unused void (*destructor)(void *))
> +{
> +	*key = malloc(sizeof(struct eal_tls_key));

sizeof(*key)

> +	if ((*key) == NULL) {
> +		RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");

"tls" -> "TLS"

> +		return -1;
> +	}
> +	(*key)->thread_index = TlsAlloc();
> +	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
> +		RTE_LOG_WIN32_ERR("TlsAlloc()");
> +		free(*key);
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +int
> +rte_thread_tls_key_delete(rte_tls_key key)
> +{
> +	if (!key) {
> +		RTE_LOG(DEBUG, EAL, "invalid tls key passed to function.\n");

"tls" -> "TLS"
Messages should start with a capital letter.
To which "function"?

> +		return -1;
> +	}
> +	if (!TlsFree(key->thread_index)) {
> +		RTE_LOG_WIN32_ERR("TlsFree()");
> +		free(key);
> +		return -1;
> +	}
> +	free(key);
> +	return 0;
> +}

Re: [dpdk-dev] [PATCH v7 2/2] eal: add generic thread-local-storage functions

[Tal Shnaiderman]

> Subject: Re: [PATCH v7 2/2] eal: add generic thread-local-storage functions
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue,  5 Jan 2021 19:06:35 +0200, Tal Shnaiderman wrote:
> > Add support for TLS functionality in EAL.
> >
> > The following functions are added:
> > rte_thread_tls_key_create - function to create a TLS data key.
> > rte_thread_tls_key_delete - function to delete a TLS data key.
> > rte_thread_tls_value_set - function to set value bound to the TLS key
> > rte_thread_tls_value_get - function to get value bound to the TLS key
> 
> "Function to" is redundant both here and in Doxygen comments.
> 
> > TLS key will be defined by the new type rte_tls_key.
> 
> Please use present tense when describing patch content.
> 
> > The API Allocates the thread local storage (TLS) key.
> 
> Typo: "allocates".
> 
> > Any thread of the process can subsequently use this key to store and
> > retrieve values that are local to the thread.
> >
> > Those functions are added in addition to TLS capability in
> > rte_per_lcore.h to allow user to use the thread reasouce destruction
> > capability in rte_thread_tls_key_create.
> 
> Err, I assumed we're abstracting pthread, otherwise the capability has
> already been there. Also, a typo: "resource".
> 
> > Windows implementation is under librte_eal/windows and implemented
> > using WIN32 API for Windows only.
> >
> > common implementation is under librte_eal/common and implemented
> using
> > pthread for UNIX compilation.
> 
> librte_eal/unix
> 
> [...]
> > +/**
> > + * Opaque pointer for TLS key.
> > + */
> > +typedef struct eal_tls_key *rte_tls_key;
> 
> Suggesting "TLS key type, an opaque pointer". Now it's unclear if there's a
> TLS key and a pointer to it or TLS key is an opaque pointer itself (which is the
> case, API-wise).
> 
> > +
> >  /**
> >   * Set core affinity of the current thread.
> >   * Support both EAL and non-EAL thread and update TLS.
> > @@ -36,4 +41,67 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
> >   */
> >  void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
> >
> > +/**
> > + * Function to create a TLS data key visible to all threads in the
> > +process
> > + * function need to be called once to create a key usable by all threads.
> 
> The sentences repeat each other and are not separated.
> 
> > + * rte_tls_key is an opaque pointer used to store the allocated key.
> > + * which is later used to get/set a value.
> > + * and optional destructor can be set to be called when a thread expires.
> 
> "expires" -> "exits", here and below.
> 
> No need to repeat parameter description in function description.
> 
> > + *
> > + * @param key
> > + *   Pointer to store the allocated rte_tls_key
> > + * @param destructor
> > + *   The function to be called when the thread expires.
> > + *   Ignored on Windows OS.
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +
> > +__rte_experimental
> > +int rte_thread_tls_key_create(rte_tls_key *key, void
> > +(*destructor)(void *));
> 
> > +
> > +/**
> > + * Function to delete a TLS data key visible to all threads in the
> > +process
> > + * rte_tls_key is the opaque pointer allocated by
> rte_thread_tls_key_create.
> > + *
> > + * @param key
> > + *   The rte_tls_key will contain the allocated key
> 
> "Will"? It's an rte_tls_key key allocated by rte_thread_tls_key_create().
> 
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +__rte_experimental
> > +int rte_thread_tls_key_delete(rte_tls_key key);
> 
> > +/**
> > + * Function to set value bound to the tls key on behalf of the
> > +calling thread
> 
> "tls" -> "TLS"
> 
> > + *
> > + * @param key
> > + *   The rte_tls_key key allocated by rte_thread_tls_key_create.
> > + * @param value
> > + *   The value bound to the rte_tls_key key for the calling thread.
> > + *
> > + * @return
> > + *   On success, zero.
> > + *   On failure, a negative number.
> > + */
> > +__rte_experimental
> > +int rte_thread_tls_value_set(rte_tls_key key, const void *value);
> > +
> > +/**
> > + * Function to get value bound to the tls key on behalf of the
> > +calling thread
> 
> "tls" -> "TLS"
> 
> > + *
> > + * @param key
> > + *   The rte_tls_key key allocated by rte_thread_tls_key_create.
> > + *
> > + * @return
> > + *   On success, value data pointer (can also be NULL).
> > + *   On failure, NULL and an error number is set in rte_errno.
> > + */
> > +__rte_experimental
> > +void *rte_thread_tls_value_get(rte_tls_key key);
> > +
> >  #endif /* _RTE_THREAD_H_ */
> 
> > diff --git a/lib/librte_eal/windows/rte_thread.c
> > b/lib/librte_eal/windows/rte_thread.c
> > new file mode 100644
> > index 0000000000..4ced283c62
> > --- /dev/null
> > +++ b/lib/librte_eal/windows/rte_thread.c
> > @@ -0,0 +1,83 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2021 Mellanox Technologies, Ltd  */
> > +
> > +#include <rte_common.h>
> > +#include <rte_errno.h>
> > +#include <rte_thread.h>
> > +#include <rte_windows.h>
> > +
> > +struct eal_tls_key {
> > +     DWORD thread_index;
> > +};
> > +
> > +int
> > +rte_thread_tls_key_create(rte_tls_key *key,
> > +             __rte_unused void (*destructor)(void *)) {
> > +     *key = malloc(sizeof(struct eal_tls_key));
> 
> sizeof(*key)
> 
> > +     if ((*key) == NULL) {
> > +             RTE_LOG(DEBUG, EAL, "Cannot allocate tls key.");
> 
> "tls" -> "TLS"
> 
> > +             return -1;
> > +     }
> > +     (*key)->thread_index = TlsAlloc();
> > +     if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
> > +             RTE_LOG_WIN32_ERR("TlsAlloc()");
> > +             free(*key);
> > +             return -1;
> > +     }
> > +     return 0;
> > +}
> > +
> > +int
> > +rte_thread_tls_key_delete(rte_tls_key key) {
> > +     if (!key) {
> > +             RTE_LOG(DEBUG, EAL, "invalid tls key passed to
> > +function.\n");
> 
> "tls" -> "TLS"
> Messages should start with a capital letter.
> To which "function"?
> 
> > +             return -1;
> > +     }
> > +     if (!TlsFree(key->thread_index)) {
> > +             RTE_LOG_WIN32_ERR("TlsFree()");
> > +             free(key);
> > +             return -1;
> > +     }
> > +     free(key);
> > +     return 0;
> > +}

Thanks for the review Dmitry, I'll fix and resend both patches.