[RFC] meter: fix ABI break due to experimental tag removal
Checks
Commit Message
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")
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Luca Boccassi <bluca@debian.org>
Cc: David Marchand <david.marchand@redhat.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Ian Stokes <ian.stokes@intel.com>
Cc: Eelco Chaudron <echaudro@redhat.com>
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(-)
Comments
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.
>
Because when enabled everything in the library must be built twice - once
for static lib and differently for a dynamic lib. Therefore unless a
library actually needs versioned symbols, we only build everything once to
save on build time.
/Bruce
On 1/29/20 1:29 PM, Ferruh Yigit wrote:
[...]
> Updated meson build system to allow the versioning,
> 'use_function_versioning = true', not sure why it was disabled by
> default.
AFAIR this is to save build time with meson. By default static build is
made and from the objects from the static build shared library is
constructed. This works unless function versioning is used - because
for the static build "attribute alias" is used and for shared build
.symver is used. So 'use_function_versioning' by default is false and
only libraries that actually use them have it set to true (and perform
separate shared build - see lib/meson.build).
I've now noticed that LPM, Distributor and Timer libraries no longer use
function versioning but have it still set in their meson.build.
With regards
Andrzej Ostruszka
On 1/29/2020 2:57 PM, Andrzej Ostruszka wrote:
> On 1/29/20 1:29 PM, Ferruh Yigit wrote:
> [...]
>> Updated meson build system to allow the versioning,
>> 'use_function_versioning = true', not sure why it was disabled by
>> default.
>
> AFAIR this is to save build time with meson. By default static build is
> made and from the objects from the static build shared library is
> constructed. This works unless function versioning is used - because
> for the static build "attribute alias" is used and for shared build
> .symver is used. So 'use_function_versioning' by default is false and
> only libraries that actually use them have it set to true (and perform
> separate shared build - see lib/meson.build).
Hi Andrzej,
Thanks for clarification, I will reduce it to the library.
>
> I've now noticed that LPM, Distributor and Timer libraries no longer use
> function versioning but have it still set in their meson.build.
Right, I can remove them.
>
> With regards
> Andrzej Ostruszka
>
On 1/29/20 5:25 PM, Ferruh Yigit wrote:
> On 1/29/2020 2:57 PM, Andrzej Ostruszka wrote:
[...]
>> I've now noticed that LPM, Distributor and Timer libraries no longer use
>> function versioning but have it still set in their meson.build.
>
> Right, I can remove them.
Bruce was kind enough to give me a nudge for that :).
If you decide to do it in one patch let me know so that I can drop my
patch in patchwork.
With regards
Andrzej Ostruszka
On 1/29/2020 4:30 PM, Andrzej Ostruszka wrote:
> On 1/29/20 5:25 PM, Ferruh Yigit wrote:
>> On 1/29/2020 2:57 PM, Andrzej Ostruszka wrote:
> [...]
>>> I've now noticed that LPM, Distributor and Timer libraries no longer use
>>> function versioning but have it still set in their meson.build.
>>
>> Right, I can remove them.
>
> Bruce was kind enough to give me a nudge for that :).
> If you decide to do it in one patch let me know so that I can drop my
> patch in patchwork.
Separate patch makes more sense, thanks for the patch.
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 <ferruh.yigit@intel.com>
> ---
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Luca Boccassi <bluca@debian.org>
> Cc: David Marchand <david.marchand@redhat.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Ian Stokes <ian.stokes@intel.com>
> Cc: Eelco Chaudron <echaudro@redhat.com>
>
> 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 <rte_common.h>
> #include <rte_log.h>
> #include <rte_cycles.h>
> +#include <rte_function_versioning.h>
>
> #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 = '<unknown 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
>
>
On 2/2/2020 6:53 PM, Neil Horman wrote:
> 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?
Yes, exactly.
>
> 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.
This will mean, even the the API was perfect it will force its users to
recompile (and re-package, re-deploy etc..) at least once, this is feeling like
we are still breaking user applications easily.
And this may create a stronger motivation by applications not to use
experimental APIs, I can't decide if this is a good thing or bad thing.
The issue is amplified in the LTS,
If we remove experimental tag in LTS, will be breaking the apps using that
experimental API, just to mature the API.
If we keep it as experimental, the API will be mature in main repo, but the LTS
has to keep exact same API as experimental up to two years.
But if we can do the versioning in the master, LTS can backport it and can have
mature version of that API in LTS without breaking the existing users.
>
> A few more nits in line
>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Neil Horman <nhorman@tuxdriver.com>
>> Cc: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Luca Boccassi <bluca@debian.org>
>> Cc: David Marchand <david.marchand@redhat.com>
>> Cc: Bruce Richardson <bruce.richardson@intel.com>
>> Cc: Ian Stokes <ian.stokes@intel.com>
>> Cc: Eelco Chaudron <echaudro@redhat.com>
>>
>> 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)
+1
>
>
>> /*
>> * 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 <rte_common.h>
>> #include <rte_log.h>
>> #include <rte_cycles.h>
>> +#include <rte_function_versioning.h>
>>
>> #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 = '<unknown 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
>>
>>
On Mon, Feb 03, 2020 at 12:53:28PM +0000, Ferruh Yigit wrote:
> On 2/2/2020 6:53 PM, Neil Horman wrote:
> > 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?
>
> Yes, exactly.
>
Ok, so we're on the same page, good.
> >
> > 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.
>
> This will mean, even the the API was perfect it will force its users to
> recompile (and re-package, re-deploy etc..) at least once, this is feeling like
Yes, thats correct.
> we are still breaking user applications easily.
we reserve that right, based on the fact that we marked these symbols as
experimental. Thats the deal we provide. Stable (non experimental) API calls
are guaranteed not to change for the ABI lifetime, and its our responsibility to
maintain that. However, experimental API's are marked as such because we can
change them (even if that change is to remove it and replace it with an
identical stable version), and application developers assume that risk by using
them. I understand the removal of an experimental symbol, replacing it with a
stable identical version is a trivial case of that change, and if we can ease
the burden, thats fine, but I don't think we need to go out of our way, or incur
any additional burden to ease that transition. We created the experimental
symbol marking to ease the burden of this sort of development on us, we should
be willing to use it.
> And this may create a stronger motivation by applications not to use
> experimental APIs, I can't decide if this is a good thing or bad thing.
>
Yeah, for stable, long term support applications, that makes sense, they
shouldn't be using experimental API's when they need the guarantee of support,
or they should be understanding of their responsibility to rebuild, when we
change the API's in both complex and trivial cases like this.
>
> The issue is amplified in the LTS,
> If we remove experimental tag in LTS, will be breaking the apps using that
> experimental API, just to mature the API.
You shouldn't be making this sort of transition in LTS. Just leave the
experimental symbol as is there, and commit the transition for the next LTS
release.
> If we keep it as experimental, the API will be mature in main repo, but the LTS
> has to keep exact same API as experimental up to two years.
Yes, thats the meaning of LTS. Things stay stable for its lifetime. And if the
only thing you are doing is replacing the experimental ABI version with a stable
version (leaving the details of the ABI unchanged), then its a moot point. The
only difference will be that the LTS release will have symbols marked as
experimental.
>
> But if we can do the versioning in the master, LTS can backport it and can have
> mature version of that API in LTS without breaking the existing users.
>
But why bother? The only thing you've changed is the version tagging. Its ok
to leave that alone in LTS, you just cant change it.
Thats part of the burden of an LTS release, it will have some drift from the
upstream head, because you have to keep things stable. So you stabilize the
upstream ABI version for this API and just never backport it to the current LTS
release.
> >
> > A few more nits in line
> >
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >> Cc: Neil Horman <nhorman@tuxdriver.com>
> >> Cc: Thomas Monjalon <thomas@monjalon.net>
> >> Cc: Luca Boccassi <bluca@debian.org>
> >> Cc: David Marchand <david.marchand@redhat.com>
> >> Cc: Bruce Richardson <bruce.richardson@intel.com>
> >> Cc: Ian Stokes <ian.stokes@intel.com>
> >> Cc: Eelco Chaudron <echaudro@redhat.com>
> >>
> >> 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)
>
> +1
>
> >
> >
> >> /*
> >> * 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 <rte_common.h>
> >> #include <rte_log.h>
> >> #include <rte_cycles.h>
> >> +#include <rte_function_versioning.h>
> >>
> >> #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 = '<unknown 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
> >>
> >>
>
>
On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > But if we can do the versioning in the master, LTS can backport it
> > and can have
> > mature version of that API in LTS without breaking the existing
> > users.
> >
>
> But why bother? The only thing you've changed is the version
> tagging. Its ok
> to leave that alone in LTS, you just cant change it.
>
> Thats part of the burden of an LTS release, it will have some drift
> from the
> upstream head, because you have to keep things stable. So you
> stabilize the
> upstream ABI version for this API and just never backport it to the
> current LTS
> release.
Hi,
A customer (OVS) explicitly and specifically requested backporting the
symbol change to 19.11, as they don't want to enable experimental APIs
in their releases. I replied that it would only be acceptable with
aliasing to keep compatibility, and Ferruh very kindly did the work to
implement that.
On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > > But if we can do the versioning in the master, LTS can backport it
> > > and can have
> > > mature version of that API in LTS without breaking the existing
> > > users.
> > >
> >
> > But why bother? The only thing you've changed is the version
> > tagging. Its ok
> > to leave that alone in LTS, you just cant change it.
> >
> > Thats part of the burden of an LTS release, it will have some drift
> > from the
> > upstream head, because you have to keep things stable. So you
> > stabilize the
> > upstream ABI version for this API and just never backport it to the
> > current LTS
> > release.
>
> Hi,
>
> A customer (OVS) explicitly and specifically requested backporting the
> symbol change to 19.11, as they don't want to enable experimental APIs
> in their releases. I replied that it would only be acceptable with
> aliasing to keep compatibility, and Ferruh very kindly did the work to
> implement that.
>
but, thats part of the agreement, no? You can't always have new features and
stability at the same time.
I get that this is an odd corner case, because strictly speaking you could waive
the ABI change that libabigail is reporting, and most application users (like
OVS) could get away with it, because their application does all the right things
to make it ok, but I don't think you can make a decsion like this for all users
based on the request of a single user.
It seems like the right thing is for OVS to augment their build time
configuration to allow developers to select the ability to use experimental apis
at compile time, and then the decision is placed in the hands of end users.
Neil
> --
> Kind regards,
> Luca Boccassi
>
On 05/02/2020 11:32, Neil Horman wrote:
> On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
>> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
>>>> But if we can do the versioning in the master, LTS can backport it
>>>> and can have
>>>> mature version of that API in LTS without breaking the existing
>>>> users.
>>>>
>>>
>>> But why bother? The only thing you've changed is the version
>>> tagging. Its ok
>>> to leave that alone in LTS, you just cant change it.
>>>
>>> Thats part of the burden of an LTS release, it will have some drift
>>> from the
>>> upstream head, because you have to keep things stable. So you
>>> stabilize the
>>> upstream ABI version for this API and just never backport it to the
>>> current LTS
>>> release.
>>
>> Hi,
>>
>> A customer (OVS) explicitly and specifically requested backporting the
>> symbol change to 19.11, as they don't want to enable experimental APIs
>> in their releases. I replied that it would only be acceptable with
>> aliasing to keep compatibility, and Ferruh very kindly did the work to
>> implement that.
>>
> but, thats part of the agreement, no? You can't always have new features and
> stability at the same time.
>
> I get that this is an odd corner case, because strictly speaking you could waive
> the ABI change that libabigail is reporting, and most application users (like
> OVS) could get away with it, because their application does all the right things
> to make it ok, but I don't think you can make a decsion like this for all users
> based on the request of a single user.
>
> It seems like the right thing is for OVS to augment their build time
> configuration to allow developers to select the ability to use experimental apis
> at compile time, and then the decision is placed in the hands of end users.
So this is not isolated case right ... it is common from API's to graduate
from experimental to mature, we _want_ that to happen, we _want_ APIs to mature.
I am worried what you are proposing will encourage a behavior whereby maintainers
to wait until the declaration of the next major ABI version to make these symbol changes,
causing these kinds of changes to queue up for that release.
And you would have to ask why?
In the case of a reasonably mature API, there will be no functional or signature change - its mature!
So what is the harm of providing an alias back to Experimental until the next ABI version is declared?
So while yes, you are 100% right - experimental mean no guarantees.
But if the API is baked, it is not going to change, so I don't see the harm.
We don't want the unnecessary triumph of policy over pragmatism.
On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
>
>
> On 05/02/2020 11:32, Neil Horman wrote:
> > On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> >> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> >>>> But if we can do the versioning in the master, LTS can backport it
> >>>> and can have
> >>>> mature version of that API in LTS without breaking the existing
> >>>> users.
> >>>>
> >>>
> >>> But why bother? The only thing you've changed is the version
> >>> tagging. Its ok
> >>> to leave that alone in LTS, you just cant change it.
> >>>
> >>> Thats part of the burden of an LTS release, it will have some drift
> >>> from the
> >>> upstream head, because you have to keep things stable. So you
> >>> stabilize the
> >>> upstream ABI version for this API and just never backport it to the
> >>> current LTS
> >>> release.
> >>
> >> Hi,
> >>
> >> A customer (OVS) explicitly and specifically requested backporting the
> >> symbol change to 19.11, as they don't want to enable experimental APIs
> >> in their releases. I replied that it would only be acceptable with
> >> aliasing to keep compatibility, and Ferruh very kindly did the work to
> >> implement that.
> >>
> > but, thats part of the agreement, no? You can't always have new features and
> > stability at the same time.
> >
> > I get that this is an odd corner case, because strictly speaking you could waive
> > the ABI change that libabigail is reporting, and most application users (like
> > OVS) could get away with it, because their application does all the right things
> > to make it ok, but I don't think you can make a decsion like this for all users
> > based on the request of a single user.
> >
> > It seems like the right thing is for OVS to augment their build time
> > configuration to allow developers to select the ability to use experimental apis
> > at compile time, and then the decision is placed in the hands of end users.
>
> So this is not isolated case right ... it is common from API's to graduate
> from experimental to mature, we _want_ that to happen, we _want_ APIs to mature.
>
Sure, I can absolutely agree with that, though I would suggest that
the maturity of the ABI is orthogonal to its labeling as such (more on
that below)
> I am worried what you are proposing will encourage a behavior whereby maintainers
> to wait until the declaration of the next major ABI version to make these symbol changes,
> causing these kinds of changes to queue up for that release.
>
I think you're probably right about that, and would make the agrument
that thats perfectly fine (again I'll clarify below)
> And you would have to ask why?
> In the case of a reasonably mature API, there will be no functional or signature change - its mature!
> So what is the harm of providing an alias back to Experimental until the next ABI version is declared?
>
From a philosophical standpoint, there is absoluely no harm, and I don't
think anyone would disagree, the harm comes from the details of the
implementation, as you've noted.
> So while yes, you are 100% right - experimental mean no guarantees.
> But if the API is baked, it is not going to change, so I don't see the harm.
>
> We don't want the unnecessary triumph of policy over pragmatism.
>
I would make the converse argument here. While I agree that when an API
is mature, theres no point in calling it experimental anymore, I would
also suggest that, if an API is mature, and not expected to change,
theres no harm in leaving its version tag as experimental for an LTS
release. This is what I was referring to above. Once an application
developer has done the work to integrate an API from DPDK into its
application, that work is done. If the API remains stable, then I
honestly don't know that they care that the version label is
EXPERIMENTAL versus 20.11 or 20.05 or whatever. They care about the API
being stable more so than its version label. As such, it
seems....reasonable to me to have developers queue their migration of
experimental APIs to official versioned APIs at the next major release
deliniation point.
I'd welcome counter arguments, but it seems pretty natural to me to make
these sorts of changes at the major release mark. People using
experimantal apis have already implicity agreed to manage changes to
them, so if we just hold it stable in an LTS release (and don't update
the version label), its just gravy for them.
Regards
Neil
>
>
>
>
>
>
On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
> On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
> >
> >
> > On 05/02/2020 11:32, Neil Horman wrote:
> > > On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> > >> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > >>>> But if we can do the versioning in the master, LTS can backport it
> > >>>> and can have mature version of that API in LTS without breaking
> > >>>> the existing users.
> > >>>>
> > >>>
> > >>> But why bother? The only thing you've changed is the version
> > >>> tagging. Its ok to leave that alone in LTS, you just cant change
> > >>> it.
> > >>>
> > >>> Thats part of the burden of an LTS release, it will have some drift
> > >>> from the upstream head, because you have to keep things stable. So
> > >>> you stabilize the upstream ABI version for this API and just never
> > >>> backport it to the current LTS release.
> > >>
> > >> Hi,
> > >>
> > >> A customer (OVS) explicitly and specifically requested backporting
> > >> the symbol change to 19.11, as they don't want to enable
> > >> experimental APIs in their releases. I replied that it would only be
> > >> acceptable with aliasing to keep compatibility, and Ferruh very
> > >> kindly did the work to implement that.
> > >>
> > > but, thats part of the agreement, no? You can't always have new
> > > features and stability at the same time.
> > >
> > > I get that this is an odd corner case, because strictly speaking you
> > > could waive the ABI change that libabigail is reporting, and most
> > > application users (like OVS) could get away with it, because their
> > > application does all the right things to make it ok, but I don't
> > > think you can make a decsion like this for all users based on the
> > > request of a single user.
> > >
> > > It seems like the right thing is for OVS to augment their build time
> > > configuration to allow developers to select the ability to use
> > > experimental apis at compile time, and then the decision is placed in
> > > the hands of end users.
> >
> > So this is not isolated case right ... it is common from API's to
> > graduate from experimental to mature, we _want_ that to happen, we
> > _want_ APIs to mature.
> >
> Sure, I can absolutely agree with that, though I would suggest that the
> maturity of the ABI is orthogonal to its labeling as such (more on that
> below)
>
> > I am worried what you are proposing will encourage a behavior whereby
> > maintainers to wait until the declaration of the next major ABI version
> > to make these symbol changes, causing these kinds of changes to queue
> > up for that release.
> >
> I think you're probably right about that, and would make the agrument
> that thats perfectly fine (again I'll clarify below)
>
> > And you would have to ask why? In the case of a reasonably mature API,
> > there will be no functional or signature change - its mature! So what
> > is the harm of providing an alias back to Experimental until the next
> > ABI version is declared?
> >
> From a philosophical standpoint, there is absoluely no harm, and I don't
> think anyone would disagree, the harm comes from the details of the
> implementation, as you've noted.
>
> > So while yes, you are 100% right - experimental mean no guarantees.
> > But if the API is baked, it is not going to change, so I don't see the
> > harm.
> >
> > We don't want the unnecessary triumph of policy over pragmatism.
> >
> I would make the converse argument here. While I agree that when an API
> is mature, theres no point in calling it experimental anymore, I would
> also suggest that, if an API is mature, and not expected to change,
> theres no harm in leaving its version tag as experimental for an LTS
> release. This is what I was referring to above. Once an application
> developer has done the work to integrate an API from DPDK into its
> application, that work is done. If the API remains stable, then I
> honestly don't know that they care that the version label is EXPERIMENTAL
> versus 20.11 or 20.05 or whatever. They care about the API being stable
> more so than its version label. As such, it seems....reasonable to me to
> have developers queue their migration of experimental APIs to official
> versioned APIs at the next major release deliniation point.
>
> I'd welcome counter arguments, but it seems pretty natural to me to make
> these sorts of changes at the major release mark. People using
> experimantal apis have already implicity agreed to manage changes to
> them, so if we just hold it stable in an LTS release (and don't update
> the version label), its just gravy for them.
>
The counter argument that I see is that while the experimental tag remains
in place the user has no way to know that an API is stable or not, and so
in many projects cannot use the API. If for example an API that is
experimental in 19.11 is unchanged through 20.05 at which point we decide
to promote it to stable. Once the change to the exp tag it is made, any
users of 19.11 can then see that it is an unchanged and now-stable ABI and
can start using it in their software, if they wish, without having to wait
for the 20.11 release. Changing the tag early allows ABIs to be potentially
used immediately.
Regards,
/Bruce
On Fri, Feb 14, 2020 at 11:36:34AM +0000, Bruce Richardson wrote:
> On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
> > On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
> > >
> > >
> > > On 05/02/2020 11:32, Neil Horman wrote:
> > > > On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> > > >> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > > >>>> But if we can do the versioning in the master, LTS can backport it
> > > >>>> and can have mature version of that API in LTS without breaking
> > > >>>> the existing users.
> > > >>>>
> > > >>>
> > > >>> But why bother? The only thing you've changed is the version
> > > >>> tagging. Its ok to leave that alone in LTS, you just cant change
> > > >>> it.
> > > >>>
> > > >>> Thats part of the burden of an LTS release, it will have some drift
> > > >>> from the upstream head, because you have to keep things stable. So
> > > >>> you stabilize the upstream ABI version for this API and just never
> > > >>> backport it to the current LTS release.
> > > >>
> > > >> Hi,
> > > >>
> > > >> A customer (OVS) explicitly and specifically requested backporting
> > > >> the symbol change to 19.11, as they don't want to enable
> > > >> experimental APIs in their releases. I replied that it would only be
> > > >> acceptable with aliasing to keep compatibility, and Ferruh very
> > > >> kindly did the work to implement that.
> > > >>
> > > > but, thats part of the agreement, no? You can't always have new
> > > > features and stability at the same time.
> > > >
> > > > I get that this is an odd corner case, because strictly speaking you
> > > > could waive the ABI change that libabigail is reporting, and most
> > > > application users (like OVS) could get away with it, because their
> > > > application does all the right things to make it ok, but I don't
> > > > think you can make a decsion like this for all users based on the
> > > > request of a single user.
> > > >
> > > > It seems like the right thing is for OVS to augment their build time
> > > > configuration to allow developers to select the ability to use
> > > > experimental apis at compile time, and then the decision is placed in
> > > > the hands of end users.
> > >
> > > So this is not isolated case right ... it is common from API's to
> > > graduate from experimental to mature, we _want_ that to happen, we
> > > _want_ APIs to mature.
> > >
> > Sure, I can absolutely agree with that, though I would suggest that the
> > maturity of the ABI is orthogonal to its labeling as such (more on that
> > below)
> >
> > > I am worried what you are proposing will encourage a behavior whereby
> > > maintainers to wait until the declaration of the next major ABI version
> > > to make these symbol changes, causing these kinds of changes to queue
> > > up for that release.
> > >
> > I think you're probably right about that, and would make the agrument
> > that thats perfectly fine (again I'll clarify below)
> >
> > > And you would have to ask why? In the case of a reasonably mature API,
> > > there will be no functional or signature change - its mature! So what
> > > is the harm of providing an alias back to Experimental until the next
> > > ABI version is declared?
> > >
> > From a philosophical standpoint, there is absoluely no harm, and I don't
> > think anyone would disagree, the harm comes from the details of the
> > implementation, as you've noted.
> >
> > > So while yes, you are 100% right - experimental mean no guarantees.
> > > But if the API is baked, it is not going to change, so I don't see the
> > > harm.
> > >
> > > We don't want the unnecessary triumph of policy over pragmatism.
> > >
> > I would make the converse argument here. While I agree that when an API
> > is mature, theres no point in calling it experimental anymore, I would
> > also suggest that, if an API is mature, and not expected to change,
> > theres no harm in leaving its version tag as experimental for an LTS
> > release. This is what I was referring to above. Once an application
> > developer has done the work to integrate an API from DPDK into its
> > application, that work is done. If the API remains stable, then I
> > honestly don't know that they care that the version label is EXPERIMENTAL
> > versus 20.11 or 20.05 or whatever. They care about the API being stable
> > more so than its version label. As such, it seems....reasonable to me to
> > have developers queue their migration of experimental APIs to official
> > versioned APIs at the next major release deliniation point.
> >
> > I'd welcome counter arguments, but it seems pretty natural to me to make
> > these sorts of changes at the major release mark. People using
> > experimantal apis have already implicity agreed to manage changes to
> > them, so if we just hold it stable in an LTS release (and don't update
> > the version label), its just gravy for them.
> >
> The counter argument that I see is that while the experimental tag remains
> in place the user has no way to know that an API is stable or not, and so
> in many projects cannot use the API. If for example an API that is
> experimental in 19.11 is unchanged through 20.05 at which point we decide
> to promote it to stable. Once the change to the exp tag it is made, any
> users of 19.11 can then see that it is an unchanged and now-stable ABI and
> can start using it in their software, if they wish, without having to wait
> for the 20.11 release. Changing the tag early allows ABIs to be potentially
> used immediately.
>
I would agree with this, however, when using an LTS release, in my mind at
least, part of the agreement there is you get stability in the fuctions that
were declared stable at the time of release. I'm not sure there should be an
expectation of additional stabilization within the lifetime of the release
(thats really what the 12 month LTS release cycle is for, no)? You never have
to wait more than a year for a new set of stable features. If you want faster
feature integration than that, you can choose to enable the experimental API's
and accept the benefits and drawbacks thereof.
That said, if (as I understand it) the goal is to provide a mechanism to allow
experimental features to be promoted to stable status, perhaps we can find a
happy middle ground. What if we were to create a construct such as this:
#pragma push_macro("ALLOW_EXPERIMENTAL_APIS")
#undef ALLOW_EXPERIMENTAL_APIS
void __rte_experimental func(...);
#pragma pop_macro("ALLOW_EXPERIMENTAL_APIS")
Such a consruct would allow the maintainer of an API to pseudo-promote an API
from a experimental to stable status, at least so far as compilation would be
concerned. Its messy and clunky, and it wouldn't change the function version at
all, but the end result would be that:
a) such a wraped experimental function would no longer issue a warning when used
during compilation/linking
and
b) provide a semi-easy grepable pattern for application writers to look for when
considering the use of an API that was previously experimental
such a construct would have to be used very judiciously, in that once its
implemented, the API has to be treated as stable, even though its 'excused' from
the normal checking, but it could provide something of the more rapid promotion
path being sought.
Thoughts?
Neil
> Regards,
> /Bruce
>
On Fri, Feb 14, 2020 at 03:48:48PM -0500, Neil Horman wrote:
> On Fri, Feb 14, 2020 at 11:36:34AM +0000, Bruce Richardson wrote:
> > On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
> > > On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
> > > >
> > > >
> > > > On 05/02/2020 11:32, Neil Horman wrote:
> > > > > On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> > > > >> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > > > >>>> But if we can do the versioning in the master, LTS can backport it
> > > > >>>> and can have mature version of that API in LTS without breaking
> > > > >>>> the existing users.
> > > > >>>>
> > > > >>>
> > > > >>> But why bother? The only thing you've changed is the version
> > > > >>> tagging. Its ok to leave that alone in LTS, you just cant change
> > > > >>> it.
> > > > >>>
> > > > >>> Thats part of the burden of an LTS release, it will have some drift
> > > > >>> from the upstream head, because you have to keep things stable. So
> > > > >>> you stabilize the upstream ABI version for this API and just never
> > > > >>> backport it to the current LTS release.
> > > > >>
> > > > >> Hi,
> > > > >>
> > > > >> A customer (OVS) explicitly and specifically requested backporting
> > > > >> the symbol change to 19.11, as they don't want to enable
> > > > >> experimental APIs in their releases. I replied that it would only be
> > > > >> acceptable with aliasing to keep compatibility, and Ferruh very
> > > > >> kindly did the work to implement that.
> > > > >>
> > > > > but, thats part of the agreement, no? You can't always have new
> > > > > features and stability at the same time.
> > > > >
> > > > > I get that this is an odd corner case, because strictly speaking you
> > > > > could waive the ABI change that libabigail is reporting, and most
> > > > > application users (like OVS) could get away with it, because their
> > > > > application does all the right things to make it ok, but I don't
> > > > > think you can make a decsion like this for all users based on the
> > > > > request of a single user.
> > > > >
> > > > > It seems like the right thing is for OVS to augment their build time
> > > > > configuration to allow developers to select the ability to use
> > > > > experimental apis at compile time, and then the decision is placed in
> > > > > the hands of end users.
> > > >
> > > > So this is not isolated case right ... it is common from API's to
> > > > graduate from experimental to mature, we _want_ that to happen, we
> > > > _want_ APIs to mature.
> > > >
> > > Sure, I can absolutely agree with that, though I would suggest that the
> > > maturity of the ABI is orthogonal to its labeling as such (more on that
> > > below)
> > >
> > > > I am worried what you are proposing will encourage a behavior whereby
> > > > maintainers to wait until the declaration of the next major ABI version
> > > > to make these symbol changes, causing these kinds of changes to queue
> > > > up for that release.
> > > >
> > > I think you're probably right about that, and would make the agrument
> > > that thats perfectly fine (again I'll clarify below)
> > >
> > > > And you would have to ask why? In the case of a reasonably mature API,
> > > > there will be no functional or signature change - its mature! So what
> > > > is the harm of providing an alias back to Experimental until the next
> > > > ABI version is declared?
> > > >
> > > From a philosophical standpoint, there is absoluely no harm, and I don't
> > > think anyone would disagree, the harm comes from the details of the
> > > implementation, as you've noted.
> > >
> > > > So while yes, you are 100% right - experimental mean no guarantees.
> > > > But if the API is baked, it is not going to change, so I don't see the
> > > > harm.
> > > >
> > > > We don't want the unnecessary triumph of policy over pragmatism.
> > > >
> > > I would make the converse argument here. While I agree that when an API
> > > is mature, theres no point in calling it experimental anymore, I would
> > > also suggest that, if an API is mature, and not expected to change,
> > > theres no harm in leaving its version tag as experimental for an LTS
> > > release. This is what I was referring to above. Once an application
> > > developer has done the work to integrate an API from DPDK into its
> > > application, that work is done. If the API remains stable, then I
> > > honestly don't know that they care that the version label is EXPERIMENTAL
> > > versus 20.11 or 20.05 or whatever. They care about the API being stable
> > > more so than its version label. As such, it seems....reasonable to me to
> > > have developers queue their migration of experimental APIs to official
> > > versioned APIs at the next major release deliniation point.
> > >
> > > I'd welcome counter arguments, but it seems pretty natural to me to make
> > > these sorts of changes at the major release mark. People using
> > > experimantal apis have already implicity agreed to manage changes to
> > > them, so if we just hold it stable in an LTS release (and don't update
> > > the version label), its just gravy for them.
> > >
> > The counter argument that I see is that while the experimental tag remains
> > in place the user has no way to know that an API is stable or not, and so
> > in many projects cannot use the API. If for example an API that is
> > experimental in 19.11 is unchanged through 20.05 at which point we decide
> > to promote it to stable. Once the change to the exp tag it is made, any
> > users of 19.11 can then see that it is an unchanged and now-stable ABI and
> > can start using it in their software, if they wish, without having to wait
> > for the 20.11 release. Changing the tag early allows ABIs to be potentially
> > used immediately.
> >
>
> I would agree with this, however, when using an LTS release, in my mind at
> least, part of the agreement there is you get stability in the fuctions that
> were declared stable at the time of release. I'm not sure there should be an
> expectation of additional stabilization within the lifetime of the release
> (thats really what the 12 month LTS release cycle is for, no)? You never have
> to wait more than a year for a new set of stable features. If you want faster
> feature integration than that, you can choose to enable the experimental API's
> and accept the benefits and drawbacks thereof.
>
> That said, if (as I understand it) the goal is to provide a mechanism to allow
> experimental features to be promoted to stable status, perhaps we can find a
> happy middle ground. What if we were to create a construct such as this:
>
> #pragma push_macro("ALLOW_EXPERIMENTAL_APIS")
> #undef ALLOW_EXPERIMENTAL_APIS
> void __rte_experimental func(...);
> #pragma pop_macro("ALLOW_EXPERIMENTAL_APIS")
>
> Such a consruct would allow the maintainer of an API to pseudo-promote an API
> from a experimental to stable status, at least so far as compilation would be
> concerned. Its messy and clunky, and it wouldn't change the function version at
> all, but the end result would be that:
> a) such a wraped experimental function would no longer issue a warning when used
> during compilation/linking
> and
> b) provide a semi-easy grepable pattern for application writers to look for when
> considering the use of an API that was previously experimental
>
> such a construct would have to be used very judiciously, in that once its
> implemented, the API has to be treated as stable, even though its 'excused' from
> the normal checking, but it could provide something of the more rapid promotion
> path being sought.
>
Sure it could work and does meet the end goal. I'm not really sure it's a
better solution than just promoting the function and putting in an alias
for it back to experimental, though.
/Bruce
On Fri, Feb 14, 2020 at 09:52:53PM +0000, Bruce Richardson wrote:
> On Fri, Feb 14, 2020 at 03:48:48PM -0500, Neil Horman wrote:
> > On Fri, Feb 14, 2020 at 11:36:34AM +0000, Bruce Richardson wrote:
> > > On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
> > > > On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
> > > > >
> > > > >
> > > > > On 05/02/2020 11:32, Neil Horman wrote:
> > > > > > On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> > > > > >> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > > > > >>>> But if we can do the versioning in the master, LTS can backport it
> > > > > >>>> and can have mature version of that API in LTS without breaking
> > > > > >>>> the existing users.
> > > > > >>>>
> > > > > >>>
> > > > > >>> But why bother? The only thing you've changed is the version
> > > > > >>> tagging. Its ok to leave that alone in LTS, you just cant change
> > > > > >>> it.
> > > > > >>>
> > > > > >>> Thats part of the burden of an LTS release, it will have some drift
> > > > > >>> from the upstream head, because you have to keep things stable. So
> > > > > >>> you stabilize the upstream ABI version for this API and just never
> > > > > >>> backport it to the current LTS release.
> > > > > >>
> > > > > >> Hi,
> > > > > >>
> > > > > >> A customer (OVS) explicitly and specifically requested backporting
> > > > > >> the symbol change to 19.11, as they don't want to enable
> > > > > >> experimental APIs in their releases. I replied that it would only be
> > > > > >> acceptable with aliasing to keep compatibility, and Ferruh very
> > > > > >> kindly did the work to implement that.
> > > > > >>
> > > > > > but, thats part of the agreement, no? You can't always have new
> > > > > > features and stability at the same time.
> > > > > >
> > > > > > I get that this is an odd corner case, because strictly speaking you
> > > > > > could waive the ABI change that libabigail is reporting, and most
> > > > > > application users (like OVS) could get away with it, because their
> > > > > > application does all the right things to make it ok, but I don't
> > > > > > think you can make a decsion like this for all users based on the
> > > > > > request of a single user.
> > > > > >
> > > > > > It seems like the right thing is for OVS to augment their build time
> > > > > > configuration to allow developers to select the ability to use
> > > > > > experimental apis at compile time, and then the decision is placed in
> > > > > > the hands of end users.
> > > > >
> > > > > So this is not isolated case right ... it is common from API's to
> > > > > graduate from experimental to mature, we _want_ that to happen, we
> > > > > _want_ APIs to mature.
> > > > >
> > > > Sure, I can absolutely agree with that, though I would suggest that the
> > > > maturity of the ABI is orthogonal to its labeling as such (more on that
> > > > below)
> > > >
> > > > > I am worried what you are proposing will encourage a behavior whereby
> > > > > maintainers to wait until the declaration of the next major ABI version
> > > > > to make these symbol changes, causing these kinds of changes to queue
> > > > > up for that release.
> > > > >
> > > > I think you're probably right about that, and would make the agrument
> > > > that thats perfectly fine (again I'll clarify below)
> > > >
> > > > > And you would have to ask why? In the case of a reasonably mature API,
> > > > > there will be no functional or signature change - its mature! So what
> > > > > is the harm of providing an alias back to Experimental until the next
> > > > > ABI version is declared?
> > > > >
> > > > From a philosophical standpoint, there is absoluely no harm, and I don't
> > > > think anyone would disagree, the harm comes from the details of the
> > > > implementation, as you've noted.
> > > >
> > > > > So while yes, you are 100% right - experimental mean no guarantees.
> > > > > But if the API is baked, it is not going to change, so I don't see the
> > > > > harm.
> > > > >
> > > > > We don't want the unnecessary triumph of policy over pragmatism.
> > > > >
> > > > I would make the converse argument here. While I agree that when an API
> > > > is mature, theres no point in calling it experimental anymore, I would
> > > > also suggest that, if an API is mature, and not expected to change,
> > > > theres no harm in leaving its version tag as experimental for an LTS
> > > > release. This is what I was referring to above. Once an application
> > > > developer has done the work to integrate an API from DPDK into its
> > > > application, that work is done. If the API remains stable, then I
> > > > honestly don't know that they care that the version label is EXPERIMENTAL
> > > > versus 20.11 or 20.05 or whatever. They care about the API being stable
> > > > more so than its version label. As such, it seems....reasonable to me to
> > > > have developers queue their migration of experimental APIs to official
> > > > versioned APIs at the next major release deliniation point.
> > > >
> > > > I'd welcome counter arguments, but it seems pretty natural to me to make
> > > > these sorts of changes at the major release mark. People using
> > > > experimantal apis have already implicity agreed to manage changes to
> > > > them, so if we just hold it stable in an LTS release (and don't update
> > > > the version label), its just gravy for them.
> > > >
> > > The counter argument that I see is that while the experimental tag remains
> > > in place the user has no way to know that an API is stable or not, and so
> > > in many projects cannot use the API. If for example an API that is
> > > experimental in 19.11 is unchanged through 20.05 at which point we decide
> > > to promote it to stable. Once the change to the exp tag it is made, any
> > > users of 19.11 can then see that it is an unchanged and now-stable ABI and
> > > can start using it in their software, if they wish, without having to wait
> > > for the 20.11 release. Changing the tag early allows ABIs to be potentially
> > > used immediately.
> > >
> >
> > I would agree with this, however, when using an LTS release, in my mind at
> > least, part of the agreement there is you get stability in the fuctions that
> > were declared stable at the time of release. I'm not sure there should be an
> > expectation of additional stabilization within the lifetime of the release
> > (thats really what the 12 month LTS release cycle is for, no)? You never have
> > to wait more than a year for a new set of stable features. If you want faster
> > feature integration than that, you can choose to enable the experimental API's
> > and accept the benefits and drawbacks thereof.
> >
> > That said, if (as I understand it) the goal is to provide a mechanism to allow
> > experimental features to be promoted to stable status, perhaps we can find a
> > happy middle ground. What if we were to create a construct such as this:
> >
> > #pragma push_macro("ALLOW_EXPERIMENTAL_APIS")
> > #undef ALLOW_EXPERIMENTAL_APIS
> > void __rte_experimental func(...);
> > #pragma pop_macro("ALLOW_EXPERIMENTAL_APIS")
> >
> > Such a consruct would allow the maintainer of an API to pseudo-promote an API
> > from a experimental to stable status, at least so far as compilation would be
> > concerned. Its messy and clunky, and it wouldn't change the function version at
> > all, but the end result would be that:
> > a) such a wraped experimental function would no longer issue a warning when used
> > during compilation/linking
> > and
> > b) provide a semi-easy grepable pattern for application writers to look for when
> > considering the use of an API that was previously experimental
> >
> > such a construct would have to be used very judiciously, in that once its
> > implemented, the API has to be treated as stable, even though its 'excused' from
> > the normal checking, but it could provide something of the more rapid promotion
> > path being sought.
> >
> Sure it could work and does meet the end goal. I'm not really sure it's a
> better solution than just promoting the function and putting in an alias
> for it back to experimental, though.
>
It probably isn't, I was just trying to think of alternative
implementations, that dont make life harder in keeping up with LTS
maintenence.
Neil
> /Bruce
>
On 14/02/2020 20:48, Neil Horman wrote:
> On Fri, Feb 14, 2020 at 11:36:34AM +0000, Bruce Richardson wrote:
>> On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
>>> On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
>>>>
>>>>
>>>> On 05/02/2020 11:32, Neil Horman wrote:
>>>>> On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
>>>>>> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
>>>>>>>> But if we can do the versioning in the master, LTS can backport it
>>>>>>>> and can have mature version of that API in LTS without breaking
>>>>>>>> the existing users.
>>>>>>>>
>>>>>>>
>>>>>>> But why bother? The only thing you've changed is the version
>>>>>>> tagging. Its ok to leave that alone in LTS, you just cant change
>>>>>>> it.
>>>>>>>
>>>>>>> Thats part of the burden of an LTS release, it will have some drift
>>>>>>> from the upstream head, because you have to keep things stable. So
>>>>>>> you stabilize the upstream ABI version for this API and just never
>>>>>>> backport it to the current LTS release.
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> A customer (OVS) explicitly and specifically requested backporting
>>>>>> the symbol change to 19.11, as they don't want to enable
>>>>>> experimental APIs in their releases. I replied that it would only be
>>>>>> acceptable with aliasing to keep compatibility, and Ferruh very
>>>>>> kindly did the work to implement that.
>>>>>>
>>>>> but, thats part of the agreement, no? You can't always have new
>>>>> features and stability at the same time.
>>>>>
>>>>> I get that this is an odd corner case, because strictly speaking you
>>>>> could waive the ABI change that libabigail is reporting, and most
>>>>> application users (like OVS) could get away with it, because their
>>>>> application does all the right things to make it ok, but I don't
>>>>> think you can make a decsion like this for all users based on the
>>>>> request of a single user.
>>>>>
>>>>> It seems like the right thing is for OVS to augment their build time
>>>>> configuration to allow developers to select the ability to use
>>>>> experimental apis at compile time, and then the decision is placed in
>>>>> the hands of end users.
>>>>
>>>> So this is not isolated case right ... it is common from API's to
>>>> graduate from experimental to mature, we _want_ that to happen, we
>>>> _want_ APIs to mature.
>>>>
>>> Sure, I can absolutely agree with that, though I would suggest that the
>>> maturity of the ABI is orthogonal to its labeling as such (more on that
>>> below)
>>>
>>>> I am worried what you are proposing will encourage a behavior whereby
>>>> maintainers to wait until the declaration of the next major ABI version
>>>> to make these symbol changes, causing these kinds of changes to queue
>>>> up for that release.
>>>>
>>> I think you're probably right about that, and would make the agrument
>>> that thats perfectly fine (again I'll clarify below)
>>>
>>>> And you would have to ask why? In the case of a reasonably mature API,
>>>> there will be no functional or signature change - its mature! So what
>>>> is the harm of providing an alias back to Experimental until the next
>>>> ABI version is declared?
>>>>
>>> From a philosophical standpoint, there is absoluely no harm, and I don't
>>> think anyone would disagree, the harm comes from the details of the
>>> implementation, as you've noted.
>>>
>>>> So while yes, you are 100% right - experimental mean no guarantees.
>>>> But if the API is baked, it is not going to change, so I don't see the
>>>> harm.
>>>>
>>>> We don't want the unnecessary triumph of policy over pragmatism.
>>>>
>>> I would make the converse argument here. While I agree that when an API
>>> is mature, theres no point in calling it experimental anymore, I would
>>> also suggest that, if an API is mature, and not expected to change,
>>> theres no harm in leaving its version tag as experimental for an LTS
>>> release. This is what I was referring to above. Once an application
>>> developer has done the work to integrate an API from DPDK into its
>>> application, that work is done. If the API remains stable, then I
>>> honestly don't know that they care that the version label is EXPERIMENTAL
>>> versus 20.11 or 20.05 or whatever. They care about the API being stable
>>> more so than its version label. As such, it seems....reasonable to me to
>>> have developers queue their migration of experimental APIs to official
>>> versioned APIs at the next major release deliniation point.
>>>
>>> I'd welcome counter arguments, but it seems pretty natural to me to make
>>> these sorts of changes at the major release mark. People using
>>> experimantal apis have already implicity agreed to manage changes to
>>> them, so if we just hold it stable in an LTS release (and don't update
>>> the version label), its just gravy for them.
>>>
>> The counter argument that I see is that while the experimental tag remains
>> in place the user has no way to know that an API is stable or not, and so
>> in many projects cannot use the API. If for example an API that is
>> experimental in 19.11 is unchanged through 20.05 at which point we decide
>> to promote it to stable. Once the change to the exp tag it is made, any
>> users of 19.11 can then see that it is an unchanged and now-stable ABI and
>> can start using it in their software, if they wish, without having to wait
>> for the 20.11 release. Changing the tag early allows ABIs to be potentially
>> used immediately.
>>
>
> I would agree with this, however, when using an LTS release, in my mind at
> least, part of the agreement there is you get stability in the fuctions that
> were declared stable at the time of release. I'm not sure there should be an
> expectation of additional stabilization within the lifetime of the release
> (thats really what the 12 month LTS release cycle is for, no)? You never have
> to wait more than a year for a new set of stable features. If you want faster
> feature integration than that, you can choose to enable the experimental API's
> and accept the benefits and drawbacks thereof.
>
> That said, if (as I understand it) the goal is to provide a mechanism to allow
> experimental features to be promoted to stable status, perhaps we can find a
> happy middle ground. What if we were to create a construct such as this:
>
> #pragma push_macro("ALLOW_EXPERIMENTAL_APIS")
> #undef ALLOW_EXPERIMENTAL_APIS
> void __rte_experimental func(...);
> #pragma pop_macro("ALLOW_EXPERIMENTAL_APIS")
>
> Such a consruct would allow the maintainer of an API to pseudo-promote an API
> from a experimental to stable status, at least so far as compilation would be
> concerned. Its messy and clunky, and it wouldn't change the function version at
> all, but the end result would be that:
> a) such a wraped experimental function would no longer issue a warning when used
> during compilation/linking
> and
> b) provide a semi-easy grepable pattern for application writers to look for when
> considering the use of an API that was previously experimental
>
> such a construct would have to be used very judiciously, in that once its
> implemented, the API has to be treated as stable, even though its 'excused' from
> the normal checking, but it could provide something of the more rapid promotion
> path being sought.
>
> Thoughts?
> Neil
>> Regards,
>> /Bruce
>>
Then method above seems clunky, and it is unclear to me if it solves the original problem.
It seems simpler to me to promote a symbol the next ABI version, and then provide an alias.
At the declaration of the next major ABI version (v21), we would then only need to check
for those symbols that are both EXPERIMENTAL and v21, to know which aliases need to be removed.
Thanks,
Ray K
On Mon, Feb 17, 2020 at 02:23:15PM +0000, Ray Kinsella wrote:
>
>
> On 14/02/2020 20:48, Neil Horman wrote:
> > On Fri, Feb 14, 2020 at 11:36:34AM +0000, Bruce Richardson wrote:
> >> On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
> >>> On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
> >>>>
> >>>>
> >>>> On 05/02/2020 11:32, Neil Horman wrote:
> >>>>> On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> >>>>>> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> >>>>>>>> But if we can do the versioning in the master, LTS can backport it
> >>>>>>>> and can have mature version of that API in LTS without breaking
> >>>>>>>> the existing users.
> >>>>>>>>
> >>>>>>>
> >>>>>>> But why bother? The only thing you've changed is the version
> >>>>>>> tagging. Its ok to leave that alone in LTS, you just cant change
> >>>>>>> it.
> >>>>>>>
> >>>>>>> Thats part of the burden of an LTS release, it will have some drift
> >>>>>>> from the upstream head, because you have to keep things stable. So
> >>>>>>> you stabilize the upstream ABI version for this API and just never
> >>>>>>> backport it to the current LTS release.
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> A customer (OVS) explicitly and specifically requested backporting
> >>>>>> the symbol change to 19.11, as they don't want to enable
> >>>>>> experimental APIs in their releases. I replied that it would only be
> >>>>>> acceptable with aliasing to keep compatibility, and Ferruh very
> >>>>>> kindly did the work to implement that.
> >>>>>>
> >>>>> but, thats part of the agreement, no? You can't always have new
> >>>>> features and stability at the same time.
> >>>>>
> >>>>> I get that this is an odd corner case, because strictly speaking you
> >>>>> could waive the ABI change that libabigail is reporting, and most
> >>>>> application users (like OVS) could get away with it, because their
> >>>>> application does all the right things to make it ok, but I don't
> >>>>> think you can make a decsion like this for all users based on the
> >>>>> request of a single user.
> >>>>>
> >>>>> It seems like the right thing is for OVS to augment their build time
> >>>>> configuration to allow developers to select the ability to use
> >>>>> experimental apis at compile time, and then the decision is placed in
> >>>>> the hands of end users.
> >>>>
> >>>> So this is not isolated case right ... it is common from API's to
> >>>> graduate from experimental to mature, we _want_ that to happen, we
> >>>> _want_ APIs to mature.
> >>>>
> >>> Sure, I can absolutely agree with that, though I would suggest that the
> >>> maturity of the ABI is orthogonal to its labeling as such (more on that
> >>> below)
> >>>
> >>>> I am worried what you are proposing will encourage a behavior whereby
> >>>> maintainers to wait until the declaration of the next major ABI version
> >>>> to make these symbol changes, causing these kinds of changes to queue
> >>>> up for that release.
> >>>>
> >>> I think you're probably right about that, and would make the agrument
> >>> that thats perfectly fine (again I'll clarify below)
> >>>
> >>>> And you would have to ask why? In the case of a reasonably mature API,
> >>>> there will be no functional or signature change - its mature! So what
> >>>> is the harm of providing an alias back to Experimental until the next
> >>>> ABI version is declared?
> >>>>
> >>> From a philosophical standpoint, there is absoluely no harm, and I don't
> >>> think anyone would disagree, the harm comes from the details of the
> >>> implementation, as you've noted.
> >>>
> >>>> So while yes, you are 100% right - experimental mean no guarantees.
> >>>> But if the API is baked, it is not going to change, so I don't see the
> >>>> harm.
> >>>>
> >>>> We don't want the unnecessary triumph of policy over pragmatism.
> >>>>
> >>> I would make the converse argument here. While I agree that when an API
> >>> is mature, theres no point in calling it experimental anymore, I would
> >>> also suggest that, if an API is mature, and not expected to change,
> >>> theres no harm in leaving its version tag as experimental for an LTS
> >>> release. This is what I was referring to above. Once an application
> >>> developer has done the work to integrate an API from DPDK into its
> >>> application, that work is done. If the API remains stable, then I
> >>> honestly don't know that they care that the version label is EXPERIMENTAL
> >>> versus 20.11 or 20.05 or whatever. They care about the API being stable
> >>> more so than its version label. As such, it seems....reasonable to me to
> >>> have developers queue their migration of experimental APIs to official
> >>> versioned APIs at the next major release deliniation point.
> >>>
> >>> I'd welcome counter arguments, but it seems pretty natural to me to make
> >>> these sorts of changes at the major release mark. People using
> >>> experimantal apis have already implicity agreed to manage changes to
> >>> them, so if we just hold it stable in an LTS release (and don't update
> >>> the version label), its just gravy for them.
> >>>
> >> The counter argument that I see is that while the experimental tag remains
> >> in place the user has no way to know that an API is stable or not, and so
> >> in many projects cannot use the API. If for example an API that is
> >> experimental in 19.11 is unchanged through 20.05 at which point we decide
> >> to promote it to stable. Once the change to the exp tag it is made, any
> >> users of 19.11 can then see that it is an unchanged and now-stable ABI and
> >> can start using it in their software, if they wish, without having to wait
> >> for the 20.11 release. Changing the tag early allows ABIs to be potentially
> >> used immediately.
> >>
> >
> > I would agree with this, however, when using an LTS release, in my mind at
> > least, part of the agreement there is you get stability in the fuctions that
> > were declared stable at the time of release. I'm not sure there should be an
> > expectation of additional stabilization within the lifetime of the release
> > (thats really what the 12 month LTS release cycle is for, no)? You never have
> > to wait more than a year for a new set of stable features. If you want faster
> > feature integration than that, you can choose to enable the experimental API's
> > and accept the benefits and drawbacks thereof.
> >
> > That said, if (as I understand it) the goal is to provide a mechanism to allow
> > experimental features to be promoted to stable status, perhaps we can find a
> > happy middle ground. What if we were to create a construct such as this:
> >
> > #pragma push_macro("ALLOW_EXPERIMENTAL_APIS")
> > #undef ALLOW_EXPERIMENTAL_APIS
> > void __rte_experimental func(...);
> > #pragma pop_macro("ALLOW_EXPERIMENTAL_APIS")
> >
> > Such a consruct would allow the maintainer of an API to pseudo-promote an API
> > from a experimental to stable status, at least so far as compilation would be
> > concerned. Its messy and clunky, and it wouldn't change the function version at
> > all, but the end result would be that:
> > a) such a wraped experimental function would no longer issue a warning when used
> > during compilation/linking
> > and
> > b) provide a semi-easy grepable pattern for application writers to look for when
> > considering the use of an API that was previously experimental
> >
> > such a construct would have to be used very judiciously, in that once its
> > implemented, the API has to be treated as stable, even though its 'excused' from
> > the normal checking, but it could provide something of the more rapid promotion
> > path being sought.
> >
> > Thoughts?
> > Neil
> >> Regards,
> >> /Bruce
> >>
>
>
> Then method above seems clunky, and it is unclear to me if it solves the original problem.
> It seems simpler to me to promote a symbol the next ABI version, and then provide an alias.
>
> At the declaration of the next major ABI version (v21), we would then only need to check
> for those symbols that are both EXPERIMENTAL and v21, to know which aliases need to be removed.
>
> Thanks,
>
> Ray K
>
i don't disagree, just looking for other potential options.
Neil
@@ -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
@@ -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")
+
/*
* 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))))
@@ -9,6 +9,7 @@
#include <rte_common.h>
#include <rte_log.h>
#include <rte_cycles.h>
+#include <rte_function_versioning.h>
#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);
@@ -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;
+};
@@ -48,7 +48,7 @@ foreach l:libraries
reason = '<unknown 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 = []