[PATCH] eal: introduce missing rte_thread wrappers

[PATCH] eal: introduce missing rte_thread wrappers

[David Vodak]

Function rte_ctrl_thread_create has been replaced by rte_thread_create_control,
encouraging Linux users to switch from the pthread_t API to the rte_thread API.
However the rte_thread API does not provide wrappers for all pthread functions.
This commit introduces equivalent functions for pthread_timedjoin_np,
pthread_getname_np and pthread_cancel.

Bugzilla ID: 1330
---
 lib/eal/include/rte_thread.h | 49 ++++++++++++++++++++++++++++++++++++
 lib/eal/unix/rte_thread.c    | 20 +++++++++++++++
 lib/eal/version.map          |  3 +++
 3 files changed, 72 insertions(+)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 8da9d4d3fb..49a068eda7 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -463,6 +463,55 @@ int rte_thread_value_set(rte_thread_key key, const void *value);
  */
 void *rte_thread_value_get(rte_thread_key key);
 
+/**
+ * Try to join with a terminated thread. If thread has not yet terminated,
+ * then the call blocks until a maximum time, specified in abstime.
+ *
+ * @param thread_id
+ *   Id of the thread.
+ *
+ * @param retval
+ *   Exit status of the thread.
+ *
+ * @param abstime
+ *   Maximum time of call blocking, measured against the CLOCK_REALTIME clock.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+int rte_thread_timedjoin_np(rte_thread_t thread_id, void **retval, const struct timespec *abstime);
+
+/**
+ * Get name of the thread.
+ *
+ * @param thread_id
+ *   Id of the thread.
+ *
+ * @param name
+ *   Name of the thread.
+ *
+ * @param size
+ *   The number of bytes available in name.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+int rte_thread_getname_np(rte_thread_t thread_id, char name[], size_t size);
+
+/**
+ * Send a cancelation request to a thread.
+ *
+ * @param thread_id
+ *   Id of the thread.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+int rte_thread_cancel(rte_thread_t thread_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 36a21ab2f9..79986bee6a 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -378,3 +378,23 @@ rte_thread_get_affinity_by_id(rte_thread_t thread_id,
 	return pthread_getaffinity_np((pthread_t)thread_id.opaque_id,
 		sizeof(*cpuset), cpuset);
 }
+
+int
+rte_thread_timedjoin_np(rte_thread_t thread_id, void **retval,
+		const struct timespec *abstime)
+{
+	return pthread_timedjoin_np((pthread_t) thread_id.opaque_id, retval,
+		abstime);
+}
+
+int
+rte_thread_getname_np(rte_thread_t thread_id, char name[], size_t size)
+{
+	return pthread_getname_np((pthread_t) thread_id.opaque_id, name, size);
+}
+
+int
+rte_thread_cancel(rte_thread_t thread_id)
+{
+	return pthread_cancel((pthread_t) thread_id.opaque_id);
+}
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 5e0cd47c82..66ac45b7de 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -310,6 +310,9 @@ DPDK_24 {
 	rte_thread_unregister;
 	rte_thread_value_get;
 	rte_thread_value_set;
+	rte_thread_timedjoin_np; # WINDOWS_NO_EXPORT
+	rte_thread_getname_np; # WINDOWS_NO_EXPORT
+	rte_thread_cancel; # WINDOWS_NO_EXPORT
 	rte_uuid_compare;
 	rte_uuid_is_null;
 	rte_uuid_parse;
-- 
2.43.0

Re: [PATCH] eal: introduce missing rte_thread wrappers

[Stephen Hemminger]

On Mon, 27 Nov 2023 10:25:02 +0100
David Vodak <vodak@cesnet.cz> wrote:

> Function rte_ctrl_thread_create has been replaced by rte_thread_create_control,
> encouraging Linux users to switch from the pthread_t API to the rte_thread API.
> However the rte_thread API does not provide wrappers for all pthread functions.
> This commit introduces equivalent functions for pthread_timedjoin_np,
> pthread_getname_np and pthread_cancel.
> 
> Bugzilla ID: 1330
> ---
>  lib/eal/include/rte_thread.h | 49 ++++++++++++++++++++++++++++++++++++
>  lib/eal/unix/rte_thread.c    | 20 +++++++++++++++
>  lib/eal/version.map          |  3 +++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index 8da9d4d3fb..49a068eda7 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -463,6 +463,55 @@ int rte_thread_value_set(rte_thread_key key, const void *value);
>   */
>  void *rte_thread_value_get(rte_thread_key key);
>  
> +/**
> + * Try to join with a terminated thread. If thread has not yet terminated,
> + * then the call blocks until a maximum time, specified in abstime.
> + *
> + * @param thread_id
> + *   Id of the thread.
> + *
> + * @param retval
> + *   Exit status of the thread.
> + *
> + * @param abstime
> + *   Maximum time of call blocking, measured against the CLOCK_REALTIME clock.
> + *
> + * @return
> + *   On success, return 0.
> + *   On failure, return a positive errno-style error number.
> + */
> +int rte_thread_timedjoin_np(rte_thread_t thread_id, void **retval, const struct timespec *abstime);
> +
> +/**
> + * Get name of the thread.
> + *
> + * @param thread_id
> + *   Id of the thread.
> + *
> + * @param name
> + *   Name of the thread.
> + *
> + * @param size
> + *   The number of bytes available in name.
> + *
> + * @return
> + *   On success, return 0.
> + *   On failure, return a positive errno-style error number.
> + */
> +int rte_thread_getname_np(rte_thread_t thread_id, char name[], size_t size);
> +
> +/**
> + * Send a cancelation request to a thread.
> + *
> + * @param thread_id
> + *   Id of the thread.
> + *
> + * @return
> + *   On success, return 0.
> + *   On failure, return a positive errno-style error number.
> + */
> +int rte_thread_cancel(rte_thread_t thread_id);

The point of the thread wrappers was to limit the use to what
was supportable on all platforms. 

Re: [PATCH] eal: introduce missing rte_thread wrappers

[Tyler Retzlaff]

On Mon, Nov 27, 2023 at 10:25:02AM +0100, David Vodak wrote:
> Function rte_ctrl_thread_create has been replaced by rte_thread_create_control,
> encouraging Linux users to switch from the pthread_t API to the rte_thread API.
> However the rte_thread API does not provide wrappers for all pthread functions.
> This commit introduces equivalent functions for pthread_timedjoin_np,
> pthread_getname_np and pthread_cancel.
> 
> Bugzilla ID: 1330
> ---

NAK this series. the rte thread API is not a POSIX emulation API.

the point of EAL is not to require applications to have to conditionally
compile code around the use of EAL API or handle "not supported"
failures it defeats the purpose of being an abstraction library.

Re: [PATCH] eal: introduce missing rte_thread wrappers

[David Vodak]

On 11/27/23 18:27, Tyler Retzlaff wrote:

> On Mon, Nov 27, 2023 at 10:25:02AM +0100, David Vodak wrote:
>> Function rte_ctrl_thread_create has been replaced by rte_thread_create_control,
>> encouraging Linux users to switch from the pthread_t API to the rte_thread API.
>> However the rte_thread API does not provide wrappers for all pthread functions.
>> This commit introduces equivalent functions for pthread_timedjoin_np,
>> pthread_getname_np and pthread_cancel.
>>
>> Bugzilla ID: 1330
>> ---
> NAK this series. the rte thread API is not a POSIX emulation API.
>
> the point of EAL is not to require applications to have to conditionally
> compile code around the use of EAL API or handle "not supported"
> failures it defeats the purpose of being an abstraction library.
Hi,

I understand that these changes may not be the best alternative. But 
what other options do we have,
if we are already using functions such as pthread_timedjoin_np? Should 
we just keep using them and
treat rte_thread_id.opaque_id as pthread_t?

Regards
David Vodak

Re: [PATCH] eal: introduce missing rte_thread wrappers

[Stephen Hemminger]

On Tue, 28 Nov 2023 09:54:02 +0100
David Vodak <vodak@cesnet.cz> wrote:

> On 11/27/23 18:27, Tyler Retzlaff wrote:
> 
> > On Mon, Nov 27, 2023 at 10:25:02AM +0100, David Vodak wrote:  
> >> Function rte_ctrl_thread_create has been replaced by rte_thread_create_control,
> >> encouraging Linux users to switch from the pthread_t API to the rte_thread API.
> >> However the rte_thread API does not provide wrappers for all pthread functions.
> >> This commit introduces equivalent functions for pthread_timedjoin_np,
> >> pthread_getname_np and pthread_cancel.
> >>
> >> Bugzilla ID: 1330
> >> ---  
> > NAK this series. the rte thread API is not a POSIX emulation API.
> >
> > the point of EAL is not to require applications to have to conditionally
> > compile code around the use of EAL API or handle "not supported"
> > failures it defeats the purpose of being an abstraction library.  
> Hi,
> 
> I understand that these changes may not be the best alternative. But 
> what other options do we have,
> if we are already using functions such as pthread_timedjoin_np? Should 
> we just keep using them and
> treat rte_thread_id.opaque_id as pthread_t?

For performance and initial isolated core model, DPDK was initially designed with a specific thread
model using dedicated threads. Over time the thread model has grown a little more
flexible with rte_ctrl threads and service lcores. But DPDK is not a generalized OS
thread model wrapper.

The problem is that DPDK API's need to make some assumptions about being
able to be not preempted and not having locking. If you allow all of pthreads
that will break.

If you want to design an application with a different thread model, then
do not expect the rest of DPDK to work. The burden of support and testing
needs to be shifted back to this application.