From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2D18BA0548; Fri, 9 Jul 2021 03:03:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E4FDE4014D; Fri, 9 Jul 2021 03:03:21 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id BAB9140143 for ; Fri, 9 Jul 2021 03:03:20 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 02BF920B7178; Thu, 8 Jul 2021 18:03:20 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 02BF920B7178 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1625792600; bh=8j373n0chlGa19i6+dfBfLHtxnaDzbgisLq1iQmF+po=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=aO9whyGtE3gP97tcJOpI2briLdvxcFUwg7jfr0EQ3nJwXTr7NsZFYBAaHwcZh+7Ir A9EPYTV7s/6Nzk4Kj4cM1vpsf+/GT54KqJshp3GDLeH7hyWP7PWAivj5zdU6cNYCgl TX9LVO/48+GJ2tFSaNxsi2O6c6HwDyna/wGkQstU= Date: Thu, 8 Jul 2021 18:03:19 -0700 From: Tyler Retzlaff To: Dmitry Kozlyuk Cc: dev@dpdk.org, thomas@monjalon.net Message-ID: <20210709010319.GB23346@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20210708192109.GA13966@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20210708234953.67906871@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210708234953.67906871@sovereign> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [dpdk-dev] RFC enabling dll/dso for dpdk on windows X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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. > > [...]