From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E6EE5A0561; Thu, 18 Mar 2021 16:45:13 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AF3A8406B4; Thu, 18 Mar 2021 16:45:13 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id DEA0C40698; Thu, 18 Mar 2021 16:45:11 +0100 (CET) IronPort-SDR: GQroCicqf1HfZm+KlrFlkoDGGUnY2RUSuPhpt3TYG7Lqx8JXj1ppmnzrbm57HntbwKv+5OW6t8 ZzwsMp9KBTXA== X-IronPort-AV: E=McAfee;i="6000,8403,9927"; a="186353597" X-IronPort-AV: E=Sophos;i="5.81,259,1610438400"; d="scan'208";a="186353597" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Mar 2021 08:45:10 -0700 IronPort-SDR: KdqkTPCtkwGm4NANMy6vU4547VEAYXUvuMHjdRccma4RZ2lSfT16UaKK451l3ZbGRt0+7ap+WY IdBNA4TYcftA== X-IronPort-AV: E=Sophos;i="5.81,259,1610438400"; d="scan'208";a="439780734" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.13.177]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 18 Mar 2021 08:45:09 -0700 Date: Thu, 18 Mar 2021 15:45:06 +0000 From: Bruce Richardson To: Thomas Monjalon Cc: David Marchand , dev , dpdk stable Message-ID: <20210318154506.GD1633@bricha3-MOBL.ger.corp.intel.com> References: <20210317093124.965624-1-thomas@monjalon.net> <10994121.sJyVhAAg4N@thomas> <20210318122827.GB1633@bricha3-MOBL.ger.corp.intel.com> <2329735.2CMtTcrZrs@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2329735.2CMtTcrZrs@thomas> Subject: Re: [dpdk-dev] [PATCH] eal: fix version macro X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 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 > > > > > Signed-off-by: Thomas Monjalon > > > > > --- > > > > > 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