From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1E9A4A04FA; Sun, 2 Feb 2020 19:54:01 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4E5ED1BFC8; Sun, 2 Feb 2020 19:54:00 +0100 (CET) Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id F39B51BFB5 for ; Sun, 2 Feb 2020 19:53:57 +0100 (CET) Received: from [107.15.85.130] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1iyKNI-00014F-JI; Sun, 02 Feb 2020 13:53:49 -0500 Date: Sun, 2 Feb 2020 19:53:34 +0100 From: Neil Horman To: Ferruh Yigit Cc: Cristian Dumitrescu , Eelco Chaudron , dev@dpdk.org, Thomas Monjalon , Luca Boccassi , David Marchand , Bruce Richardson , Ian Stokes Message-ID: <20200202185334.GA6783@localhost.localdomain> References: <20200129122953.2016199-1-ferruh.yigit@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200129122953.2016199-1-ferruh.yigit@intel.com> User-Agent: Mutt/1.12.1 (2019-06-15) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 Wed, Jan 29, 2020 at 12:29:53PM +0000, Ferruh Yigit wrote: > Duplicated the existing symbol and versioned one as experimental and > other as stable. > > Created VERSION_SYMBOL_EXPERIMENTAL helper macro. > > Updated the 'check-experimental-syms.sh' buildtool, which was > complaining that the symbol is in EXPERIMENTAL tag in .map file but it > is not in the .experimental section (__rte_experimental tag is missing). > Updated tool in a way it won't complain if the symbol in the > EXPERIMENTAL tag duplicated in some other block in .map file (versioned) > > Updated meson build system to allow the versioning, > 'use_function_versioning = true', not sure why it was disabled by > default. > > Fixes: 30512af820fe ("meter: remove experimental flag from RFC4115 trTCM API") > I'm not sure I understand the purpose here. I think I understand what you are trying to do. I believe that you are trying to move the experimental symbols in librte_meter to be part of the official ABI, while still maintaining backward compatibility with applications built against the experimental symbol versions, correct? I can understand the desire, and I'm not exactly opposed to providing a mechanism for that, but it seems somewhat complex, and to be honest, part of the drawback to using experimental symbols as an application developer is understanding that you may need to rebuild when those symbols change (even if the change is the symbol removal and replacement with an identical one that has a versioned tag). I think the right answer for people encountering this condition is to just rebuild. Otherwise, we are creating an environment in which we are implicitly communicating to users that we are maintaining a modicum of stability in experimental symobls. A few more nits in line > Signed-off-by: Ferruh Yigit > --- > Cc: Neil Horman > Cc: Thomas Monjalon > Cc: Luca Boccassi > Cc: David Marchand > Cc: Bruce Richardson > Cc: Ian Stokes > Cc: Eelco Chaudron > > I didn't like the idea of duplicating the symbol but not able to find to > alias it without duplicating, if there is a way please share. > > Also not tested with old binaries, only verified the symbols in new > binaries. > --- > buildtools/check-experimental-syms.sh | 3 +- > .../common/include/rte_function_versioning.h | 4 ++ > lib/librte_meter/rte_meter.c | 59 ++++++++++++++++++- > lib/librte_meter/rte_meter_version.map | 8 +++ > lib/meson.build | 2 +- > 5 files changed, 71 insertions(+), 5 deletions(-) > > diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-experimental-syms.sh > index f3603e5ba..35c4bf006 100755 > --- a/buildtools/check-experimental-syms.sh > +++ b/buildtools/check-experimental-syms.sh > @@ -26,7 +26,8 @@ ret=0 > for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE |cut -d ' ' -f 3` > do > if grep -q "\.text.*[[:space:]]$SYM$" $DUMPFILE && > - ! grep -q "\.text\.experimental.*[[:space:]]$SYM$" $DUMPFILE > + ! grep -q "\.text\.experimental.*[[:space:]]$SYM$" $DUMPFILE && > + $LIST_SYMBOL -s $SYM $MAPFILE | grep -q EXPERIMENTAL > then > cat >&2 <<- END_OF_MESSAGE > $SYM is not flagged as experimental > diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h > index c924351d5..ab102b06e 100644 > --- a/lib/librte_eal/common/include/rte_function_versioning.h > +++ b/lib/librte_eal/common/include/rte_function_versioning.h > @@ -46,6 +46,9 @@ > */ > #define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n)) > > + > +#define VERSION_SYMBOL_EXPERIMENTAL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@EXPERIMENTAL") > + I don't know that you want to make a explicit clone of this macro. instead what you might want to do is something like: #define __VERSION_SYMBOL(b, e, n, t) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@" RTE_STR(t) "_" RTE_STR(n)) #define VERSION_SYMBOL(b, e, n) __VERSION_SYMBOL(b, e, n, DPDK) Thats not exactly right, but you get the idea, then you can emulate VERSION_SYMBOL_EXPERIMENTAL with __VERSION_SYMBOL(b, e, n, EXPERIMENTAL) > /* > * BIND_DEFAULT_SYMBOL > * Creates a symbol version entry instructing the linker to bind references to > @@ -79,6 +82,7 @@ > * No symbol versioning in use > */ > #define VERSION_SYMBOL(b, e, n) > +#define VERSION_SYMBOL_EXPERIMENTAL(b, e) > #define __vsym > #define BIND_DEFAULT_SYMBOL(b, e, n) > #define MAP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p)))) > diff --git a/lib/librte_meter/rte_meter.c b/lib/librte_meter/rte_meter.c > index da01429a8..5244537fa 100644 > --- a/lib/librte_meter/rte_meter.c > +++ b/lib/librte_meter/rte_meter.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > > #include "rte_meter.h" > > @@ -119,8 +120,8 @@ rte_meter_trtcm_config(struct rte_meter_trtcm *m, > return 0; > } > > -int > -rte_meter_trtcm_rfc4115_profile_config( > +static int > +rte_meter_trtcm_rfc4115_profile_config_( > struct rte_meter_trtcm_rfc4115_profile *p, > struct rte_meter_trtcm_rfc4115_params *params) > { > @@ -145,7 +146,35 @@ rte_meter_trtcm_rfc4115_profile_config( > } > > int > -rte_meter_trtcm_rfc4115_config( > +rte_meter_trtcm_rfc4115_profile_config_s( > + struct rte_meter_trtcm_rfc4115_profile *p, > + struct rte_meter_trtcm_rfc4115_params *params); > +int > +rte_meter_trtcm_rfc4115_profile_config_s( > + struct rte_meter_trtcm_rfc4115_profile *p, > + struct rte_meter_trtcm_rfc4115_params *params) > +{ > + return rte_meter_trtcm_rfc4115_profile_config_(p, params); > +} > +BIND_DEFAULT_SYMBOL(rte_meter_trtcm_rfc4115_profile_config, _s, 20.0.1); > +MAP_STATIC_SYMBOL(int rte_meter_trtcm_rfc4115_profile_config(struct rte_meter_trtcm_rfc4115_profile *p, > + struct rte_meter_trtcm_rfc4115_params *params), rte_meter_trtcm_rfc4115_profile_config_s); > + > +int > +rte_meter_trtcm_rfc4115_profile_config_e( > + struct rte_meter_trtcm_rfc4115_profile *p, > + struct rte_meter_trtcm_rfc4115_params *params); > +int > +rte_meter_trtcm_rfc4115_profile_config_e( > + struct rte_meter_trtcm_rfc4115_profile *p, > + struct rte_meter_trtcm_rfc4115_params *params) > +{ > + return rte_meter_trtcm_rfc4115_profile_config_(p, params); > +} > +VERSION_SYMBOL_EXPERIMENTAL(rte_meter_trtcm_rfc4115_profile_config, _e); > + > +static int > +rte_meter_trtcm_rfc4115_config_( > struct rte_meter_trtcm_rfc4115 *m, > struct rte_meter_trtcm_rfc4115_profile *p) > { > @@ -160,3 +189,27 @@ rte_meter_trtcm_rfc4115_config( > > return 0; > } > + > +int > +rte_meter_trtcm_rfc4115_config_s(struct rte_meter_trtcm_rfc4115 *m, > + struct rte_meter_trtcm_rfc4115_profile *p); > +int > +rte_meter_trtcm_rfc4115_config_s(struct rte_meter_trtcm_rfc4115 *m, > + struct rte_meter_trtcm_rfc4115_profile *p) > +{ > + return rte_meter_trtcm_rfc4115_config_(m, p); > +} > +BIND_DEFAULT_SYMBOL(rte_meter_trtcm_rfc4115_config, _s, 20.0.1); > +MAP_STATIC_SYMBOL(int rte_meter_trtcm_rfc4115_config(struct rte_meter_trtcm_rfc4115 *m, > + struct rte_meter_trtcm_rfc4115_profile *p), rte_meter_trtcm_rfc4115_config_s); > + > +int > +rte_meter_trtcm_rfc4115_config_e(struct rte_meter_trtcm_rfc4115 *m, > + struct rte_meter_trtcm_rfc4115_profile *p); > +int > +rte_meter_trtcm_rfc4115_config_e(struct rte_meter_trtcm_rfc4115 *m, > + struct rte_meter_trtcm_rfc4115_profile *p) > +{ > + return rte_meter_trtcm_rfc4115_config_(m, p); > +} > +VERSION_SYMBOL_EXPERIMENTAL(rte_meter_trtcm_rfc4115_config, _e); > diff --git a/lib/librte_meter/rte_meter_version.map b/lib/librte_meter/rte_meter_version.map > index fc12cc0bf..b4b043174 100644 > --- a/lib/librte_meter/rte_meter_version.map > +++ b/lib/librte_meter/rte_meter_version.map > @@ -20,4 +20,12 @@ DPDK_20.0.1 { > rte_meter_trtcm_rfc4115_color_blind_check; > rte_meter_trtcm_rfc4115_config; > rte_meter_trtcm_rfc4115_profile_config; > + > } DPDK_20.0; > + > +EXPERIMENTAL { > + global: > + > + rte_meter_trtcm_rfc4115_config; > + rte_meter_trtcm_rfc4115_profile_config; > +}; > diff --git a/lib/meson.build b/lib/meson.build > index 0af3efab2..553964831 100644 > --- a/lib/meson.build > +++ b/lib/meson.build > @@ -48,7 +48,7 @@ foreach l:libraries > reason = '' # set if build == false to explain why > name = l > allow_experimental_apis = false > - use_function_versioning = false > + use_function_versioning = true > sources = [] > headers = [] > includes = [] > -- > 2.24.1 > >