[dpdk-dev] RFC enabling dll/dso for dpdk on windows

[dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Tyler Retzlaff]

hi folks,

we would like to submit a a patch series that makes dll/dso for dpdk
work on windows. there are two differences in the windows platform that
would need to be address through enhancements to dpdk.

(1) windows dynamic objects don't export sufficient information for
    tls variables and the windows loader and runtime would need to be
    enhanced in order to perform runtime linking. [1][2]

(2) importing exported data symbols from a dll/dso on windows requires
    that the symbol be decorated with dllimport. optionally loading
    performance of dll/dso is also further improved by decorating
    exported function symbols. [3]

for (1) a novel approach is proposed where a new set of per_lcore
macros are introduced and used to replace existing macros with some
adjustment to declaration/definition usage is made. of note

    * on linux the new macros would expand compatibly to maintain abi
      of existing exported tls variables. since windows dynamic
      linking has never worked there is no compatibility concern for
      existing windows binaries.

    * the existing macros would be retained for api compatibility
      potentially with the intent of deprecating them at a later time.

    * new macros would be "internal" to dpdk they should not be
      available to applications as a part of the stable api.

for (2) we would propose the introduction and use of two macros to
allow decoration of exported data symbols. these macro would be or
similarly named __rte_import and __rte_export. of note

    * on linux the macros would expand empty but optionally
      in the future__rte_export could be enhanced to expand to
      __attribute__((visibility("default"))) enabling the use of gcc
      -fvisibility=hidden in dpdk to improve dso load times. [4][5]

    * on windows the macros would trivially expand to
      __declspec(dllimport) and __declspec(dllexport)

    * library meson.build files would need to define a preprocessor
      knob to control decoration internal/external to libraries
      exporting data symbols to ensure optimal code generation for
      accesses.

the following is the base list of proposed macro additions for the new
per_lcore macros a new header is proposed named `rte_tls.h'

__rte_export
__rte_import

  have already been explained in (2)

__rte_thread_local

  is trivially expanded to __thread or _Thread_local or
  __declspec(thread) as appropriate.

RTE_DEFINE_TLS(vartype, varname, value)
RTE_DEFINE_TLS_EXPORT(vartype, varname, value)
RTE_DECLARE_TLS(vartype, varname)
RTE_DECLARE_TLS_IMPORT(vartype, varname)

  are roughly equivalent to RTE_DEFINE_PER_LCORE and
  RTE_DECLARE_PER_LCORE where the difference in the new macros are.

    * separate macros for exported vs non-exported variables.

    * DEFINE macros require initialization value as a parameter instead
      of the assignment usage. `RTE_DEFINE_PER_LCORE(t, n) = x;' there
      was no reasonable way to expand the windows variant of the macro
      to maintain assignment so it was parameterized to allow
      flexibility.

RTE_TLS(varname)

  is the equivalent of RTE_PER_LCORE to allow r/w access to the variable
  on linux the expansion is the same for windows it is non-trivial.

we look forward to feedback on this proposal, once we have initial
feedback the series will be submitted where further review can take
place.

thanks

1.  https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-1/compiler-error-c2492?view=msvc-160
2. https://docs.microsoft.com/en-us/windows/win32/debug/pe-format
3.  https://docs.microsoft.com/en-us/cpp/build/importing-into-an-application-using-declspec-dllimport?view=msvc-160
4. https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Function-Attributes.html
5. https://gcc.gnu.org/wiki/Visibility

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Thomas Monjalon]

08/07/2021 21:21, Tyler Retzlaff:
> (2) importing exported data symbols from a dll/dso on windows requires
>     that the symbol be decorated with dllimport. optionally loading
>     performance of dll/dso is also further improved by decorating
>     exported function symbols. [3]
> 
> for (2) we would propose the introduction and use of two macros to
> allow decoration of exported data symbols. these macro would be or
> similarly named __rte_import and __rte_export. of note

That's the same symbol declared in a single place
which is exported and imported.
So I don't understand the need for 2 macros.

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Dmitry Kozlyuk]

Hi Tyler,

2021-07-08 12:21 (UTC-0700), Tyler Retzlaff:
> hi folks,
> 
> we would like to submit a a patch series that makes dll/dso for dpdk
> work on windows. there are two differences in the windows platform that
> would need to be address through enhancements to dpdk.
> 
> (1) windows dynamic objects don't export sufficient information for
>     tls variables and the windows loader and runtime would need to be
>     enhanced in order to perform runtime linking. [1][2]

When will the new loader be available?
Will it be ported to Server 2019?
Will this functionality require compiler support
(you mention that accessing such variables will be "non-trivial")?
 
> (2) importing exported data symbols from a dll/dso on windows requires
>     that the symbol be decorated with dllimport. optionally loading
>     performance of dll/dso is also further improved by decorating
>     exported function symbols. [3]

Does it affect ABI?

It is also a huge code change, although a mechanical one.
Is it required? All exported symbols are listed in .map/def, after all.

> for (1) a novel approach is proposed where a new set of per_lcore
> macros are introduced and used to replace existing macros with some
> adjustment to declaration/definition usage is made. of note
> 
>     * on linux the new macros would expand compatibly to maintain abi
>       of existing exported tls variables. since windows dynamic
>       linking has never worked there is no compatibility concern for
>       existing windows binaries.
> 
>     * the existing macros would be retained for api compatibility
>       potentially with the intent of deprecating them at a later time.
> 
>     * new macros would be "internal" to dpdk they should not be
>       available to applications as a part of the stable api.
> 
> for (2) we would propose the introduction and use of two macros to
> allow decoration of exported data symbols. these macro would be or
> similarly named __rte_import and __rte_export. of note
> 
>     * on linux the macros would expand empty but optionally
>       in the future__rte_export could be enhanced to expand to
>       __attribute__((visibility("default"))) enabling the use of gcc
>       -fvisibility=hidden in dpdk to improve dso load times. [4][5]
> 
>     * on windows the macros would trivially expand to
>       __declspec(dllimport) and __declspec(dllexport)
> 
>     * library meson.build files would need to define a preprocessor
>       knob to control decoration internal/external to libraries
>       exporting data symbols to ensure optimal code generation for
>       accesses.

Either you mean a macro to switch __rte_export between dllimport/dllexport
or I don't understand this point. BTW, what will __rte_export be for static
build?

> 
> the following is the base list of proposed macro additions for the new
> per_lcore macros a new header is proposed named `rte_tls.h'

When rte_thread_key*() family of functions was introduced as rte_tls_*(),
Jerin objected that there's a confusion with Transport Layer Security.
How about RTE_THREAD_VAR, etc?

> __rte_export
> __rte_import
> 
>   have already been explained in (2)
> 
> __rte_thread_local
> 
>   is trivially expanded to __thread or _Thread_local or
>   __declspec(thread) as appropriate.
> 
> RTE_DEFINE_TLS(vartype, varname, value)
> RTE_DEFINE_TLS_EXPORT(vartype, varname, value)
> RTE_DECLARE_TLS(vartype, varname)
> RTE_DECLARE_TLS_IMPORT(vartype, varname)
> 
>   are roughly equivalent to RTE_DEFINE_PER_LCORE and
>   RTE_DECLARE_PER_LCORE where the difference in the new macros are.
> 
>     * separate macros for exported vs non-exported variables.

Is it necessary, i.e. can' RTE_DECLARE/DEFINE_TLS compose with other
attributes, like __rte_experimental and __rte_deprecated?

>     * DEFINE macros require initialization value as a parameter instead
>       of the assignment usage. `RTE_DEFINE_PER_LCORE(t, n) = x;' there
>       was no reasonable way to expand the windows variant of the macro
>       to maintain assignment so it was parameterized to allow
>       flexibility.
> 
> RTE_TLS(varname)
> 
>   is the equivalent of RTE_PER_LCORE to allow r/w access to the variable
>   on linux the expansion is the same for windows it is non-trivial.
> [...]

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Tyler Retzlaff]

On Thu, Jul 08, 2021 at 10:39:13PM +0200, Thomas Monjalon wrote:
> 08/07/2021 21:21, Tyler Retzlaff:
> > (2) importing exported data symbols from a dll/dso on windows requires
> >     that the symbol be decorated with dllimport. optionally loading
> >     performance of dll/dso is also further improved by decorating
> >     exported function symbols. [3]
> > 
> > for (2) we would propose the introduction and use of two macros to
> > allow decoration of exported data symbols. these macro would be or
> > similarly named __rte_import and __rte_export. of note
> 
> That's the same symbol declared in a single place
> which is exported and imported.
> So I don't understand the need for 2 macros.

i may be misinterpreting your reply. you're saying there is no need for
2 because we use .def files?

strictly speaking when exporting C symbols this is true. so yes, we
could introduce only __rte_import and not bother with __rte_export.

is that what you meant?

i don't have any objection to just __rte_import alone but it is
mandatory for data symbols.

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Tyler Retzlaff]

On Thu, Jul 08, 2021 at 11:49:53PM +0300, Dmitry Kozlyuk wrote:
> Hi Tyler,
> 
> 2021-07-08 12:21 (UTC-0700), Tyler Retzlaff:
> > hi folks,
> > 
> > we would like to submit a a patch series that makes dll/dso for dpdk
> > work on windows. there are two differences in the windows platform that
> > would need to be address through enhancements to dpdk.
> > 
> > (1) windows dynamic objects don't export sufficient information for
> >     tls variables and the windows loader and runtime would need to be
> >     enhanced in order to perform runtime linking. [1][2]
> 
> When will the new loader be available?

the solution i have prototyped does not directly export the tls variables
and instead relies on exports of tls offsets within a module.  no loader
change or new os is required.

> Will it be ported to Server 2019?

not necessary (as per above)

> Will this functionality require compiler support

the prototype was developed using windows clang, mingw code compiles but
i did not try to run it. i suspect it is okay though haven't examine any
side-effects when using emul tls like mingw does. anyway mingw dll's
don't work now and it probably shouldn't block them being available with
clang.

> (you mention that accessing such variables will be "non-trivial")?

the solution involves exporting offsets that then allow explicit tls
accesses relative to the gs segment. it's non-trivial in the sense that
none of the normal explicit tls functions in windows are used and the
compiler doesn't generate the code for implicit tls access. the overhead
is relatively tolerable (one or two additional dereferences).

>  
> > (2) importing exported data symbols from a dll/dso on windows requires
> >     that the symbol be decorated with dllimport. optionally loading
> >     performance of dll/dso is also further improved by decorating
> >     exported function symbols. [3]
> 
> Does it affect ABI?

the data symbols are already part of the abi for linux. this just allows
them to be properly accessed when exported from dll on windows.
surprisingly lld-link doesn't fail when building dll's now which it should
in the absence of a __declspec(dllimport) ms link would.

on windows now the tls variables are exported but not useful with this
change we would choose not to export them at all and each exported tls
variable would be replaced with a new variable.

one nit (which we will get separate feedback on) is how to export
symbols only on windows (and don't export them on linux) because similar
to the tls variables linux has no use for my new variables.

> 
> It is also a huge code change, although a mechanical one.
> Is it required? All exported symbols are listed in .map/def, after all.

if broad sweeping mechanical change is a sensitive issue we can limit
the change to just the data symbols which are required. but keeping in
mind there is a penalty on load time when the function symbols are not
decorated. ultimately we would like them all properly decorated but we
don't need to push it now since we're just trying to enable the
functionality.

> 
> > for (1) a novel approach is proposed where a new set of per_lcore
> > macros are introduced and used to replace existing macros with some
> > adjustment to declaration/definition usage is made. of note
> > 
> >     * on linux the new macros would expand compatibly to maintain abi
> >       of existing exported tls variables. since windows dynamic
> >       linking has never worked there is no compatibility concern for
> >       existing windows binaries.
> > 
> >     * the existing macros would be retained for api compatibility
> >       potentially with the intent of deprecating them at a later time.
> > 
> >     * new macros would be "internal" to dpdk they should not be
> >       available to applications as a part of the stable api.
> > 
> > for (2) we would propose the introduction and use of two macros to
> > allow decoration of exported data symbols. these macro would be or
> > similarly named __rte_import and __rte_export. of note
> > 
> >     * on linux the macros would expand empty but optionally
> >       in the future__rte_export could be enhanced to expand to
> >       __attribute__((visibility("default"))) enabling the use of gcc
> >       -fvisibility=hidden in dpdk to improve dso load times. [4][5]
> > 
> >     * on windows the macros would trivially expand to
> >       __declspec(dllimport) and __declspec(dllexport)
> > 
> >     * library meson.build files would need to define a preprocessor
> >       knob to control decoration internal/external to libraries
> >       exporting data symbols to ensure optimal code generation for
> >       accesses.
> 
> Either you mean a macro to switch __rte_export between dllimport/dllexport
> or I don't understand this point. BTW, what will __rte_export be for static
> build?

there are two import cases that a library like eal has when it exports a
data symbol.

e.g. if eal exports a variable where the variable is used both within
eal and outside of eal you want different expansions of __rte_import.

when consuming the variable within eal if __declspec(import) is used you
will get less-optimal codegen (because the code is generated for
imported access). however outside of eal you need the __declspec(import)
to generate the correct code to access the exported data.

i haven't looked into how gcc/ld deals with this. maybe ld is just
smarter and figures out when to generate the optimal code.

static build doesn't really get negatively impacted by __rte_export but
when statically linking the ms linker will complain with warnings that
can be suppressed without harm.

it's still something that is on my mind (and i don't want to make it an
issue that blocks this proposal) but i'm starting to lean toward a build
time option where either static or dynamic build is requested instead of
cobbling both together out of the same build product.  but that is
really off topic for this change.
> 
> > 
> > the following is the base list of proposed macro additions for the new
> > per_lcore macros a new header is proposed named `rte_tls.h'
> 
> When rte_thread_key*() family of functions was introduced as rte_tls_*(),
> Jerin objected that there's a confusion with Transport Layer Security.
> How about RTE_THREAD_VAR, etc?

no objection, one of the reason i posted the set of macros from the
prototype was so people could offer up suggestions on better namespace.

> 
> > __rte_export
> > __rte_import
> > 
> >   have already been explained in (2)
> > 
> > __rte_thread_local
> > 
> >   is trivially expanded to __thread or _Thread_local or
> >   __declspec(thread) as appropriate.
> > 
> > RTE_DEFINE_TLS(vartype, varname, value)
> > RTE_DEFINE_TLS_EXPORT(vartype, varname, value)
> > RTE_DECLARE_TLS(vartype, varname)
> > RTE_DECLARE_TLS_IMPORT(vartype, varname)
> > 
> >   are roughly equivalent to RTE_DEFINE_PER_LCORE and
> >   RTE_DECLARE_PER_LCORE where the difference in the new macros are.
> > 
> >     * separate macros for exported vs non-exported variables.
> 
> Is it necessary, i.e. can' RTE_DECLARE/DEFINE_TLS compose with other
> attributes, like __rte_experimental and __rte_deprecated?

it's necessary in so far as the existing per-lcore variables that are
not imported/export can still have storage class specifier like static
applied without jumping through hoops.

i tried for some time to have a single declare/define macro but dealing
with 'static' being used made the problem hard. i can't just "shut-off"
the nested __rte_export/__rte_import expansion if static is put in front
of the macro or parameterized.

> 
> >     * DEFINE macros require initialization value as a parameter instead
> >       of the assignment usage. `RTE_DEFINE_PER_LCORE(t, n) = x;' there
> >       was no reasonable way to expand the windows variant of the macro
> >       to maintain assignment so it was parameterized to allow
> >       flexibility.
> > 
> > RTE_TLS(varname)
> > 
> >   is the equivalent of RTE_PER_LCORE to allow r/w access to the variable
> >   on linux the expansion is the same for windows it is non-trivial.
> > [...]

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Thomas Monjalon]

09/07/2021 02:16, Tyler Retzlaff:
> On Thu, Jul 08, 2021 at 10:39:13PM +0200, Thomas Monjalon wrote:
> > 08/07/2021 21:21, Tyler Retzlaff:
> > > (2) importing exported data symbols from a dll/dso on windows requires
> > >     that the symbol be decorated with dllimport. optionally loading
> > >     performance of dll/dso is also further improved by decorating
> > >     exported function symbols. [3]
> > > 
> > > for (2) we would propose the introduction and use of two macros to
> > > allow decoration of exported data symbols. these macro would be or
> > > similarly named __rte_import and __rte_export. of note
> > 
> > That's the same symbol declared in a single place
> > which is exported and imported.
> > So I don't understand the need for 2 macros.
> 
> i may be misinterpreting your reply. you're saying there is no need for
> 2 because we use .def files?
> 
> strictly speaking when exporting C symbols this is true. so yes, we
> could introduce only __rte_import and not bother with __rte_export.
> 
> is that what you meant?
> 
> i don't have any objection to just __rte_import alone but it is
> mandatory for data symbols.

It may be my misunderstanding.
The function is declared only once in the .h
so I don't understand where these 2 macros are used.

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Tyler Retzlaff]

On Fri, Jul 09, 2021 at 01:34:06PM +0200, Thomas Monjalon wrote:
> 09/07/2021 02:16, Tyler Retzlaff:
> > On Thu, Jul 08, 2021 at 10:39:13PM +0200, Thomas Monjalon wrote:
> > > 08/07/2021 21:21, Tyler Retzlaff:
> > > > (2) importing exported data symbols from a dll/dso on windows requires
> > > >     that the symbol be decorated with dllimport. optionally loading
> > > >     performance of dll/dso is also further improved by decorating
> > > >     exported function symbols. [3]
> > > > 
> > > > for (2) we would propose the introduction and use of two macros to
> > > > allow decoration of exported data symbols. these macro would be or
> > > > similarly named __rte_import and __rte_export. of note
> > > 
> > > That's the same symbol declared in a single place
> > > which is exported and imported.
> > > So I don't understand the need for 2 macros.
> > 
> > i may be misinterpreting your reply. you're saying there is no need for
> > 2 because we use .def files?
> > 
> > strictly speaking when exporting C symbols this is true. so yes, we
> > could introduce only __rte_import and not bother with __rte_export.
> > 
> > is that what you meant?
> > 
> > i don't have any objection to just __rte_import alone but it is
> > mandatory for data symbols.
> 
> It may be my misunderstanding.
> The function is declared only once in the .h
> so I don't understand where these 2 macros are used.
> 

okay, i think the question is about mechanically where they are used and
i'll limit my answer to data symbols since that's what we need them for.

the export is typically used at the point of definition and the import
at declaration. so ignoring anything about tls the following example
hopefully illustrates.

let's start with a module i'll just call it eal.dll it is declaring a
data symbol that will be exported and may be referenced internally within
eal.dll itself but also externally by other modules outside of eal.dll.

    // eal.c
    __rte_export
    int eal_some_var;

    // eal.h
    #ifndef BUILDING_EAL
    __rte_import
    #endif
    extern int eal_some_var;

    // eal_other.c
    #define BUILDING_EAL
    #include <eal.h>

    eal_some_var = 100;

we also have another module app.exe that consumes the variable
exported from eal.dll.

    // app.c
    #include <eal.h>

    eal_some_var = 200;

as mentioned in my first reply you can get away with not using
__rte_export because dpdk also lists the exports in the .map/.def files
even for data symbols but it is required for import [1]. the relevant
text from the documentation is quoted here.

    Using __declspec(dllimport) is optional on function declarations,
    but the compiler produces more efficient code if you use this
    keyword.  However, you must use __declspec(dllimport) for the
    importing executable to access the DLL's public data symbols and
    objects. Note that the users of your DLL still need to link with
    an import library.

    specifically "you must use __declspec(dllimport) for the importing
    executable to access the DLL's public data symbols" is relevant.

the other internal/external conditional compile for BUILDING_EAL is
applied to avoid generating the code for the extra layer of indirection
mentioned here [2] when using the variable in the same module in which
it was defined.

1. https://docs.microsoft.com/en-us/cpp/build/importing-into-an-application-using-declspec-dllimport?view=msvc-160
2. https://docs.microsoft.com/en-us/cpp/build/importing-data-using-declspec-dllimport?view=msvc-160

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Dmitry Kozlyuk]

2021-07-08 18:03 (UTC-0700), Tyler Retzlaff:
> On Thu, Jul 08, 2021 at 11:49:53PM +0300, Dmitry Kozlyuk wrote:
> > Hi Tyler,
> > 
> > 2021-07-08 12:21 (UTC-0700), Tyler Retzlaff:  
> > > hi folks,
> > > 
> > > we would like to submit a a patch series that makes dll/dso for dpdk
> > > work on windows. there are two differences in the windows platform that
> > > would need to be address through enhancements to dpdk.
> > > 
> > > (1) windows dynamic objects don't export sufficient information for
> > >     tls variables and the windows loader and runtime would need to be
> > >     enhanced in order to perform runtime linking. [1][2]  
> > 
> > When will the new loader be available?  
> 
> the solution i have prototyped does not directly export the tls variables
> and instead relies on exports of tls offsets within a module.  no loader
> change or new os is required.
> 
> > Will it be ported to Server 2019?  
> 
> not necessary (as per above)
> 
> > Will this functionality require compiler support  
> 
> the prototype was developed using windows clang, mingw code compiles but
> i did not try to run it. i suspect it is okay though haven't examine any
> side-effects when using emul tls like mingw does. anyway mingw dll's
> don't work now and it probably shouldn't block them being available with
> clang.

AFAIK it's the opposite. MinGW can handle TLS varibale export from DLL,
although with "__emutls." prefix and some performance penalty.
Clang can't at all, despite compiling and linking without an issue.

No, it is not acceptable to add a generic feature supported by only one
compiler. (FWIW, I'm displeased even by mlx5 being tied to clang.)
Particularly, I don't understand how could MinGW and clang coexist
if they export different sets of symbols. Apps will need to know
if it's MingW- or clang-compiled DPDK? Sounds messy.

> > (you mention that accessing such variables will be "non-trivial")?  
> 
> the solution involves exporting offsets that then allow explicit tls
> accesses relative to the gs segment. it's non-trivial in the sense that
> none of the normal explicit tls functions in windows are used and the
> compiler doesn't generate the code for implicit tls access. the overhead
> is relatively tolerable (one or two additional dereferences).

A thorough benchmark will be required. I'm afraid that inline assembly
(which %gs mention suggests) can impact optimization of the code nearby.
Ideally it should be a DPDK performance autotest.

> 
> >    
> > > (2) importing exported data symbols from a dll/dso on windows requires
> > >     that the symbol be decorated with dllimport. optionally loading
> > >     performance of dll/dso is also further improved by decorating
> > >     exported function symbols. [3]  
> > 
> > Does it affect ABI?  
> 
> the data symbols are already part of the abi for linux. this just allows
> them to be properly accessed when exported from dll on windows.
> surprisingly lld-link doesn't fail when building dll's now which it should
> in the absence of a __declspec(dllimport) ms link would.
> 
> on windows now the tls variables are exported but not useful with this
> change we would choose not to export them at all and each exported tls
> variable would be replaced with a new variable.
> 
> one nit (which we will get separate feedback on) is how to export
> symbols only on windows (and don't export them on linux) because similar
> to the tls variables linux has no use for my new variables.

There's already WINDOWS_NO_EXPORT mark in .map to generate .def,
likewise, .map for Linux/FreeBSD could be generated from a basic .map
with similar marks.

> > 
> > It is also a huge code change, although a mechanical one.
> > Is it required? All exported symbols are listed in .map/def, after all.  
> 
> if broad sweeping mechanical change is a sensitive issue we can limit
> the change to just the data symbols which are required. but keeping in
> mind there is a penalty on load time when the function symbols are not
> decorated. ultimately we would like them all properly decorated but we
> don't need to push it now since we're just trying to enable the
> functionality.

I was asking in connection with the previous question about ABI,
because 21.11 ABI freeze may be a two-year one. Since ABI is not affected
for Unix and for Windows we don't maintain it currently, there is no rush for
the change at least.

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Tyler Retzlaff]

On Fri, Jul 16, 2021 at 12:40:35PM +0300, Dmitry Kozlyuk wrote:
> 2021-07-08 18:03 (UTC-0700), Tyler Retzlaff:
> > On Thu, Jul 08, 2021 at 11:49:53PM +0300, Dmitry Kozlyuk wrote:
> > > Hi Tyler,
> > > 
> > > 2021-07-08 12:21 (UTC-0700), Tyler Retzlaff:  
> > > > hi folks,
> > > > 
> > > > we would like to submit a a patch series that makes dll/dso for dpdk
> > > > work on windows. there are two differences in the windows platform that
> > > > would need to be address through enhancements to dpdk.
> > > > 
> > > > (1) windows dynamic objects don't export sufficient information for
> > > >     tls variables and the windows loader and runtime would need to be
> > > >     enhanced in order to perform runtime linking. [1][2]  
> > > 
> > > When will the new loader be available?  
> > 
> > the solution i have prototyped does not directly export the tls variables
> > and instead relies on exports of tls offsets within a module.  no loader
> > change or new os is required.
> > 
> > > Will it be ported to Server 2019?  
> > 
> > not necessary (as per above)
> > 
> > > Will this functionality require compiler support  
> > 
> > the prototype was developed using windows clang, mingw code compiles but
> > i did not try to run it. i suspect it is okay though haven't examine any
> > side-effects when using emul tls like mingw does. anyway mingw dll's
> > don't work now and it probably shouldn't block them being available with
> > clang.
> 
> AFAIK it's the opposite. MinGW can handle TLS varibale export from DLL,
> although with "__emutls." prefix and some performance penalty.
> Clang can't at all, despite compiling and linking without an issue.

mingw emutls just makes it compile allowing the variables to be exported,
the binaries still won't work without loader support. or are you saying
they do?

> 
> No, it is not acceptable to add a generic feature supported by only one
> compiler. (FWIW, I'm displeased even by mlx5 being tied to clang.)
> Particularly, I don't understand how could MinGW and clang coexist
> if they export different sets of symbols. Apps will need to know
> if it's MingW- or clang-compiled DPDK? Sounds messy.

it doesn't seem reasonable to reject the feature because mingw may or
may not work.  mingw binaries are not worse off if the feature can be
enabled with clang. either way it is untested i am uncertain if it will
work with mingw and have no time budget to test it. if it made mingw
built binaries "worse" i would agree with you but the worst case
scenario is that it works exactly as well as it does now.

> 
> > > (you mention that accessing such variables will be "non-trivial")?  
> > 
> > the solution involves exporting offsets that then allow explicit tls
> > accesses relative to the gs segment. it's non-trivial in the sense that
> > none of the normal explicit tls functions in windows are used and the
> > compiler doesn't generate the code for implicit tls access. the overhead
> > is relatively tolerable (one or two additional dereferences).
> 
> A thorough benchmark will be required. I'm afraid that inline assembly
> (which %gs mention suggests) can impact optimization of the code nearby.
> Ideally it should be a DPDK performance autotest.

no inline assembly is used, only compiler intrinsics which is superior
because it allows the compiler an opportunity to perform optimization
which inline assembly does not. whether or not clang or mingw will
optimize i have no idea. this is aside from the expected additional code
generated for cross dll boundary accesses.

let's catch up at the sync and discuss your concerns.

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Dmitry Kozlyuk]

2021-07-18 20:45 (UTC-0700), Tyler Retzlaff:
> On Fri, Jul 16, 2021 at 12:40:35PM +0300, Dmitry Kozlyuk wrote:
> > 2021-07-08 18:03 (UTC-0700), Tyler Retzlaff:  
> > > On Thu, Jul 08, 2021 at 11:49:53PM +0300, Dmitry Kozlyuk wrote:  
> > > > Hi Tyler,
> > > > 
> > > > 2021-07-08 12:21 (UTC-0700), Tyler Retzlaff:    
> > > > > hi folks,
> > > > > 
> > > > > we would like to submit a a patch series that makes dll/dso for dpdk
> > > > > work on windows. there are two differences in the windows platform that
> > > > > would need to be address through enhancements to dpdk.
> > > > > 
> > > > > (1) windows dynamic objects don't export sufficient information for
> > > > >     tls variables and the windows loader and runtime would need to be
> > > > >     enhanced in order to perform runtime linking. [1][2]    
> > > > 
> > > > When will the new loader be available?    
> > > 
> > > the solution i have prototyped does not directly export the tls variables
> > > and instead relies on exports of tls offsets within a module.  no loader
> > > change or new os is required.
> > >   
> > > > Will it be ported to Server 2019?    
> > > 
> > > not necessary (as per above)
> > >   
> > > > Will this functionality require compiler support    
> > > 
> > > the prototype was developed using windows clang, mingw code compiles but
> > > i did not try to run it. i suspect it is okay though haven't examine any
> > > side-effects when using emul tls like mingw does. anyway mingw dll's
> > > don't work now and it probably shouldn't block them being available with
> > > clang.  
> > 
> > AFAIK it's the opposite. MinGW can handle TLS varibale export from DLL,
> > although with "__emutls." prefix and some performance penalty.
> > Clang can't at all, despite compiling and linking without an issue.  
> 
> mingw emutls just makes it compile allowing the variables to be exported,
> the binaries still won't work without loader support. or are you saying
> they do?
> 
> > 
> > No, it is not acceptable to add a generic feature supported by only one
> > compiler. (FWIW, I'm displeased even by mlx5 being tied to clang.)
> > Particularly, I don't understand how could MinGW and clang coexist
> > if they export different sets of symbols. Apps will need to know
> > if it's MingW- or clang-compiled DPDK? Sounds messy.  
> 
> it doesn't seem reasonable to reject the feature because mingw may or
> may not work.  mingw binaries are not worse off if the feature can be
> enabled with clang. either way it is untested i am uncertain if it will
> work with mingw and have no time budget to test it. if it made mingw
> built binaries "worse" i would agree with you but the worst case
> scenario is that it works exactly as well as it does now.

My mistake, I thought we exported __emutls.VARNAME symbols (in which case
MinGW DLLs would work without loader change). Since we don't, then you're
right, we have the same set of exported symbols and can enable this feature.
I'm willing to share the effort and help with MinGW.

> 
> >   
> > > > (you mention that accessing such variables will be "non-trivial")?    
> > > 
> > > the solution involves exporting offsets that then allow explicit tls
> > > accesses relative to the gs segment. it's non-trivial in the sense that
> > > none of the normal explicit tls functions in windows are used and the
> > > compiler doesn't generate the code for implicit tls access. the overhead
> > > is relatively tolerable (one or two additional dereferences).  
> > 
> > A thorough benchmark will be required. I'm afraid that inline assembly
> > (which %gs mention suggests) can impact optimization of the code nearby.
> > Ideally it should be a DPDK performance autotest.  
> 
> no inline assembly is used, only compiler intrinsics which is superior
> because it allows the compiler an opportunity to perform optimization
> which inline assembly does not. whether or not clang or mingw will
> optimize i have no idea. this is aside from the expected additional code
> generated for cross dll boundary accesses.
> 
> let's catch up at the sync and discuss your concerns.

I think there are none left, unless you have some.
Waiting for you to post the actual code.

Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows

[Tyler Retzlaff]

On Mon, Jul 19, 2021 at 12:12:12PM +0300, Dmitry Kozlyuk wrote:
> > 
> > mingw emutls just makes it compile allowing the variables to be exported,
> > the binaries still won't work without loader support. or are you saying
> > they do?
> > 
> > > 
> > > No, it is not acceptable to add a generic feature supported by only one
> > > compiler. (FWIW, I'm displeased even by mlx5 being tied to clang.)
> > > Particularly, I don't understand how could MinGW and clang coexist
> > > if they export different sets of symbols. Apps will need to know
> > > if it's MingW- or clang-compiled DPDK? Sounds messy.  
> > 
> > it doesn't seem reasonable to reject the feature because mingw may or
> > may not work.  mingw binaries are not worse off if the feature can be
> > enabled with clang. either way it is untested i am uncertain if it will
> > work with mingw and have no time budget to test it. if it made mingw
> > built binaries "worse" i would agree with you but the worst case
> > scenario is that it works exactly as well as it does now.
> 
> My mistake, I thought we exported __emutls.VARNAME symbols (in which case
> MinGW DLLs would work without loader change). Since we don't, then you're
> right, we have the same set of exported symbols and can enable this feature.
> I'm willing to share the effort and help with MinGW.

sounds good to me, i'll work on a draft patch with you and when we have
something acceptable we'll formally post it.

thanks for helping out.