patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime
@ 2021-02-05 19:39 Bruce Richardson
  2021-02-05 20:05 ` Thomas Monjalon
  2021-02-16 15:13 ` [dpdk-stable] [PATCH v2] " Bruce Richardson
  0 siblings, 2 replies; 9+ messages in thread
From: Bruce Richardson @ 2021-02-05 19:39 UTC (permalink / raw)
  To: dev
  Cc: thomas, david.marchand, Bruce Richardson, stable, Ray Kinsella,
	Neil Horman, Kevin Laatz

For using a DPDK application, such as OVS, which is dynamically linked, the
DPDK version in use should always report the actual version, not the
version used at build time. This incorrect behaviour can be seen by
building OVS against one version of DPDK and running it against a later
one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
dpdk_version returned will be the build version not the currently running
one - which can be verified using the DPDK telemetry library client.

  $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
  dpdk_version        : "DPDK 20.11.0-rc4"

  $ echo quit | sudo dpdk-telemetry.py
  Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
  {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
  -->

To fix this, we need to convert the rte_version() function, and any other
necessary parts of the rte_version.h, to be actual functions in EAL, not
just inlines/macros. The only complication in doing so is that telemetry
library cannot call rte_version() directly, and instead needs the version
string passed in on init.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

---
While new functions are technically supposed to be experimental, I don't
believe it should apply in this case
---
 lib/librte_eal/common/meson.build    |  2 +
 lib/librte_eal/common/rte_version.c  | 46 ++++++++++++++++++++
 lib/librte_eal/freebsd/eal.c         |  1 +
 lib/librte_eal/include/rte_version.h | 63 ++++++++++++++++------------
 lib/librte_eal/linux/eal.c           |  1 +
 lib/librte_eal/version.map           |  7 ++++
 lib/librte_telemetry/rte_telemetry.h |  2 +-
 lib/librte_telemetry/telemetry.c     | 12 +++---
 8 files changed, 101 insertions(+), 33 deletions(-)
 create mode 100644 lib/librte_eal/common/rte_version.c

diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 39d4272976..70bd854fec 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -36,6 +36,7 @@ if is_windows
 		'rte_random.c',
 		'rte_reciprocal.c',
 		'rte_service.c',
+		'rte_version.c',
 	)
 	subdir_done()
 endif
@@ -79,6 +80,7 @@ sources += files(
 	'rte_random.c',
 	'rte_reciprocal.c',
 	'rte_service.c',
+	'rte_version.c',
 )

 if is_linux
diff --git a/lib/librte_eal/common/rte_version.c b/lib/librte_eal/common/rte_version.c
new file mode 100644
index 0000000000..4ae5d66c89
--- /dev/null
+++ b/lib/librte_eal/common/rte_version.c
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+
+#include <rte_version.h>
+
+const char *
+rte_version_prefix(void) { return RTE_VER_PREFIX; }
+
+unsigned int
+rte_version_year(void) { return RTE_VER_YEAR; }
+
+unsigned int
+rte_version_month(void) { return RTE_VER_MONTH; }
+
+unsigned int
+rte_version_minor(void) { return RTE_VER_MINOR; }
+
+const char *
+rte_version_suffix(void) { return RTE_VER_SUFFIX; }
+
+unsigned int
+rte_version_release(void) { return RTE_VER_RELEASE; }
+
+const char *
+rte_version(void)
+{
+	static char version[32];
+	if (version[0] != 0)
+		return version;
+	if (strlen(RTE_VER_SUFFIX) == 0)
+		snprintf(version, sizeof(version), "%s %d.%02d.%d",
+				RTE_VER_PREFIX,
+				RTE_VER_YEAR,
+				RTE_VER_MONTH,
+				RTE_VER_MINOR);
+		else
+			snprintf(version, sizeof(version), "%s %d.%02d.%d%s%d",
+				RTE_VER_PREFIX,
+				RTE_VER_YEAR,
+				RTE_VER_MONTH,
+				RTE_VER_MINOR,
+				RTE_VER_SUFFIX,
+				RTE_VER_RELEASE);
+	return version;
+}
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 51478358c7..3f4dde6f85 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -943,6 +943,7 @@ rte_eal_init(int argc, char **argv)
 	if (!internal_conf->no_telemetry) {
 		const char *error_str = NULL;
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
+				rte_version(),
 				&internal_conf->ctrl_cpuset, &error_str)
 				!= 0) {
 			rte_eal_init_alert(error_str);
diff --git a/lib/librte_eal/include/rte_version.h b/lib/librte_eal/include/rte_version.h
index f7a3a1ebcf..2f3f727b46 100644
--- a/lib/librte_eal/include/rte_version.h
+++ b/lib/librte_eal/include/rte_version.h
@@ -28,38 +28,47 @@ extern "C" {
  * All version numbers in one to compare with RTE_VERSION_NUM()
  */
 #define RTE_VERSION RTE_VERSION_NUM( \
-			RTE_VER_YEAR, \
-			RTE_VER_MONTH, \
-			RTE_VER_MINOR, \
-			RTE_VER_RELEASE)
+			rte_version_year(), \
+			rte_version_month(), \
+			rte_version_minor(), \
+			rte_version_release())
+
+/**
+ * Function to return DPDK version prefix string
+ */
+const char *rte_version_prefix(void);
+
+/**
+ * Function to return DPDK version year
+ */
+unsigned int rte_version_year(void);
+
+/**
+ * Function to return DPDK version month
+ */
+unsigned int rte_version_month(void);
+
+/**
+ * Function to return DPDK minor version number
+ */
+unsigned int rte_version_minor(void);
+
+/**
+ * Function to return DPDK version suffix for any release candidates
+ */
+const char *rte_version_suffix(void);
+
+/**
+ * Function to return DPDK version release candidate value
+ */
+unsigned int rte_version_release(void);

 /**
  * Function returning version string
  * @return
- *     string
+ *     DPDK version string
  */
-static inline const char *
-rte_version(void)
-{
-	static char version[32];
-	if (version[0] != 0)
-		return version;
-	if (strlen(RTE_VER_SUFFIX) == 0)
-		snprintf(version, sizeof(version), "%s %d.%02d.%d",
-			RTE_VER_PREFIX,
-			RTE_VER_YEAR,
-			RTE_VER_MONTH,
-			RTE_VER_MINOR);
-	else
-		snprintf(version, sizeof(version), "%s %d.%02d.%d%s%d",
-			RTE_VER_PREFIX,
-			RTE_VER_YEAR,
-			RTE_VER_MONTH,
-			RTE_VER_MINOR,
-			RTE_VER_SUFFIX,
-			RTE_VER_RELEASE);
-	return version;
-}
+const char *rte_version(void);

 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 32b48c3de9..3492926117 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1316,6 +1316,7 @@ rte_eal_init(int argc, char **argv)
 	if (!internal_conf->no_telemetry) {
 		const char *error_str = NULL;
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
+				rte_version(),
 				&internal_conf->ctrl_cpuset, &error_str)
 				!= 0) {
 			rte_eal_init_alert(error_str);
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index fce90a112f..756c60ed1d 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -199,6 +199,13 @@ DPDK_21 {
 	rte_uuid_is_null;
 	rte_uuid_parse;
 	rte_uuid_unparse;
+	rte_version;
+	rte_version_minor;
+	rte_version_month;
+	rte_version_prefix;
+	rte_version_release;
+	rte_version_suffix;
+	rte_version_year;
 	rte_vfio_clear_group;
 	rte_vfio_container_create;
 	rte_vfio_container_destroy;
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index 76172222c9..f6c3992a9c 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -311,7 +311,7 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
  */
 __rte_experimental
 int
-rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
+rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
 		const char **err_str);

 /**
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index b142729da4..14b4ff5ea7 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -15,7 +15,6 @@
 #include <rte_string_fns.h>
 #include <rte_common.h>
 #include <rte_spinlock.h>
-#include <rte_version.h>

 #include "rte_telemetry.h"
 #include "telemetry_json.h"
@@ -48,6 +47,8 @@ struct socket {
 static struct socket v2_socket; /* socket for v2 telemetry */
 static struct socket v1_socket; /* socket for v1 telemetry */
 #endif /* !RTE_EXEC_ENV_WINDOWS */
+
+static const char *telemetry_version; /* save rte_version */
 static char telemetry_log_error[1024]; /* Will contain error on init failure */
 /* list of command callbacks, with one command registered by default */
 static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
@@ -105,7 +106,7 @@ json_info(const char *cmd __rte_unused, const char *params __rte_unused,
 		struct rte_tel_data *d)
 {
 	rte_tel_data_start_dict(d);
-	rte_tel_data_add_dict_string(d, "version", rte_version());
+	rte_tel_data_add_dict_string(d, "version", telemetry_version);
 	rte_tel_data_add_dict_int(d, "pid", getpid());
 	rte_tel_data_add_dict_int(d, "max_output_len", MAX_OUTPUT_LEN);
 	return 0;
@@ -303,7 +304,7 @@ client_handler(void *sock_id)
 	char info_str[1024];
 	snprintf(info_str, sizeof(info_str),
 			"{\"version\":\"%s\",\"pid\":%d,\"max_output_len\":%d}",
-			rte_version(), getpid(), MAX_OUTPUT_LEN);
+			telemetry_version, getpid(), MAX_OUTPUT_LEN);
 	if (write(s, info_str, strlen(info_str)) < 0) {
 		close(s);
 		return NULL;
@@ -481,9 +482,10 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 #endif /* !RTE_EXEC_ENV_WINDOWS */

 int32_t
-rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
-		const char **err_str)
+rte_telemetry_init(const char *runtime_dir, const char *rte_version,
+		rte_cpuset_t *cpuset, const char **err_str)
 {
+	telemetry_version = rte_version;
 #ifndef RTE_EXEC_ENV_WINDOWS
 	if (telemetry_v2_init(runtime_dir, cpuset) != 0) {
 		*err_str = telemetry_log_error;
--
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime
  2021-02-05 19:39 [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime Bruce Richardson
@ 2021-02-05 20:05 ` Thomas Monjalon
  2021-02-05 21:26   ` Bruce Richardson
  2021-02-16 15:13 ` [dpdk-stable] [PATCH v2] " Bruce Richardson
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2021-02-05 20:05 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, david.marchand, Bruce Richardson, stable, Ray Kinsella,
	Neil Horman, Kevin Laatz

05/02/2021 20:39, Bruce Richardson:
> For using a DPDK application, such as OVS, which is dynamically linked, the
> DPDK version in use should always report the actual version, not the
> version used at build time. This incorrect behaviour can be seen by
> building OVS against one version of DPDK and running it against a later
> one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> dpdk_version returned will be the build version not the currently running
> one - which can be verified using the DPDK telemetry library client.
> 
>   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
>   dpdk_version        : "DPDK 20.11.0-rc4"
> 
>   $ echo quit | sudo dpdk-telemetry.py
>   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
>   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}

Nice demonstration.

>  __rte_experimental
>  int
> -rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
> +rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
>  		const char **err_str);

It is changing the API.
As it is experimental, you just need to mention it in the release notes.

It is the fix. Do you think it should be merged quickly?
Or wait for 21.05?



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime
  2021-02-05 20:05 ` Thomas Monjalon
@ 2021-02-05 21:26   ` Bruce Richardson
  2021-02-09 12:34     ` Thomas Monjalon
  0 siblings, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2021-02-05 21:26 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, david.marchand, stable, Ray Kinsella, Neil Horman, Kevin Laatz

On Fri, Feb 05, 2021 at 09:05:43PM +0100, Thomas Monjalon wrote:
> 05/02/2021 20:39, Bruce Richardson:
> > For using a DPDK application, such as OVS, which is dynamically linked, the
> > DPDK version in use should always report the actual version, not the
> > version used at build time. This incorrect behaviour can be seen by
> > building OVS against one version of DPDK and running it against a later
> > one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> > dpdk_version returned will be the build version not the currently running
> > one - which can be verified using the DPDK telemetry library client.
> > 
> >   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
> >   dpdk_version        : "DPDK 20.11.0-rc4"
> > 
> >   $ echo quit | sudo dpdk-telemetry.py
> >   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
> >   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
> 
> Nice demonstration.
> 
> >  __rte_experimental
> >  int
> > -rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
> > +rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
> >  		const char **err_str);
> 
> It is changing the API.
> As it is experimental, you just need to mention it in the release notes.

I don't think I actually need to mention it there, because this API is more
"INTERNAL" than "EXPERIMENTAL". It's called automatically from
rte_eal_init(). I've done up patch http://patches.dpdk.org/patch/87806/
to correct this, including a RN addition. That should remove the need for a
doc update for this patch.

> 
> It is the fix. Do you think it should be merged quickly?
> Or wait for 21.05?
> 

I'm not sure either way to be honest. Given the bug has been around so
long, it's not exactly urgent. On the other hand, to get the fix the user
needs to rebuild their app, so having it sooner is nicer, and will mean it
would make the next LTS point release. Overall, though, I'm fine whichever
you decide.

/Bruce

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime
  2021-02-05 21:26   ` Bruce Richardson
@ 2021-02-09 12:34     ` Thomas Monjalon
  2021-02-09 12:36       ` Bruce Richardson
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2021-02-09 12:34 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: stable, dev, david.marchand, stable, Ray Kinsella, Neil Horman,
	Kevin Laatz

05/02/2021 22:26, Bruce Richardson:
> On Fri, Feb 05, 2021 at 09:05:43PM +0100, Thomas Monjalon wrote:
> > 05/02/2021 20:39, Bruce Richardson:
> > > For using a DPDK application, such as OVS, which is dynamically linked, the
> > > DPDK version in use should always report the actual version, not the
> > > version used at build time. This incorrect behaviour can be seen by
> > > building OVS against one version of DPDK and running it against a later
> > > one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> > > dpdk_version returned will be the build version not the currently running
> > > one - which can be verified using the DPDK telemetry library client.
> > > 
> > >   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
> > >   dpdk_version        : "DPDK 20.11.0-rc4"
> > > 
> > >   $ echo quit | sudo dpdk-telemetry.py
> > >   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
> > >   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
> > 
> > Nice demonstration.
> > 
> > >  __rte_experimental
> > >  int
> > > -rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
> > > +rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
> > >  		const char **err_str);
> > 
> > It is changing the API.
> > As it is experimental, you just need to mention it in the release notes.
> 
> I don't think I actually need to mention it there, because this API is more
> "INTERNAL" than "EXPERIMENTAL". It's called automatically from
> rte_eal_init(). I've done up patch http://patches.dpdk.org/patch/87806/
> to correct this, including a RN addition. That should remove the need for a
> doc update for this patch.
> 
> > 
> > It is the fix. Do you think it should be merged quickly?
> > Or wait for 21.05?
> > 
> 
> I'm not sure either way to be honest. Given the bug has been around so
> long, it's not exactly urgent. On the other hand, to get the fix the user
> needs to rebuild their app, so having it sooner is nicer, and will mean it
> would make the next LTS point release. Overall, though, I'm fine whichever
> you decide.


There is not much help available to close 21.02, so I won't take any risk.
I'll merge this fix in 21.05.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime
  2021-02-09 12:34     ` Thomas Monjalon
@ 2021-02-09 12:36       ` Bruce Richardson
  0 siblings, 0 replies; 9+ messages in thread
From: Bruce Richardson @ 2021-02-09 12:36 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: stable, dev, david.marchand, Ray Kinsella, Neil Horman, Kevin Laatz

On Tue, Feb 09, 2021 at 01:34:40PM +0100, Thomas Monjalon wrote:
> 05/02/2021 22:26, Bruce Richardson:
> > On Fri, Feb 05, 2021 at 09:05:43PM +0100, Thomas Monjalon wrote:
> > > 05/02/2021 20:39, Bruce Richardson:
> > > > For using a DPDK application, such as OVS, which is dynamically linked, the
> > > > DPDK version in use should always report the actual version, not the
> > > > version used at build time. This incorrect behaviour can be seen by
> > > > building OVS against one version of DPDK and running it against a later
> > > > one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> > > > dpdk_version returned will be the build version not the currently running
> > > > one - which can be verified using the DPDK telemetry library client.
> > > > 
> > > >   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
> > > >   dpdk_version        : "DPDK 20.11.0-rc4"
> > > > 
> > > >   $ echo quit | sudo dpdk-telemetry.py
> > > >   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
> > > >   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
> > > 
> > > Nice demonstration.
> > > 
> > > >  __rte_experimental
> > > >  int
> > > > -rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
> > > > +rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
> > > >  		const char **err_str);
> > > 
> > > It is changing the API.
> > > As it is experimental, you just need to mention it in the release notes.
> > 
> > I don't think I actually need to mention it there, because this API is more
> > "INTERNAL" than "EXPERIMENTAL". It's called automatically from
> > rte_eal_init(). I've done up patch http://patches.dpdk.org/patch/87806/
> > to correct this, including a RN addition. That should remove the need for a
> > doc update for this patch.
> > 
> > > 
> > > It is the fix. Do you think it should be merged quickly?
> > > Or wait for 21.05?
> > > 
> > 
> > I'm not sure either way to be honest. Given the bug has been around so
> > long, it's not exactly urgent. On the other hand, to get the fix the user
> > needs to rebuild their app, so having it sooner is nicer, and will mean it
> > would make the next LTS point release. Overall, though, I'm fine whichever
> > you decide.
> 
> 
> There is not much help available to close 21.02, so I won't take any risk.
> I'll merge this fix in 21.05.
> 
> 

I agree that it's not urgent enough to risk taking in 21.02. Taking it in
21.05 is fine, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-stable] [PATCH v2] eal: fix querying DPDK version at runtime
  2021-02-05 19:39 [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime Bruce Richardson
  2021-02-05 20:05 ` Thomas Monjalon
@ 2021-02-16 15:13 ` Bruce Richardson
  2021-03-15 22:24   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
  2021-03-17  8:40   ` [dpdk-stable] " David Marchand
  1 sibling, 2 replies; 9+ messages in thread
From: Bruce Richardson @ 2021-02-16 15:13 UTC (permalink / raw)
  To: dev; +Cc: thomas, david.marchand, mdr, kevin.laatz, Bruce Richardson, stable

For using a DPDK application, such as OVS, which is dynamically linked, the
DPDK version in use should always report the actual version, not the
version used at build time. This incorrect behaviour can be seen by
building OVS against one version of DPDK and running it against a later
one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
dpdk_version returned will be the build version not the currently running
one - which can be verified using the DPDK telemetry library client.

  $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
  dpdk_version        : "DPDK 20.11.0-rc4"

  $ echo quit | sudo dpdk-telemetry.py
  Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
  {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
  -->

To fix this, we need to convert the rte_version() function, and any other
necessary parts of the rte_version.h, to be actual functions in EAL, not
just inlines/macros. The only complication in doing so is that telemetry
library cannot call rte_version() directly, and instead needs the version
string passed in on init.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
v2:
	rebased on top of main for 21.05 consideration.
---
 lib/librte_eal/common/meson.build    |  2 +
 lib/librte_eal/common/rte_version.c  | 46 ++++++++++++++++++++
 lib/librte_eal/freebsd/eal.c         |  1 +
 lib/librte_eal/include/rte_version.h | 63 ++++++++++++++++------------
 lib/librte_eal/linux/eal.c           |  1 +
 lib/librte_eal/version.map           |  7 ++++
 lib/librte_telemetry/rte_telemetry.h |  2 +-
 lib/librte_telemetry/telemetry.c     | 12 +++---
 8 files changed, 101 insertions(+), 33 deletions(-)
 create mode 100644 lib/librte_eal/common/rte_version.c

diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 39d427297..70bd854fe 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -36,6 +36,7 @@ if is_windows
 		'rte_random.c',
 		'rte_reciprocal.c',
 		'rte_service.c',
+		'rte_version.c',
 	)
 	subdir_done()
 endif
@@ -79,6 +80,7 @@ sources += files(
 	'rte_random.c',
 	'rte_reciprocal.c',
 	'rte_service.c',
+	'rte_version.c',
 )

 if is_linux
diff --git a/lib/librte_eal/common/rte_version.c b/lib/librte_eal/common/rte_version.c
new file mode 100644
index 000000000..4ae5d66c8
--- /dev/null
+++ b/lib/librte_eal/common/rte_version.c
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+
+#include <rte_version.h>
+
+const char *
+rte_version_prefix(void) { return RTE_VER_PREFIX; }
+
+unsigned int
+rte_version_year(void) { return RTE_VER_YEAR; }
+
+unsigned int
+rte_version_month(void) { return RTE_VER_MONTH; }
+
+unsigned int
+rte_version_minor(void) { return RTE_VER_MINOR; }
+
+const char *
+rte_version_suffix(void) { return RTE_VER_SUFFIX; }
+
+unsigned int
+rte_version_release(void) { return RTE_VER_RELEASE; }
+
+const char *
+rte_version(void)
+{
+	static char version[32];
+	if (version[0] != 0)
+		return version;
+	if (strlen(RTE_VER_SUFFIX) == 0)
+		snprintf(version, sizeof(version), "%s %d.%02d.%d",
+				RTE_VER_PREFIX,
+				RTE_VER_YEAR,
+				RTE_VER_MONTH,
+				RTE_VER_MINOR);
+		else
+			snprintf(version, sizeof(version), "%s %d.%02d.%d%s%d",
+				RTE_VER_PREFIX,
+				RTE_VER_YEAR,
+				RTE_VER_MONTH,
+				RTE_VER_MINOR,
+				RTE_VER_SUFFIX,
+				RTE_VER_RELEASE);
+	return version;
+}
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 51478358c..3f4dde6f8 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -943,6 +943,7 @@ rte_eal_init(int argc, char **argv)
 	if (!internal_conf->no_telemetry) {
 		const char *error_str = NULL;
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
+				rte_version(),
 				&internal_conf->ctrl_cpuset, &error_str)
 				!= 0) {
 			rte_eal_init_alert(error_str);
diff --git a/lib/librte_eal/include/rte_version.h b/lib/librte_eal/include/rte_version.h
index f7a3a1ebc..2f3f727b4 100644
--- a/lib/librte_eal/include/rte_version.h
+++ b/lib/librte_eal/include/rte_version.h
@@ -28,38 +28,47 @@ extern "C" {
  * All version numbers in one to compare with RTE_VERSION_NUM()
  */
 #define RTE_VERSION RTE_VERSION_NUM( \
-			RTE_VER_YEAR, \
-			RTE_VER_MONTH, \
-			RTE_VER_MINOR, \
-			RTE_VER_RELEASE)
+			rte_version_year(), \
+			rte_version_month(), \
+			rte_version_minor(), \
+			rte_version_release())
+
+/**
+ * Function to return DPDK version prefix string
+ */
+const char *rte_version_prefix(void);
+
+/**
+ * Function to return DPDK version year
+ */
+unsigned int rte_version_year(void);
+
+/**
+ * Function to return DPDK version month
+ */
+unsigned int rte_version_month(void);
+
+/**
+ * Function to return DPDK minor version number
+ */
+unsigned int rte_version_minor(void);
+
+/**
+ * Function to return DPDK version suffix for any release candidates
+ */
+const char *rte_version_suffix(void);
+
+/**
+ * Function to return DPDK version release candidate value
+ */
+unsigned int rte_version_release(void);

 /**
  * Function returning version string
  * @return
- *     string
+ *     DPDK version string
  */
-static inline const char *
-rte_version(void)
-{
-	static char version[32];
-	if (version[0] != 0)
-		return version;
-	if (strlen(RTE_VER_SUFFIX) == 0)
-		snprintf(version, sizeof(version), "%s %d.%02d.%d",
-			RTE_VER_PREFIX,
-			RTE_VER_YEAR,
-			RTE_VER_MONTH,
-			RTE_VER_MINOR);
-	else
-		snprintf(version, sizeof(version), "%s %d.%02d.%d%s%d",
-			RTE_VER_PREFIX,
-			RTE_VER_YEAR,
-			RTE_VER_MONTH,
-			RTE_VER_MINOR,
-			RTE_VER_SUFFIX,
-			RTE_VER_RELEASE);
-	return version;
-}
+const char *rte_version(void);

 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 32b48c3de..349292611 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1316,6 +1316,7 @@ rte_eal_init(int argc, char **argv)
 	if (!internal_conf->no_telemetry) {
 		const char *error_str = NULL;
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
+				rte_version(),
 				&internal_conf->ctrl_cpuset, &error_str)
 				!= 0) {
 			rte_eal_init_alert(error_str);
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index fce90a112..756c60ed1 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -199,6 +199,13 @@ DPDK_21 {
 	rte_uuid_is_null;
 	rte_uuid_parse;
 	rte_uuid_unparse;
+	rte_version;
+	rte_version_minor;
+	rte_version_month;
+	rte_version_prefix;
+	rte_version_release;
+	rte_version_suffix;
+	rte_version_year;
 	rte_vfio_clear_group;
 	rte_vfio_container_create;
 	rte_vfio_container_destroy;
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index f8e54dc68..f7c8534b8 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -311,7 +311,7 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help);
  */
 __rte_internal
 int
-rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
+rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
 		const char **err_str);

 /**
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index b142729da..14b4ff5ea 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -15,7 +15,6 @@
 #include <rte_string_fns.h>
 #include <rte_common.h>
 #include <rte_spinlock.h>
-#include <rte_version.h>

 #include "rte_telemetry.h"
 #include "telemetry_json.h"
@@ -48,6 +47,8 @@ struct socket {
 static struct socket v2_socket; /* socket for v2 telemetry */
 static struct socket v1_socket; /* socket for v1 telemetry */
 #endif /* !RTE_EXEC_ENV_WINDOWS */
+
+static const char *telemetry_version; /* save rte_version */
 static char telemetry_log_error[1024]; /* Will contain error on init failure */
 /* list of command callbacks, with one command registered by default */
 static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
@@ -105,7 +106,7 @@ json_info(const char *cmd __rte_unused, const char *params __rte_unused,
 		struct rte_tel_data *d)
 {
 	rte_tel_data_start_dict(d);
-	rte_tel_data_add_dict_string(d, "version", rte_version());
+	rte_tel_data_add_dict_string(d, "version", telemetry_version);
 	rte_tel_data_add_dict_int(d, "pid", getpid());
 	rte_tel_data_add_dict_int(d, "max_output_len", MAX_OUTPUT_LEN);
 	return 0;
@@ -303,7 +304,7 @@ client_handler(void *sock_id)
 	char info_str[1024];
 	snprintf(info_str, sizeof(info_str),
 			"{\"version\":\"%s\",\"pid\":%d,\"max_output_len\":%d}",
-			rte_version(), getpid(), MAX_OUTPUT_LEN);
+			telemetry_version, getpid(), MAX_OUTPUT_LEN);
 	if (write(s, info_str, strlen(info_str)) < 0) {
 		close(s);
 		return NULL;
@@ -481,9 +482,10 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 #endif /* !RTE_EXEC_ENV_WINDOWS */

 int32_t
-rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
-		const char **err_str)
+rte_telemetry_init(const char *runtime_dir, const char *rte_version,
+		rte_cpuset_t *cpuset, const char **err_str)
 {
+	telemetry_version = rte_version;
 #ifndef RTE_EXEC_ENV_WINDOWS
 	if (telemetry_v2_init(runtime_dir, cpuset) != 0) {
 		*err_str = telemetry_log_error;
--
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] eal: fix querying DPDK version at runtime
  2021-02-16 15:13 ` [dpdk-stable] [PATCH v2] " Bruce Richardson
@ 2021-03-15 22:24   ` Thomas Monjalon
  2021-03-17  8:40   ` [dpdk-stable] " David Marchand
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2021-03-15 22:24 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, david.marchand, mdr, kevin.laatz, stable

16/02/2021 16:13, Bruce Richardson:
> For using a DPDK application, such as OVS, which is dynamically linked, the
> DPDK version in use should always report the actual version, not the
> version used at build time. This incorrect behaviour can be seen by
> building OVS against one version of DPDK and running it against a later
> one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> dpdk_version returned will be the build version not the currently running
> one - which can be verified using the DPDK telemetry library client.
> 
>   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
>   dpdk_version        : "DPDK 20.11.0-rc4"
> 
>   $ echo quit | sudo dpdk-telemetry.py
>   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
>   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
>   -->
> 
> To fix this, we need to convert the rte_version() function, and any other
> necessary parts of the rte_version.h, to be actual functions in EAL, not
> just inlines/macros. The only complication in doing so is that telemetry
> library cannot call rte_version() directly, and instead needs the version
> string passed in on init.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> v2:
> 	rebased on top of main for 21.05 consideration.

Applied, thanks




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH v2] eal: fix querying DPDK version at runtime
  2021-02-16 15:13 ` [dpdk-stable] [PATCH v2] " Bruce Richardson
  2021-03-15 22:24   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
@ 2021-03-17  8:40   ` David Marchand
  2021-03-17  9:33     ` Thomas Monjalon
  1 sibling, 1 reply; 9+ messages in thread
From: David Marchand @ 2021-03-17  8:40 UTC (permalink / raw)
  To: Bruce Richardson, Thomas Monjalon
  Cc: dev, Ray Kinsella, Kevin Laatz, dpdk stable

On Tue, Feb 16, 2021 at 4:13 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> For using a DPDK application, such as OVS, which is dynamically linked, the
> DPDK version in use should always report the actual version, not the
> version used at build time. This incorrect behaviour can be seen by
> building OVS against one version of DPDK and running it against a later
> one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> dpdk_version returned will be the build version not the currently running
> one - which can be verified using the DPDK telemetry library client.
>
>   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
>   dpdk_version        : "DPDK 20.11.0-rc4"
>
>   $ echo quit | sudo dpdk-telemetry.py
>   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
>   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
>   -->
>
> To fix this, we need to convert the rte_version() function, and any other
> necessary parts of the rte_version.h, to be actual functions in EAL, not
> just inlines/macros. The only complication in doing so is that telemetry
> library cannot call rte_version() directly, and instead needs the version
> string passed in on init.
>
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org


> diff --git a/lib/librte_eal/include/rte_version.h b/lib/librte_eal/include/rte_version.h
> index f7a3a1ebc..2f3f727b4 100644
> --- a/lib/librte_eal/include/rte_version.h
> +++ b/lib/librte_eal/include/rte_version.h
> @@ -28,38 +28,47 @@ extern "C" {
>   * All version numbers in one to compare with RTE_VERSION_NUM()
>   */
>  #define RTE_VERSION RTE_VERSION_NUM( \
> -                       RTE_VER_YEAR, \
> -                       RTE_VER_MONTH, \
> -                       RTE_VER_MINOR, \
> -                       RTE_VER_RELEASE)
> +                       rte_version_year(), \
> +                       rte_version_month(), \
> +                       rte_version_minor(), \
> +                       rte_version_release())

It breaks SPDK and applications relying on RTE_VERSION in preprocessor
directives.
RTE_VERSION* macros should be left alone, and applications that need
the runtime value should call rte_version().

See logs in https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/2f636aaf4ce244eba20844f9ff006033/log_upload_file/2021/3/dpdk_6857cb635821_2021-03-17_06-34-34_NA.zip

  CC lib/env_dpdk/pci_virtio.o
  CC lib/env_dpdk/pci_vmd.o
  CC lib/env_dpdk/pci_idxd.o
In file included from env_internal.h:42:0,
                 from pci_vmd.c:34:
/dpdk/build/include/rte_version.h:31:20: error: missing binary
operator before token "("
    rte_version_year(), \
                    ^
/dpdk/build/include/rte_version.h:25:36: note: in definition of macro
'RTE_VERSION_NUM'
 #define RTE_VERSION_NUM(a,b,c,d) ((a) << 24 | (b) << 16 | (c) << 8 | (d))
                                    ^
env_internal.h:49:5: note: in expansion of macro 'RTE_VERSION'
 #if RTE_VERSION < RTE_VERSION_NUM(19, 11, 0, 0)
     ^~~~~~~~~~~
In file included from env_internal.h:42:0,
                 from pci.c:34:
/dpdk/build/include/rte_version.h:31:20: error: missing binary
operator before token "("
    rte_version_year(), \
                    ^


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH v2] eal: fix querying DPDK version at runtime
  2021-03-17  8:40   ` [dpdk-stable] " David Marchand
@ 2021-03-17  9:33     ` Thomas Monjalon
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2021-03-17  9:33 UTC (permalink / raw)
  To: Bruce Richardson, David Marchand
  Cc: dev, Ray Kinsella, Kevin Laatz, dpdk stable

17/03/2021 09:40, David Marchand:
> On Tue, Feb 16, 2021 at 4:13 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > For using a DPDK application, such as OVS, which is dynamically linked, the
> > DPDK version in use should always report the actual version, not the
> > version used at build time. This incorrect behaviour can be seen by
> > building OVS against one version of DPDK and running it against a later
> > one. Using "ovs-vsctl list Open_vSwitch" to query basic info, the
> > dpdk_version returned will be the build version not the currently running
> > one - which can be verified using the DPDK telemetry library client.
> >
> >   $ sudo ovs-vsctl list Open_vSwitch | grep dpdk_version
> >   dpdk_version        : "DPDK 20.11.0-rc4"
> >
> >   $ echo quit | sudo dpdk-telemetry.py
> >   Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
> >   {"version": "DPDK 21.02.0-rc2", "pid": 405659, "max_output_len": 16384}
> >   -->
> >
> > To fix this, we need to convert the rte_version() function, and any other
> > necessary parts of the rte_version.h, to be actual functions in EAL, not
> > just inlines/macros. The only complication in doing so is that telemetry
> > library cannot call rte_version() directly, and instead needs the version
> > string passed in on init.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> 
> 
> > diff --git a/lib/librte_eal/include/rte_version.h b/lib/librte_eal/include/rte_version.h
> > index f7a3a1ebc..2f3f727b4 100644
> > --- a/lib/librte_eal/include/rte_version.h
> > +++ b/lib/librte_eal/include/rte_version.h
> > @@ -28,38 +28,47 @@ extern "C" {
> >   * All version numbers in one to compare with RTE_VERSION_NUM()
> >   */
> >  #define RTE_VERSION RTE_VERSION_NUM( \
> > -                       RTE_VER_YEAR, \
> > -                       RTE_VER_MONTH, \
> > -                       RTE_VER_MINOR, \
> > -                       RTE_VER_RELEASE)
> > +                       rte_version_year(), \
> > +                       rte_version_month(), \
> > +                       rte_version_minor(), \
> > +                       rte_version_release())
> 
> It breaks SPDK and applications relying on RTE_VERSION in preprocessor
> directives.
> RTE_VERSION* macros should be left alone, and applications that need
> the runtime value should call rte_version().

Yes, sorry for not catching it before merge.

> See logs in https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/2f636aaf4ce244eba20844f9ff006033/log_upload_file/2021/3/dpdk_6857cb635821_2021-03-17_06-34-34_NA.zip

My bad ignoring SPDK build, probably because of other issues there were.

I've sent a fix:
https://patches.dpdk.org/project/dpdk/patch/20210317093124.965624-1-thomas@monjalon.net/



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-17  9:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 19:39 [dpdk-stable] [PATCH] eal: fix querying DPDK version at runtime Bruce Richardson
2021-02-05 20:05 ` Thomas Monjalon
2021-02-05 21:26   ` Bruce Richardson
2021-02-09 12:34     ` Thomas Monjalon
2021-02-09 12:36       ` Bruce Richardson
2021-02-16 15:13 ` [dpdk-stable] [PATCH v2] " Bruce Richardson
2021-03-15 22:24   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2021-03-17  8:40   ` [dpdk-stable] " David Marchand
2021-03-17  9:33     ` 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).