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. > 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. Considering the versioning used for the PMD libs, such easy versioning is my preferred choice, FWIW. > > This role is currently held by the third digit but since there's a confusion > with DPDK revisions, it won't be used internally by the PMD. Hopefully this > fourth digit will remain unused (otherwise I can add as many digits as > necessary to make it acceptable, I'll then re-consider the SHA1 idea :) hehe :-) Marcelo
