[dpdk-dev] [PATCH v2 3/4] net/mlx: version rdma-core glue libraries

Adrien Mazarguil adrien.mazarguil at 6wind.com
Tue Feb 6 12:06:38 CET 2018


On Mon, Feb 05, 2018 at 03:06:19PM -0200, Marcelo Ricardo Leitner wrote:
> On Mon, Feb 05, 2018 at 04:54:55PM +0100, Adrien Mazarguil wrote:
> > On Mon, Feb 05, 2018 at 01:29:42PM -0200, Marcelo Ricardo Leitner wrote:
> > > On Mon, Feb 05, 2018 at 03:59:18PM +0100, Adrien Mazarguil wrote:
> > > > On Mon, Feb 05, 2018 at 12:37:34PM -0200, Marcelo Ricardo Leitner wrote:
> > > > > On Mon, Feb 05, 2018 at 03:16:21PM +0100, Thomas Monjalon wrote:
> > > > > > 05/02/2018 14:44, Adrien Mazarguil:
> ...
> > > > > > > Using a weak one like CRC32 for a shorter name poses a risk of
> > > > > > > collision. Moreover the next time someone decides to update all version
> > > > > > > notices or modify a comment will impact that hash. We'd need to isolate the
> > > > > > > symbol definition itself, ignore parameter names in function prototypes and
> > > > > > > only then we may get a somewhat meaningful hash describing a given ABI.
> > > > > 
> > > > > That's what I meant with stricter. Yes it would catch such
> > > > > situations, but you tell me on how much we want to protect/restrict
> > > > > here.  Do you see a reason for building only the dpdk/pmd side and not
> > > > > the glue library at a time?
> > > > 
> > > > No, they're always built together. We're only adding this versioning to
> > > > avoid issues when users somehow end up with several DPDK versions installed
> > > > on their system, or with leftovers of previous releases lying around. That's
> > > > all we need to solve here. dlopen()'ing the proper file takes care of that,
> > > > the symbol version number check afterward is performed just in case.
> > > 
> > > Interesting. These leftovers probably wouldn't be there if it wasn't
> > > versioned in the first place. :-)
> > 
> > Seriously, we can't assume users will do everything using neat packages and
> > may run an unfortunate "make install" from the DPDK source tree without
> > noticing they wrecked their system. Someone will have to mop the ensuing but
> > preventable bug reports.
> > 
> > > > > > > Given the added complexity, is there really a problem with simple version
> > > > > > > numbers we increment every time something gets modified? (Note this is
> > > > > > > already how our .map files work, they're not generated automatically)
> > > > > > 
> > > > > > Our map files show the major version where a symbol was introduced.
> > > > > > It is simple because no symbol can be introduced in a minor version.
> > > > > > 
> > > > > > > How about keeping things as is?
> > > > > 
> > > > > I don't really see the need of unique filenames. The next patch is
> > > > > already leveraging RTE_EAL_PMD_PATH, which if versioned should be
> > > > > enough for this, no?
> > > > 
> > > > As you said, "if" versioned. As an undocumented empty string by default,
> > > > there's no way to be sure. Leaving the PMD version its internal but
> > > > (unfortunately) exposed bits will certainly prevent mistakes.
> > > > 
> > > > > > You are using 18.02.1 while it is introduced in 18.02.0.
> > > > > > If you don't want to correlate the .so version number with DPDK version
> > > > > > number, maybe that 1, 2, 3 would be a simpler choice (less confusing).
> > > > > 
> > > > > +1
> > > > 
> > > > Then are you fine with the "18.02.0" suffix?
> > > 
> > > Not really, sorry. It was more for the "1, 2, 3" sequence or tying it
> > > to dpdk version.
> > > 
> > > With the latest replies, I don't think the reasoning is enough to
> > > justify these extra checks, but I won't oppose to including it.
> > 
> > 18.02.0 makes it tied to the current release number, so I guess we agree.
> 
> It makes them equal, but not tied. If nobody patches it, when 18.02.1
> is out, the glue lib will still be 18.02.0.

Well this must be understood as "this plug-in implements 18.02.0's mlx4 glue
ABI", which remains true (and compatible) with subsequent DPDK releases as
long as the glue code is not updated.

Note this is no different from a single-digit suffix, which wouldn't be
updated either if the ABI isn't. Again, these initial digits are needed
because otherwise there is already a confusion with stable branches that
implement different ABIs and are therefore incompatible:

 librte_pmd_mlx4_glue.so.17.02.1
 librte_pmd_mlx4_glue.so.17.11.1
 librte_pmd_mlx4_glue.so.18.02.0

With a single digit, all of them would be named "librte_pmd_mlx4_glue.so.1",
rendering versioning basically useless.

> > The idea for now is this part remains tied to the DPDK release.
> > 
> > If a new ABI version is needed in a subsequent commit, the initial part gets
> > bumped to the current WIP DPDK release (say, 42.02.0). If subsequent
> > intermediate commits break the glue ABI, a fourth digit is added
> > (e.g. 42.02.0.1).
> 
> I'll defer this to other project developers. This is more about a
> project standard than anything here. I could even argue that this glue
> should be named after the pmd lib, such as
>    ./usr/local/lib/librte_pmd_mlx4_glue.so.1.1
> The fact of not providing the _glue.so symlink is enough to avoid
> others from linking against it. But it's more of a project standard
> than a technical decision, I guess, weather this lib is seen as a
> plugin or as a (private) library.

I think you nailed it, I call it a "plug-in" because dlopen() is manually
performed on it, however it's in fact a private library whose API is not
exposed and no application is supposed to use directly.

For this reason, while up to package maintainers, my suggestion is to not
install it in a public location like "/usr/local/lib" but configure
RTE_EAL_PMD_PATH to some DPDK-specific path, e.g. "/usr/share/dpdk/pmd",
which is possible since patch 4/4 of this series.

> Considering the versioning used for the PMD libs, such easy versioning
> is my preferred choice, FWIW.

Problem remains that the DPDK projects manages its own backports/stable
releases system instead of relying on package maintainers for that, so
properly versioning things from the beginning to avoid collisions is really
always a concern. Had backports not been a requirement in the first place,
I agree a single digit would have been enough.

My suggestion of using 18.02.0 (instead of 18.02.1) stands. It addresses
Thomas' concern by properly matching the DPDK release the ABI was last
updated for and mine for the backports issues mentioned above. Let's go with
that and move on.

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list