DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mk: enable next abi in static libs
@ 2015-07-02 22:05 Thomas Monjalon
  2015-07-06 13:18 ` Thomas Monjalon
  2015-07-08 14:55 ` [dpdk-dev] [PATCH v2 0/2] next abi option Thomas Monjalon
  0 siblings, 2 replies; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-02 22:05 UTC (permalink / raw)
  To: dev

When a change makes really hard to keep ABI compatibility,
instead of waiting next release to break the ABI, it is smoother
to introduce the new code and enable it only for static libraries.
The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
When the release is out, a dynamically linked application can use
the new shared libraries without rebuild while developpers can prepare
their application for the next ABI by reading the deprecation notice
and easily testing the new code.
When starting the next release cycle, the "ifdefs" will be removed
and the ABI break will be marked by incrementing LIBABIVER.

The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
It is automatically enabled for static libraries and disabled for
shared libraries.
It can be forced to another value by editing the generated .config file.
It shouldn't be enabled for shared libraries because it would break the
ABI without changing the version number LIBABIVER. That's why a warning
is printed in this case.

The guideline is also updated to integrate this new possibility.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 doc/guides/guidelines/versioning.rst | 2 ++
 lib/Makefile                         | 4 ++++
 mk/rte.sdkconfig.mk                  | 3 +++
 pkg/dpdk.spec                        | 1 +
 scripts/validate-abi.sh              | 2 ++
 5 files changed, 12 insertions(+)

diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
index a1c9368..6bc2a8e 100644
--- a/doc/guides/guidelines/versioning.rst
+++ b/doc/guides/guidelines/versioning.rst
@@ -57,6 +57,8 @@ being provided. The requirements for doing so are:
 
 #. A full deprecation cycle, as explained above, must be made to offer
    downstream consumers sufficient warning of the change.
+   The changes may be shown and used in static builds before the deprecation
+   cycle by conditioning them with RTE_NEXT_ABI option.
 
 #. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
    incorporated must be incremented in parallel with the ABI changes
diff --git a/lib/Makefile b/lib/Makefile
index 5f480f9..ebf56ba 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -31,6 +31,10 @@
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+ifeq '$(CONFIG_RTE_BUILD_SHARED_LIB)$(CONFIG_RTE_NEXT_ABI)' 'yy'
+$(info WARNING: Shared libraries versioning is tainted!)
+endif
+
 DIRS-y += librte_compat
 DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_MALLOC) += librte_malloc
diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index f8d95b1..135825c 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -77,6 +77,9 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT)
 		$(CPP) -undef -P -x assembler-with-cpp \
 		-ffreestanding \
 		-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
+		printf 'CONFIG_RTE_NEXT_ABI=' >> $(RTE_OUTPUT)/.config_tmp ; \
+		sed -n 's,CONFIG_RTE_BUILD_SHARED_LIB=,,p' $(RTE_OUTPUT)/.config_tmp | \
+		tr 'yn' 'ny' >> $(RTE_OUTPUT)/.config_tmp ; \
 		if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; then \
 			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
 			cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig ; \
diff --git a/pkg/dpdk.spec b/pkg/dpdk.spec
index 5f6ec6a..fb71ccc 100644
--- a/pkg/dpdk.spec
+++ b/pkg/dpdk.spec
@@ -82,6 +82,7 @@ make O=%{target} T=%{target} 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
 sed -ri 's,(LIBRTE_PMD_XENVIRT=).*,\1y,'   %{target}/.config
diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
index 1747b8b..4476433 100755
--- a/scripts/validate-abi.sh
+++ b/scripts/validate-abi.sh
@@ -157,6 +157,7 @@ git checkout $TAG1
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
 
@@ -198,6 +199,7 @@ git checkout $TAG2
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
 
-- 
2.4.2

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-02 22:05 [dpdk-dev] [PATCH] mk: enable next abi in static libs Thomas Monjalon
@ 2015-07-06 13:18 ` Thomas Monjalon
  2015-07-06 13:35   ` Neil Horman
  2015-07-08 14:55 ` [dpdk-dev] [PATCH v2 0/2] next abi option Thomas Monjalon
  1 sibling, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-06 13:18 UTC (permalink / raw)
  To: dev

Any comment or ack?

2015-07-03 00:05, Thomas Monjalon:
> When a change makes really hard to keep ABI compatibility,
> instead of waiting next release to break the ABI, it is smoother
> to introduce the new code and enable it only for static libraries.
> The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> When the release is out, a dynamically linked application can use
> the new shared libraries without rebuild while developpers can prepare
> their application for the next ABI by reading the deprecation notice
> and easily testing the new code.
> When starting the next release cycle, the "ifdefs" will be removed
> and the ABI break will be marked by incrementing LIBABIVER.
> 
> The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> It is automatically enabled for static libraries and disabled for
> shared libraries.
> It can be forced to another value by editing the generated .config file.
> It shouldn't be enabled for shared libraries because it would break the
> ABI without changing the version number LIBABIVER. That's why a warning
> is printed in this case.
> 
> The guideline is also updated to integrate this new possibility.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-06 13:18 ` Thomas Monjalon
@ 2015-07-06 13:35   ` Neil Horman
  2015-07-06 13:49     ` Thomas Monjalon
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Horman @ 2015-07-06 13:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> Any comment or ack?
> 
> 2015-07-03 00:05, Thomas Monjalon:
> > When a change makes really hard to keep ABI compatibility,
> > instead of waiting next release to break the ABI, it is smoother
> > to introduce the new code and enable it only for static libraries.
> > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > When the release is out, a dynamically linked application can use
> > the new shared libraries without rebuild while developpers can prepare
> > their application for the next ABI by reading the deprecation notice
> > and easily testing the new code.
> > When starting the next release cycle, the "ifdefs" will be removed
> > and the ABI break will be marked by incrementing LIBABIVER.
> > 
> > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > It is automatically enabled for static libraries and disabled for
> > shared libraries.
> > It can be forced to another value by editing the generated .config file.
> > It shouldn't be enabled for shared libraries because it would break the
> > ABI without changing the version number LIBABIVER. That's why a warning
> > is printed in this case.
> > 
> > The guideline is also updated to integrate this new possibility.
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> 
Yeah, I'm not sure why this is necessecary.  That is to say, if you want to
introduce a new ABI operation prior to the old one being removed, that is precisely what
the versioning macros are for, and can be used to map the static api to the new
version. e.g, given function X that you want to enhance in an ABI breaking way:

1) Separate function X to X_v1 and X_v2
2) Map X_v2 to X@DPDK_v2, map X_v1 to X@DPDK_v1
3) Map the static symbol X to X_v2
4) Post the deprecation notice of X for release 3 immediately

Splitting the static ABI from the shared ABI just means that applications will
have the opportunity to isolate themselves to one kind of build, which is a bad
idea.

Neil

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-06 13:35   ` Neil Horman
@ 2015-07-06 13:49     ` Thomas Monjalon
  2015-07-06 18:22       ` Neil Horman
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-06 13:49 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-07-06 09:35, Neil Horman:
> On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > Any comment or ack?
> > 
> > 2015-07-03 00:05, Thomas Monjalon:
> > > When a change makes really hard to keep ABI compatibility,
> > > instead of waiting next release to break the ABI, it is smoother
> > > to introduce the new code and enable it only for static libraries.
> > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > When the release is out, a dynamically linked application can use
> > > the new shared libraries without rebuild while developpers can prepare
> > > their application for the next ABI by reading the deprecation notice
> > > and easily testing the new code.
> > > When starting the next release cycle, the "ifdefs" will be removed
> > > and the ABI break will be marked by incrementing LIBABIVER.
> > > 
> > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > It is automatically enabled for static libraries and disabled for
> > > shared libraries.
> > > It can be forced to another value by editing the generated .config file.
> > > It shouldn't be enabled for shared libraries because it would break the
> > > ABI without changing the version number LIBABIVER. That's why a warning
> > > is printed in this case.
> > > 
> > > The guideline is also updated to integrate this new possibility.
> > > 
> > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > 
> > 
> Yeah, I'm not sure why this is necessecary.  That is to say, if you want to

It's explained at the beginning:
"When a change makes really hard to keep ABI compatibility", e.g. mbuf change.

> introduce a new ABI operation prior to the old one being removed, that is precisely what
> the versioning macros are for, and can be used to map the static api to the new
> version. e.g, given function X that you want to enhance in an ABI breaking way:
> 
> 1) Separate function X to X_v1 and X_v2
> 2) Map X_v2 to X@DPDK_v2, map X_v1 to X@DPDK_v1
> 3) Map the static symbol X to X_v2
> 4) Post the deprecation notice of X for release 3 immediately

We cannot do that for mbuf change.

> Splitting the static ABI from the shared ABI just means that applications will
> have the opportunity to isolate themselves to one kind of build, which is a bad
> idea.

It helps to be prepared for the next release by testing the app with static libraries.
We agreed to allow API breaking for important changes like mbuf rework.
This option NEXT_ABI is a step between announcement and effective ABI breaking.

I think it's a reasonnable approach. But if nobody ack it, I'm perfectly OK to
drop it and related features (unified packet type and interrupt mode).

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-06 13:49     ` Thomas Monjalon
@ 2015-07-06 18:22       ` Neil Horman
  2015-07-06 21:44         ` Thomas Monjalon
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Horman @ 2015-07-06 18:22 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> 2015-07-06 09:35, Neil Horman:
> > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > Any comment or ack?
> > > 
> > > 2015-07-03 00:05, Thomas Monjalon:
> > > > When a change makes really hard to keep ABI compatibility,
> > > > instead of waiting next release to break the ABI, it is smoother
> > > > to introduce the new code and enable it only for static libraries.
> > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > When the release is out, a dynamically linked application can use
> > > > the new shared libraries without rebuild while developpers can prepare
> > > > their application for the next ABI by reading the deprecation notice
> > > > and easily testing the new code.
> > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > 
> > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > It is automatically enabled for static libraries and disabled for
> > > > shared libraries.
> > > > It can be forced to another value by editing the generated .config file.
> > > > It shouldn't be enabled for shared libraries because it would break the
> > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > is printed in this case.
> > > > 
> > > > The guideline is also updated to integrate this new possibility.
> > > > 
> > > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > 
> > > 
> > Yeah, I'm not sure why this is necessecary.  That is to say, if you want to
> 
> It's explained at the beginning:
> "When a change makes really hard to keep ABI compatibility", e.g. mbuf change.
> 

Thats not what I was referring to.  I was referring to the need to split out
ABI's based on a separate config item only for static libraries.  I understand
that sometimes you want a 'preview' of the next abi.

> > introduce a new ABI operation prior to the old one being removed, that is precisely what
> > the versioning macros are for, and can be used to map the static api to the new
> > version. e.g, given function X that you want to enhance in an ABI breaking way:
> > 
> > 1) Separate function X to X_v1 and X_v2
> > 2) Map X_v2 to X@DPDK_v2, map X_v1 to X@DPDK_v1
> > 3) Map the static symbol X to X_v2
> > 4) Post the deprecation notice of X for release 3 immediately
> 
> We cannot do that for mbuf change.
> 
You can actually, its just alot of work.  Also, I know this doesn't relate very
closely to the subject, and I apologize, I was really just reacting to the
immediately preceding sentence in the origional post.

> > Splitting the static ABI from the shared ABI just means that applications will
> > have the opportunity to isolate themselves to one kind of build, which is a bad
> > idea.
> 
> It helps to be prepared for the next release by testing the app with static libraries.
> We agreed to allow API breaking for important changes like mbuf rework.
> This option NEXT_ABI is a step between announcement and effective ABI breaking.
> 
> I think it's a reasonnable approach. But if nobody ack it, I'm perfectly OK to
> drop it and related features (unified packet type and interrupt mode).
> 

I'd be ok with it iff:

1) It applies to static and shared ABI's together.  That is to say that setting
the NEXT_ABI config flag creates the same ABI changes regardless of other build
configuration.  It needs to be used in such a way that a consistent ABI is
presented when set, otherwise it won't be useful.

2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
the next ABI and the one after that, and the one after that, or it won't provide
an appropriate preview for anyone.


If we can meet those two standards, it would likely be a useful feature to have.
Neil

> 

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-06 18:22       ` Neil Horman
@ 2015-07-06 21:44         ` Thomas Monjalon
  2015-07-07 11:14           ` Neil Horman
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-06 21:44 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-07-06 14:22, Neil Horman:
> On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> > 2015-07-06 09:35, Neil Horman:
> > > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > > Any comment or ack?
> > > > 
> > > > 2015-07-03 00:05, Thomas Monjalon:
> > > > > When a change makes really hard to keep ABI compatibility,
> > > > > instead of waiting next release to break the ABI, it is smoother
> > > > > to introduce the new code and enable it only for static libraries.
> > > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > > When the release is out, a dynamically linked application can use
> > > > > the new shared libraries without rebuild while developpers can prepare
> > > > > their application for the next ABI by reading the deprecation notice
> > > > > and easily testing the new code.
> > > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > > 
> > > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > > It is automatically enabled for static libraries and disabled for
> > > > > shared libraries.
> > > > > It can be forced to another value by editing the generated .config file.
> > > > > It shouldn't be enabled for shared libraries because it would break the
> > > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > > is printed in this case.
> > > > > 
> > > > > The guideline is also updated to integrate this new possibility.
[...]
> I'd be ok with it iff:
> 
> 1) It applies to static and shared ABI's together.  That is to say that setting
> the NEXT_ABI config flag creates the same ABI changes regardless of other build
> configuration.  It needs to be used in such a way that a consistent ABI is
> presented when set, otherwise it won't be useful.

Yes the option trigger exactly the same ABI for static and shared libraries.
But it's too complicated (at least for 2.1) to make LIBABIVER and version map
dependant of this build-time option.
That's why, it should not be enabled to deploy shared libraries, though it can
be used for tests and development.
As static libraries are almost never packaged, they will be built and linked
at the same time. That's why users of static libraries tend to prefer the
newest ABI, which is the default in this case.

> 2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
> the next ABI and the one after that, and the one after that, or it won't provide
> an appropriate preview for anyone.

If you mean the next ABI must be promoted as the standard ABI in the next release,
yes: ifdefs will be cleaned when starting a new release.
Thanks, I learnt the english word hodgepodge :)

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-06 21:44         ` Thomas Monjalon
@ 2015-07-07 11:14           ` Neil Horman
  2015-07-07 12:46             ` Thomas Monjalon
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Horman @ 2015-07-07 11:14 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Mon, Jul 06, 2015 at 11:44:59PM +0200, Thomas Monjalon wrote:
> 2015-07-06 14:22, Neil Horman:
> > On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> > > 2015-07-06 09:35, Neil Horman:
> > > > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > > > Any comment or ack?
> > > > > 
> > > > > 2015-07-03 00:05, Thomas Monjalon:
> > > > > > When a change makes really hard to keep ABI compatibility,
> > > > > > instead of waiting next release to break the ABI, it is smoother
> > > > > > to introduce the new code and enable it only for static libraries.
> > > > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > > > When the release is out, a dynamically linked application can use
> > > > > > the new shared libraries without rebuild while developpers can prepare
> > > > > > their application for the next ABI by reading the deprecation notice
> > > > > > and easily testing the new code.
> > > > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > > > 
> > > > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > > > It is automatically enabled for static libraries and disabled for
> > > > > > shared libraries.
> > > > > > It can be forced to another value by editing the generated .config file.
> > > > > > It shouldn't be enabled for shared libraries because it would break the
> > > > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > > > is printed in this case.
> > > > > > 
> > > > > > The guideline is also updated to integrate this new possibility.
> [...]
> > I'd be ok with it iff:
> > 
> > 1) It applies to static and shared ABI's together.  That is to say that setting
> > the NEXT_ABI config flag creates the same ABI changes regardless of other build
> > configuration.  It needs to be used in such a way that a consistent ABI is
> > presented when set, otherwise it won't be useful.
> 
> Yes the option trigger exactly the same ABI for static and shared libraries.
> But it's too complicated (at least for 2.1) to make LIBABIVER and version map
> dependant of this build-time option.

No, I think thats a bridge too far.  I'm not sure whats difficult about
overriding LIBABIVER in lib.rte.mk and bump all numbers 1 higher (or better
still just add a .1 to the end of it), by checking CONFIG_NEXT_ABI

As for maintaining the version map, I don't see any problem with duplicating the
map files, to a -next variant, and changing the CPU_LDFLAGS in rte.lib.mk based
on the NEXT_ABI config option again.

In fact, if this is a thing that people want, that might be beneficial, as
something else occurs to me.  I think you're going to want this to be a mandated
piece of the update process.  That is to say, if someone wants to deprecate an
aspect of the ABI, or change it, I think you'll want to mandate that they submit
the change at the same time they submit the deprecation notice, and simply
protect it with this NEXT_ABI config option.  That provides several advantages:

1) It ensures that the notice is submitted at the same time as the actual change
2) It ensures that the NEXT_ABI provides a complete view of what the next ABI
version looks like, not just a partial view of it

Adding a *-version-next.map file for each library makes adhering to the above
easier, and allows for an easy converstion, in that when its time to officially
update the ABI, fixing the version map is a matter of copying
<library>-version-next.map to <library>-version.map.

The use case that I'm thinking of here is as such:

Consider two ABI modifying updates, A and B.

The author of A writes his changes, submits them with appropriate ifdefs for
CONFIG_NEXT_ABI, along with a deprecation notice.

The author of B writes his changes, but doesn't submit them, instead submitting
only a deprecation notice, with plans to post the actuall patches after the
deprecation notice is shipped in a release

After the release CONFIG_NEXT_ABI exposes the ABI changes made by A, but not by
B (because they don't yet exist in the code).  I think to give users a complete
view of the NEXT_ABI, changes to the ABI, should be done by submitting the patch
(gated on the NEXT_ABI config option), along with the deprecation notice, at the
same time.  That way the ABI view is complete and consistent.  And if you do the
above bits with the cloned version map and LIBABIVER bump, its also consistent
between shared and static libraries.

> That's why, it should not be enabled to deploy shared libraries, though it can
> be used for tests and development.
> As static libraries are almost never packaged, they will be built and linked
> at the same time. That's why users of static libraries tend to prefer the
> newest ABI, which is the default in this case.
> 
> > 2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
> > the next ABI and the one after that, and the one after that, or it won't provide
> > an appropriate preview for anyone.
> 
> If you mean the next ABI must be promoted as the standard ABI in the next release,
> yes: ifdefs will be cleaned when starting a new release.
> Thanks, I learnt the english word hodgepodge :)
> 
Je-mexcuse, une meli-melo? :)

I mean't what you indicate yes, and in addition to that, I just wanted to
clarify that this option could strictly _only_ apply to the very next ABI.  That
is to say, someone can't use this without also posting an ABI deprecation
notice, or we would find ourselves in a situation where something would only be
available in NEXT_ABI for more than one release, which would be unacceptable.
But I think we're saying the same thing.

> 

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-07 11:14           ` Neil Horman
@ 2015-07-07 12:46             ` Thomas Monjalon
  2015-07-07 13:11               ` Neil Horman
  2015-07-07 13:44               ` Neil Horman
  0 siblings, 2 replies; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-07 12:46 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Thanks Neil, we are making good progress.

2015-07-07 07:14, Neil Horman:
> On Mon, Jul 06, 2015 at 11:44:59PM +0200, Thomas Monjalon wrote:
> > 2015-07-06 14:22, Neil Horman:
> > > On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> > > > 2015-07-06 09:35, Neil Horman:
> > > > > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > > > > Any comment or ack?
> > > > > > 
> > > > > > 2015-07-03 00:05, Thomas Monjalon:
> > > > > > > When a change makes really hard to keep ABI compatibility,
> > > > > > > instead of waiting next release to break the ABI, it is smoother
> > > > > > > to introduce the new code and enable it only for static libraries.
> > > > > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > > > > When the release is out, a dynamically linked application can use
> > > > > > > the new shared libraries without rebuild while developpers can prepare
> > > > > > > their application for the next ABI by reading the deprecation notice
> > > > > > > and easily testing the new code.
> > > > > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > > > > 
> > > > > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > > > > It is automatically enabled for static libraries and disabled for
> > > > > > > shared libraries.
> > > > > > > It can be forced to another value by editing the generated .config file.
> > > > > > > It shouldn't be enabled for shared libraries because it would break the
> > > > > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > > > > is printed in this case.
> > > > > > > 
> > > > > > > The guideline is also updated to integrate this new possibility.
> > [...]
> > > I'd be ok with it iff:
> > > 
> > > 1) It applies to static and shared ABI's together.  That is to say that setting
> > > the NEXT_ABI config flag creates the same ABI changes regardless of other build
> > > configuration.  It needs to be used in such a way that a consistent ABI is
> > > presented when set, otherwise it won't be useful.
> > 
> > Yes the option trigger exactly the same ABI for static and shared libraries.
> > But it's too complicated (at least for 2.1) to make LIBABIVER and version map
> > dependant of this build-time option.
> 
> No, I think thats a bridge too far.  I'm not sure whats difficult about
> overriding LIBABIVER in lib.rte.mk and bump all numbers 1 higher (or better
> still just add a .1 to the end of it), by checking CONFIG_NEXT_ABI

Good idea, I will submit a v2 which adds .1.

> As for maintaining the version map, I don't see any problem with duplicating the
> map files, to a -next variant, and changing the CPU_LDFLAGS in rte.lib.mk based
> on the NEXT_ABI config option again.

OK

> In fact, if this is a thing that people want, that might be beneficial, as
> something else occurs to me.  I think you're going to want this to be a mandated
> piece of the update process.  That is to say, if someone wants to deprecate an
> aspect of the ABI, or change it, I think you'll want to mandate that they submit
> the change at the same time they submit the deprecation notice, and simply
> protect it with this NEXT_ABI config option.  That provides several advantages:

For the release 2.1, we have some deprecation notices without code. It was
the policy agreed in 2.0 release.
Maybe we can force code to be submitted with deprecation notices, starting
with release 2.2.
It needs to be amended in v2 of this patch.

> 1) It ensures that the notice is submitted at the same time as the actual change
> 2) It ensures that the NEXT_ABI provides a complete view of what the next ABI
> version looks like, not just a partial view of it

Yes it would be probably useful.

> Adding a *-version-next.map file for each library makes adhering to the above
> easier, and allows for an easy converstion, in that when its time to officially
> update the ABI, fixing the version map is a matter of copying
> <library>-version-next.map to <library>-version.map.

OK

[...]
> > That's why, it should not be enabled to deploy shared libraries, though it can
> > be used for tests and development.
> > As static libraries are almost never packaged, they will be built and linked
> > at the same time. That's why users of static libraries tend to prefer the
> > newest ABI, which is the default in this case.
> > 
> > > 2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
> > > the next ABI and the one after that, and the one after that, or it won't provide
> > > an appropriate preview for anyone.
> > 
> > If you mean the next ABI must be promoted as the standard ABI in the next release,
> > yes: ifdefs will be cleaned when starting a new release.
> > Thanks, I learnt the english word hodgepodge :)
> > 
> Je-mexcuse, une meli-melo? :)

Oui un meli-melo, un ramassis. un beau bordel en somme.

> I mean't what you indicate yes, and in addition to that, I just wanted to
> clarify that this option could strictly _only_ apply to the very next ABI.  That
> is to say, someone can't use this without also posting an ABI deprecation
> notice, or we would find ourselves in a situation where something would only be
> available in NEXT_ABI for more than one release, which would be unacceptable.
> But I think we're saying the same thing.

Yes. I'll try to make it clear in v2.

Neil, in the meantime, could you please help to check ABI breakage in the HEAD?

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-07 12:46             ` Thomas Monjalon
@ 2015-07-07 13:11               ` Neil Horman
  2015-07-07 13:44               ` Neil Horman
  1 sibling, 0 replies; 24+ messages in thread
From: Neil Horman @ 2015-07-07 13:11 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Jul 07, 2015 at 05:46:08AM -0700, Thomas Monjalon wrote:
> Thanks Neil, we are making good progress.
> 
> 2015-07-07 07:14, Neil Horman:
> > On Mon, Jul 06, 2015 at 11:44:59PM +0200, Thomas Monjalon wrote:
> > > 2015-07-06 14:22, Neil Horman:
> > > > On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> > > > > 2015-07-06 09:35, Neil Horman:
> > > > > > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > > > > > Any comment or ack?
> > > > > > > 
> > > > > > > 2015-07-03 00:05, Thomas Monjalon:
> > > > > > > > When a change makes really hard to keep ABI compatibility,
> > > > > > > > instead of waiting next release to break the ABI, it is smoother
> > > > > > > > to introduce the new code and enable it only for static libraries.
> > > > > > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > > > > > When the release is out, a dynamically linked application can use
> > > > > > > > the new shared libraries without rebuild while developpers can prepare
> > > > > > > > their application for the next ABI by reading the deprecation notice
> > > > > > > > and easily testing the new code.
> > > > > > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > > > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > > > > > 
> > > > > > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > > > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > > > > > It is automatically enabled for static libraries and disabled for
> > > > > > > > shared libraries.
> > > > > > > > It can be forced to another value by editing the generated .config file.
> > > > > > > > It shouldn't be enabled for shared libraries because it would break the
> > > > > > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > > > > > is printed in this case.
> > > > > > > > 
> > > > > > > > The guideline is also updated to integrate this new possibility.
> > > [...]
> > > > I'd be ok with it iff:
> > > > 
> > > > 1) It applies to static and shared ABI's together.  That is to say that setting
> > > > the NEXT_ABI config flag creates the same ABI changes regardless of other build
> > > > configuration.  It needs to be used in such a way that a consistent ABI is
> > > > presented when set, otherwise it won't be useful.
> > > 
> > > Yes the option trigger exactly the same ABI for static and shared libraries.
> > > But it's too complicated (at least for 2.1) to make LIBABIVER and version map
> > > dependant of this build-time option.
> > 
> > No, I think thats a bridge too far.  I'm not sure whats difficult about
> > overriding LIBABIVER in lib.rte.mk and bump all numbers 1 higher (or better
> > still just add a .1 to the end of it), by checking CONFIG_NEXT_ABI
> 
> Good idea, I will submit a v2 which adds .1.
> 
> > As for maintaining the version map, I don't see any problem with duplicating the
> > map files, to a -next variant, and changing the CPU_LDFLAGS in rte.lib.mk based
> > on the NEXT_ABI config option again.
> 
> OK
> 
> > In fact, if this is a thing that people want, that might be beneficial, as
> > something else occurs to me.  I think you're going to want this to be a mandated
> > piece of the update process.  That is to say, if someone wants to deprecate an
> > aspect of the ABI, or change it, I think you'll want to mandate that they submit
> > the change at the same time they submit the deprecation notice, and simply
> > protect it with this NEXT_ABI config option.  That provides several advantages:
> 
> For the release 2.1, we have some deprecation notices without code. It was
> the policy agreed in 2.0 release.
Right, we're stuck with that for the next release, but this idea of yours I
think will help make sure we get both together in the future.

> Maybe we can force code to be submitted with deprecation notices, starting
> with release 2.2.
> It needs to be amended in v2 of this patch.
Yes, I think that makes sense

> 
> > 1) It ensures that the notice is submitted at the same time as the actual change
> > 2) It ensures that the NEXT_ABI provides a complete view of what the next ABI
> > version looks like, not just a partial view of it
> 
> Yes it would be probably useful.
> 
> > Adding a *-version-next.map file for each library makes adhering to the above
> > easier, and allows for an easy converstion, in that when its time to officially
> > update the ABI, fixing the version map is a matter of copying
> > <library>-version-next.map to <library>-version.map.
> 
> OK
> 
> [...]
> > > That's why, it should not be enabled to deploy shared libraries, though it can
> > > be used for tests and development.
> > > As static libraries are almost never packaged, they will be built and linked
> > > at the same time. That's why users of static libraries tend to prefer the
> > > newest ABI, which is the default in this case.
> > > 
> > > > 2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
> > > > the next ABI and the one after that, and the one after that, or it won't provide
> > > > an appropriate preview for anyone.
> > > 
> > > If you mean the next ABI must be promoted as the standard ABI in the next release,
> > > yes: ifdefs will be cleaned when starting a new release.
> > > Thanks, I learnt the english word hodgepodge :)
> > > 
> > Je-mexcuse, une meli-melo? :)
> 
> Oui un meli-melo, un ramassis. un beau bordel en somme.
> 
exactement! :)

> > I mean't what you indicate yes, and in addition to that, I just wanted to
> > clarify that this option could strictly _only_ apply to the very next ABI.  That
> > is to say, someone can't use this without also posting an ABI deprecation
> > notice, or we would find ourselves in a situation where something would only be
> > available in NEXT_ABI for more than one release, which would be unacceptable.
> > But I think we're saying the same thing.
> 
> Yes. I'll try to make it clear in v2.
> 
> Neil, in the meantime, could you please help to check ABI breakage in the HEAD?
I'll try to take a look today
Neil

> 

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-07 12:46             ` Thomas Monjalon
  2015-07-07 13:11               ` Neil Horman
@ 2015-07-07 13:44               ` Neil Horman
  2015-07-10 16:07                 ` Mcnamara, John
  1 sibling, 1 reply; 24+ messages in thread
From: Neil Horman @ 2015-07-07 13:44 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Jul 07, 2015 at 05:46:08AM -0700, Thomas Monjalon wrote:
> Thanks Neil, we are making good progress.
> 
> 2015-07-07 07:14, Neil Horman:
> > On Mon, Jul 06, 2015 at 11:44:59PM +0200, Thomas Monjalon wrote:
> > > 2015-07-06 14:22, Neil Horman:
> > > > On Mon, Jul 06, 2015 at 03:49:50PM +0200, Thomas Monjalon wrote:
> > > > > 2015-07-06 09:35, Neil Horman:
> > > > > > On Mon, Jul 06, 2015 at 03:18:51PM +0200, Thomas Monjalon wrote:
> > > > > > > Any comment or ack?
> > > > > > > 
> > > > > > > 2015-07-03 00:05, Thomas Monjalon:
> > > > > > > > When a change makes really hard to keep ABI compatibility,
> > > > > > > > instead of waiting next release to break the ABI, it is smoother
> > > > > > > > to introduce the new code and enable it only for static libraries.
> > > > > > > > The flag RTE_NEXT_ABI may be used to "ifdef" the new code.
> > > > > > > > When the release is out, a dynamically linked application can use
> > > > > > > > the new shared libraries without rebuild while developpers can prepare
> > > > > > > > their application for the next ABI by reading the deprecation notice
> > > > > > > > and easily testing the new code.
> > > > > > > > When starting the next release cycle, the "ifdefs" will be removed
> > > > > > > > and the ABI break will be marked by incrementing LIBABIVER.
> > > > > > > > 
> > > > > > > > The new option CONFIG_RTE_NEXT_ABI is not defined in the configuration
> > > > > > > > templates because it is deduced from CONFIG_RTE_BUILD_SHARED_LIB.
> > > > > > > > It is automatically enabled for static libraries and disabled for
> > > > > > > > shared libraries.
> > > > > > > > It can be forced to another value by editing the generated .config file.
> > > > > > > > It shouldn't be enabled for shared libraries because it would break the
> > > > > > > > ABI without changing the version number LIBABIVER. That's why a warning
> > > > > > > > is printed in this case.
> > > > > > > > 
> > > > > > > > The guideline is also updated to integrate this new possibility.
> > > [...]
> > > > I'd be ok with it iff:
> > > > 
> > > > 1) It applies to static and shared ABI's together.  That is to say that setting
> > > > the NEXT_ABI config flag creates the same ABI changes regardless of other build
> > > > configuration.  It needs to be used in such a way that a consistent ABI is
> > > > presented when set, otherwise it won't be useful.
> > > 
> > > Yes the option trigger exactly the same ABI for static and shared libraries.
> > > But it's too complicated (at least for 2.1) to make LIBABIVER and version map
> > > dependant of this build-time option.
> > 
> > No, I think thats a bridge too far.  I'm not sure whats difficult about
> > overriding LIBABIVER in lib.rte.mk and bump all numbers 1 higher (or better
> > still just add a .1 to the end of it), by checking CONFIG_NEXT_ABI
> 
> Good idea, I will submit a v2 which adds .1.
> 
> > As for maintaining the version map, I don't see any problem with duplicating the
> > map files, to a -next variant, and changing the CPU_LDFLAGS in rte.lib.mk based
> > on the NEXT_ABI config option again.
> 
> OK
> 
> > In fact, if this is a thing that people want, that might be beneficial, as
> > something else occurs to me.  I think you're going to want this to be a mandated
> > piece of the update process.  That is to say, if someone wants to deprecate an
> > aspect of the ABI, or change it, I think you'll want to mandate that they submit
> > the change at the same time they submit the deprecation notice, and simply
> > protect it with this NEXT_ABI config option.  That provides several advantages:
> 
> For the release 2.1, we have some deprecation notices without code. It was
> the policy agreed in 2.0 release.
> Maybe we can force code to be submitted with deprecation notices, starting
> with release 2.2.
> It needs to be amended in v2 of this patch.
> 
> > 1) It ensures that the notice is submitted at the same time as the actual change
> > 2) It ensures that the NEXT_ABI provides a complete view of what the next ABI
> > version looks like, not just a partial view of it
> 
> Yes it would be probably useful.
> 
> > Adding a *-version-next.map file for each library makes adhering to the above
> > easier, and allows for an easy converstion, in that when its time to officially
> > update the ABI, fixing the version map is a matter of copying
> > <library>-version-next.map to <library>-version.map.
> 
> OK
> 
> [...]
> > > That's why, it should not be enabled to deploy shared libraries, though it can
> > > be used for tests and development.
> > > As static libraries are almost never packaged, they will be built and linked
> > > at the same time. That's why users of static libraries tend to prefer the
> > > newest ABI, which is the default in this case.
> > > 
> > > > 2) It only applies to the next ABI.  That is to say, it can't be a hodgepodge of
> > > > the next ABI and the one after that, and the one after that, or it won't provide
> > > > an appropriate preview for anyone.
> > > 
> > > If you mean the next ABI must be promoted as the standard ABI in the next release,
> > > yes: ifdefs will be cleaned when starting a new release.
> > > Thanks, I learnt the english word hodgepodge :)
> > > 
> > Je-mexcuse, une meli-melo? :)
> 
> Oui un meli-melo, un ramassis. un beau bordel en somme.
> 
> > I mean't what you indicate yes, and in addition to that, I just wanted to
> > clarify that this option could strictly _only_ apply to the very next ABI.  That
> > is to say, someone can't use this without also posting an ABI deprecation
> > notice, or we would find ourselves in a situation where something would only be
> > available in NEXT_ABI for more than one release, which would be unacceptable.
> > But I think we're saying the same thing.
> 
> Yes. I'll try to make it clear in v2.
> 
> Neil, in the meantime, could you please help to check ABI breakage in the HEAD?
> 
Took a look, the only ABI break I see that we need to worry about is the one
introduced in commit 8eecb3295aed0a979def52245564d03be172a83c. It adds a
bitfield called lro into the existing uint8_t there, but does so in the middle
of the set, which pushes the other bits around, breaking ABI.  It should have
been added to the end.

Neil

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

* [dpdk-dev] [PATCH v2 0/2] next abi option
  2015-07-02 22:05 [dpdk-dev] [PATCH] mk: enable next abi in static libs Thomas Monjalon
  2015-07-06 13:18 ` Thomas Monjalon
@ 2015-07-08 14:55 ` Thomas Monjalon
  2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 1/2] mk: remove variables identical to config ones Thomas Monjalon
                     ` (2 more replies)
  1 sibling, 3 replies; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-08 14:55 UTC (permalink / raw)
  To: nhorman; +Cc: dev

This is the second version of the NEXT_ABI policy.
It can now be used for shared and static libraries.

While updating rte.lib.mk, it appeared that some useless
(and not consistent) variables were used for some config
options. The first patch clean them.

Thomas Monjalon (2):
  mk: remove variables identical to config ones
  mk: enable next abi preview

 config/common_bsdapp                 |  5 +++++
 config/common_linuxapp               |  5 +++++
 doc/guides/guidelines/versioning.rst | 12 +++++++++---
 mk/exec-env/bsdapp/rte.vars.mk       |  4 ++--
 mk/exec-env/linuxapp/rte.vars.mk     |  4 ++--
 mk/rte.lib.mk                        | 16 +++++++++-------
 mk/rte.sdkbuild.mk                   |  2 +-
 mk/rte.sharelib.mk                   |  8 ++++----
 mk/rte.vars.mk                       |  8 --------
 pkg/dpdk.spec                        |  1 +
 scripts/validate-abi.sh              |  2 ++
 11 files changed, 40 insertions(+), 27 deletions(-)

-- 
2.4.2

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

* [dpdk-dev] [PATCH v2 1/2] mk: remove variables identical to config ones
  2015-07-08 14:55 ` [dpdk-dev] [PATCH v2 0/2] next abi option Thomas Monjalon
@ 2015-07-08 14:55   ` Thomas Monjalon
  2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 2/2] mk: enable next abi preview Thomas Monjalon
  2015-07-08 16:50   ` [dpdk-dev] [PATCH v2 0/2] next abi option Neil Horman
  2 siblings, 0 replies; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-08 14:55 UTC (permalink / raw)
  To: nhorman; +Cc: dev

CONFIG_RTE_BUILD_SHARED_LIB and CONFIG_RTE_BUILD_COMBINE_LIBS does not
have quotes in their values (only y or n). That's why the variables
RTE_BUILD_SHARED_LIB and RTE_BUILD_COMBINE_LIBS are always identical to
their CONFIG_ counterpart, and are useless.
In order to have consistent naming of config options in the makefiles,
these options are removed and the "CONFIG_ prefixed" variables are used.

Fixes: e25e4d7ef16b ("mk: shared libraries")
Fixes: 4d3d79e7a5c6 ("mk: combined library")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/exec-env/bsdapp/rte.vars.mk   |  4 ++--
 mk/exec-env/linuxapp/rte.vars.mk |  4 ++--
 mk/rte.lib.mk                    | 12 ++++++------
 mk/rte.sdkbuild.mk               |  2 +-
 mk/rte.sharelib.mk               |  8 ++++----
 mk/rte.vars.mk                   |  8 --------
 6 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/mk/exec-env/bsdapp/rte.vars.mk b/mk/exec-env/bsdapp/rte.vars.mk
index aed0e18..47a673e 100644
--- a/mk/exec-env/bsdapp/rte.vars.mk
+++ b/mk/exec-env/bsdapp/rte.vars.mk
@@ -39,7 +39,7 @@
 #
 # examples for RTE_EXEC_ENV: linuxapp, bsdapp
 #
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 EXECENV_CFLAGS  = -pthread -fPIC
 else
 EXECENV_CFLAGS  = -pthread
@@ -49,7 +49,7 @@ EXECENV_LDFLAGS =
 EXECENV_LDLIBS  = -lexecinfo
 EXECENV_ASFLAGS =
 
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 EXECENV_LDLIBS += -lgcc_s
 endif
 
diff --git a/mk/exec-env/linuxapp/rte.vars.mk b/mk/exec-env/linuxapp/rte.vars.mk
index e5af318..5fd7d85 100644
--- a/mk/exec-env/linuxapp/rte.vars.mk
+++ b/mk/exec-env/linuxapp/rte.vars.mk
@@ -39,7 +39,7 @@
 #
 # examples for RTE_EXEC_ENV: linuxapp, bsdapp
 #
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 EXECENV_CFLAGS  = -pthread -fPIC
 else
 EXECENV_CFLAGS  = -pthread
@@ -51,7 +51,7 @@ EXECENV_LDFLAGS = --no-as-needed
 EXECENV_LDLIBS  = -lrt -lm
 EXECENV_ASFLAGS =
 
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 EXECENV_LDLIBS += -lgcc_s
 endif
 
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index 25aa989..fff62a7 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -37,7 +37,7 @@ include $(RTE_SDK)/mk/internal/rte.depdirs-pre.mk
 
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 
 LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
 CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
@@ -87,7 +87,7 @@ O_TO_S_DO = @set -e; \
 	$(O_TO_S) && \
 	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
 
-ifeq ($(RTE_BUILD_SHARED_LIB),n)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
 O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
 O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
@@ -110,7 +110,7 @@ lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
 #
 # Archive objects in .a file if needed
 #
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
 ifeq ($(LIBABIVER),)
 	@echo "Must Specify a $(LIB) ABI version"
@@ -130,7 +130,7 @@ endif
 		$(depfile_newer)),\
 		$(O_TO_S_DO))
 
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
+ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
 	$(if $(or \
         $(file_missing),\
         $(call cmdline_changed,$(O_TO_C_STR)),\
@@ -153,7 +153,7 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
 	    $(depfile_missing),\
 	    $(depfile_newer)),\
 	    $(O_TO_A_DO))
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
+ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
 	$(if $(or \
         $(file_missing),\
         $(call cmdline_changed,$(O_TO_C_STR)),\
@@ -171,7 +171,7 @@ $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
 	@echo "  INSTALL-LIB $(LIB)"
 	@[ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib
 	$(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 	$(Q)ln -s -f $< $(RTE_OUTPUT)/lib/$(LIBSONAME)
 endif
 
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index 352f738..38ec7bd 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -93,7 +93,7 @@ $(ROOTDIRS-y):
 	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
 	@echo "== Build $@"
 	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
-	@if [ $@ = drivers -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
+	@if [ $@ = drivers -a $(CONFIG_RTE_BUILD_COMBINE_LIBS) = y ]; then \
 		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
 	fi
 
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index de53558..7bb7219 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -34,8 +34,8 @@ include $(RTE_SDK)/mk/internal/rte.build-pre.mk
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
 
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 LIB_ONE := lib$(RTE_LIBNAME).so
 else
 LIB_ONE := lib$(RTE_LIBNAME).a
@@ -75,8 +75,8 @@ O_TO_A_DO = @set -e; \
 # Archive objects to share library
 #
 
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 $(LIB_ONE): FORCE
 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
 	$(O_TO_S_DO)
diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
index d2f01b6..0469064 100644
--- a/mk/rte.vars.mk
+++ b/mk/rte.vars.mk
@@ -63,14 +63,6 @@ ifneq ($(BUILDING_RTE_SDK),)
   RTE_TOOLCHAIN := $(CONFIG_RTE_TOOLCHAIN:"%"=%)
   RTE_TARGET := $(RTE_ARCH)-$(RTE_MACHINE)-$(RTE_EXEC_ENV)-$(RTE_TOOLCHAIN)
   RTE_SDK_BIN := $(RTE_OUTPUT)
-  RTE_BUILD_SHARED_LIB := $(CONFIG_RTE_BUILD_SHARED_LIB:"%"=%)
-  ifeq ($(RTE_BUILD_SHARED_LIB),)
-    RTE_BUILD_SHARED_LIB := n
-  endif
-  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
-  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
-    RTE_BUILD_COMBINE_LIBS := n
-  endif
 endif
 
 RTE_LIBNAME := $(CONFIG_RTE_LIBNAME:"%"=%)
-- 
2.4.2

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

* [dpdk-dev] [PATCH v2 2/2] mk: enable next abi preview
  2015-07-08 14:55 ` [dpdk-dev] [PATCH v2 0/2] next abi option Thomas Monjalon
  2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 1/2] mk: remove variables identical to config ones Thomas Monjalon
@ 2015-07-08 14:55   ` Thomas Monjalon
  2015-07-08 16:44     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
  2015-07-08 16:50   ` [dpdk-dev] [PATCH v2 0/2] next abi option Neil Horman
  2 siblings, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-08 14:55 UTC (permalink / raw)
  To: nhorman; +Cc: dev

When a change makes really hard to keep ABI compatibility,
instead of waiting next release to break the ABI, it is smoother
to introduce the new code as a preview and disable it when packaging.
The flag RTE_NEXT_ABI must be used to "ifdef" the new code.
When the release is out, a dynamically linked application can use
the new shared libraries with the old ABI while developpers can prepare
their application for the next ABI by reading the deprecation notice
and easily testing the new code.
When starting the next release cycle, the "ifdefs" will be removed
and the ABI break will be marked by incrementing LIBABIVER. The map
files will also be updated.

The default value is enabled to be developer compliant.
The packagers must disable it as done in pkg/dpdk.spec.
When enabled, all shared library numbers are incremented by appending
a minor .1 to the old ABI number. In the next release, only impacted
libraries will have a major +1 increment.
The impacted libraries must provide an alternative map file to use
with this option.

The ABI policy is updated.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 config/common_bsdapp                 |  5 +++++
 config/common_linuxapp               |  5 +++++
 doc/guides/guidelines/versioning.rst | 12 +++++++++---
 mk/rte.lib.mk                        |  6 ++++--
 pkg/dpdk.spec                        |  1 +
 scripts/validate-abi.sh              |  2 ++
 6 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/config/common_bsdapp b/config/common_bsdapp
index 78754b2..a4e3262 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -90,6 +90,11 @@ CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_LIBNAME=intel_dpdk
 
 #
+# Use newest code breaking previous ABI
+#
+CONFIG_RTE_NEXT_ABI=y
+
+#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/config/common_linuxapp b/config/common_linuxapp
index f5646e0..050bf35 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -90,6 +90,11 @@ CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_LIBNAME="intel_dpdk"
 
 #
+# Use newest code breaking previous ABI
+#
+CONFIG_RTE_NEXT_ABI=y
+
+#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
index ea789cb..8a739dd 100644
--- a/doc/guides/guidelines/versioning.rst
+++ b/doc/guides/guidelines/versioning.rst
@@ -55,12 +55,18 @@ being provided. The requirements for doing so are:
 #. At least 3 acknowledgments of the need to do so must be made on the
    dpdk.org mailing list.
 
+#. 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.
 
-#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
-   incorporated must be incremented in parallel with the ABI changes
-   themselves.
+#. 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
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index fff62a7..0ad32cb 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -37,11 +37,13 @@ include $(RTE_SDK)/mk/internal/rte.depdirs-pre.mk
 
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
-ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
+ifeq ($(CONFIG_RTE_NEXT_ABI),y)
+LIB := $(LIB).1
+endif
 CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
-
 endif
 
 
diff --git a/pkg/dpdk.spec b/pkg/dpdk.spec
index 5f6ec6a..fb71ccc 100644
--- a/pkg/dpdk.spec
+++ b/pkg/dpdk.spec
@@ -82,6 +82,7 @@ make O=%{target} T=%{target} 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
 sed -ri 's,(LIBRTE_PMD_XENVIRT=).*,\1y,'   %{target}/.config
diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
index 1747b8b..4476433 100755
--- a/scripts/validate-abi.sh
+++ b/scripts/validate-abi.sh
@@ -157,6 +157,7 @@ git checkout $TAG1
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
 
@@ -198,6 +199,7 @@ git checkout $TAG2
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
 
-- 
2.4.2

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

* [dpdk-dev] [PATCH v3] mk: enable next abi preview
  2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 2/2] mk: enable next abi preview Thomas Monjalon
@ 2015-07-08 16:44     ` Thomas Monjalon
  2015-07-13  7:32       ` Mcnamara, John
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-08 16:44 UTC (permalink / raw)
  To: nhorman; +Cc: dev

When a change makes really hard to keep ABI compatibility,
instead of waiting next release to break the ABI, it is smoother
to introduce the new code as a preview and disable it when packaging.
The flag RTE_NEXT_ABI must be used to "ifdef" the new code.
When the release is out, a dynamically linked application can use
the new shared libraries with the old ABI while developpers can prepare
their application for the next ABI by reading the deprecation notice
and easily testing the new code.
When starting the next release cycle, the "ifdefs" will be removed
and the ABI break will be marked by incrementing LIBABIVER. The map
files will also be updated.

The default value is enabled to be developer compliant.
The packagers must disable it as done in pkg/dpdk.spec.
When enabled, all shared library numbers are incremented by appending
a minor .1 to the old ABI number. In the next release, only impacted
libraries will have a major +1 increment.
The impacted libraries must provide an alternative map file to use
with this option.

The ABI policy is updated.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
v3 change:
- fix .so symbolic link to .so.x.1

 config/common_bsdapp                 |  5 +++++
 config/common_linuxapp               |  5 +++++
 doc/guides/guidelines/versioning.rst | 12 +++++++++---
 mk/rte.lib.mk                        |  9 +++++----
 pkg/dpdk.spec                        |  1 +
 scripts/validate-abi.sh              |  2 ++
 6 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/config/common_bsdapp b/config/common_bsdapp
index 78754b2..a4e3262 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -90,6 +90,11 @@ CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_LIBNAME=intel_dpdk
 
 #
+# Use newest code breaking previous ABI
+#
+CONFIG_RTE_NEXT_ABI=y
+
+#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/config/common_linuxapp b/config/common_linuxapp
index f5646e0..050bf35 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -90,6 +90,11 @@ CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_LIBNAME="intel_dpdk"
 
 #
+# Use newest code breaking previous ABI
+#
+CONFIG_RTE_NEXT_ABI=y
+
+#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/doc/guides/guidelines/versioning.rst b/doc/guides/guidelines/versioning.rst
index ea789cb..8a739dd 100644
--- a/doc/guides/guidelines/versioning.rst
+++ b/doc/guides/guidelines/versioning.rst
@@ -55,12 +55,18 @@ being provided. The requirements for doing so are:
 #. At least 3 acknowledgments of the need to do so must be made on the
    dpdk.org mailing list.
 
+#. 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.
 
-#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
-   incorporated must be incremented in parallel with the ABI changes
-   themselves.
+#. 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
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index fff62a7..f15de9b 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -37,11 +37,13 @@ include $(RTE_SDK)/mk/internal/rte.depdirs-pre.mk
 
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
-ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
 LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))
+ifeq ($(CONFIG_RTE_NEXT_ABI),y)
+LIB := $(LIB).1
+endif
 CPU_LDFLAGS += --version-script=$(SRCDIR)/$(EXPORT_MAP)
-
 endif
 
 
@@ -167,12 +169,11 @@ endif
 # install lib in $(RTE_OUTPUT)/lib
 #
 $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
-	$(eval LIBSONAME := $(basename $(LIB)))
 	@echo "  INSTALL-LIB $(LIB)"
 	@[ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib
 	$(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
-	$(Q)ln -s -f $< $(RTE_OUTPUT)/lib/$(LIBSONAME)
+	$(Q)ln -s -f $< $(basename $(basename $@))
 endif
 
 #
diff --git a/pkg/dpdk.spec b/pkg/dpdk.spec
index 5f6ec6a..fb71ccc 100644
--- a/pkg/dpdk.spec
+++ b/pkg/dpdk.spec
@@ -82,6 +82,7 @@ make O=%{target} T=%{target} 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
 sed -ri 's,(LIBRTE_PMD_XENVIRT=).*,\1y,'   %{target}/.config
diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
index 1747b8b..4476433 100755
--- a/scripts/validate-abi.sh
+++ b/scripts/validate-abi.sh
@@ -157,6 +157,7 @@ git checkout $TAG1
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
 
@@ -198,6 +199,7 @@ git checkout $TAG2
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
+sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
 sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
 
-- 
2.4.2

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

* Re: [dpdk-dev] [PATCH v2 0/2] next abi option
  2015-07-08 14:55 ` [dpdk-dev] [PATCH v2 0/2] next abi option Thomas Monjalon
  2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 1/2] mk: remove variables identical to config ones Thomas Monjalon
  2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 2/2] mk: enable next abi preview Thomas Monjalon
@ 2015-07-08 16:50   ` Neil Horman
  2015-07-08 22:58     ` Thomas Monjalon
  2 siblings, 1 reply; 24+ messages in thread
From: Neil Horman @ 2015-07-08 16:50 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Wed, Jul 08, 2015 at 04:55:21PM +0200, Thomas Monjalon wrote:
> This is the second version of the NEXT_ABI policy.
> It can now be used for shared and static libraries.
> 
> While updating rte.lib.mk, it appeared that some useless
> (and not consistent) variables were used for some config
> options. The first patch clean them.
> 
> Thomas Monjalon (2):
>   mk: remove variables identical to config ones
>   mk: enable next abi preview
> 
>  config/common_bsdapp                 |  5 +++++
>  config/common_linuxapp               |  5 +++++
>  doc/guides/guidelines/versioning.rst | 12 +++++++++---
>  mk/exec-env/bsdapp/rte.vars.mk       |  4 ++--
>  mk/exec-env/linuxapp/rte.vars.mk     |  4 ++--
>  mk/rte.lib.mk                        | 16 +++++++++-------
>  mk/rte.sdkbuild.mk                   |  2 +-
>  mk/rte.sharelib.mk                   |  8 ++++----
>  mk/rte.vars.mk                       |  8 --------
>  pkg/dpdk.spec                        |  1 +
>  scripts/validate-abi.sh              |  2 ++
>  11 files changed, 40 insertions(+), 27 deletions(-)
> 
> -- 
> 2.4.2
> 
> 
For the series
Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

* Re: [dpdk-dev] [PATCH v2 0/2] next abi option
  2015-07-08 16:50   ` [dpdk-dev] [PATCH v2 0/2] next abi option Neil Horman
@ 2015-07-08 22:58     ` Thomas Monjalon
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-08 22:58 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

2015-07-08 12:50, Neil Horman:
> On Wed, Jul 08, 2015 at 04:55:21PM +0200, Thomas Monjalon wrote:
> > This is the second version of the NEXT_ABI policy.
> > It can now be used for shared and static libraries.
> > 
> > While updating rte.lib.mk, it appeared that some useless
> > (and not consistent) variables were used for some config
> > options. The first patch clean them.
> > 
> > Thomas Monjalon (2):
> >   mk: remove variables identical to config ones
> >   mk: enable next abi preview
> 
> For the series
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Applied, thanks Neil for your help

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-07 13:44               ` Neil Horman
@ 2015-07-10 16:07                 ` Mcnamara, John
  2015-07-11 14:19                   ` Neil Horman
  0 siblings, 1 reply; 24+ messages in thread
From: Mcnamara, John @ 2015-07-10 16:07 UTC (permalink / raw)
  To: Neil Horman, Thomas Monjalon; +Cc: dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, July 7, 2015 2:44 PM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
> 
> On Tue, Jul 07, 2015 at 05:46:08AM -0700, Thomas Monjalon wrote:
> > Neil, in the meantime, could you please help to check ABI breakage in
> the HEAD?
> >
> Took a look, the only ABI break I see that we need to worry about is the
> one introduced in commit 8eecb3295aed0a979def52245564d03be172a83c. It adds
> a bitfield called lro into the existing uint8_t there, but does so in the
> middle of the set, which pushes the other bits around, breaking ABI.  It
> should have been added to the end.

Hi,

Is it okay to submit a patch to move it to the end?

John.
-- 

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-10 16:07                 ` Mcnamara, John
@ 2015-07-11 14:19                   ` Neil Horman
  2015-07-13 10:14                     ` Mcnamara, John
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Horman @ 2015-07-11 14:19 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev

On Fri, Jul 10, 2015 at 04:07:53PM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Tuesday, July 7, 2015 2:44 PM
> > To: Thomas Monjalon
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
> > 
> > On Tue, Jul 07, 2015 at 05:46:08AM -0700, Thomas Monjalon wrote:
> > > Neil, in the meantime, could you please help to check ABI breakage in
> > the HEAD?
> > >
> > Took a look, the only ABI break I see that we need to worry about is the
> > one introduced in commit 8eecb3295aed0a979def52245564d03be172a83c. It adds
> > a bitfield called lro into the existing uint8_t there, but does so in the
> > middle of the set, which pushes the other bits around, breaking ABI.  It
> > should have been added to the end.
> 
> Hi,
> 
> Is it okay to submit a patch to move it to the end?
> 
Assuming that fixes the problem, I think thats the only thing you can do right
now.  I expect that will work, but I would run it through the ABI checker to be
certain
Neil


> John.
> -- 
> 
> 
> 
> 

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

* Re: [dpdk-dev] [PATCH v3] mk: enable next abi preview
  2015-07-08 16:44     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
@ 2015-07-13  7:32       ` Mcnamara, John
  2015-07-13  8:48         ` Thomas Monjalon
  0 siblings, 1 reply; 24+ messages in thread
From: Mcnamara, John @ 2015-07-13  7:32 UTC (permalink / raw)
  To: Thomas Monjalon, nhorman; +Cc: dev


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, July 8, 2015 5:44 PM
> To: nhorman@tuxdriver.com
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3] mk: enable next abi preview
> 
> --- a/scripts/validate-abi.sh
> +++ b/scripts/validate-abi.sh
> @@ -157,6 +157,7 @@ git checkout $TAG1
>  # Make sure we configure SHARED libraries  # Also turn off IGB and KNI as
> those require kernel headers to build  sed -i -e"$
> a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
> +sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
>  sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET  sed -i
> -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
> 

Hi,

This change to enable CONFIG_RTE_NEXT_ABI=n breaks validate-abi.sh because master won't compile with CONFIG_RTE_BUILD_SHARED_LIB=y and CONFIG_RTE_NEXT_ABI=n:

     make uninstall && \
     make T=x86_64-native-linuxapp-gcc install -j \
          CONFIG_RTE_BUILD_SHARED_LIB=y CONFIG_RTE_NEXT_ABI=n

Change was made in: 506f51cc0da7e057ac31e15048ba3b8015112226

John.
-- 

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

* Re: [dpdk-dev] [PATCH v3] mk: enable next abi preview
  2015-07-13  7:32       ` Mcnamara, John
@ 2015-07-13  8:48         ` Thomas Monjalon
  2015-07-13  9:02           ` [dpdk-dev] [PATCH] mk: fix shared lib build with stable abi Thomas Monjalon
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-13  8:48 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev

2015-07-13 07:32, Mcnamara, John:
> This change to enable CONFIG_RTE_NEXT_ABI=n breaks validate-abi.sh
> because master won't compile with CONFIG_RTE_BUILD_SHARED_LIB=y and
> CONFIG_RTE_NEXT_ABI=n:

My bad. I thought I was testing both cases (next ABI and stable one) but
it appears only the "next one" was tested.

The error is trivial:
-       $(Q)ln -s -f $< $(RTE_OUTPUT)/lib/$(LIBSONAME)
+       $(Q)ln -s -f $< $(basename $(basename $@))

The double basename should apply to NEXT_ABI case only.

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

* [dpdk-dev] [PATCH] mk: fix shared lib build with stable abi
  2015-07-13  8:48         ` Thomas Monjalon
@ 2015-07-13  9:02           ` Thomas Monjalon
  2015-07-13  9:24             ` Mcnamara, John
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-13  9:02 UTC (permalink / raw)
  To: John McNamara; +Cc: dev

When next ABI is enabled, the shared lib extension is .so.x.1.
That's why a double basename was introduced.
But the "ifeq NEXT_ABI" was forgotten, removing the .so
extension when NEXT_ABI is disabled.
It was preventing the linker from finding the .so libraries.

Fixes: 506f51cc0da7 ("mk: enable next abi preview")

Reported-by: John McNamara <john.mcnamara@intel.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.lib.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index f15de9b..9ff5cce 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -173,7 +173,11 @@ $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
 	@[ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib
 	$(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
+ifeq ($(CONFIG_RTE_NEXT_ABI),y)
 	$(Q)ln -s -f $< $(basename $(basename $@))
+else
+	$(Q)ln -s -f $< $(basename $@)
+endif
 endif
 
 #
-- 
2.4.2

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

* Re: [dpdk-dev] [PATCH] mk: fix shared lib build with stable abi
  2015-07-13  9:02           ` [dpdk-dev] [PATCH] mk: fix shared lib build with stable abi Thomas Monjalon
@ 2015-07-13  9:24             ` Mcnamara, John
  2015-07-13  9:32               ` Thomas Monjalon
  0 siblings, 1 reply; 24+ messages in thread
From: Mcnamara, John @ 2015-07-13  9:24 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, July 13, 2015 10:02 AM
> To: Mcnamara, John
> Cc: dev@dpdk.org
> Subject: [PATCH] mk: fix shared lib build with stable abi
> 
> When next ABI is enabled, the shared lib extension is .so.x.1.
> That's why a double basename was introduced.
> But the "ifeq NEXT_ABI" was forgotten, removing the .so extension when
> NEXT_ABI is disabled.
> It was preventing the linker from finding the .so libraries.
> 
> Fixes: 506f51cc0da7 ("mk: enable next abi preview")
> 
> Reported-by: John McNamara <john.mcnamara@intel.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

That fixes it. Thanks.

Acked-by: John McNamara <john.mcnamara@intel.com>

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

* Re: [dpdk-dev] [PATCH] mk: fix shared lib build with stable abi
  2015-07-13  9:24             ` Mcnamara, John
@ 2015-07-13  9:32               ` Thomas Monjalon
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Monjalon @ 2015-07-13  9:32 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev

> > When next ABI is enabled, the shared lib extension is .so.x.1.
> > That's why a double basename was introduced.
> > But the "ifeq NEXT_ABI" was forgotten, removing the .so extension when
> > NEXT_ABI is disabled.
> > It was preventing the linker from finding the .so libraries.
> > 
> > Fixes: 506f51cc0da7 ("mk: enable next abi preview")
> > 
> > Reported-by: John McNamara <john.mcnamara@intel.com>
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> That fixes it. Thanks.
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>

Applied, thanks.
Now that next ABI can be disabled, patches using it may be applied :)

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

* Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
  2015-07-11 14:19                   ` Neil Horman
@ 2015-07-13 10:14                     ` Mcnamara, John
  0 siblings, 0 replies; 24+ messages in thread
From: Mcnamara, John @ 2015-07-13 10:14 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Saturday, July 11, 2015 3:20 PM
> To: Mcnamara, John
> Cc: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
> 
> On Fri, Jul 10, 2015 at 04:07:53PM +0000, Mcnamara, John wrote:
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > > Sent: Tuesday, July 7, 2015 2:44 PM
> > > To: Thomas Monjalon
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] mk: enable next abi in static libs
> > >
> ...
>
> > Is it okay to submit a patch to move it to the end?
> >
> Assuming that fixes the problem, I think thats the only thing you can do
> right now.  I expect that will work, but I would run it through the ABI
> checker to be certain.

Hi Neil,

I ran the change through the abi checker before and after and the lro field no longer generates a Medium Severity ABI warning. I'll submit a patch to move the field.

John.
-- 

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

end of thread, other threads:[~2015-07-13 10:14 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-02 22:05 [dpdk-dev] [PATCH] mk: enable next abi in static libs Thomas Monjalon
2015-07-06 13:18 ` Thomas Monjalon
2015-07-06 13:35   ` Neil Horman
2015-07-06 13:49     ` Thomas Monjalon
2015-07-06 18:22       ` Neil Horman
2015-07-06 21:44         ` Thomas Monjalon
2015-07-07 11:14           ` Neil Horman
2015-07-07 12:46             ` Thomas Monjalon
2015-07-07 13:11               ` Neil Horman
2015-07-07 13:44               ` Neil Horman
2015-07-10 16:07                 ` Mcnamara, John
2015-07-11 14:19                   ` Neil Horman
2015-07-13 10:14                     ` Mcnamara, John
2015-07-08 14:55 ` [dpdk-dev] [PATCH v2 0/2] next abi option Thomas Monjalon
2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 1/2] mk: remove variables identical to config ones Thomas Monjalon
2015-07-08 14:55   ` [dpdk-dev] [PATCH v2 2/2] mk: enable next abi preview Thomas Monjalon
2015-07-08 16:44     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
2015-07-13  7:32       ` Mcnamara, John
2015-07-13  8:48         ` Thomas Monjalon
2015-07-13  9:02           ` [dpdk-dev] [PATCH] mk: fix shared lib build with stable abi Thomas Monjalon
2015-07-13  9:24             ` Mcnamara, John
2015-07-13  9:32               ` Thomas Monjalon
2015-07-08 16:50   ` [dpdk-dev] [PATCH v2 0/2] next abi option Neil Horman
2015-07-08 22:58     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).