patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: David Marchand <david.marchand@redhat.com>, dev <dev@dpdk.org>,
	dpdk stable <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH] eal: fix version macro
Date: Thu, 18 Mar 2021 12:28:27 +0000	[thread overview]
Message-ID: <20210318122827.GB1633@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <10994121.sJyVhAAg4N@thomas>

On Wed, Mar 17, 2021 at 11:01:25AM +0100, Thomas Monjalon wrote:
> 17/03/2021 10:48, David Marchand:
> > On Wed, Mar 17, 2021 at 10:31 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > The macro RTE_VERSION is broken since updated with function calls.
> > > It is a build-time version number, and must be built with macros.
> > > For a run-time version number, there is the function rte_version().
> > >
> > > Fixes: 5b637a848195 ("eal: fix querying DPDK version at runtime")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: David Marchand <david.marchand@redhat.com>
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > >  lib/librte_eal/include/rte_version.h | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/lib/librte_eal/include/rte_version.h b/lib/librte_eal/include/rte_version.h
> > > index 2f3f727b46..736c5703be 100644
> > > --- a/lib/librte_eal/include/rte_version.h
> > > +++ b/lib/librte_eal/include/rte_version.h
> > > @@ -28,10 +28,10 @@ extern "C" {
> > >   * All version numbers in one to compare with RTE_VERSION_NUM()
> > >   */
> > >  #define RTE_VERSION RTE_VERSION_NUM( \
> > > -                       rte_version_year(), \
> > > -                       rte_version_month(), \
> > > -                       rte_version_minor(), \
> > > -                       rte_version_release())
> > > +                       RTE_VER_YEAR, \
> > > +                       RTE_VER_MONTH, \
> > > +                       RTE_VER_MINOR, \
> > > +                       RTE_VER_RELEASE)
> > >
> > >  /**
> > >   * Function to return DPDK version prefix string
> > 
> > The original patch wanted to fix rte_version() at runtime.
> > I don't see the need to keep the rte_version_XXX exports now that
> > RTE_VERSION is reverted.
> 
> I think it may help to query the version numbers at runtime,
> in "if" condition. Is there another way I'm missing?
> We may argue that the runtime version number should not be used
> to decide how to behave in an application.
> 
I would also tend toward keeping them, for the same reason that runtime is
definitely to be preferred over build time, and they are not like to be
much of a maintenance burden.

Also, next time we have an ABI break, I wonder if the existing macros
should be renamed to have an RTE_BUILD_VER_ prefix, to make it clear that
it's the build version only that is being reported rather than the version
actually being used. Similarly the functions could be renamed to have
rte_runtime_ prefix, ensuring that in all cases the user is clear whether
they are getting the build version or the runtime version.

/Bruce

  reply	other threads:[~2021-03-18 12:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17  9:31 Thomas Monjalon
2021-03-17  9:48 ` David Marchand
2021-03-17 10:01   ` Thomas Monjalon
2021-03-18 12:28     ` Bruce Richardson [this message]
2021-03-18 14:41       ` Thomas Monjalon
2021-03-18 15:45         ` Bruce Richardson
2021-03-17 15:36 ` David Marchand
2021-03-17 15:39   ` Thomas Monjalon

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=20210318122827.GB1633@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).