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 7291D4291F; Tue, 11 Apr 2023 22:34:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F3F6140E2D; Tue, 11 Apr 2023 22:34:17 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id A589B40DFD for ; Tue, 11 Apr 2023 22:34:15 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id C50452171F97; Tue, 11 Apr 2023 13:34:14 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com C50452171F97 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1681245254; bh=i1LumoCyW2uvGBnA54UYq1e8ZM3oFewMAhihML9fm7I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=s2CdCneda2tU1Livu/rtCerecPcBxfRKCkxhfT6IqLToSd1TgCKwPhUt5xFruvG2+ 9PGm+6lXqnPNbrMfyFzjYz9piXNoxaSCeJnNVxN6oWExOWOEGAZ58G0zFnB2CPJFrd ieJtvPxzMZLCThqtDXyrP+9dORsfzEUFoev736z4= Date: Tue, 11 Apr 2023 13:34:14 -0700 From: Tyler Retzlaff To: Bruce Richardson Cc: dev@dpdk.org, david.marchand@redhat.com, thomas@monjalon.net, mb@smartsharesystems.com, konstantin.ananyev@huawei.com Subject: Re: [PATCH v3 11/11] telemetry: avoid expanding versioned symbol macros on msvc Message-ID: <20230411203414.GA9591@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1680558751-17931-1-git-send-email-roretzla@linux.microsoft.com> <1680741919-22102-1-git-send-email-roretzla@linux.microsoft.com> <1680741919-22102-12-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Tue, Apr 11, 2023 at 11:24:07AM +0100, Bruce Richardson wrote: > On Wed, Apr 05, 2023 at 05:45:19PM -0700, Tyler Retzlaff wrote: > > Windows does not support versioned symbols. Fortunately Windows also > > doesn't have an exported stable ABI. > > > > Export rte_tel_data_add_array_int -> rte_tel_data_add_array_int_24 > > and rte_tel_data_add_dict_int -> rte_tel_data_add_dict_int_v24 > > functions. > > > > Windows does have a way to achieve similar versioning for symbols but it > > is not a simple #define so it will be done as a work package later. > > > > Signed-off-by: Tyler Retzlaff > > --- > > lib/telemetry/telemetry_data.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c > > index 2bac2de..284c16e 100644 > > --- a/lib/telemetry/telemetry_data.c > > +++ b/lib/telemetry/telemetry_data.c > > @@ -82,8 +82,16 @@ > > /* mark the v23 function as the older version, and v24 as the default version */ > > VERSION_SYMBOL(rte_tel_data_add_array_int, _v23, 23); > > BIND_DEFAULT_SYMBOL(rte_tel_data_add_array_int, _v24, 24); > > +#ifndef RTE_TOOLCHAIN_MSVC > > MAP_STATIC_SYMBOL(int rte_tel_data_add_array_int(struct rte_tel_data *d, > > int64_t x), rte_tel_data_add_array_int_v24); > > +#else > > +int > > +rte_tel_data_add_array_int(struct rte_tel_data *d, int64_t x) > > +{ > > + return rte_tel_data_add_array_int_v24(d, x); > > +} > > +#endif > > > > Can't see any general way to do this from the versioning header file, so > agree that we need some changes here. Rather than defining a public > funcion, we could keep the diff reduced by just using a macro alias here, > right? For example: > > #ifdef RTE_TOOLCHAIN_MSVC > #define rte_tel_data_add_array_int rte_tel_data_add_array_int_v24 > #else > MAP_STATIC_SYMBOL(int rte_tel_data_add_array_int(struct rte_tel_data *d, > int64_t x), rte_tel_data_add_array_int_v24); > #endif > > If this is a temporary measure, I'd tend towards the shortest solution that > can work. However, no strong opinions, so, either using functions as you > have it, or macros: so i have to leave it as it is the reason being the version.map -> exports.def generation does not handle this. the .def only contains the rte_tel_data_add_array_int symbol. if we expand it away to the _v24 name the link will fail. let's consume the change as-is for now and i will work on the generalized solution when changes are integrated that actually make the windows dso/dll functional. > > Acked-by: Bruce Richardson