diff mbox series

[v2,9/9] telemetry: avoid expanding versioned symbol macros on msvc

Message ID 1680638847-26430-10-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series msvc integration changes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-x86_64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Tyler Retzlaff April 4, 2023, 8:07 p.m. UTC 
  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 <roretzla@linux.microsoft.com>
---
 lib/telemetry/telemetry_data.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Bruce Richardson April 5, 2023, 10:56 a.m. UTC | #1 
On Tue, Apr 04, 2023 at 01:07:27PM -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 <roretzla@linux.microsoft.com>

Does this require a change in telemetry itself? Can it be done via the
header file with the versioning macros in it, so it would apply to any
other versioned functions we have in DPDK?

/Bruce

> ---
>  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
>  
>  int
>  rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)
> @@ -220,8 +228,16 @@
>  /* mark the v23 function as the older version, and v24 as the default version */
>  VERSION_SYMBOL(rte_tel_data_add_dict_int, _v23, 23);
>  BIND_DEFAULT_SYMBOL(rte_tel_data_add_dict_int, _v24, 24);
> +#ifndef RTE_TOOLCHAIN_MSVC
>  MAP_STATIC_SYMBOL(int rte_tel_data_add_dict_int(struct rte_tel_data *d,
>  		const char *name, int64_t val), rte_tel_data_add_dict_int_v24);
> +#else
> +int
> +rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int64_t val)
> +{
> +	return rte_tel_data_add_dict_int_v24(d, name, val);
> +}
> +#endif
>  
>  int
>  rte_tel_data_add_dict_uint(struct rte_tel_data *d,
> -- 
> 1.8.3.1
>
Tyler Retzlaff April 5, 2023, 4:02 p.m. UTC | #2 
On Wed, Apr 05, 2023 at 11:56:05AM +0100, Bruce Richardson wrote:
> On Tue, Apr 04, 2023 at 01:07:27PM -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 <roretzla@linux.microsoft.com>
> 
> Does this require a change in telemetry itself? Can it be done via the
> header file with the versioning macros in it, so it would apply to any
> other versioned functions we have in DPDK?

i didn't spend a lot of time thinking if the existing macros could be
made to expand in the way needed. there is a way of doing versioning on
windows but it is foreign to how this symbol versioning scheme works so
i plan to investigate it separately after i get unit tests running.

for now i know what i'm doing is ugly but i need to get protection of
unit tests so i'm doing minimal changes to get to that point. if you're
not comfortable with this going in on a temporary basis i can remove it
from this series and we can work on it as a separated patch set.

my bar is pretty low here, as long as it doesn't break any existing
linux/gcc/clang etc ok, if msvc is not right i'll take a second pass
and design each stop-gap properly. it already doesn't work so things
aren't made worse.

let me know if i need to carve this out of the series.

ty

> 
> /Bruce
> 
> > ---
> >  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
> >  
> >  int
> >  rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)
> > @@ -220,8 +228,16 @@
> >  /* mark the v23 function as the older version, and v24 as the default version */
> >  VERSION_SYMBOL(rte_tel_data_add_dict_int, _v23, 23);
> >  BIND_DEFAULT_SYMBOL(rte_tel_data_add_dict_int, _v24, 24);
> > +#ifndef RTE_TOOLCHAIN_MSVC
> >  MAP_STATIC_SYMBOL(int rte_tel_data_add_dict_int(struct rte_tel_data *d,
> >  		const char *name, int64_t val), rte_tel_data_add_dict_int_v24);
> > +#else
> > +int
> > +rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int64_t val)
> > +{
> > +	return rte_tel_data_add_dict_int_v24(d, name, val);
> > +}
> > +#endif
> >  
> >  int
> >  rte_tel_data_add_dict_uint(struct rte_tel_data *d,
> > -- 
> > 1.8.3.1
> >
Bruce Richardson April 5, 2023, 4:17 p.m. UTC | #3 
On Wed, Apr 05, 2023 at 09:02:10AM -0700, Tyler Retzlaff wrote:
> On Wed, Apr 05, 2023 at 11:56:05AM +0100, Bruce Richardson wrote:
> > On Tue, Apr 04, 2023 at 01:07:27PM -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 <roretzla@linux.microsoft.com>
> > 
> > Does this require a change in telemetry itself? Can it be done via the
> > header file with the versioning macros in it, so it would apply to any
> > other versioned functions we have in DPDK?
> 
> i didn't spend a lot of time thinking if the existing macros could be
> made to expand in the way needed. there is a way of doing versioning on
> windows but it is foreign to how this symbol versioning scheme works so
> i plan to investigate it separately after i get unit tests running.
> 
> for now i know what i'm doing is ugly but i need to get protection of
> unit tests so i'm doing minimal changes to get to that point. if you're
> not comfortable with this going in on a temporary basis i can remove it
> from this series and we can work on it as a separated patch set.
> 
> my bar is pretty low here, as long as it doesn't break any existing
> linux/gcc/clang etc ok, if msvc is not right i'll take a second pass
> and design each stop-gap properly. it already doesn't work so things
> aren't made worse.
> 
> let me know if i need to carve this out of the series.
> 
It's not that ugly. :-) If no other clear solution is apparent, I can certainly
live with this.

/Bruce
diff mbox series

Patch

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
 
 int
 rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)
@@ -220,8 +228,16 @@ 
 /* mark the v23 function as the older version, and v24 as the default version */
 VERSION_SYMBOL(rte_tel_data_add_dict_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_dict_int, _v24, 24);
+#ifndef RTE_TOOLCHAIN_MSVC
 MAP_STATIC_SYMBOL(int rte_tel_data_add_dict_int(struct rte_tel_data *d,
 		const char *name, int64_t val), rte_tel_data_add_dict_int_v24);
+#else
+int
+rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int64_t val)
+{
+	return rte_tel_data_add_dict_int_v24(d, name, val);
+}
+#endif
 
 int
 rte_tel_data_add_dict_uint(struct rte_tel_data *d,