From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 3E4781B2A8 for ; Mon, 5 Feb 2018 18:06:23 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 495FF2820C; Mon, 5 Feb 2018 17:06:22 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-10.gru2.redhat.com [10.97.116.10]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AD7505D6B4; Mon, 5 Feb 2018 17:06:21 +0000 (UTC) Received: by localhost.localdomain (Postfix, from userid 1000) id BE364181132; Mon, 5 Feb 2018 15:06:19 -0200 (-02) Date: Mon, 5 Feb 2018 15:06:19 -0200 From: Marcelo Ricardo Leitner To: Adrien Mazarguil Cc: Thomas Monjalon , "Van Haaren, Harry" , "dev@dpdk.org" , Shahaf Shuler , Nelio Laranjeiro Message-ID: <20180205170619.GI27676@localhost.localdomain> References: <20180202144736.8239-1-adrien.mazarguil@6wind.com> <20180205125806.GE27676@localhost.localdomain> <20180205134446.GG4256@6wind.com> <1992025.8JADqR79Gx@xps> <20180205143734.GG27676@localhost.localdomain> <20180205145917.GJ4256@6wind.com> <20180205152942.GH27676@localhost.localdomain> <20180205155455.GN4256@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180205155455.GN4256@6wind.com> User-Agent: Mutt/1.9.1 (2017-09-22) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 05 Feb 2018 17:06:22 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 3/4] net/mlx: version rdma-core glue libraries X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Feb 2018 17:06:23 -0000 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