[v16,0/9] eal: Add EAL API for threading

  From: Narcisa Vasile <navasile@microsoft.com>

EAL thread API

**Problem Statement**
DPDK currently uses the pthread interface to create and manage threads.
Windows does not support the POSIX thread programming model,
so it currently
relies on a header file that hides the Windows calls under
pthread matched interfaces. Given that EAL should isolate the environment
specifics from the applications and libraries and mediate
all the communication with the operating systems, a new EAL interface
is needed for thread management.

**Goals**
* Introduce a generic EAL API for threading support that will remove
  the current Windows pthread.h shim.
* Replace references to pthread_* across the DPDK codebase with the new
  RTE_THREAD_* API.
* Allow users to choose between using the RTE_THREAD_* API or a
  3rd party thread library through a configuration option.

**Design plan**
New API main files:
* rte_thread.h (librte_eal/include)
* rte_thread.c (librte_eal/windows)
* rte_thread.c (librte_eal/common)

**A schematic example of the design**
--------------------------------------------------
lib/librte_eal/include/rte_thread.h
int rte_thread_create();

lib/librte_eal/common/rte_thread.c
int rte_thread_create() 
{
	return pthread_create();
}

lib/librte_eal/windows/rte_thread.c
int rte_thread_create() 
{
	return CreateThread();
}
-----------------------------------------------------

**Thread attributes**

When or after a thread is created, specific characteristics of the thread
can be adjusted. Given that the thread characteristics that are of interest
for DPDK applications are affinity and priority, the following structure
that represents thread attributes has been defined:

typedef struct
{
	enum rte_thread_priority priority;
	rte_cpuset_t cpuset;
} rte_thread_attr_t;

The *rte_thread_create()* function can optionally receive
an rte_thread_attr_t
object that will cause the thread to be created with the
affinity and priority
described by the attributes object. If no rte_thread_attr_t is passed
(parameter is NULL), the default affinity and priority are used.
An rte_thread_attr_t object can also be set to the default values
by calling *rte_thread_attr_init()*.

*Priority* is represented through an enum that currently advertises
two values for priority:
	- RTE_THREAD_PRIORITY_NORMAL
	- RTE_THREAD_PRIORITY_REALTIME_CRITICAL
The enum can be extended to allow for multiple priority levels.
rte_thread_set_priority      - sets the priority of a thread
rte_thread_get_priority      - retrieves the priority of a thread
                               from the OS
rte_thread_attr_set_priority - updates an rte_thread_attr_t object
                               with a new value for priority

The user can choose thread priority through an EAL parameter,
when starting an application.  If EAL parameter is not used,
the per-platform default value for thread priority is used.
Otherwise administrator has an option to set one of available options:
 --thread-prio normal
 --thread-prio realtime

Example:
./dpdk-l2fwd -l 0-3 -n 4 –thread-prio normal -- -q 8 -p ffff

*Affinity* is described by the already known “rte_cpuset_t” type.
rte_thread_attr_set/get_affinity - sets/gets the affinity field in a
                                   rte_thread_attr_t object
rte_thread_set/get_affinity      – sets/gets the affinity of a thread

**Errors**
A translation function that maps Windows error codes to errno-style
error codes is provided. 

**Future work**
The long term plan is for EAL to provide full threading support:
* Add support for conditional variables
* Additional functionality offered by pthread_*
  (such as pthread_setname_np, etc.)

v16:
- Fix warning on freebsd by adding cast
- Change affinity unit test to consider ases when the requested CPU
  are not available on the system.
- Fix priority unit test to avoid termination of thread before the
  priority is checked.

v15:
- Add try_lock mutex functionality. If the mutex is already owned by a
  different thread, the function returns immediately. Otherwise, 
  the mutex will be acquired.
- Add function for getting the priority of a thread.
  An auxiliary function that translates the OS priority to the
  EAL accepted ones is added.
- Fix unit tests logging, add descriptive asserts that mark test failures.
  Verify mutex locking, verify barrier return values. Add test for
  statically initialized mutexes.
- Fix Alpine build by removing the use of pthread_attr_set_affinity() and
  using pthread_set_affinity() after the thread is created.

v14:
- Remove patch "eal: add EAL argument for setting thread priority"
  This will be added later when enabling the new threading API.
- Remove priority enum value "_UNDEFINED". NORMAL is used
  as the default.
- Fix issue with thread return value.

v13:
 - Fix syntax error in unit tests

v12:
 - Fix freebsd warning about initializer in unit tests

v11:
 - Add unit tests for thread API
 - Rebase

v10:
 - Remove patch no. 10. It will be broken down in subpatches 
   and sent as a different patchset that depends on this one.
   This is done due to the ABI breaks that would be caused by patch 10.
 - Replace unix/rte_thread.c with common/rte_thread.c
 - Remove initializations that may prevent compiler from issuing useful
   warnings.
 - Remove rte_thread_types.h and rte_windows_thread_types.h
 - Remove unneeded priority macros (EAL_THREAD_PRIORITY*)
 - Remove functions that retrieves thread handle from process handle
 - Remove rte_thread_cancel() until same behavior is obtained on
   all platforms.
 - Fix rte_thread_detach() function description,
   return value and remove empty line.
 - Reimplement mutex functions. Add compatible representation for mutex
   identifier. Add macro to replace static mutex initialization instances.
 - Fix commit messages (lines too long, remove unicode symbols)

v9:
- Sign patches

v8:
- Rebase
- Add rte_thread_detach() API
- Set default priority, when user did not specify a value

v7:
Based on DmitryK's review:
- Change thread id representation
- Change mutex id representation
- Implement static mutex inititalizer for Windows
- Change barrier identifier representation
- Improve commit messages
- Add missing doxygen comments
- Split error translation function
- Improve name for affinity function
- Remove cpuset_size parameter
- Fix eal_create_cpu_map function
- Map EAL priority values to OS specific values
- Add thread wrapper for start routine
- Do not export rte_thread_cancel() on Windows
- Cleanup, fix comments, fix typos.

v6:
- improve error-translation function
- call the error translation function in rte_thread_value_get()

v5:
- update cover letter with more details on the priority argument

v4:
- fix function description
- rebase

v3:
- rebase

v2:
- revert changes that break ABI 
- break up changes into smaller patches
- fix coding style issues
- fix issues with errors
- fix parameter type in examples/kni.c

Narcisa Vasile (9):
  eal: add basic threading functions
  eal: add thread attributes
  eal/windows: translate Windows errors to errno-style errors
  eal: implement functions for thread affinity management
  eal: implement thread priority management functions
  eal: add thread lifetime management
  eal: implement functions for mutex management
  eal: implement functions for thread barrier management
  Add unit tests for thread API

 app/test/meson.build            |   2 +
 app/test/test_threads.c         | 372 ++++++++++++++++++
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     | 497 ++++++++++++++++++++++++
 lib/eal/include/rte_thread.h    | 435 ++++++++++++++++++++-
 lib/eal/unix/meson.build        |   1 -
 lib/eal/unix/rte_thread.c       |  92 -----
 lib/eal/version.map             |  22 ++
 lib/eal/windows/eal_lcore.c     | 176 ++++++---
 lib/eal/windows/eal_windows.h   |  10 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 656 ++++++++++++++++++++++++++++++--
 12 files changed, 2093 insertions(+), 173 deletions(-)
 create mode 100644 app/test/test_threads.c
 create mode 100644 lib/eal/common/rte_thread.c
 delete mode 100644 lib/eal/unix/rte_thread.c
  

09/10/2021 09:41, Narcisa Ana Maria Vasile:
> From: Narcisa Vasile <navasile@microsoft.com>
> 
> EAL thread API
> 
> **Problem Statement**
> DPDK currently uses the pthread interface to create and manage threads.
> Windows does not support the POSIX thread programming model,
> so it currently
> relies on a header file that hides the Windows calls under
> pthread matched interfaces. Given that EAL should isolate the environment
> specifics from the applications and libraries and mediate
> all the communication with the operating systems, a new EAL interface
> is needed for thread management.
> 
> **Goals**
> * Introduce a generic EAL API for threading support that will remove
>   the current Windows pthread.h shim.
> * Replace references to pthread_* across the DPDK codebase with the new
>   RTE_THREAD_* API.
> * Allow users to choose between using the RTE_THREAD_* API or a
>   3rd party thread library through a configuration option.
> 
> **Design plan**
> New API main files:
> * rte_thread.h (librte_eal/include)
> * rte_thread.c (librte_eal/windows)
> * rte_thread.c (librte_eal/common)

Why this file is not in lib/eal/unix/ ?


> **A schematic example of the design**
> --------------------------------------------------
> lib/librte_eal/include/rte_thread.h
> int rte_thread_create();
> 
> lib/librte_eal/common/rte_thread.c
> int rte_thread_create() 
> {
> 	return pthread_create();
> }
> 
> lib/librte_eal/windows/rte_thread.c
> int rte_thread_create() 
> {
> 	return CreateThread();
> }
> -----------------------------------------------------

We must have the same error code, no matter the underlying implementation.
So you cannot return directly pthread or win32 error codes.


> **Thread attributes**
> 
> When or after a thread is created, specific characteristics of the thread
> can be adjusted. Given that the thread characteristics that are of interest
> for DPDK applications are affinity and priority, the following structure
> that represents thread attributes has been defined:
> 
> typedef struct
> {
> 	enum rte_thread_priority priority;
> 	rte_cpuset_t cpuset;
> } rte_thread_attr_t;
> 
> The *rte_thread_create()* function can optionally receive
> an rte_thread_attr_t
> object that will cause the thread to be created with the
> affinity and priority
> described by the attributes object. If no rte_thread_attr_t is passed
> (parameter is NULL), the default affinity and priority are used.
> An rte_thread_attr_t object can also be set to the default values
> by calling *rte_thread_attr_init()*.
> 
> *Priority* is represented through an enum that currently advertises
> two values for priority:
> 	- RTE_THREAD_PRIORITY_NORMAL
> 	- RTE_THREAD_PRIORITY_REALTIME_CRITICAL

The priority level realtime should never used.

I am not sure about handling the priority so precisely.
I think we can abstract the priority need through different functions.
We already have the function rte_ctrl_thread_create() where priority
should be fixed.
I think we have only 2 types of threads:
	- control thread (interrupt, timer, IPC)
	- datapath lcore (created in rte_eal_init, including service cores)
It means we need only one new function for datapath thread creation.

> The enum can be extended to allow for multiple priority levels.
> rte_thread_set_priority      - sets the priority of a thread
> rte_thread_get_priority      - retrieves the priority of a thread
>                                from the OS
> rte_thread_attr_set_priority - updates an rte_thread_attr_t object
>                                with a new value for priority
> 
> The user can choose thread priority through an EAL parameter,
> when starting an application.  If EAL parameter is not used,
> the per-platform default value for thread priority is used.
> Otherwise administrator has an option to set one of available options:
>  --thread-prio normal
>  --thread-prio realtime

I don't think we need such feature.
Anyway, it is a new feature, so it is beyond the initial replacement goal.


> Example:
> ./dpdk-l2fwd -l 0-3 -n 4 –thread-prio normal -- -q 8 -p ffff
> 
> *Affinity* is described by the already known “rte_cpuset_t” type.
> rte_thread_attr_set/get_affinity - sets/gets the affinity field in a
>                                    rte_thread_attr_t object
> rte_thread_set/get_affinity      – sets/gets the affinity of a thread
> 
> **Errors**
> A translation function that maps Windows error codes to errno-style
> error codes is provided. 
> 
> **Future work**
> The long term plan is for EAL to provide full threading support:
> * Add support for conditional variables
> * Additional functionality offered by pthread_*
>   (such as pthread_setname_np, etc.)
  

On Tue, Oct 12, 2021 at 06:07:06PM +0200, Thomas Monjalon wrote:
> 09/10/2021 09:41, Narcisa Ana Maria Vasile:
> > From: Narcisa Vasile <navasile@microsoft.com>
> > 
> > EAL thread API
> > 
> > **Problem Statement**
> > DPDK currently uses the pthread interface to create and manage threads.
> > Windows does not support the POSIX thread programming model,
> > so it currently
> > relies on a header file that hides the Windows calls under
> > pthread matched interfaces. Given that EAL should isolate the environment
> > specifics from the applications and libraries and mediate
> > all the communication with the operating systems, a new EAL interface
> > is needed for thread management.
> > 
> > **Goals**
> > * Introduce a generic EAL API for threading support that will remove
> >   the current Windows pthread.h shim.
> > * Replace references to pthread_* across the DPDK codebase with the new
> >   RTE_THREAD_* API.
> > * Allow users to choose between using the RTE_THREAD_* API or a
> >   3rd party thread library through a configuration option.
> > 
> > **Design plan**
> > New API main files:
> > * rte_thread.h (librte_eal/include)
> > * rte_thread.c (librte_eal/windows)
> > * rte_thread.c (librte_eal/common)
> 
> Why this file is not in lib/eal/unix/ ?
> 
  
  Thank you Thomas for reviewing these patches!
  Your guidance is very much appreciated as I want to bring this patchset on the
  good path towards merging.
  Based on community meeting discussions, multiple users have requested an option
  to allow them to use a 3rd party thread library. At the same time, we want to remove
  the Windows shim that we currently use for threading in DPDK,
  so we decided to do the following:
  
  A new rte_thread_* API is introduced, which will be used uniformly across DPDK.
	  - for unix-based platforms, the code in eal/common will be compiled
	  - for windows, the code in eal/windows will be compiled.
  For all cases when a 3rd party library is needed, the code from eal/common will be
  used. For example, if winpthreads or pthreads4w are used, at build time the code from
  'eal/common' will be selected and the rte_thread_* API will point to pthread_*
  functions described by the "pthread.h" header file provided by the 3rd party library.
 
  Using a 3rd party library will not require any changes in the DPDK code,
  except for adding an option in the meson files. 
  Therefore code from common will work both on unix and windows.
  Nick explains even better in his RFC from that time: [RFC] pthread on Windows - Patchwork (dpdk.org).
  
  I will improve the cover letter and the commit messages to better explain this.
> 
> > **A schematic example of the design**
> > --------------------------------------------------
> > lib/librte_eal/include/rte_thread.h
> > int rte_thread_create();
> > 
> > lib/librte_eal/common/rte_thread.c
> > int rte_thread_create() 
> > {
> > 	return pthread_create();
> > }
> > 
> > lib/librte_eal/windows/rte_thread.c
> > int rte_thread_create() 
> > {
> > 	return CreateThread();
> > }
> > -----------------------------------------------------
> 
> We must have the same error code, no matter the underlying implementation.
> So you cannot return directly pthread or win32 error codes.
> 

  The approach here is to translate the Windows errors to POSIX-style ones to have
  uniformity across the entire threading module.

> 
> > **Thread attributes**
> > 
> > When or after a thread is created, specific characteristics of the thread
> > can be adjusted. Given that the thread characteristics that are of interest
> > for DPDK applications are affinity and priority, the following structure
> > that represents thread attributes has been defined:
> > 
> > typedef struct
> > {
> > 	enum rte_thread_priority priority;
> > 	rte_cpuset_t cpuset;
> > } rte_thread_attr_t;
> > 
> > The *rte_thread_create()* function can optionally receive
> > an rte_thread_attr_t
> > object that will cause the thread to be created with the
> > affinity and priority
> > described by the attributes object. If no rte_thread_attr_t is passed
> > (parameter is NULL), the default affinity and priority are used.
> > An rte_thread_attr_t object can also be set to the default values
> > by calling *rte_thread_attr_init()*.
> > 
> > *Priority* is represented through an enum that currently advertises
> > two values for priority:
> > 	- RTE_THREAD_PRIORITY_NORMAL
> > 	- RTE_THREAD_PRIORITY_REALTIME_CRITICAL
> 
> The priority level realtime should never used.
> 
> I am not sure about handling the priority so precisely.
> I think we can abstract the priority need through different functions.
> We already have the function rte_ctrl_thread_create() where priority
> should be fixed.
> I think we have only 2 types of threads:
> 	- control thread (interrupt, timer, IPC)
> 	- datapath lcore (created in rte_eal_init, including service cores)
> It means we need only one new function for datapath thread creation.
> 

I'll explain better the intent here on you other comment in patch 2. 

> > The enum can be extended to allow for multiple priority levels.
> > rte_thread_set_priority      - sets the priority of a thread
> > rte_thread_get_priority      - retrieves the priority of a thread
> >                                from the OS
> > rte_thread_attr_set_priority - updates an rte_thread_attr_t object
> >                                with a new value for priority
> > 
> > The user can choose thread priority through an EAL parameter,
> > when starting an application.  If EAL parameter is not used,
> > the per-platform default value for thread priority is used.
> > Otherwise administrator has an option to set one of available options:
> >  --thread-prio normal
> >  --thread-prio realtime
> 
> I don't think we need such feature.
> Anyway, it is a new feature, so it is beyond the initial replacement goal.
> 
> 

  True, this is not part of this patchset, I will correct the cover letter.
  
> > Example:
> > ./dpdk-l2fwd -l 0-3 -n 4 –thread-prio normal -- -q 8 -p ffff
> > 
> > *Affinity* is described by the already known “rte_cpuset_t” type.
> > rte_thread_attr_set/get_affinity - sets/gets the affinity field in a
> >                                    rte_thread_attr_t object
> > rte_thread_set/get_affinity      – sets/gets the affinity of a thread
> > 
> > **Errors**
> > A translation function that maps Windows error codes to errno-style
> > error codes is provided. 
> > 
> > **Future work**
> > The long term plan is for EAL to provide full threading support:
> > * Add support for conditional variables
> > * Additional functionality offered by pthread_*
> >   (such as pthread_setname_np, etc.)
> 
>