[RFC] config: remove RTE_NEXT_ABI

[RFC] config: remove RTE_NEXT_ABI

[Ferruh Yigit]

After experimental API process defined do we still need RTE_NEXT_ABI
config and process which has similar targets?

Are distros disable experimental APIs when delivering DPDK? And is there
any config required to control this, as RTE_NEXT_ABI intended to do?

Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Luca Boccassi <bluca@debian.org>
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 config/common_base                     |  5 -----
 devtools/test-build.sh                 |  2 --
 devtools/validate-abi.sh               |  1 -
 doc/guides/contributing/versioning.rst | 10 ----------
 mk/rte.lib.mk                          |  5 -----
 pkg/dpdk.spec                          |  1 -
 6 files changed, 24 deletions(-)

diff --git a/config/common_base b/config/common_base
index ad03cf433..6b867f6a9 100644
--- a/config/common_base
+++ b/config/common_base
@@ -41,11 +41,6 @@ CONFIG_RTE_ARCH_STRICT_ALIGN=n
 CONFIG_RTE_BUILD_SHARED_LIB=n
 
 #
-# Use newest code breaking previous ABI
-#
-CONFIG_RTE_NEXT_ABI=y
-
-#
 # Major ABI to overwrite library specific LIBABIVER
 #
 CONFIG_RTE_MAJOR_ABI=
diff --git a/devtools/test-build.sh b/devtools/test-build.sh
index 3362edcc5..22b4e1a98 100755
--- a/devtools/test-build.sh
+++ b/devtools/test-build.sh
@@ -154,8 +154,6 @@ config () # <directory> <target> <options>
 		# Built-in options (lowercase)
 		! echo $3 | grep -q '+default' || \
 		sed -ri 's,(RTE_MACHINE=")native,\1default,' $1/.config
-		echo $3 | grep -q '+next' || \
-		sed -ri           's,(NEXT_ABI=)y,\1n,' $1/.config
 		! echo $3 | grep -q '+shared' || \
 		sed -ri         's,(SHARED_LIB=)n,\1y,' $1/.config
 		! echo $3 | grep -q '+debug' || ( \
diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh
index 138436d93..a64edf92f 100755
--- a/devtools/validate-abi.sh
+++ b/devtools/validate-abi.sh
@@ -105,7 +105,6 @@ set_log_file() {
 fixup_config() {
 	local conf=config/defconfig_$target
 	cmd sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" $conf
-	cmd sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" $conf
 	cmd sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" $conf
 	cmd sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" $conf
 	cmd sed -i -e"$ a\CONFIG_RTE_KNI_KMOD=n" $conf
diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
index c495294db..59ff0e8b7 100644
--- a/doc/guides/contributing/versioning.rst
+++ b/doc/guides/contributing/versioning.rst
@@ -91,19 +91,9 @@ being provided. The requirements for doing so are:
      interest" be sought for each deprecation, for example: from NIC vendors,
      CPU vendors, end-users, etc.
 
-#. The changes (including an alternative map file) must be gated with
-   the ``RTE_NEXT_ABI`` option, and provided with a deprecation notice at the
-   same time.
-   It will become the default ABI in the next release.
-
 #. A full deprecation cycle, as explained above, must be made to offer
    downstream consumers sufficient warning of the change.
 
-#. At the beginning of the next release cycle, every ``RTE_NEXT_ABI``
-   conditions will be removed, the ``LIBABIVER`` variable in the makefile(s)
-   where the ABI is changed will be incremented, and the map files will
-   be updated.
-
 Note that the above process for ABI deprecation should not be undertaken
 lightly. ABI stability is extremely important for downstream consumers of the
 DPDK, especially when distributed in shared object form. Every effort should
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index c696a2174..8ac26face 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -20,11 +20,6 @@ endif
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
 ifeq ($(EXTLIB_BUILD),n)
-ifeq ($(CONFIG_RTE_MAJOR_ABI),)
-ifeq ($(CONFIG_RTE_NEXT_ABI),y)
-LIB := $(LIB).1
-endif
-endif
 CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
 endif
 endif
diff --git a/pkg/dpdk.spec b/pkg/dpdk.spec
index 4d3b5745c..d118f0463 100644
--- a/pkg/dpdk.spec
+++ b/pkg/dpdk.spec
@@ -84,7 +84,6 @@ make O=%{target} T=%{config} config
 sed -ri 's,(RTE_MACHINE=).*,\1%{machine},' %{target}/.config
 sed -ri 's,(RTE_APP_TEST=).*,\1n,'         %{target}/.config
 sed -ri 's,(RTE_BUILD_SHARED_LIB=).*,\1y,' %{target}/.config
-sed -ri 's,(RTE_NEXT_ABI=).*,\1n,'         %{target}/.config
 sed -ri 's,(LIBRTE_VHOST=).*,\1y,'         %{target}/.config
 sed -ri 's,(LIBRTE_PMD_PCAP=).*,\1y,'      %{target}/.config
 make O=%{target} %{?_smp_mflags}
-- 
2.13.6

Re: [RFC] config: remove RTE_NEXT_ABI

[Luca Boccassi]

On Wed, 2018-03-07 at 17:44 +0000, Ferruh Yigit wrote:
> After experimental API process defined do we still need RTE_NEXT_ABI
> config and process which has similar targets?
> 
> Are distros disable experimental APIs when delivering DPDK? And is
> there
> any config required to control this, as RTE_NEXT_ABI intended to do?

I tried to tinker with not exporting experimental APIs - but the
problem is intra-project dependencies, iow: librte_foo has a
foo_experimental API that librte_bar uses, so if librte_foo
foo_experimental symbol is not available everything breaks down. I need
to spend a bit more on this problem but -ENOTIME

> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Luca Boccassi <bluca@debian.org>
> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  config/common_base                     |  5 -----
>  devtools/test-build.sh                 |  2 --
>  devtools/validate-abi.sh               |  1 -
>  doc/guides/contributing/versioning.rst | 10 ----------
>  mk/rte.lib.mk                          |  5 -----
>  pkg/dpdk.spec                          |  1 -
>  6 files changed, 24 deletions(-)
> 
> diff --git a/config/common_base b/config/common_base
> index ad03cf433..6b867f6a9 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -41,11 +41,6 @@ CONFIG_RTE_ARCH_STRICT_ALIGN=n
>  CONFIG_RTE_BUILD_SHARED_LIB=n
>  
>  #
> -# Use newest code breaking previous ABI
> -#
> -CONFIG_RTE_NEXT_ABI=y
> -
> -#
>  # Major ABI to overwrite library specific LIBABIVER
>  #
>  CONFIG_RTE_MAJOR_ABI=
> diff --git a/devtools/test-build.sh b/devtools/test-build.sh
> index 3362edcc5..22b4e1a98 100755
> --- a/devtools/test-build.sh
> +++ b/devtools/test-build.sh
> @@ -154,8 +154,6 @@ config () # <directory> <target> <options>
>  		# Built-in options (lowercase)
>  		! echo $3 | grep -q '+default' || \
>  		sed -ri 's,(RTE_MACHINE=")native,\1default,'
> $1/.config
> -		echo $3 | grep -q '+next' || \
> -		sed -ri           's,(NEXT_ABI=)y,\1n,' $1/.config
>  		! echo $3 | grep -q '+shared' || \
>  		sed -ri         's,(SHARED_LIB=)n,\1y,' $1/.config
>  		! echo $3 | grep -q '+debug' || ( \
> diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh
> index 138436d93..a64edf92f 100755
> --- a/devtools/validate-abi.sh
> +++ b/devtools/validate-abi.sh
> @@ -105,7 +105,6 @@ set_log_file() {
>  fixup_config() {
>  	local conf=config/defconfig_$target
>  	cmd sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" $conf
> -	cmd sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" $conf
>  	cmd sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" $conf
>  	cmd sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" $conf
>  	cmd sed -i -e"$ a\CONFIG_RTE_KNI_KMOD=n" $conf
> diff --git a/doc/guides/contributing/versioning.rst
> b/doc/guides/contributing/versioning.rst
> index c495294db..59ff0e8b7 100644
> --- a/doc/guides/contributing/versioning.rst
> +++ b/doc/guides/contributing/versioning.rst
> @@ -91,19 +91,9 @@ being provided. The requirements for doing so are:
>       interest" be sought for each deprecation, for example: from NIC
> vendors,
>       CPU vendors, end-users, etc.
>  
> -#. The changes (including an alternative map file) must be gated
> with
> -   the ``RTE_NEXT_ABI`` option, and provided with a deprecation
> notice at the
> -   same time.
> -   It will become the default ABI in the next release.
> -
>  #. A full deprecation cycle, as explained above, must be made to
> offer
>     downstream consumers sufficient warning of the change.
>  
> -#. At the beginning of the next release cycle, every
> ``RTE_NEXT_ABI``
> -   conditions will be removed, the ``LIBABIVER`` variable in the
> makefile(s)
> -   where the ABI is changed will be incremented, and the map files
> will
> -   be updated.
> -
>  Note that the above process for ABI deprecation should not be
> undertaken
>  lightly. ABI stability is extremely important for downstream
> consumers of the
>  DPDK, especially when distributed in shared object form. Every
> effort should
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index c696a2174..8ac26face 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -20,11 +20,6 @@ endif
>  ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
>  LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
>  ifeq ($(EXTLIB_BUILD),n)
> -ifeq ($(CONFIG_RTE_MAJOR_ABI),)
> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
> -LIB := $(LIB).1
> -endif
> -endif
>  CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
>  endif
>  endif
> diff --git a/pkg/dpdk.spec b/pkg/dpdk.spec
> index 4d3b5745c..d118f0463 100644
> --- a/pkg/dpdk.spec
> +++ b/pkg/dpdk.spec
> @@ -84,7 +84,6 @@ make O=%{target} T=%{config} config
>  sed -ri 's,(RTE_MACHINE=).*,\1%{machine},' %{target}/.config
>  sed -ri 's,(RTE_APP_TEST=).*,\1n,'         %{target}/.config
>  sed -ri 's,(RTE_BUILD_SHARED_LIB=).*,\1y,' %{target}/.config
> -sed -ri 's,(RTE_NEXT_ABI=).*,\1n,'         %{target}/.config
>  sed -ri 's,(LIBRTE_VHOST=).*,\1y,'         %{target}/.config
>  sed -ri 's,(LIBRTE_PMD_PCAP=).*,\1y,'      %{target}/.config
>  make O=%{target} %{?_smp_mflags}

-- 
Kind regards,
Luca Boccassi

Re: [RFC] config: remove RTE_NEXT_ABI

[Thomas Monjalon]

07/03/2018 18:44, Ferruh Yigit:
> After experimental API process defined do we still need RTE_NEXT_ABI
> config and process which has similar targets?

They are different targets.
Experimental API is always enabled but may be avoided by applications.
Next ABI can be used to break ABI without notice and disabled to keep
old ABI compatibility. It is almost never used because it is preferred
to keep ABI compatibility with rte_compat macros, or wait a deprecation
period after notice.

Re: [RFC] config: remove RTE_NEXT_ABI

[Ferruh Yigit]

On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> 07/03/2018 18:44, Ferruh Yigit:
>> After experimental API process defined do we still need RTE_NEXT_ABI
>> config and process which has similar targets?
> 
> They are different targets.
> Experimental API is always enabled but may be avoided by applications.
> Next ABI can be used to break ABI without notice and disabled to keep
> old ABI compatibility. It is almost never used because it is preferred
> to keep ABI compatibility with rte_compat macros, or wait a deprecation
> period after notice.

OK, I see.

Shouldn't we disable it by default at least? Otherwise who is not paying
attention to this config option will get and ABI/API break.

Re: [RFC] config: remove RTE_NEXT_ABI

[Thomas Monjalon]

08/03/2018 12:43, Ferruh Yigit:
> On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> > 07/03/2018 18:44, Ferruh Yigit:
> >> After experimental API process defined do we still need RTE_NEXT_ABI
> >> config and process which has similar targets?
> > 
> > They are different targets.
> > Experimental API is always enabled but may be avoided by applications.
> > Next ABI can be used to break ABI without notice and disabled to keep
> > old ABI compatibility. It is almost never used because it is preferred
> > to keep ABI compatibility with rte_compat macros, or wait a deprecation
> > period after notice.
> 
> OK, I see.
> 
> Shouldn't we disable it by default at least? Otherwise who is not paying
> attention to this config option will get and ABI/API break.

Yes I think you are right, it can be disabled by default.

Re: [RFC] config: remove RTE_NEXT_ABI

[Neil Horman]

On Thu, Mar 08, 2018 at 04:17:00PM +0100, Thomas Monjalon wrote:
> 08/03/2018 12:43, Ferruh Yigit:
> > On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> > > 07/03/2018 18:44, Ferruh Yigit:
> > >> After experimental API process defined do we still need RTE_NEXT_ABI
> > >> config and process which has similar targets?
> > > 
> > > They are different targets.
> > > Experimental API is always enabled but may be avoided by applications.
> > > Next ABI can be used to break ABI without notice and disabled to keep
> > > old ABI compatibility. It is almost never used because it is preferred
> > > to keep ABI compatibility with rte_compat macros, or wait a deprecation
> > > period after notice.
> > 
> > OK, I see.
> > 
> > Shouldn't we disable it by default at least? Otherwise who is not paying
> > attention to this config option will get and ABI/API break.
> 
> Yes I think you are right, it can be disabled by default.
> 
I would agree, there seems to be overlap here, and the experimental tagging can
cover what the NEXT_API flag is meant to do.  It can be removed I think.
Neil

> 
> 
> 

Re: [RFC] config: remove RTE_NEXT_ABI

[Thomas Monjalon]

08/03/2018 16:35, Neil Horman:
> On Thu, Mar 08, 2018 at 04:17:00PM +0100, Thomas Monjalon wrote:
> > 08/03/2018 12:43, Ferruh Yigit:
> > > On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> > > > 07/03/2018 18:44, Ferruh Yigit:
> > > >> After experimental API process defined do we still need RTE_NEXT_ABI
> > > >> config and process which has similar targets?
> > > > 
> > > > They are different targets.
> > > > Experimental API is always enabled but may be avoided by applications.
> > > > Next ABI can be used to break ABI without notice and disabled to keep
> > > > old ABI compatibility. It is almost never used because it is preferred
> > > > to keep ABI compatibility with rte_compat macros, or wait a deprecation
> > > > period after notice.
> > > 
> > > OK, I see.
> > > 
> > > Shouldn't we disable it by default at least? Otherwise who is not paying
> > > attention to this config option will get and ABI/API break.
> > 
> > Yes I think you are right, it can be disabled by default.
> > 
> I would agree, there seems to be overlap here, and the experimental tagging can
> cover what the NEXT_API flag is meant to do.  It can be removed I think.

It is not NEXT_API but NEXT_ABI.
Why do you think it overlaps experimental API tagging?

Re: [RFC] config: remove RTE_NEXT_ABI

[Neil Horman]

On Thu, Mar 08, 2018 at 05:04:01PM +0100, Thomas Monjalon wrote:
> 08/03/2018 16:35, Neil Horman:
> > On Thu, Mar 08, 2018 at 04:17:00PM +0100, Thomas Monjalon wrote:
> > > 08/03/2018 12:43, Ferruh Yigit:
> > > > On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> > > > > 07/03/2018 18:44, Ferruh Yigit:
> > > > >> After experimental API process defined do we still need RTE_NEXT_ABI
> > > > >> config and process which has similar targets?
> > > > > 
> > > > > They are different targets.
> > > > > Experimental API is always enabled but may be avoided by applications.
> > > > > Next ABI can be used to break ABI without notice and disabled to keep
> > > > > old ABI compatibility. It is almost never used because it is preferred
> > > > > to keep ABI compatibility with rte_compat macros, or wait a deprecation
> > > > > period after notice.
> > > > 
> > > > OK, I see.
> > > > 
> > > > Shouldn't we disable it by default at least? Otherwise who is not paying
> > > > attention to this config option will get and ABI/API break.
> > > 
> > > Yes I think you are right, it can be disabled by default.
> > > 
> > I would agree, there seems to be overlap here, and the experimental tagging can
> > cover what the NEXT_API flag is meant to do.  It can be removed I think.
> 
> It is not NEXT_API but NEXT_ABI.
Sorry, typo, though I'm sure you got that, since the former doesn't exist,
right?
> Why do you think it overlaps experimental API tagging?

I assert that because the compat lib has macros to map common symbols to version
specific ones.  That is to say, if you change a data structure, you can setup
the API calls that use said structure such that version 1 or the symbol maps to
an internal function that uses the old structure, while version 2 maps to an
internal function that uses the new symbol

That is to say, if you're planning on introducing ABI changes, the experimental
API tagging can be used to implement what the NEXT_ABI macro does.

Neil

> 
> 
> 

Re: [RFC] config: remove RTE_NEXT_ABI

[Thomas Monjalon]

08/03/2018 20:40, Neil Horman:
> On Thu, Mar 08, 2018 at 05:04:01PM +0100, Thomas Monjalon wrote:
> > 08/03/2018 16:35, Neil Horman:
> > > On Thu, Mar 08, 2018 at 04:17:00PM +0100, Thomas Monjalon wrote:
> > > > 08/03/2018 12:43, Ferruh Yigit:
> > > > > On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> > > > > > 07/03/2018 18:44, Ferruh Yigit:
> > > > > >> After experimental API process defined do we still need RTE_NEXT_ABI
> > > > > >> config and process which has similar targets?
> > > > > > 
> > > > > > They are different targets.
> > > > > > Experimental API is always enabled but may be avoided by applications.
> > > > > > Next ABI can be used to break ABI without notice and disabled to keep
> > > > > > old ABI compatibility. It is almost never used because it is preferred
> > > > > > to keep ABI compatibility with rte_compat macros, or wait a deprecation
> > > > > > period after notice.
> > > > > 
> > > > > OK, I see.
> > > > > 
> > > > > Shouldn't we disable it by default at least? Otherwise who is not paying
> > > > > attention to this config option will get and ABI/API break.
> > > > 
> > > > Yes I think you are right, it can be disabled by default.
> > > > 
> > > I would agree, there seems to be overlap here, and the experimental tagging can
> > > cover what the NEXT_API flag is meant to do.  It can be removed I think.
> > 
> > It is not NEXT_API but NEXT_ABI.
> Sorry, typo, though I'm sure you got that, since the former doesn't exist,
> right?
> > Why do you think it overlaps experimental API tagging?
> 
> I assert that because the compat lib has macros to map common symbols to version
> specific ones.  That is to say, if you change a data structure, you can setup
> the API calls that use said structure such that version 1 or the symbol maps to
> an internal function that uses the old structure, while version 2 maps to an
> internal function that uses the new symbol
> 
> That is to say, if you're planning on introducing ABI changes, the experimental
> API tagging can be used to implement what the NEXT_ABI macro does.

It is a different usage.
Experimental API tagging is for new functions.
rte_compat is used to avoid breaking the ABI when changing old code.
NEXT_ABI has been used in the past to disable an ABI breakage, which was
not possible to mitigate with rte_compat because impacting too many functions.

I am not saying that I like NEXT_ABI, but it could be useful exceptionnally.

Re: [RFC] config: remove RTE_NEXT_ABI

[Neil Horman]

On Thu, Mar 08, 2018 at 10:34:14PM +0100, Thomas Monjalon wrote:
> 08/03/2018 20:40, Neil Horman:
> > On Thu, Mar 08, 2018 at 05:04:01PM +0100, Thomas Monjalon wrote:
> > > 08/03/2018 16:35, Neil Horman:
> > > > On Thu, Mar 08, 2018 at 04:17:00PM +0100, Thomas Monjalon wrote:
> > > > > 08/03/2018 12:43, Ferruh Yigit:
> > > > > > On 3/8/2018 8:05 AM, Thomas Monjalon wrote:
> > > > > > > 07/03/2018 18:44, Ferruh Yigit:
> > > > > > >> After experimental API process defined do we still need RTE_NEXT_ABI
> > > > > > >> config and process which has similar targets?
> > > > > > > 
> > > > > > > They are different targets.
> > > > > > > Experimental API is always enabled but may be avoided by applications.
> > > > > > > Next ABI can be used to break ABI without notice and disabled to keep
> > > > > > > old ABI compatibility. It is almost never used because it is preferred
> > > > > > > to keep ABI compatibility with rte_compat macros, or wait a deprecation
> > > > > > > period after notice.
> > > > > > 
> > > > > > OK, I see.
> > > > > > 
> > > > > > Shouldn't we disable it by default at least? Otherwise who is not paying
> > > > > > attention to this config option will get and ABI/API break.
> > > > > 
> > > > > Yes I think you are right, it can be disabled by default.
> > > > > 
> > > > I would agree, there seems to be overlap here, and the experimental tagging can
> > > > cover what the NEXT_API flag is meant to do.  It can be removed I think.
> > > 
> > > It is not NEXT_API but NEXT_ABI.
> > Sorry, typo, though I'm sure you got that, since the former doesn't exist,
> > right?
> > > Why do you think it overlaps experimental API tagging?
> > 
> > I assert that because the compat lib has macros to map common symbols to version
> > specific ones.  That is to say, if you change a data structure, you can setup
> > the API calls that use said structure such that version 1 or the symbol maps to
> > an internal function that uses the old structure, while version 2 maps to an
> > internal function that uses the new symbol
> > 
> > That is to say, if you're planning on introducing ABI changes, the experimental
> > API tagging can be used to implement what the NEXT_ABI macro does.
> 
> It is a different usage.
> Experimental API tagging is for new functions.
> rte_compat is used to avoid breaking the ABI when changing old code.
> NEXT_ABI has been used in the past to disable an ABI breakage, which was
> not possible to mitigate with rte_compat because impacting too many functions.
> 
Thats not entirely true.  It _is_ used to manage ABI changes when backwards
compatibiilty needs to be preserved. It _can_be_ used for experimental abi
management.  That is to say, if you want to modify an existing ABI symbol, you
can do so by writing a new function, and then exporting the new function as the
old symbol with the @EXPERIMENTAL version.  Not saying we have to do that, but
we certainly can, and can eliminate NEXT_ABI in the process.

> I am not saying that I like NEXT_ABI, but it could be useful exceptionnally.
> 
Well, if the consensus is that it should be kept, its no skin off my nose, but
the discussion was around removing NEXT_ABI, and I was copied, so I thought I'd
add my $0.02

Neil

> 
> 

[PATCH] config: disable RTE_NEXT_ABI by default

[Ferruh Yigit]

Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
the current release and these APIs are targetted for further release.

RTE_NEXT_ABI shouldn't be enabled by default.

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: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 config/common_base | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/common_base b/config/common_base
index 2e888d13b..dbd0c9ae9 100644
--- a/config/common_base
+++ b/config/common_base
@@ -43,7 +43,7 @@ CONFIG_RTE_BUILD_SHARED_LIB=n
 #
 # Use newest code breaking previous ABI
 #
-CONFIG_RTE_NEXT_ABI=y
+CONFIG_RTE_NEXT_ABI=n
 
 #
 # Major ABI to overwrite library specific LIBABIVER
-- 
2.17.1

Re: [PATCH] config: disable RTE_NEXT_ABI by default

[Luca Boccassi]

On Thu, 2018-10-04 at 16:43 +0100, Ferruh Yigit wrote:
> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
> the current release and these APIs are targetted for further release.
> 
> RTE_NEXT_ABI shouldn't be enabled by default.
> 
> 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: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  config/common_base | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/config/common_base b/config/common_base
> index 2e888d13b..dbd0c9ae9 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -43,7 +43,7 @@ CONFIG_RTE_BUILD_SHARED_LIB=n
>  #
>  # Use newest code breaking previous ABI
>  #
> -CONFIG_RTE_NEXT_ABI=y
> +CONFIG_RTE_NEXT_ABI=n
>  
>  #
>  # Major ABI to overwrite library specific LIBABIVER

Acked-by: Luca Boccassi <bluca@debian.org>

-- 
Kind regards,
Luca Boccassi

[PATCH v2] config: disable RTE_NEXT_ABI by default

[Ferruh Yigit]

Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
the current release and these APIs are targeted for further release.

RTE_NEXT_ABI shouldn't be enabled by default.

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: Christian Ehrhardt <christian.ehrhardt@canonical.com>

v2:
* fix typo in commit log
---
 config/common_base | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/common_base b/config/common_base
index 2e888d13b..dbd0c9ae9 100644
--- a/config/common_base
+++ b/config/common_base
@@ -43,7 +43,7 @@ CONFIG_RTE_BUILD_SHARED_LIB=n
 #
 # Use newest code breaking previous ABI
 #
-CONFIG_RTE_NEXT_ABI=y
+CONFIG_RTE_NEXT_ABI=n
 
 #
 # Major ABI to overwrite library specific LIBABIVER
-- 
2.17.1

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Thomas Monjalon]

04/10/2018 17:48, Ferruh Yigit:
> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
> the current release and these APIs are targeted for further release.

It seems nobody is using it in last releases.

> RTE_NEXT_ABI shouldn't be enabled by default.

The reason for having it enabled by default is that when you build DPDK
yourself, you probably want the latest features.
If packaged properly for stability, it is easy to disable it in
the package recipe.

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Ferruh Yigit]

On 10/4/2018 4:10 PM, Thomas Monjalon wrote:
> 04/10/2018 17:48, Ferruh Yigit:
>> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
>> the current release and these APIs are targeted for further release.
> 
> It seems nobody is using it in last releases.
> 
>> RTE_NEXT_ABI shouldn't be enabled by default.
> 
> The reason for having it enabled by default is that when you build DPDK
> yourself, you probably want the latest features.
> If packaged properly for stability, it is easy to disable it in
> the package recipe.

My concern was (if this has been used), user may get unstable APIs and without
explicitly being aware of it.

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Thomas Monjalon]

04/10/2018 17:28, Ferruh Yigit:
> On 10/4/2018 4:10 PM, Thomas Monjalon wrote:
> > 04/10/2018 17:48, Ferruh Yigit:
> >> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
> >> the current release and these APIs are targeted for further release.
> > 
> > It seems nobody is using it in last releases.
> > 
> >> RTE_NEXT_ABI shouldn't be enabled by default.
> > 
> > The reason for having it enabled by default is that when you build DPDK
> > yourself, you probably want the latest features.
> > If packaged properly for stability, it is easy to disable it in
> > the package recipe.
> 
> My concern was (if this has been used), user may get unstable APIs and without
> explicitly being aware of it.

I am OK with both defaults (enabled or disabled).

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Bruce Richardson]

On Thu, Oct 04, 2018 at 05:55:34PM +0200, Thomas Monjalon wrote:
> 04/10/2018 17:28, Ferruh Yigit:
> > On 10/4/2018 4:10 PM, Thomas Monjalon wrote:
> > > 04/10/2018 17:48, Ferruh Yigit:
> > >> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
> > >> the current release and these APIs are targeted for further release.
> > > 
> > > It seems nobody is using it in last releases.
> > > 
> > >> RTE_NEXT_ABI shouldn't be enabled by default.
> > > 
> > > The reason for having it enabled by default is that when you build DPDK
> > > yourself, you probably want the latest features.
> > > If packaged properly for stability, it is easy to disable it in
> > > the package recipe.
> > 
> > My concern was (if this has been used), user may get unstable APIs and without
> > explicitly being aware of it.
> 
> I am OK with both defaults (enabled or disabled).
> 
I'd keep it as is. As said, I'm not sure it's being used right now anyway.

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Ferruh Yigit]

On 10/5/2018 10:13 AM, Bruce Richardson wrote:
> On Thu, Oct 04, 2018 at 05:55:34PM +0200, Thomas Monjalon wrote:
>> 04/10/2018 17:28, Ferruh Yigit:
>>> On 10/4/2018 4:10 PM, Thomas Monjalon wrote:
>>>> 04/10/2018 17:48, Ferruh Yigit:
>>>>> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
>>>>> the current release and these APIs are targeted for further release.
>>>>
>>>> It seems nobody is using it in last releases.
>>>>
>>>>> RTE_NEXT_ABI shouldn't be enabled by default.
>>>>
>>>> The reason for having it enabled by default is that when you build DPDK
>>>> yourself, you probably want the latest features.
>>>> If packaged properly for stability, it is easy to disable it in
>>>> the package recipe.
>>>
>>> My concern was (if this has been used), user may get unstable APIs and without
>>> explicitly being aware of it.
>>
>> I am OK with both defaults (enabled or disabled).
>>
> I'd keep it as is. As said, I'm not sure it's being used right now anyway.

No, not used right now.
But I think we can use it, did you able to find chance to check:

https://mails.dpdk.org/archives/dev/2018-October/114372.html

Option D.

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Neil Horman]

On Fri, Oct 05, 2018 at 11:17:30AM +0100, Ferruh Yigit wrote:
> On 10/5/2018 10:13 AM, Bruce Richardson wrote:
> > On Thu, Oct 04, 2018 at 05:55:34PM +0200, Thomas Monjalon wrote:
> >> 04/10/2018 17:28, Ferruh Yigit:
> >>> On 10/4/2018 4:10 PM, Thomas Monjalon wrote:
> >>>> 04/10/2018 17:48, Ferruh Yigit:
> >>>>> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
> >>>>> the current release and these APIs are targeted for further release.
> >>>>
> >>>> It seems nobody is using it in last releases.
> >>>>
> >>>>> RTE_NEXT_ABI shouldn't be enabled by default.
> >>>>
> >>>> The reason for having it enabled by default is that when you build DPDK
> >>>> yourself, you probably want the latest features.
> >>>> If packaged properly for stability, it is easy to disable it in
> >>>> the package recipe.
> >>>
> >>> My concern was (if this has been used), user may get unstable APIs and without
> >>> explicitly being aware of it.
> >>
> >> I am OK with both defaults (enabled or disabled).
> >>
> > I'd keep it as is. As said, I'm not sure it's being used right now anyway.
> 
> No, not used right now.
> But I think we can use it, did you able to find chance to check:
> 
> https://mails.dpdk.org/archives/dev/2018-October/114372.html
> 
> Option D.
> 

Just to propose something else, We also have the ALLOW_EXPERIMENTAL_API flag
that we IIRC default to on.  Would it be worth consolidating these two
mechanisms into one?  Currently ALLOW_EXPERIMENTAL_API lets us flag symbols that
are not yet stable, and it seems to work well.  It does not however let us
simply define out structures/variables that might adversely affect the ABI.
Would it be worth considering adding a macro (something like
__rte_experimental_symbol()), that allows a variable/struct to be defined if
ALLOW_EXPERIMENTAL_API is set, and squashed otherwise?

Neil

Re: [PATCH v2] config: disable RTE_NEXT_ABI by default

[Ferruh Yigit]

On 10/5/2018 12:30 PM, Neil Horman wrote:
> On Fri, Oct 05, 2018 at 11:17:30AM +0100, Ferruh Yigit wrote:
>> On 10/5/2018 10:13 AM, Bruce Richardson wrote:
>>> On Thu, Oct 04, 2018 at 05:55:34PM +0200, Thomas Monjalon wrote:
>>>> 04/10/2018 17:28, Ferruh Yigit:
>>>>> On 10/4/2018 4:10 PM, Thomas Monjalon wrote:
>>>>>> 04/10/2018 17:48, Ferruh Yigit:
>>>>>>> Enabling RTE_NEXT_ABI means to enable APIs that break the ABI for
>>>>>>> the current release and these APIs are targeted for further release.
>>>>>>
>>>>>> It seems nobody is using it in last releases.
>>>>>>
>>>>>>> RTE_NEXT_ABI shouldn't be enabled by default.
>>>>>>
>>>>>> The reason for having it enabled by default is that when you build DPDK
>>>>>> yourself, you probably want the latest features.
>>>>>> If packaged properly for stability, it is easy to disable it in
>>>>>> the package recipe.
>>>>>
>>>>> My concern was (if this has been used), user may get unstable APIs and without
>>>>> explicitly being aware of it.
>>>>
>>>> I am OK with both defaults (enabled or disabled).
>>>>
>>> I'd keep it as is. As said, I'm not sure it's being used right now anyway.
>>
>> No, not used right now.
>> But I think we can use it, did you able to find chance to check:
>>
>> https://mails.dpdk.org/archives/dev/2018-October/114372.html
>>
>> Option D.
>>
> 
> Just to propose something else, We also have the ALLOW_EXPERIMENTAL_API flag
> that we IIRC default to on.  Would it be worth consolidating these two
> mechanisms into one?  Currently ALLOW_EXPERIMENTAL_API lets us flag symbols that
> are not yet stable, and it seems to work well.  It does not however let us
> simply define out structures/variables that might adversely affect the ABI.
> Would it be worth considering adding a macro (something like
> __rte_experimental_symbol()), that allows a variable/struct to be defined if
> ALLOW_EXPERIMENTAL_API is set, and squashed otherwise?

RTE_NEXT_ABI is not just for symbols.

If there a new API foo(), __rte_experimental works fine to mark it experimental.

But if there is an _existing API_
"bar(char)",

and we plan to change it to
"bar(int, int)",

to publish the change early in this release we need RTE_NEXT_ABI ifdef since
both can't exist together, so it will be used as:

Release N:

 #ifdef RTE_NEXT_ABI
 bar(int, int);
 #else
 bar(char);
 #endif


Release N + 1:

 bar(int, int);