DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Kinsella, Ray" <ray.kinsella@intel.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>,
	Luca Boccassi <bluca@debian.org>
Cc: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	David Marchand <david.marchand@redhat.com>,
	"Christian Ehrhardt" <christian.ehrhardt@canonical.com>,
	Timothy Redaelli <tredaelli@redhat.com>,
	Kevin Traynor <ktraynor@redhat.com>, dpdk-dev <dev@dpdk.org>,
	"Laatz, Kevin" <kevin.laatz@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Neil Horman <nhorman@redhat.com>
Subject: Re: [dpdk-dev] How to manage new APIs added after major ABI release?
Date: Tue, 10 Dec 2019 17:01:08 +0000	[thread overview]
Message-ID: <BN6PR11MB41457EF7AF3202E6C8323273905B0@BN6PR11MB4145.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20191210163216.GA119@bricha3-MOBL.ger.corp.intel.com>

First, good find, much kudos.
To fix, I tend to agree with Bruce.

The SONAME should be comprised of the library name and the major version number, period.
As described in the DPDK documentation.

I too would be concerned about our forgetting to make the change in 20.11.

Also, what would happen when DPDK 20.05 roles around.
In DPDK 20.05's case the SONAME would still be 20.0
However the library file name would be stamped libeal_20.2.so ... 

Better to drop the minor version to avoid any ambiguity.
I would suggest a respin of DPDK 19.11 to fix.

Ray K

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday 10 December 2019 16:32
> To: Luca Boccassi <bluca@debian.org>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Kinsella, Ray
> <ray.kinsella@intel.com>; Thomas Monjalon <thomas@monjalon.net>; David
> Marchand <david.marchand@redhat.com>; Christian Ehrhardt
> <christian.ehrhardt@canonical.com>; Timothy Redaelli
> <tredaelli@redhat.com>; Kevin Traynor <ktraynor@redhat.com>; dpdk-dev
> <dev@dpdk.org>; Laatz, Kevin <kevin.laatz@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Neil Horman <nhorman@redhat.com>
> Subject: Re: [dpdk-dev] How to manage new APIs added after major ABI
> release?
> 
> On Tue, Dec 10, 2019 at 04:20:41PM +0000, Luca Boccassi wrote:
> > On Tue, 2019-12-10 at 15:46 +0000, Bruce Richardson wrote:
> > > On Tue, Dec 10, 2019 at 03:03:51PM +0000, Luca Boccassi wrote:
> > > > On Tue, 2019-12-10 at 14:36 +0000, Bruce Richardson wrote:
> > > > > On Tue, Dec 10, 2019 at 12:40:53PM +0000, Ferruh Yigit wrote:
> > > > > > On 12/10/2019 12:04 PM, Bruce Richardson wrote:
> > > > > > > On Tue, Dec 10, 2019 at 11:56:28AM +0000, Ferruh Yigit
> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > With new process, the major ABI releases will be
> > > > > > > > compatible until it is deprecated (until next LTS for
> > > > > > > > now), like current ABI version is 20 in DPDK_19.11 and
> > > > > > > > DPDK versions until DPDK_20.11 will be ABI compatible
> with
> > > > > > > > this version.
> > > > > > > >
> > > > > > > > But if we introduce a new API after major ABI, say in
> > > > > > > > 20.02 release, are we allowed to break the ABI for that
> > > > > > > > API before DPDK_20.11?
> > > > > > > >
> > > > > > > > If we allow it break, following problem will be observed:
> > > > > > > > Assume an application using .so.20.1 library, and using
> > > > > > > > the new API introduced in 20.02, lets say foo(), but when
> > > > > > > > application switches to .so.20.2 (released via
> > > > > > > > DPDK_20.05), application will fail because of ABI
> breakage
> > > > > > > > in foo().
> > > > > > > >
> > > > > > > > I think it is fair that application expects forward
> > > > > > > > compatibility in minor versions of a shared library.
> > > > > > > > Like if application linked against .so.20.2, fair to
> > > > > > > > expect .so.20.3, .so.20.4 etc will work fine. I think
> > > > > > > > currently only .so.20.0 is fully forward compatible.
> > > > > > > >
> > > > > > > > If we all agree on this, we may need to tweak the process
> > > > > > > > a little, but before diving into implementation details,
> I
> > > > > > > > would like to be sure we are in same page.
> > > > > > > >
> > > > > > >
> > > > > > > Well, any new API's generally come in as experimental, in
> > > > > > > which case changes are allowed, and breakage can be
> > > > > > > expected. If they are not experiemental, then the ABI
> policy
> > > > > > > applies to them in that they cannot change since they are
> > > > > > > part of the .21 ABI, even if that ABI is not fully complete
> > > > > > > yet. For any application only using stable, non-
> > > > > > > experimental functions, forward compatibility must be
> > > > > > > maintained IMHO.
> > > > > > >
> > > > > >
> > > > > > Talking about not experimental APIs, experimental ones free
> > > > > > from the process.
> > > > > >
> > > > > > And when and API added in 20.02 (ABI_20.1) it is kind of
> still
> > > > > > ABI_20, because it should be supported for following
> ABI_20.x,
> > > > > > instead of calling it ABI_21, and this minor tweak (and mind
> > > > > > shift) in .map files can be our solution.
> > > > >
> > > > > Related at what to do with adding versions between major ABI
> > > > > versions, when investigating with Kevin the ABI checking we
> have
> > > > > made an unpleasant
> > > > > discovery:
> > > > >
> > > > > This minor version bumping from 20.0 to 20.1 has apparently
> > > > > already broken our ABI according to libabigail.
> > > > >
> > > > > The Gory Details [skip to the end for suggestions to fix]
> > > > > ------------------------------------------------------------
> > > > >
> > > > > The reason for this is that the soversion encoded in each
> > > > > library
> > > > > -
> > > > > whether
> > > > > built with meson or make - is the full 20.0 version, not just
> > > > > the major ABI
> > > > > .20 part. Then when apps link against DPDK, they actually
> encode
> > > > > the 20.0.
> > > > >
> > > > > So what this means is that currently - using a make build as an
> > > > > example here - ldd on the latest head build gives:
> > > > >
> > > > >  LD_LIBRARY_PATH=$(pwd)/x86_64-native-linux-gcc/lib ldd x86_64-
> > > > > native-linux-gcc/app/testpmd | head
> > > > >         linux-vdso.so.1 (0x00007fff6813d000)
> > > > >         librte_pmd_bond.so.20.1 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pmd_bond.so.20.1
> (0x00007f36d723c000)
> > > > >         librte_pmd_dpaa.so.20.1 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pmd_dpaa.so.20.1
> (0x00007f36d7229000)
> > > > >         librte_mempool_dpaa.so.20.1 =>
> > > > > /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_mempool_dpaa.so.20.1
> > > > > (0x00007f36d7224000)
> > > > >         librte_pmd_ixgbe.so.20.1 =>
> /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pmd_ixgbe.so.20.1
> > > > > (0x00007f36d71ba000)
> > > > >         librte_pmd_i40e.so.20.1 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pmd_i40e.so.20.1
> (0x00007f36d7126000)
> > > > >         librte_pmd_bnxt.so.20.1 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pmd_bnxt.so.20.1
> (0x00007f36d70e5000)
> > > > >         librte_pmd_softnic.so.20.1 =>
> > > > > /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pmd_softnic.so.20.1
> > > > > (0x00007f36d70b7000)
> > > > >         librte_flow_classify.so.0.201 =>
> > > > > /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_flow_classify.so.0.201
> > > > > (0x00007f36d70b1000)
> > > > >         librte_pipeline.so.20.1 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc/lib/librte_pipeline.so.20.1
> > > > > (0x00007f36d7088000) ...
> > > > >
> > > > > Similarly ldd on a 19.11 checkout gives:
> > > > >
> > > > > LD_LIBRARY_PATH=$(pwd)/x86_64-native-linux-gcc_v19.11/lib ldd
> > > > > x86_64-
> > > > > native-linux-gcc_v19.11/app/testpmd | head
> > > > >         linux-vdso.so.1 (0x00007ffc2a964000)
> > > > >         librte_pmd_bond.so.20.0 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pmd_bond.so.20.0
> > > > > (0x00007fd4dc6b6000)
> > > > >         librte_pmd_dpaa.so.20.0 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pmd_dpaa.so.20.0
> > > > > (0x00007fd4dc6a3000)
> > > > >         librte_mempool_dpaa.so.20.0 =>
> > > > > /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_mempool_dpaa.so.20.0
> > > > > (0x00007fd4dc69e000)
> > > > >         librte_pmd_ixgbe.so.20.0 =>
> /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pmd_ixgbe.so.20.0
> > > > > (0x00007fd4dc634000)
> > > > >         librte_pmd_i40e.so.20.0 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pmd_i40e.so.20.0
> > > > > (0x00007fd4dc5a0000)
> > > > >         librte_pmd_bnxt.so.20.0 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pmd_bnxt.so.20.0
> > > > > (0x00007fd4dc55d000)
> > > > >         librte_pmd_softnic.so.20.0 =>
> > > > > /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pmd_softnic.so.20.0
> > > > > (0x00007fd4dc531000)
> > > > >         librte_flow_classify.so.0.200 =>
> > > > > /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_flow_classify.so.0.200
> > > > > (0x00007fd4dc52b000)
> > > > >         librte_pipeline.so.20.0 => /home/bruce/dpdk.org/x86_64-
> > > > > native-linux-gcc_v19.11/lib/librte_pipeline.so.20.0
> > > > > (0x00007fd4dc502000)
> > > > >
> > > > > The final check - using the 19.11 compiled testpmd with the
> > > > > library path set to 20.02 versionned libs:
> > > > >
> > > > > LD_LIBRARY_PATH=$(pwd)/x86_64-native-linux-gcc/lib ldd x86_64-
> > > > > native-
> > > > > linux-gcc_v19.11/app/testpmd | head
> > > > >         linux-vdso.so.1 (0x00007ffc711fc000)
> > > > >         librte_pmd_bond.so.20.0 => not found
> > > > >         librte_pmd_dpaa.so.20.0 => not found
> > > > >         librte_mempool_dpaa.so.20.0 => not found
> > > > >         librte_pmd_ixgbe.so.20.0 => not found
> > > > >         librte_pmd_i40e.so.20.0 => not found
> > > > >         librte_pmd_bnxt.so.20.0 => not found
> > > > >         librte_pmd_softnic.so.20.0 => not found
> > > > >         librte_flow_classify.so.0.200 => not found
> > > > >         librte_pipeline.so.20.0 => not found
> > > > >
> > > > > Fixing This
> > > > > -----------
> > > > >
> > > > > To fix this, we need to ensure that the SONAME remains constant
> > > > > across the releases. Therefore, I currently see two options:
> > > > >
> > > > > 1. keep 20.0 as the version and soname across all releases in
> > > > > 2020, i.e.
> > > > >   just revert the ABIVERSION change patch. Trouble there is how
> > > > > to track
> > > > >   20.02 vs 20.05 etc. etc.
> > > > >
> > > > > 2. remove the .0, .1 from the SONAMES stored in the libraries.
> > > > > This
> > > > > has the
> > > > >   advantage of keeping the existing planned schemes, but has
> the
> > > > > really big
> > > > >   downside of breaking ABI compatibility with anyone who has
> > > > > already
> > > > >   compiled with 19.11.
> > > > >
> > > > > Personally, of the two options - unless someone can come up
> with
> > > > > a third option - I'd tend towards the second, fixing the builds
> > > > > to remove the
> > > > > .0 in
> > > > > the soname, and releasing that ASAP as 19.11.1 before 19.11
> gets
> > > > > widespread adoption. Since this ABI stability is new, teething
> > > > > problems may be expected.
> > > > >
> > > > > Thoughts and comments?
> > > > > /Bruce
> > > > >
> > > > > BTW: For meson, the patch for option 2 is just to remove the
> > > > > so_version variable and all references to it from
> > > > > lib/meson.build and drivers/meson.build. Haven't looked into a
> > > > > "make" fix yet.
> > > >
> > > > Hi,
> > > >
> > > > With libtool and its (arguably arcane) format, only the first
> > > > digit is the ABI current version and gets encoded in the elf
> > > > header. The other digits can be used to track compatible minor
> > > > increments, and are mostly ignored. On the system a symlink
> > > > libfoo.so.major -> libfoo.so.major.minor is added.
> > > >
> > > > Eg:
> > > >
> > > > $ readelf -d /usr/lib/x86_64-linux-gnu/libzmq.so.5.2.3 | grep
> > > > SONAME
> > > >  0x000000000000000e (SONAME)             Library soname:
> > > > [libzmq.so.5]
> > > > $ ls -l /usr/lib/x86_64-linux-gnu/libzmq.so.5
> > > > lrwxrwxrwx 1 root root 15 Dec 31  2014 /usr/lib/x86_64-linux-
> > > > gnu/libzmq.so.5 -> libzmq.so.5.2.3
> > > >
> > > > Can we do the same? Not sure what the right incantation is for
> > > > Meson, but it should be possibly.
> > > >
> > >
> > > That's essentially option 2, and it's still an ABI break because
> > > existing
> > > builds of 19.11 have the soname will the full version number in it.
> > > The
> > > default behaviour for meson is exactly how you described it, except
> > > that
> > > previously we needed more exact control over the version info (for
> > > your
> > > dpdk-specific versions in the sonames) and so overrode the
> soversion
> > > explicitly. The fix for meson is to remove this overriding i.e.
> > > remove
> > > "soversion:" parameter for each shared_library() call.
> > >
> > >
> > > > Also, we should leave the current at 20.0 - let's not break
> > > > compatibility already, please :-)
> > > >
> > >
> > > If we do this, maybe we can use 20.0.1 and 20.0.2 version numbers?
> >
> > Yes, that's what I meant - IMHO we should just take the hit and use
> the
> > slightly weird 20.0 format until next year, and add a third digit for
> > compatible updates. Then for v21 we can drop it.
> >
> My concern with that is us forgetting, because we'll put in place hacks
> to
> have the soversion be the first two numbers of the version. Then we
> need to
> remember to remove those hacks before 20.11 goes out or we'll end up
> with
> 21.0 being the soversion again.
> 
> For that reason, I'd rather see us fix it now before 19.11 gets widely
> adopted.
> 
> /Bruce

  reply	other threads:[~2019-12-12  9:55 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10 11:56 Ferruh Yigit
2019-12-10 12:04 ` Bruce Richardson
2019-12-10 12:40   ` Ferruh Yigit
2019-12-10 14:36     ` Bruce Richardson
2019-12-10 15:03       ` Luca Boccassi
2019-12-10 15:46         ` Bruce Richardson
2019-12-10 16:20           ` Luca Boccassi
2019-12-10 16:32             ` Bruce Richardson
2019-12-10 17:01               ` Kinsella, Ray [this message]
2019-12-10 17:04               ` Thomas Monjalon
2019-12-10 18:22                 ` Luca Boccassi
2019-12-10 23:34                   ` Bruce Richardson
2019-12-10 16:39             ` Thomas Monjalon
2019-12-10 17:00               ` Bruce Richardson
2019-12-10 15:04       ` Ferruh Yigit
2019-12-10 15:37         ` Ferruh Yigit
2019-12-10 15:40       ` Kinsella, Ray
2019-12-11 13:32       ` Neil Horman
2019-12-11 13:11 ` Neil Horman
2019-12-11 13:29   ` Thomas Monjalon
2019-12-11 13:30   ` Ferruh Yigit
2019-12-11 14:34     ` Neil Horman
2019-12-11 15:29       ` Ferruh Yigit
2019-12-11 15:02     ` Thomas Monjalon
2019-12-11 15:17       ` Bruce Richardson
2019-12-11 15:46       ` Ferruh Yigit
2019-12-11 15:55         ` Bruce Richardson
2019-12-11 16:30           ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN6PR11MB41457EF7AF3202E6C8323273905B0@BN6PR11MB4145.namprd11.prod.outlook.com \
    --to=ray.kinsella@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=bluca@debian.org \
    --cc=bruce.richardson@intel.com \
    --cc=christian.ehrhardt@canonical.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=kevin.laatz@intel.com \
    --cc=ktraynor@redhat.com \
    --cc=nhorman@redhat.com \
    --cc=thomas@monjalon.net \
    --cc=tredaelli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).