DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
@ 2020-01-29 12:29 Ferruh Yigit
  2020-01-29 14:46 ` Bruce Richardson
                   ` (3 more replies)
  0 siblings, 4 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-01-29 12:29 UTC (permalink / raw)
  To: Neil Horman, Cristian Dumitrescu, Eelco Chaudron
  Cc: dev, Thomas Monjalon, Luca Boccassi, David Marchand,
	Bruce Richardson, Ian Stokes

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(-)

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")
+
 /*
  * 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


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-01-29 12:29 [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal Ferruh Yigit
@ 2020-01-29 14:46 ` Bruce Richardson
  2020-01-29 14:57 ` Andrzej Ostruszka
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 31+ messages in thread
From: Bruce Richardson @ 2020-01-29 14:46 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Neil Horman, Cristian Dumitrescu, Eelco Chaudron, dev,
	Thomas Monjalon, Luca Boccassi, David Marchand, Ian Stokes

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-01-29 12:29 [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal Ferruh Yigit
  2020-01-29 14:46 ` Bruce Richardson
@ 2020-01-29 14:57 ` Andrzej Ostruszka
  2020-01-29 16:25   ` Ferruh Yigit
  2020-01-29 16:43 ` [dpdk-dev] [RFC v2] " Ferruh Yigit
  2020-02-02 18:53 ` [dpdk-dev] [RFC] " Neil Horman
  3 siblings, 1 reply; 31+ messages in thread
From: Andrzej Ostruszka @ 2020-01-29 14:57 UTC (permalink / raw)
  To: dev

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-01-29 14:57 ` Andrzej Ostruszka
@ 2020-01-29 16:25   ` Ferruh Yigit
  2020-01-29 16:30     ` Andrzej Ostruszka
  0 siblings, 1 reply; 31+ messages in thread
From: Ferruh Yigit @ 2020-01-29 16:25 UTC (permalink / raw)
  To: Andrzej Ostruszka, dev

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
> 


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-01-29 16:25   ` Ferruh Yigit
@ 2020-01-29 16:30     ` Andrzej Ostruszka
  2020-01-29 16:40       ` Ferruh Yigit
  0 siblings, 1 reply; 31+ messages in thread
From: Andrzej Ostruszka @ 2020-01-29 16:30 UTC (permalink / raw)
  To: Ferruh Yigit, dev

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-01-29 16:30     ` Andrzej Ostruszka
@ 2020-01-29 16:40       ` Ferruh Yigit
  0 siblings, 0 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-01-29 16:40 UTC (permalink / raw)
  To: Andrzej Ostruszka, dev

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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-29 12:29 [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal Ferruh Yigit
  2020-01-29 14:46 ` Bruce Richardson
  2020-01-29 14:57 ` Andrzej Ostruszka
@ 2020-01-29 16:43 ` " Ferruh Yigit
  2020-01-29 17:49   ` Andrzej Ostruszka
  2020-01-30 12:33   ` Thomas Monjalon
  2020-02-02 18:53 ` [dpdk-dev] [RFC] " Neil Horman
  3 siblings, 2 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-01-29 16:43 UTC (permalink / raw)
  To: Neil Horman, Cristian Dumitrescu, Eelco Chaudron
  Cc: dev, Thomas Monjalon, Luca Boccassi, David Marchand,
	Bruce Richardson, Ian Stokes, Andrzej Ostruszka

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)

Enabled function versioning for meson build for the library.

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>
Cc: Andrzej Ostruszka <amo@semihalf.com>

v2:
* allow versioning only for meter library, instead of enabling it
  globally

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/meson.build                  |  1 +
 lib/librte_meter/rte_meter.c                  | 59 ++++++++++++++++++-
 lib/librte_meter/rte_meter_version.map        |  8 +++
 5 files changed, 71 insertions(+), 4 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")
+
 /*
  * 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/meson.build b/lib/librte_meter/meson.build
index 646fd4d43..fce036843 100644
--- a/lib/librte_meter/meson.build
+++ b/lib/librte_meter/meson.build
@@ -3,3 +3,4 @@
 
 sources = files('rte_meter.c')
 headers = files('rte_meter.h')
+use_function_versioning = true
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;
+};
-- 
2.24.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-29 16:43 ` [dpdk-dev] [RFC v2] " Ferruh Yigit
@ 2020-01-29 17:49   ` Andrzej Ostruszka
  2020-01-30 12:33   ` Thomas Monjalon
  1 sibling, 0 replies; 31+ messages in thread
From: Andrzej Ostruszka @ 2020-01-29 17:49 UTC (permalink / raw)
  To: Ferruh Yigit, Neil Horman, Cristian Dumitrescu, Eelco Chaudron
  Cc: dev, Thomas Monjalon, Luca Boccassi, David Marchand,
	Bruce Richardson, Ian Stokes

On 1/29/20 5:43 PM, Ferruh Yigit wrote:
[...]
> 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);

You might want to mark these version symbols with __vsym macro.  Without
this shared lib build with LTO might remove them (due to long standing
gcc bug - it does not have proper way to mark .symver in internal
representation and so does not recognize that function is used).

This comment is global - for all symbols mentioned in BIND_/VERSION_ macros.

Out of curiosity - why do you need separate declaration just before
definition?

With regards
Andrzej Ostruszka

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-29 16:43 ` [dpdk-dev] [RFC v2] " Ferruh Yigit
  2020-01-29 17:49   ` Andrzej Ostruszka
@ 2020-01-30 12:33   ` Thomas Monjalon
  2020-01-30 12:57     ` Luca Boccassi
  2020-01-31  9:25     ` Ferruh Yigit
  1 sibling, 2 replies; 31+ messages in thread
From: Thomas Monjalon @ 2020-01-30 12:33 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Neil Horman, Cristian Dumitrescu, Eelco Chaudron, dev,
	Luca Boccassi, David Marchand, Bruce Richardson, Ian Stokes,
	Andrzej Ostruszka

Hi,

I disagree with the need of this patch.
The symbol was experimental, meaning we can change it.
Removing experimental tag is not an ABI break.


29/01/2020 17:43, Ferruh Yigit:
> Duplicated the existing symbol and versioned one as experimental and
> other as stable.
[..]
> --- 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;
> +};





^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 12:33   ` Thomas Monjalon
@ 2020-01-30 12:57     ` Luca Boccassi
  2020-01-30 14:17       ` Thomas Monjalon
  2020-01-31  9:25     ` Ferruh Yigit
  1 sibling, 1 reply; 31+ messages in thread
From: Luca Boccassi @ 2020-01-30 12:57 UTC (permalink / raw)
  To: Thomas Monjalon, Ferruh Yigit
  Cc: Neil Horman, Cristian Dumitrescu, Eelco Chaudron, dev,
	David Marchand, Bruce Richardson, Ian Stokes, Andrzej Ostruszka

On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
> Hi,
> 
> I disagree with the need of this patch.
> The symbol was experimental, meaning we can change it.
> Removing experimental tag is not an ABI break.

Hi,

This symbol change was requested for backport in 19.11.x, and
experimental or not I'm not too keen on backward incompatible changes
to the public interface in an _LTS point release_. The compromise was
to see if we could support both symbols version, which makes the change
backward compatible.

If you prefer not to have this patch in mainline I'm also fine in
taking it just for the LTS. I agree with you that it is not required
for mainline releases (although nicer for me if it's a backport rather
than a new change).

> 29/01/2020 17:43, Ferruh Yigit:
> > Duplicated the existing symbol and versioned one as experimental
> > and
> > other as stable.
> 
> [..]
> > --- 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;
> > +};
> 
> 
> 
> 
> 
-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 12:57     ` Luca Boccassi
@ 2020-01-30 14:17       ` Thomas Monjalon
  2020-01-30 14:21         ` Luca Boccassi
  0 siblings, 1 reply; 31+ messages in thread
From: Thomas Monjalon @ 2020-01-30 14:17 UTC (permalink / raw)
  To: Ferruh Yigit, Luca Boccassi
  Cc: Neil Horman, Cristian Dumitrescu, Eelco Chaudron, dev,
	David Marchand, Bruce Richardson, Ian Stokes, Andrzej Ostruszka

30/01/2020 13:57, Luca Boccassi:
> On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
> > Hi,
> > 
> > I disagree with the need of this patch.
> > The symbol was experimental, meaning we can change it.
> > Removing experimental tag is not an ABI break.
> 
> Hi,
> 
> This symbol change was requested for backport in 19.11.x, and
> experimental or not I'm not too keen on backward incompatible changes
> to the public interface in an _LTS point release_. The compromise was
> to see if we could support both symbols version, which makes the change
> backward compatible.
> 
> If you prefer not to have this patch in mainline I'm also fine in
> taking it just for the LTS. I agree with you that it is not required
> for mainline releases (although nicer for me if it's a backport rather
> than a new change).

I would like to avoid opening the door for maintaining the experimental ABI
in the mainline. Please take it directly in the LTS.

The next question is to know whether we really want to have such patch in LTS.
Anyway, 19.11.0 has this symbol as experimental.
How adding a non-experimental version of the function in 19.11.1 will change
the ABI status of the whole 19.11 branch?



> > 29/01/2020 17:43, Ferruh Yigit:
> > > Duplicated the existing symbol and versioned one as experimental
> > > and
> > > other as stable.
> > 
> > [..]
> > > --- 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;
> > > +};




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 14:17       ` Thomas Monjalon
@ 2020-01-30 14:21         ` Luca Boccassi
  2020-01-30 15:55           ` Thomas Monjalon
  0 siblings, 1 reply; 31+ messages in thread
From: Luca Boccassi @ 2020-01-30 14:21 UTC (permalink / raw)
  To: Thomas Monjalon, Ferruh Yigit
  Cc: Neil Horman, Cristian Dumitrescu, Eelco Chaudron, dev,
	David Marchand, Bruce Richardson, Ian Stokes, Andrzej Ostruszka

On Thu, 2020-01-30 at 15:17 +0100, Thomas Monjalon wrote:
> 30/01/2020 13:57, Luca Boccassi:
> > On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
> > > Hi,
> > > 
> > > I disagree with the need of this patch.
> > > The symbol was experimental, meaning we can change it.
> > > Removing experimental tag is not an ABI break.
> > 
> > Hi,
> > 
> > This symbol change was requested for backport in 19.11.x, and
> > experimental or not I'm not too keen on backward incompatible
> > changes
> > to the public interface in an _LTS point release_. The compromise
> > was
> > to see if we could support both symbols version, which makes the
> > change
> > backward compatible.
> > 
> > If you prefer not to have this patch in mainline I'm also fine in
> > taking it just for the LTS. I agree with you that it is not
> > required
> > for mainline releases (although nicer for me if it's a backport
> > rather
> > than a new change).
> 
> I would like to avoid opening the door for maintaining the
> experimental ABI
> in the mainline. Please take it directly in the LTS.
> 
> The next question is to know whether we really want to have such
> patch in LTS.
> Anyway, 19.11.0 has this symbol as experimental.
> How adding a non-experimental version of the function in 19.11.1 will
> change
> the ABI status of the whole 19.11 branch?

The problem is not adding the new symbol, but removing the experimental
one. Changing the version of the symbol was requested by OVS for
inclusion in 19.11.

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 14:21         ` Luca Boccassi
@ 2020-01-30 15:55           ` Thomas Monjalon
  2020-01-30 16:04             ` Luca Boccassi
  0 siblings, 1 reply; 31+ messages in thread
From: Thomas Monjalon @ 2020-01-30 15:55 UTC (permalink / raw)
  To: Luca Boccassi
  Cc: Ferruh Yigit, Neil Horman, Cristian Dumitrescu, Eelco Chaudron,
	dev, David Marchand, Bruce Richardson, Ian Stokes,
	Andrzej Ostruszka

30/01/2020 15:21, Luca Boccassi:
> On Thu, 2020-01-30 at 15:17 +0100, Thomas Monjalon wrote:
> > 30/01/2020 13:57, Luca Boccassi:
> > > On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
> > > > Hi,
> > > > 
> > > > I disagree with the need of this patch.
> > > > The symbol was experimental, meaning we can change it.
> > > > Removing experimental tag is not an ABI break.
> > > 
> > > Hi,
> > > 
> > > This symbol change was requested for backport in 19.11.x, and
> > > experimental or not I'm not too keen on backward incompatible
> > > changes
> > > to the public interface in an _LTS point release_. The compromise
> > > was
> > > to see if we could support both symbols version, which makes the
> > > change
> > > backward compatible.
> > > 
> > > If you prefer not to have this patch in mainline I'm also fine in
> > > taking it just for the LTS. I agree with you that it is not
> > > required
> > > for mainline releases (although nicer for me if it's a backport
> > > rather
> > > than a new change).
> > 
> > I would like to avoid opening the door for maintaining the
> > experimental ABI
> > in the mainline. Please take it directly in the LTS.
> > 
> > The next question is to know whether we really want to have such
> > patch in LTS.
> > Anyway, 19.11.0 has this symbol as experimental.
> > How adding a non-experimental version of the function in 19.11.1 will
> > change
> > the ABI status of the whole 19.11 branch?
> 
> The problem is not adding the new symbol, but removing the experimental
> one. Changing the version of the symbol was requested by OVS for
> inclusion in 19.11.

Yes, sorry, this is what I meant.
Given 19.11.0 already has the symbol as experimental,
and that applications like OVS had to accept it as experimental,
why removing experimental tag in 19.11.1?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 15:55           ` Thomas Monjalon
@ 2020-01-30 16:04             ` Luca Boccassi
  2020-01-30 16:15               ` Eelco Chaudron
  0 siblings, 1 reply; 31+ messages in thread
From: Luca Boccassi @ 2020-01-30 16:04 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Ferruh Yigit, Neil Horman, Cristian Dumitrescu, Eelco Chaudron,
	dev, David Marchand, Bruce Richardson, Ian Stokes,
	Andrzej Ostruszka

On Thu, 2020-01-30 at 16:55 +0100, Thomas Monjalon wrote:
> 30/01/2020 15:21, Luca Boccassi:
> > On Thu, 2020-01-30 at 15:17 +0100, Thomas Monjalon wrote:
> > > 30/01/2020 13:57, Luca Boccassi:
> > > > On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
> > > > > Hi,
> > > > > 
> > > > > I disagree with the need of this patch.
> > > > > The symbol was experimental, meaning we can change it.
> > > > > Removing experimental tag is not an ABI break.
> > > > 
> > > > Hi,
> > > > 
> > > > This symbol change was requested for backport in 19.11.x, and
> > > > experimental or not I'm not too keen on backward incompatible
> > > > changes
> > > > to the public interface in an _LTS point release_. The
> > > > compromise
> > > > was
> > > > to see if we could support both symbols version, which makes
> > > > the
> > > > change
> > > > backward compatible.
> > > > 
> > > > If you prefer not to have this patch in mainline I'm also fine
> > > > in
> > > > taking it just for the LTS. I agree with you that it is not
> > > > required
> > > > for mainline releases (although nicer for me if it's a backport
> > > > rather
> > > > than a new change).
> > > 
> > > I would like to avoid opening the door for maintaining the
> > > experimental ABI
> > > in the mainline. Please take it directly in the LTS.
> > > 
> > > The next question is to know whether we really want to have such
> > > patch in LTS.
> > > Anyway, 19.11.0 has this symbol as experimental.
> > > How adding a non-experimental version of the function in 19.11.1
> > > will
> > > change
> > > the ABI status of the whole 19.11 branch?
> > 
> > The problem is not adding the new symbol, but removing the
> > experimental
> > one. Changing the version of the symbol was requested by OVS for
> > inclusion in 19.11.
> 
> Yes, sorry, this is what I meant.
> Given 19.11.0 already has the symbol as experimental,
> and that applications like OVS had to accept it as experimental,
> why removing experimental tag in 19.11.1?

I think it was mentioned that it was preferred not to suppress the
compiler warning to avoid any accidental use in the future, but the OVS
maintainer(s) should answer as I might remember wrongly.

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 16:04             ` Luca Boccassi
@ 2020-01-30 16:15               ` Eelco Chaudron
  2020-01-30 20:20                 ` Thomas Monjalon
  0 siblings, 1 reply; 31+ messages in thread
From: Eelco Chaudron @ 2020-01-30 16:15 UTC (permalink / raw)
  To: Luca Boccassi
  Cc: Thomas Monjalon, Ferruh Yigit, Neil Horman, Cristian Dumitrescu,
	dev, David Marchand, Bruce Richardson, Ian Stokes,
	Andrzej Ostruszka



On 30 Jan 2020, at 17:04, Luca Boccassi wrote:

> On Thu, 2020-01-30 at 16:55 +0100, Thomas Monjalon wrote:
>> 30/01/2020 15:21, Luca Boccassi:
>>> On Thu, 2020-01-30 at 15:17 +0100, Thomas Monjalon wrote:
>>>> 30/01/2020 13:57, Luca Boccassi:
>>>>> On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I disagree with the need of this patch.
>>>>>> The symbol was experimental, meaning we can change it.
>>>>>> Removing experimental tag is not an ABI break.
>>>>>
>>>>> Hi,
>>>>>
>>>>> This symbol change was requested for backport in 19.11.x, and
>>>>> experimental or not I'm not too keen on backward incompatible
>>>>> changes
>>>>> to the public interface in an _LTS point release_. The
>>>>> compromise
>>>>> was
>>>>> to see if we could support both symbols version, which makes
>>>>> the
>>>>> change
>>>>> backward compatible.
>>>>>
>>>>> If you prefer not to have this patch in mainline I'm also fine
>>>>> in
>>>>> taking it just for the LTS. I agree with you that it is not
>>>>> required
>>>>> for mainline releases (although nicer for me if it's a backport
>>>>> rather
>>>>> than a new change).
>>>>
>>>> I would like to avoid opening the door for maintaining the
>>>> experimental ABI
>>>> in the mainline. Please take it directly in the LTS.
>>>>
>>>> The next question is to know whether we really want to have such
>>>> patch in LTS.
>>>> Anyway, 19.11.0 has this symbol as experimental.
>>>> How adding a non-experimental version of the function in 19.11.1
>>>> will
>>>> change
>>>> the ABI status of the whole 19.11 branch?
>>>
>>> The problem is not adding the new symbol, but removing the
>>> experimental
>>> one. Changing the version of the symbol was requested by OVS for
>>> inclusion in 19.11.
>>
>> Yes, sorry, this is what I meant.
>> Given 19.11.0 already has the symbol as experimental,
>> and that applications like OVS had to accept it as experimental,
>> why removing experimental tag in 19.11.1?
>
> I think it was mentioned that it was preferred not to suppress the
> compiler warning to avoid any accidental use in the future, but the 
> OVS
> maintainer(s) should answer as I might remember wrongly.

Yes this is the reason, OVS compiles with -Werror so we would like to 
avoid the warnings. You can not disable them per include, it’s global 
for all of DPDK.

//Eelco


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 16:15               ` Eelco Chaudron
@ 2020-01-30 20:20                 ` Thomas Monjalon
  2020-02-03 11:10                   ` Eelco Chaudron
  0 siblings, 1 reply; 31+ messages in thread
From: Thomas Monjalon @ 2020-01-30 20:20 UTC (permalink / raw)
  To: Luca Boccassi, Eelco Chaudron
  Cc: Ferruh Yigit, Neil Horman, Cristian Dumitrescu, dev,
	David Marchand, Bruce Richardson, Ian Stokes, Andrzej Ostruszka

30/01/2020 17:15, Eelco Chaudron:
> On 30 Jan 2020, at 17:04, Luca Boccassi wrote:
> > On Thu, 2020-01-30 at 16:55 +0100, Thomas Monjalon wrote:
> >> 30/01/2020 15:21, Luca Boccassi:
> >>> On Thu, 2020-01-30 at 15:17 +0100, Thomas Monjalon wrote:
> >>>> 30/01/2020 13:57, Luca Boccassi:
> >>>>> On Thu, 2020-01-30 at 13:33 +0100, Thomas Monjalon wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I disagree with the need of this patch.
> >>>>>> The symbol was experimental, meaning we can change it.
> >>>>>> Removing experimental tag is not an ABI break.
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> This symbol change was requested for backport in 19.11.x, and
> >>>>> experimental or not I'm not too keen on backward incompatible
> >>>>> changes
> >>>>> to the public interface in an _LTS point release_. The
> >>>>> compromise
> >>>>> was
> >>>>> to see if we could support both symbols version, which makes
> >>>>> the
> >>>>> change
> >>>>> backward compatible.
> >>>>>
> >>>>> If you prefer not to have this patch in mainline I'm also fine
> >>>>> in
> >>>>> taking it just for the LTS. I agree with you that it is not
> >>>>> required
> >>>>> for mainline releases (although nicer for me if it's a backport
> >>>>> rather
> >>>>> than a new change).
> >>>>
> >>>> I would like to avoid opening the door for maintaining the
> >>>> experimental ABI
> >>>> in the mainline. Please take it directly in the LTS.
> >>>>
> >>>> The next question is to know whether we really want to have such
> >>>> patch in LTS.
> >>>> Anyway, 19.11.0 has this symbol as experimental.
> >>>> How adding a non-experimental version of the function in 19.11.1
> >>>> will
> >>>> change
> >>>> the ABI status of the whole 19.11 branch?
> >>>
> >>> The problem is not adding the new symbol, but removing the
> >>> experimental
> >>> one. Changing the version of the symbol was requested by OVS for
> >>> inclusion in 19.11.
> >>
> >> Yes, sorry, this is what I meant.
> >> Given 19.11.0 already has the symbol as experimental,
> >> and that applications like OVS had to accept it as experimental,
> >> why removing experimental tag in 19.11.1?
> >
> > I think it was mentioned that it was preferred not to suppress the
> > compiler warning to avoid any accidental use in the future, but the 
> > OVS
> > maintainer(s) should answer as I might remember wrongly.
> 
> Yes this is the reason, OVS compiles with -Werror so we would like to 
> avoid the warnings. You can not disable them per include, it’s global 
> for all of DPDK.

Yes but anyway OVS must accept the experimental function as the next release
will use it with DPDK 19.11.0.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 12:33   ` Thomas Monjalon
  2020-01-30 12:57     ` Luca Boccassi
@ 2020-01-31  9:25     ` Ferruh Yigit
  1 sibling, 0 replies; 31+ messages in thread
From: Ferruh Yigit @ 2020-01-31  9:25 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Neil Horman, Cristian Dumitrescu, Eelco Chaudron, dev,
	Luca Boccassi, David Marchand, Bruce Richardson, Ian Stokes,
	Andrzej Ostruszka

On 1/30/2020 12:33 PM, Thomas Monjalon wrote:
> Hi,
> 
> I disagree with the need of this patch.
> The symbol was experimental, meaning we can change it.
> Removing experimental tag is not an ABI break.

In theory what you said is correct, this is experimental API and anything can
happen to it, this is the contract.

And when we need to change an experimental API, the users will be affected,
there is no escape from it.

But for this case limitation is more mechanical I believe,
because API is not changing at all, and it is becoming mature.
So we are breaking the user application because the experimental API they are
using becoming mature without any change.
This looks annoying from the user perspective, and if we can prevent this break,
I am for preventing it.

> 
> 
> 29/01/2020 17:43, Ferruh Yigit:
>> Duplicated the existing symbol and versioned one as experimental and
>> other as stable.
> [..]
>> --- 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;
>> +};
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-01-29 12:29 [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal Ferruh Yigit
                   ` (2 preceding siblings ...)
  2020-01-29 16:43 ` [dpdk-dev] [RFC v2] " Ferruh Yigit
@ 2020-02-02 18:53 ` " Neil Horman
  2020-02-03 12:53   ` Ferruh Yigit
  3 siblings, 1 reply; 31+ messages in thread
From: Neil Horman @ 2020-02-02 18:53 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Cristian Dumitrescu, Eelco Chaudron, dev, Thomas Monjalon,
	Luca Boccassi, David Marchand, Bruce Richardson, Ian Stokes

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
> 
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC v2] meter: fix ABI break due to experimental tag removal
  2020-01-30 20:20                 ` Thomas Monjalon
@ 2020-02-03 11:10                   ` Eelco Chaudron
  0 siblings, 0 replies; 31+ messages in thread
From: Eelco Chaudron @ 2020-02-03 11:10 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Luca Boccassi, Ferruh Yigit, Neil Horman, Cristian Dumitrescu,
	dev, David Marchand, Bruce Richardson, Ian Stokes,
	Andrzej Ostruszka



On 30 Jan 2020, at 21:20, Thomas Monjalon wrote:

<SNIP>

>>>>
>>>> Yes, sorry, this is what I meant.
>>>> Given 19.11.0 already has the symbol as experimental,
>>>> and that applications like OVS had to accept it as experimental,
>>>> why removing experimental tag in 19.11.1?
>>>
>>> I think it was mentioned that it was preferred not to suppress the
>>> compiler warning to avoid any accidental use in the future, but the
>>> OVS
>>> maintainer(s) should answer as I might remember wrongly.
>>
>> Yes this is the reason, OVS compiles with -Werror so we would like to
>> avoid the warnings. You can not disable them per include, it’s 
>> global
>> for all of DPDK.
>
> Yes but anyway OVS must accept the experimental function as the next 
> release
> will use it with DPDK 19.11.0.

Yes, we do for now, but we would like to remove it ASAP as it opens up 
the code base for the experimental APIs to slip in…


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-02 18:53 ` [dpdk-dev] [RFC] " Neil Horman
@ 2020-02-03 12:53   ` Ferruh Yigit
  2020-02-04 12:02     ` Neil Horman
  0 siblings, 1 reply; 31+ messages in thread
From: Ferruh Yigit @ 2020-02-03 12:53 UTC (permalink / raw)
  To: Neil Horman
  Cc: Cristian Dumitrescu, Eelco Chaudron, dev, Thomas Monjalon,
	Luca Boccassi, David Marchand, Bruce Richardson, Ian Stokes

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
>>
>>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-03 12:53   ` Ferruh Yigit
@ 2020-02-04 12:02     ` Neil Horman
  2020-02-05 10:04       ` Luca Boccassi
  0 siblings, 1 reply; 31+ messages in thread
From: Neil Horman @ 2020-02-04 12:02 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Cristian Dumitrescu, Eelco Chaudron, dev, Thomas Monjalon,
	Luca Boccassi, David Marchand, Bruce Richardson, Ian Stokes

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
> >>
> >>
> 
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-04 12:02     ` Neil Horman
@ 2020-02-05 10:04       ` Luca Boccassi
  2020-02-05 11:32         ` Neil Horman
  0 siblings, 1 reply; 31+ messages in thread
From: Luca Boccassi @ 2020-02-05 10:04 UTC (permalink / raw)
  To: Neil Horman, Ferruh Yigit
  Cc: Cristian Dumitrescu, Eelco Chaudron, dev, Thomas Monjalon,
	David Marchand, Bruce Richardson, Ian Stokes

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.

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-05 10:04       ` Luca Boccassi
@ 2020-02-05 11:32         ` Neil Horman
  2020-02-13 17:40           ` Ray Kinsella
  0 siblings, 1 reply; 31+ messages in thread
From: Neil Horman @ 2020-02-05 11:32 UTC (permalink / raw)
  To: Luca Boccassi
  Cc: Ferruh Yigit, Cristian Dumitrescu, Eelco Chaudron, dev,
	Thomas Monjalon, David Marchand, Bruce Richardson, Ian Stokes

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
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-05 11:32         ` Neil Horman
@ 2020-02-13 17:40           ` Ray Kinsella
  2020-02-14  2:40             ` Neil Horman
  0 siblings, 1 reply; 31+ messages in thread
From: Ray Kinsella @ 2020-02-13 17:40 UTC (permalink / raw)
  To: Neil Horman, Luca Boccassi
  Cc: Ferruh Yigit, Cristian Dumitrescu, Eelco Chaudron, dev,
	Thomas Monjalon, David Marchand, Bruce Richardson, Ian Stokes



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. 







^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-13 17:40           ` Ray Kinsella
@ 2020-02-14  2:40             ` Neil Horman
  2020-02-14 11:36               ` Bruce Richardson
  0 siblings, 1 reply; 31+ messages in thread
From: Neil Horman @ 2020-02-14  2:40 UTC (permalink / raw)
  To: Ray Kinsella
  Cc: Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu, Eelco Chaudron,
	dev, Thomas Monjalon, David Marchand, Bruce Richardson,
	Ian Stokes

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

> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-14  2:40             ` Neil Horman
@ 2020-02-14 11:36               ` Bruce Richardson
  2020-02-14 20:48                 ` Neil Horman
  0 siblings, 1 reply; 31+ messages in thread
From: Bruce Richardson @ 2020-02-14 11:36 UTC (permalink / raw)
  To: Neil Horman
  Cc: Ray Kinsella, Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu,
	Eelco Chaudron, dev, Thomas Monjalon, David Marchand, Ian Stokes

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-14 11:36               ` Bruce Richardson
@ 2020-02-14 20:48                 ` Neil Horman
  2020-02-14 21:52                   ` Bruce Richardson
  2020-02-17 14:23                   ` Ray Kinsella
  0 siblings, 2 replies; 31+ messages in thread
From: Neil Horman @ 2020-02-14 20:48 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Ray Kinsella, Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu,
	Eelco Chaudron, dev, Thomas Monjalon, David Marchand, Ian Stokes

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
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-14 20:48                 ` Neil Horman
@ 2020-02-14 21:52                   ` Bruce Richardson
  2020-02-15 13:43                     ` Neil Horman
  2020-02-17 14:23                   ` Ray Kinsella
  1 sibling, 1 reply; 31+ messages in thread
From: Bruce Richardson @ 2020-02-14 21:52 UTC (permalink / raw)
  To: Neil Horman
  Cc: Ray Kinsella, Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu,
	Eelco Chaudron, dev, Thomas Monjalon, David Marchand, Ian Stokes

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-14 21:52                   ` Bruce Richardson
@ 2020-02-15 13:43                     ` Neil Horman
  0 siblings, 0 replies; 31+ messages in thread
From: Neil Horman @ 2020-02-15 13:43 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Ray Kinsella, Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu,
	Eelco Chaudron, dev, Thomas Monjalon, David Marchand, Ian Stokes

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
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-14 20:48                 ` Neil Horman
  2020-02-14 21:52                   ` Bruce Richardson
@ 2020-02-17 14:23                   ` Ray Kinsella
  2020-02-17 15:37                     ` Neil Horman
  1 sibling, 1 reply; 31+ messages in thread
From: Ray Kinsella @ 2020-02-17 14:23 UTC (permalink / raw)
  To: Neil Horman, Bruce Richardson
  Cc: Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu, Eelco Chaudron,
	dev, Thomas Monjalon, David Marchand, Ian Stokes



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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
  2020-02-17 14:23                   ` Ray Kinsella
@ 2020-02-17 15:37                     ` Neil Horman
  0 siblings, 0 replies; 31+ messages in thread
From: Neil Horman @ 2020-02-17 15:37 UTC (permalink / raw)
  To: Ray Kinsella
  Cc: Bruce Richardson, Luca Boccassi, Ferruh Yigit,
	Cristian Dumitrescu, Eelco Chaudron, dev, Thomas Monjalon,
	David Marchand, Ian Stokes

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, back to index

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 12:29 [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal Ferruh Yigit
2020-01-29 14:46 ` Bruce Richardson
2020-01-29 14:57 ` Andrzej Ostruszka
2020-01-29 16:25   ` Ferruh Yigit
2020-01-29 16:30     ` Andrzej Ostruszka
2020-01-29 16:40       ` Ferruh Yigit
2020-01-29 16:43 ` [dpdk-dev] [RFC v2] " Ferruh Yigit
2020-01-29 17:49   ` Andrzej Ostruszka
2020-01-30 12:33   ` Thomas Monjalon
2020-01-30 12:57     ` Luca Boccassi
2020-01-30 14:17       ` Thomas Monjalon
2020-01-30 14:21         ` Luca Boccassi
2020-01-30 15:55           ` Thomas Monjalon
2020-01-30 16:04             ` Luca Boccassi
2020-01-30 16:15               ` Eelco Chaudron
2020-01-30 20:20                 ` Thomas Monjalon
2020-02-03 11:10                   ` Eelco Chaudron
2020-01-31  9:25     ` Ferruh Yigit
2020-02-02 18:53 ` [dpdk-dev] [RFC] " Neil Horman
2020-02-03 12:53   ` Ferruh Yigit
2020-02-04 12:02     ` Neil Horman
2020-02-05 10:04       ` Luca Boccassi
2020-02-05 11:32         ` Neil Horman
2020-02-13 17:40           ` Ray Kinsella
2020-02-14  2:40             ` Neil Horman
2020-02-14 11:36               ` Bruce Richardson
2020-02-14 20:48                 ` Neil Horman
2020-02-14 21:52                   ` Bruce Richardson
2020-02-15 13:43                     ` Neil Horman
2020-02-17 14:23                   ` Ray Kinsella
2020-02-17 15:37                     ` Neil Horman

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox