From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 2CD8A1B408 for ; Tue, 6 Feb 2018 12:06:52 +0100 (CET) Received: by mail-wm0-f49.google.com with SMTP id v123so3006012wmd.5 for ; Tue, 06 Feb 2018 03:06:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=6IL6aZD7Lrs73327KLy+7FbMZ4ZRwqcCVRtv943m1bs=; b=YXQNzpufmcL14Tja2Amr/u2FTAcayIhsFkSAwIks1alZE5euCggFWFu0SmMR6sB6qv 5q6DsyXYaIguHhcf40uZFjxp6wh9YRYCbPKgHs4Ys33UEZZt8ZTUdDzOK44RHEBpUSMo uoHda37dQYgcwNN6Nymdo9HlNgXcLyHzjNSCTm/22HiZkg0YMT4LiRZndB+mww6i8AqS 32lg7SWXEdGJryfu/Hf9HXmTA1DtKl3HHSzp9gc463o0bRlUOFRduEB5lJVkEmmXq95c OI0In5eXIlIO9vZgz9pHfp1NySTKcjAhsrSEJNw7gLqsdh36VyZ2CGsniQKYYY5NHz6J YAxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=6IL6aZD7Lrs73327KLy+7FbMZ4ZRwqcCVRtv943m1bs=; b=VmNl69RNdxU5jOyZBhjwP4ArhomLb2Un7FXhlrzM8EMdwemREMq4oHruZqxliskdoS z42ebQmJgQJiC27ntYYRXM0PgvKa+LdESt7SXYgOnygFfC+ibTX0GmnH5GBMqDQMK/7Q 40Dig6hn8B1mv5rzPYUBesCjEnm9Zygm+Qo0H247oN6mJ7R9ZMIiwKrDiD4o//T718+h mIztapbCPOooI3h53Ph6MnQtRHm3TzDf9svyMKBtBDiFGzzvRjk1jLFsEhjmMyCwsknM jWgnv4dkf+bF/Wl8ObXvtlAGiNjCVIfrr4gyQa8YyHJXCRo+FNFK+NjatPyuM3BkuCk2 6tIQ== X-Gm-Message-State: APf1xPDuo4UEueOryvW9ABuJCz7p/EzNWKuHF8tqZyBRnKUvLJuFGHEz TsIOhiyoCMBG+jYBnkUOx1e1JmEl X-Google-Smtp-Source: AH8x226OjoDn7OGG/flTnnPfXhaJagWQkxxP/D9/TAB2/xGBwsKFSHJsoR0kX4UbRBWcthmBXLcMVw== X-Received: by 10.80.158.110 with SMTP id z101mr3222157ede.89.1517915211786; Tue, 06 Feb 2018 03:06:51 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id j17sm9941929ede.84.2018.02.06.03.06.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Feb 2018 03:06:50 -0800 (PST) Date: Tue, 6 Feb 2018 12:06:38 +0100 From: Adrien Mazarguil To: Marcelo Ricardo Leitner Cc: Thomas Monjalon , "Van Haaren, Harry" , "dev@dpdk.org" , Shahaf Shuler , Nelio Laranjeiro Message-ID: <20180206110638.GS4256@6wind.com> 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> <20180205170619.GI27676@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180205170619.GI27676@localhost.localdomain> 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: Tue, 06 Feb 2018 11:06:52 -0000 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