| Message ID | f9d29234-4837-86b5-605b-cfedf84b2fbd@mayadata.io |
|---|---|
| State | New |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series |
|
| Related | show |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | fail | apply issues |
Hi Nick, > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Nick Connolly > Sent: Monday, October 19, 2020 2:59 AM > To: dev@dpdk.org > Subject: [EXTERNAL] [dpdk-dev] [RFC] pthread on Windows > > > The proposed changes are: > > 1. An EAL implementation of pthread with a new rte_pthread API. > 2. The DPDK code (libs, examples, drivers, apps, tests, etc) needs to > be modified to use the new rte_pthread API. > 3. There needs to be an option for apps to use an external pthread > library as an alternative to the EAL implementation. > 4. Eventually, apps can opt in to using the rte_pthread API if desired. > > Item #3 isn't dependent on #1 and #2 - it can be implemented now, > allowing forward progress to be made without blocking on #1 and #2 which > may take longer to resolve. > > One concern I have with starting on #3 first is that with this patch, we make pthread semantics mandatory for DPDK core. When new code which references pthread API is later added to DPDK core, and that functionality doesn’t yet have a Windows emulation in EAL, DPDK core may take the dependency on a certain pthread semantics that (a) not implemented before, and (b) is hard to emulate. That could represent a problem later, when we introduce the “EAL threads” API layer with a more loose semantics (which can be backed by either external pthread library, or by emulation on Windows). Given that a compile flag is not part of any patch submission that introduces such new pthread dependency, how do we detect this problem during said submission? Do we know if there is a test or submit requirements which ensures that DPDK compiles on all platforms/environments (including this flag to use external pthread library) to catch new pthread dependencies, prior to accepting any new patch? Khoa.
Hi Khoa, On 29/10/2020 21:19, Khoa To wrote: >> -----Original Message----- >> From: dev <dev-bounces@dpdk.org> On Behalf Of Nick Connolly >> Sent: Monday, October 19, 2020 2:59 AM >> To: dev@dpdk.org >> Subject: [EXTERNAL] [dpdk-dev] [RFC] pthread on Windows >> >> >> The proposed changes are: >> >> 1. An EAL implementation of pthread with a new rte_pthread API. >> 2. The DPDK code (libs, examples, drivers, apps, tests, etc) needs to >> be modified to use the new rte_pthread API. >> 3. There needs to be an option for apps to use an external pthread >> library as an alternative to the EAL implementation. >> 4. Eventually, apps can opt in to using the rte_pthread API if desired. >> >> Item #3 isn't dependent on #1 and #2 - it can be implemented now, >> allowing forward progress to be made without blocking on #1 and #2 which >> may take longer to resolve. >> >> > One concern I have with starting on #3 first is that with this patch, we make pthread semantics mandatory for DPDK core. When new code which references pthread API is later added to DPDK core, and that functionality doesn’t yet have a Windows emulation in EAL, DPDK core may take the dependency on a certain pthread semantics that (a) not implemented before, and (b) is hard to emulate. > > That could represent a problem later, when we introduce the “EAL threads” API layer with a more loose semantics (which can be backed by either external pthread library, or by emulation on Windows). > > Given that a compile flag is not part of any patch submission that introduces such new pthread dependency, how do we detect this problem during said submission? > > Do we know if there is a test or submit requirements which ensures that DPDK compiles on all platforms/environments (including this flag to use external pthread library) to catch new pthread dependencies, prior to accepting any new patch? > > Khoa. I think we are ok here ... the patch doesn't change any dependencies, or make pthreads semantics mandatory for DPDK core. Any changes to DPDK core will be built and tested against the Windows EAL in exactly the same way as currently and the same standards of correctness apply. Any enhancements needed by the DPDK core will need to go into the Windows EAL as currently. All that the patch does is provide the flexibility to use an external library to provide part of the functionality of the EAL if the environment requires it (for example to fit with the application's threading model). So, why bother with the #3 patch now instead of waiting for #1 and #2? Well, based on my experience getting the SPDK running on Windows, I suspect it will be some time before #1 and #2 are done. There's nothing inherently difficult, but there are a number of details to work through and reach agreement about. The patch provides a way of maintaining progress on Windows whilst #1 and #2 are figured out, for example by making it easy for Datapath to use the pthreads4w library in their environment. Regards, Nick
+Dmitry, Harini Hi Nick, > -----Original Message----- > From: Nick Connolly <nick.connolly@mayadata.io> > Sent: Monday, November 2, 2020 3:17 AM > To: Khoa To <khot@microsoft.com>; dev@dpdk.org > Subject: Re: [EXTERNAL] [dpdk-dev] [RFC] pthread on Windows > > Hi Khoa, > > On 29/10/2020 21:19, Khoa To wrote: > >> -----Original Message----- > >> From: dev <dev-bounces@dpdk.org> On Behalf Of Nick Connolly > >> Sent: Monday, October 19, 2020 2:59 AM > >> To: dev@dpdk.org > >> Subject: [EXTERNAL] [dpdk-dev] [RFC] pthread on Windows > >> > >> > >> The proposed changes are: > >> > >> 1. An EAL implementation of pthread with a new rte_pthread API. > >> 2. The DPDK code (libs, examples, drivers, apps, tests, etc) needs to > >> be modified to use the new rte_pthread API. > >> 3. There needs to be an option for apps to use an external pthread > >> library as an alternative to the EAL implementation. > >> 4. Eventually, apps can opt in to using the rte_pthread API if desired. > >> > >> Item #3 isn't dependent on #1 and #2 - it can be implemented now, > >> allowing forward progress to be made without blocking on #1 and #2 > which > >> may take longer to resolve. > >> > >> > > One concern I have with starting on #3 first is that with this patch, we make > pthread semantics mandatory for DPDK core. When new code which > references pthread API is later added to DPDK core, and that functionality > doesn’t yet have a Windows emulation in EAL, DPDK core may take the > dependency on a certain pthread semantics that (a) not implemented > before, and (b) is hard to emulate. > > > > That could represent a problem later, when we introduce the “EAL > threads” API layer with a more loose semantics (which can be backed by > either external pthread library, or by emulation on Windows). > > > > Given that a compile flag is not part of any patch submission that introduces > such new pthread dependency, how do we detect this problem during said > submission? > > > > Do we know if there is a test or submit requirements which ensures that > DPDK compiles on all platforms/environments (including this flag to use > external pthread library) to catch new pthread dependencies, prior to > accepting any new patch? > > > > Khoa. > > I think we are ok here ... the patch doesn't change any dependencies, or > make pthreads semantics mandatory for DPDK core. Any changes to DPDK > core will be built and tested against the Windows EAL in exactly the > same way as currently and the same standards of correctness apply. Any > enhancements needed by the DPDK core will need to go into the Windows > EAL as currently. All that the patch does is provide the flexibility to > use an external library to provide part of the functionality of the EAL > if the environment requires it (for example to fit with the > application's threading model). Yes, I agree with you that the patch doesn't change any dependencies for DPDK core. It does, however, enable someone to submit patches that relies on external library dependencies, without that being obvious in the patch submission. Since I am not too familiar with DPDK patch submission process, I think we just need to confirm at the next Windows DPDK community call (or on this thread) that "the same standards of correctness apply" means patches are tested to compile on all supported platforms without any special flag, before they are accepted. Khoa.
Hi Khoa, As far as I can see, the DPDK Performance Test Lab at University of New Hampshire provides the guarantee that the standard tree will build just fine on Windows (https://lab.dpdk.org/results/dashboard/status/) - in particular, the Windows-Compile-DPDK-Meson test. Regards, Nick On 03/11/2020 22:34, Khoa To wrote: > +Dmitry, Harini > > Hi Nick, > >> -----Original Message----- >> From: Nick Connolly <nick.connolly@mayadata.io> >> Sent: Monday, November 2, 2020 3:17 AM >> To: Khoa To <khot@microsoft.com>; dev@dpdk.org >> Subject: Re: [EXTERNAL] [dpdk-dev] [RFC] pthread on Windows >> >> Hi Khoa, >> >> On 29/10/2020 21:19, Khoa To wrote: >>>> -----Original Message----- >>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Nick Connolly >>>> Sent: Monday, October 19, 2020 2:59 AM >>>> To: dev@dpdk.org >>>> Subject: [EXTERNAL] [dpdk-dev] [RFC] pthread on Windows >>>> >>>> >>>> The proposed changes are: >>>> >>>> 1. An EAL implementation of pthread with a new rte_pthread API. >>>> 2. The DPDK code (libs, examples, drivers, apps, tests, etc) needs to >>>> be modified to use the new rte_pthread API. >>>> 3. There needs to be an option for apps to use an external pthread >>>> library as an alternative to the EAL implementation. >>>> 4. Eventually, apps can opt in to using the rte_pthread API if desired. >>>> >>>> Item #3 isn't dependent on #1 and #2 - it can be implemented now, >>>> allowing forward progress to be made without blocking on #1 and #2 >> which >>>> may take longer to resolve. >>>> >>>> >>> One concern I have with starting on #3 first is that with this patch, we make >> pthread semantics mandatory for DPDK core. When new code which >> references pthread API is later added to DPDK core, and that functionality >> doesn’t yet have a Windows emulation in EAL, DPDK core may take the >> dependency on a certain pthread semantics that (a) not implemented >> before, and (b) is hard to emulate. >>> That could represent a problem later, when we introduce the “EAL >> threads” API layer with a more loose semantics (which can be backed by >> either external pthread library, or by emulation on Windows). >>> Given that a compile flag is not part of any patch submission that introduces >> such new pthread dependency, how do we detect this problem during said >> submission? >>> Do we know if there is a test or submit requirements which ensures that >> DPDK compiles on all platforms/environments (including this flag to use >> external pthread library) to catch new pthread dependencies, prior to >> accepting any new patch? >>> Khoa. >> I think we are ok here ... the patch doesn't change any dependencies, or >> make pthreads semantics mandatory for DPDK core. Any changes to DPDK >> core will be built and tested against the Windows EAL in exactly the >> same way as currently and the same standards of correctness apply. Any >> enhancements needed by the DPDK core will need to go into the Windows >> EAL as currently. All that the patch does is provide the flexibility to >> use an external library to provide part of the functionality of the EAL >> if the environment requires it (for example to fit with the >> application's threading model). > Yes, I agree with you that the patch doesn't change any dependencies for DPDK core. > It does, however, enable someone to submit patches that relies on external library > dependencies, without that being obvious in the patch submission. > > Since I am not too familiar with DPDK patch submission process, I think we just need > to confirm at the next Windows DPDK community call (or on this thread) that > "the same standards of correctness apply" means patches are tested to compile on > all supported platforms without any special flag, before they are accepted. > > Khoa. > > > > > > > > >
diff --git a/app/meson.build b/app/meson.build index eb74f215a..8f415084d 100644 --- a/app/meson.build +++ b/app/meson.build @@ -49,6 +49,7 @@ foreach app:apps + '_rte_' + d) endforeach dep_objs += lib_execinfo + dep_objs += get_variable('eal_' + get_option('default_library') + '_deps') link_libs = [] if get_option('default_library') == 'static' diff --git a/config/meson.build b/config/meson.build index 69f2aeb60..3554c2f90 100644 --- a/config/meson.build +++ b/config/meson.build @@ -298,3 +298,13 @@ if get_option('b_lto') add_project_link_arguments('-Wno-lto-type-mismatch', language: 'c') endif endif + +# Include EAL dependencies +foreach p : get_option('eal_deps') + name = p.strip() + project = subproject(name) + project_dep = project.get_variable(name + '_dep') + eal_shared_deps += project.get_variable(name + 'shared_dep', project_dep) + eal_static_deps += project.get_variable(name + 'static_dep', project_dep) + eal_exclude_list += project.get_variable('eal_header_files', []) +endforeach diff --git a/drivers/meson.build b/drivers/meson.build index 5f9526557..7592866c3 100644 --- a/drivers/meson.build +++ b/drivers/meson.build @@ -99,6 +99,8 @@ foreach subpath:subdirs static_deps += [get_variable('static_rte_' + d)] endif endforeach + shared_deps += eal_shared_deps + static_deps += eal_static_deps endif if not build diff --git a/examples/meson.build b/examples/meson.build index 245d98575..6d07c1f2c 100644 --- a/examples/meson.build +++ b/examples/meson.build @@ -95,6 +95,7 @@ foreach example: examples endif dep_objs += [get_variable(var_name)] endforeach + dep_objs += get_variable('eal_' + get_option('default_library') + '_deps') if allow_experimental_apis cflags += '-DALLOW_EXPERIMENTAL_API' endif diff --git a/lib/librte_eal/windows/include/eal/meson.build b/lib/librte_eal/windows/include/eal/meson.build new file mode 100644 index 000000000..75fdbf758 --- /dev/null +++ b/lib/librte_eal/windows/include/eal/meson.build @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2020, MayaData Inc. All rights reserved. +# Copyright (c) 2020, DataCore Software Corporation. All rights reserved. + +# Copy the headers to the build directory to implement the exclude list. + +includes += include_directories('.') + +foreach h : eal_headers + if not eal_exclude_list.contains(h) + if h.startswith('rte_') or not eal_exclude_list.contains('*') + configure_file(copy: true, input: join_paths('..', h), output: h) + endif + endif +endforeach + +if not eal_exclude_list.contains('*') + subdir('netinet') + subdir('sys') +endif diff --git a/lib/librte_eal/windows/include/eal/netinet/meson.build b/lib/librte_eal/windows/include/eal/netinet/meson.build new file mode 100644 index 000000000..b7722adf9 --- /dev/null +++ b/lib/librte_eal/windows/include/eal/netinet/meson.build @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2020, MayaData Inc. All rights reserved. +# Copyright (c) 2020, DataCore Software Corporation. All rights reserved. + +# Copy the headers to the build directory to implement the exclude list. + +includes += include_directories('.') + +foreach h : eal_netinet_headers + if not eal_exclude_list.contains('netinet/' + h) + configure_file(copy: true, input: join_paths('../../netinet', h), output: h) + endif +endforeach diff --git a/lib/librte_eal/windows/include/eal/sys/meson.build b/lib/librte_eal/windows/include/eal/sys/meson.build new file mode 100644 index 000000000..295c4e7bf --- /dev/null +++ b/lib/librte_eal/windows/include/eal/sys/meson.build @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2020, MayaData Inc. All rights reserved. +# Copyright (c) 2020, DataCore Software Corporation. All rights reserved. + +# Copy the headers to the build directory to implement the exclude list. + +includes += include_directories('.') + +foreach h : eal_sys_headers + if not eal_exclude_list.contains('sys/' + h) + configure_file(copy: true, input: join_paths('../../sys', h), output: h) + endif +endforeach diff --git a/lib/librte_eal/windows/include/meson.build b/lib/librte_eal/windows/include/meson.build index b3534b025..07a747363 100644 --- a/lib/librte_eal/windows/include/meson.build +++ b/lib/librte_eal/windows/include/meson.build @@ -1,10 +1,35 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright 2020 Mellanox Technologies, Ltd -includes += include_directories('.') - -headers += files( +eal_headers = [ 'rte_os.h', 'rte_virt2phys.h', 'rte_windows.h', -) +] + +headers += files(eal_headers) + +eal_headers += [ + 'dirent.h', + 'fnmatch.h', + 'getopt.h', + 'pthread.h', + 'regex.h', + 'sched.h', + 'unistd.h', +] + +eal_sys_headers = [ + 'queue.h', +] + +eal_netinet_headers = [ + 'in.h', + 'ip.h', +] + +if eal_exclude_list.length() == 0 + includes += include_directories('.') +else + subdir('eal') +endif diff --git a/lib/meson.build b/lib/meson.build index e5597f174..b4cf1b07c 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -91,6 +91,8 @@ foreach l:libraries shared_deps += [get_variable('shared_rte_' + d)] static_deps += [get_variable('static_rte_' + d)] endforeach + shared_deps += eal_shared_deps + static_deps += eal_static_deps endif if not build diff --git a/meson.build b/meson.build index 61d9a4f5f..13f50517f 100644 --- a/meson.build +++ b/meson.build @@ -21,6 +21,9 @@ dpdk_drivers = [] dpdk_extra_ldflags = [] dpdk_libs_disabled = [] dpdk_drvs_disabled = [] +eal_shared_deps = [] +eal_static_deps = [] +eal_exclude_list = [] abi_version_file = files('ABI_VERSION') if host_machine.cpu_family().startswith('x86') @@ -31,19 +34,20 @@ elif host_machine.cpu_family().startswith('ppc') arch_subdir = 'ppc' endif +# do configuration and get tool paths +subdir('buildtools') +subdir('config') + # configure the build, and make sure configs here and in config folder are # able to be included in any file. We also store a global array of include dirs # for passing to pmdinfogen scripts global_inc = include_directories('.', 'config', 'lib/librte_eal/include', - 'lib/librte_eal/@0@/include'.format(host_machine.system()), + 'lib/librte_eal/@0@/include@1@'.format(host_machine.system(), + (eal_exclude_list.length() != 0) ? '/eal' : ''), 'lib/librte_eal/@0@/include'.format(arch_subdir), ) -# do configuration and get tool paths -subdir('buildtools') -subdir('config') - # build libs and drivers subdir('buildtools/pmdinfogen') subdir('lib') diff --git a/meson_options.txt b/meson_options.txt index 9bf18ab6b..b28eb8ba3 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -6,6 +6,8 @@ option('disable_drivers', type: 'string', value: '', description: 'Comma-separated list of drivers to explicitly disable.') option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>', description: 'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.') +option('eal_deps', type: 'array', value: [], + description: 'Environment Abstraction Layer dependencies') option('enable_docs', type: 'boolean', value: false, description: 'build documentation') option('enable_kmods', type: 'boolean', value: false, diff --git a/subprojects/test/meson.build b/subprojects/test/meson.build new file mode 100644 index 000000000..911626f23 --- /dev/null +++ b/subprojects/test/meson.build @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2020, MayaData Inc. All rights reserved. +# Copyright (c) 2020, DataCore Software Corporation. All rights reserved. + +project('TEST', 'C', + version: '0.0.0', + license: 'BSD', + default_options: ['buildtype=release','default_library=static'], + meson_version: '>= 0.47.1' +) + +cc = meson.get_compiler('c') + +includes = include_directories('.') + +eal_header_files =[ + 'pthread.h', + 'sched.h', +] + +test_static_dep = declare_dependency( + include_directories : includes, + link_args : [ '-lws2_32', '-lrpcrt4' ], +) + +test_shared_dep = declare_dependency( + include_directories : includes, + link_args : [ '-lws2_32', '-lrpcrt4' ], +) + +test_dep = test_static_dep
As part of the DPDK on Windows work, the following proposal has been put together and we would welcome community feedback. The heart of the issue is that, unlike Linux and FreeBSD, Windows does not claim to be POSIX conformant: some functionality is missing, some is present and some has different semantics. Support for pthread is missing on Windows and this is currently handled with an EAL header file that wraps the underlying Windows calls. It only implements functionality that is actually used by the DPDK and does so in as lightweight a way as possible. The guidance we have received so far is that the EAL is meant to be an isolation layer between the DPDK and the underlying environment, such that: * All accesses to the operating system (apart from basic libc functionality) should pass through the EAL. * The DPDK code should have no POSIX dependencies outside of the EAL. This means that pthread support needs to be hidden behind a new rte_* interface and the DPDK code must be modified to use it. As the Windows work progresses there will be other areas of POSIX support that need to be treated in a similar manner. [As an aside, an alternative would be to decide that there is at least a partial implied POSIX dependency in the DPDK code base and the EAL is meant to provide an isolation layer for functionality that is outside of this. This is the approach taken by WPDK to run SPDK on Windows (https://github.com/wpdk/wpdk)] In the absence of a Microsoft supported pthread library, there are a number of projects available that could be used, but they each have compromises and trade-offs. The support in the EAL maps very closely to Windows calls, but only implements a limited subset of functionality. The proposal includes the option to build the DPDK using an external threads package for applications with more extensive requirements, such as condition variables or thread cancellation. The proposed changes are: 1. An EAL implementation of pthread with a new rte_pthread API. 2. The DPDK code (libs, examples, drivers, apps, tests, etc) needs to be modified to use the new rte_pthread API. 3. There needs to be an option for apps to use an external pthread library as an alternative to the EAL implementation. 4. Eventually, apps can opt in to using the rte_pthread API if desired. Item #3 isn't dependent on #1 and #2 - it can be implemented now, allowing forward progress to be made without blocking on #1 and #2 which may take longer to resolve. The detailed proposal for #3 is as follows: 1. Add a meson build option to specify a list of subprojects to be combined with the EAL. 2. If no subprojects are specified, then the build should complete exactly as currently, using the support built in to the EAL. 3. If a subproject is specified then just the relevant functionality in the EAL is replaced at build time, leaving the rest of the EAL unchanged. 4. A subproject can specify the list of headers it provides, overriding the ones in the EAL (e.g. pthreads4w provides pthread.h and sched.h). 5. This allows development to continue on the EAL version of pthread and also allows an application to use an external library. This can be implemented with only very minor changes to the meson build files (see the illustrative set of diffs below). We would welcome your input on these proposals. Regards, Nick (on behalf of DPDK on Windows) ----------------------------------------------------------------------