DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Marcelo Ricardo Leitner <mleitner@redhat.com>,
	"Van Haaren, Harry" <harry.van.haaren@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Shahaf Shuler <shahafs@mellanox.com>,
	Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v2 3/4] net/mlx: version rdma-core glue libraries
Date: Mon, 05 Feb 2018 15:16:21 +0100	[thread overview]
Message-ID: <1992025.8JADqR79Gx@xps> (raw)
In-Reply-To: <20180205134446.GG4256@6wind.com>

05/02/2018 14:44, Adrien Mazarguil:
> On Mon, Feb 05, 2018 at 10:58:06AM -0200, Marcelo Ricardo Leitner wrote:
> > On Mon, Feb 05, 2018 at 12:24:23PM +0000, Van Haaren, Harry wrote:
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Marcelo Ricardo Leitner
> > > > Sent: Monday, February 5, 2018 12:14 PM
> > > > To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > Cc: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Shahaf Shuler
> > > > <shahafs@mellanox.com>; Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > Subject: Re: [dpdk-dev] [PATCH v2 3/4] net/mlx: version rdma-core glue
> > > > libraries
> > > > 
> > > > On Mon, Feb 05, 2018 at 12:24:02PM +0100, Adrien Mazarguil wrote:
> > > > > On Sun, Feb 04, 2018 at 03:29:38PM +0100, Thomas Monjalon wrote:
> > > > > > 02/02/2018 17:46, Adrien Mazarguil:
> > > > > > > --- a/drivers/net/mlx4/Makefile
> > > > > > > +++ b/drivers/net/mlx4/Makefile
> > > > > > > @@ -33,7 +33,9 @@ include $(RTE_SDK)/mk/rte.vars.mk
> > > > > > >
> > > > > > >  # Library name.
> > > > > > >  LIB = librte_pmd_mlx4.a
> > > > > > > -LIB_GLUE = librte_pmd_mlx4_glue.so
> > > > > > > +LIB_GLUE = $(LIB_GLUE_BASE).$(LIB_GLUE_VERSION)
> > > > > > > +LIB_GLUE_BASE = librte_pmd_mlx4_glue.so
> > > > > > > +LIB_GLUE_VERSION = 18.02.1
> > > > > >
> > > > > > You should use the version number of the release, i.e. 18.02.0
> > > > > > Ideally, you should retrieve it from rte_version.h.
> > > > >
> > > > > Keep in mind this only needs to be updated when the glue API gets
> > > > modified,
> > > > > and this "18.02.1" string may remain unmodified for subsequent DPDK
> > > > > releases, probably as long as the PMD doesn't use any new rdma-core calls.
> > > > >
> > > > > We've already backported this patch to 17.02 and 17.11, both requiring
> > > > > different sets of Verbs calls and thus a different version, hence the
> > > > added
> > > > > "18.02" as a starting point. The last digit may have to be modified
> > > > possibly
> > > > > several times between official DPDK releases while work is being done on
> > > > the
> > > > > PMD (i.e. per commit).
> > > > >
> > > > > In short it's not meant to follow DPDK's public versioning scheme. If you
> > > > > really think it should, doing so will make things more complex in the
> > > > > Makefile, which will have to parse rte_version.h. What's your opinion?
> > > > 
> > > > What about appending date +%s output to it? It would be stricter and
> > > > automated.
> > > 
> > > Adding current timestamp or date into a build breaks reproducibility of builds, so is
> > > generally not recommended.
> > 
> > Then the sha1sum of mlx4_glue.h.
> > With this the size check I mentioned on the other patch would become
> > redundant and unnecessary.
> 
> Using a strong hash algorithm to version a library/symbol, while possible,
> seems a bit overkill and results in ugliness:
> 
>  librte_pmd_mlx4.so.c4ca4eaf2fe975ead83453458f4f56db49e724f3
> 
> 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.
> 
> 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?

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).

  reply	other threads:[~2018-02-05 14:16 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-02 15:16 [dpdk-dev] [PATCH v1 0/4] net/mlx: enhance rdma-core glue configuration Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 1/4] net/mlx: add debug checks to glue structure Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 2/4] net/mlx: fix missing includes for rdma-core glue Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 3/4] net/mlx: version rdma-core glue libraries Adrien Mazarguil
2018-02-02 15:16 ` [dpdk-dev] [PATCH v1 4/4] net/mlx: make rdma-core glue path configurable Adrien Mazarguil
2018-02-02 16:46 ` [dpdk-dev] [PATCH v2 0/4] net/mlx: enhance rdma-core glue configuration Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 1/4] net/mlx: add debug checks to glue structure Adrien Mazarguil
2018-02-05 12:27     ` Marcelo Ricardo Leitner
2018-02-05 13:31       ` Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 2/4] net/mlx: fix missing includes for rdma-core glue Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 3/4] net/mlx: version rdma-core glue libraries Adrien Mazarguil
2018-02-04 14:29     ` Thomas Monjalon
2018-02-05 11:24       ` Adrien Mazarguil
2018-02-05 12:13         ` Marcelo Ricardo Leitner
2018-02-05 12:24           ` Van Haaren, Harry
2018-02-05 12:43             ` Marcelo Ricardo Leitner
2018-02-05 12:58             ` Marcelo Ricardo Leitner
2018-02-05 13:44               ` Adrien Mazarguil
2018-02-05 14:16                 ` Thomas Monjalon [this message]
2018-02-05 14:33                   ` Adrien Mazarguil
2018-02-05 14:37                   ` Marcelo Ricardo Leitner
2018-02-05 14:59                     ` Adrien Mazarguil
2018-02-05 15:29                       ` Marcelo Ricardo Leitner
2018-02-05 15:54                         ` Adrien Mazarguil
2018-02-05 17:06                           ` Marcelo Ricardo Leitner
2018-02-06 11:06                             ` Adrien Mazarguil
2018-02-02 16:46   ` [dpdk-dev] [PATCH v2 4/4] net/mlx: make rdma-core glue path configurable Adrien Mazarguil
2018-02-02 16:52   ` [dpdk-dev] [PATCH v2 0/4] net/mlx: enhance rdma-core glue configuration Nélio Laranjeiro
2018-02-06 11:31   ` Shahaf Shuler

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=1992025.8JADqR79Gx@xps \
    --to=thomas@monjalon.net \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    --cc=mleitner@redhat.com \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=shahafs@mellanox.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).