* [dpdk-stable] [PATCH] eal: fix version macro
@ 2021-03-17 9:31 Thomas Monjalon
2021-03-17 9:48 ` David Marchand
2021-03-17 15:36 ` David Marchand
0 siblings, 2 replies; 8+ messages in thread
From: Thomas Monjalon @ 2021-03-17 9:31 UTC (permalink / raw)
To: dev; +Cc: stable, David Marchand, Bruce Richardson
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
--
2.30.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-17 9:31 [dpdk-stable] [PATCH] eal: fix version macro Thomas Monjalon @ 2021-03-17 9:48 ` David Marchand 2021-03-17 10:01 ` Thomas Monjalon 2021-03-17 15:36 ` David Marchand 1 sibling, 1 reply; 8+ messages in thread From: David Marchand @ 2021-03-17 9:48 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, dpdk stable, Bruce Richardson 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. -- David Marchand ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-17 9:48 ` David Marchand @ 2021-03-17 10:01 ` Thomas Monjalon 2021-03-18 12:28 ` Bruce Richardson 0 siblings, 1 reply; 8+ messages in thread From: Thomas Monjalon @ 2021-03-17 10:01 UTC (permalink / raw) To: David Marchand; +Cc: dev, dpdk stable, Bruce Richardson 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-17 10:01 ` Thomas Monjalon @ 2021-03-18 12:28 ` Bruce Richardson 2021-03-18 14:41 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Bruce Richardson @ 2021-03-18 12:28 UTC (permalink / raw) To: Thomas Monjalon; +Cc: David Marchand, dev, dpdk stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-18 12:28 ` Bruce Richardson @ 2021-03-18 14:41 ` Thomas Monjalon 2021-03-18 15:45 ` Bruce Richardson 0 siblings, 1 reply; 8+ messages in thread From: Thomas Monjalon @ 2021-03-18 14:41 UTC (permalink / raw) To: Bruce Richardson; +Cc: David Marchand, dev, dpdk stable 18/03/2021 13:28, Bruce Richardson: > 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. I am fine with such rename, but that's already quite clear that a macro is at build time, and a function is usually evaluated at runtime. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-18 14:41 ` Thomas Monjalon @ 2021-03-18 15:45 ` Bruce Richardson 0 siblings, 0 replies; 8+ messages in thread From: Bruce Richardson @ 2021-03-18 15:45 UTC (permalink / raw) To: Thomas Monjalon; +Cc: David Marchand, dev, dpdk stable On Thu, Mar 18, 2021 at 03:41:35PM +0100, Thomas Monjalon wrote: > 18/03/2021 13:28, Bruce Richardson: > > 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. > > I am fine with such rename, > but that's already quite clear that a macro is at build time, > and a function is usually evaluated at runtime. > If we take the existing rte_version function, without checking the source code, one has no way of checking if that is resolved at runtime (as it is now) or at compile-time (as it was). However, if we assume that that is a bug and that all such functions should be run-time operations, then there is no difficulty. /Bruce ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-17 9:31 [dpdk-stable] [PATCH] eal: fix version macro Thomas Monjalon 2021-03-17 9:48 ` David Marchand @ 2021-03-17 15:36 ` David Marchand 2021-03-17 15:39 ` Thomas Monjalon 1 sibling, 1 reply; 8+ messages in thread From: David Marchand @ 2021-03-17 15:36 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, dpdk stable, Bruce Richardson 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> Acked-by: David Marchand <david.marchand@redhat.com> Let's discuss the versioning of the new symbols in the other thread. Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH] eal: fix version macro 2021-03-17 15:36 ` David Marchand @ 2021-03-17 15:39 ` Thomas Monjalon 0 siblings, 0 replies; 8+ messages in thread From: Thomas Monjalon @ 2021-03-17 15:39 UTC (permalink / raw) To: David Marchand, Bruce Richardson; +Cc: dev, dpdk stable 17/03/2021 16:36, 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> > > Acked-by: David Marchand <david.marchand@redhat.com> Applied quickly because the compilation of some apps are broken. > Let's discuss the versioning of the new symbols in the other thread. OK, in this thread: https://inbox.dpdk.org/dev/20210317151535.1412311-1-thomas@monjalon.net/ ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-18 15:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-17 9:31 [dpdk-stable] [PATCH] eal: fix version macro Thomas Monjalon 2021-03-17 9:48 ` David Marchand 2021-03-17 10:01 ` Thomas Monjalon 2021-03-18 12:28 ` Bruce Richardson 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
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).