DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions
@ 2021-02-22 21:25 Jie
  2021-02-22 22:24 ` Dmitry Kozlyuk
  2021-02-23 23:01 ` [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports " Jie Zhou
  0 siblings, 2 replies; 9+ messages in thread
From: Jie @ 2021-02-22 21:25 UTC (permalink / raw)
  To: dev; +Cc: dmitry.kozliuk, roretzla

From: Jie Zhou <jizh@microsoft.com>

This patch allows the same set of rte_metrics_tel_* functions to be
exported no matter JANSSON is available or not, by doing following:
1.	Leverage dpdk_conf to set configuration flag RTE_HAVE_JANSSON
when Jansson dependency is found.
2.	In rte_metrics_telemetry.c, leverage RTE_HAVE_JANSSON to handle the
case when JANSSON is not available by adding stubs for all the instances.
3.	In meson.build, per dpdk\doc\guides\rel_notes\release_20_05.rst,
it is claimed that "Telemetry library is no longer dependent on the
external Jansson library, which allows Telemetry be enabled by default.",
thus make the deps and includes of Telemetry as not conditional anymore.

Signed-off-by: Jie Zhou <jizh@microsoft.com>
---
 config/meson.build                         |  5 ++
 lib/librte_metrics/meson.build             | 13 +++---
 lib/librte_metrics/rte_metrics_telemetry.c | 53 ++++++++++++++++++++++
 lib/librte_metrics/rte_metrics_telemetry.h |  2 +-
 4 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 3cf560b8a..892bd9677 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -292,6 +292,11 @@ if is_freebsd
 	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
 endif
 
+jansson = dependency('jansson', required: false, method: 'pkg-config')
+if jansson.found()
+	dpdk_conf.set('RTE_HAVE_JANSSON', 1)
+endif
+
 if is_windows
 	# VirtualAlloc2() is available since Windows 10 / Server 2016.
 	add_project_arguments('-D_WIN32_WINNT=0x0A00', language: 'c')
diff --git a/lib/librte_metrics/meson.build b/lib/librte_metrics/meson.build
index 28a8cc115..b13604c88 100644
--- a/lib/librte_metrics/meson.build
+++ b/lib/librte_metrics/meson.build
@@ -4,11 +4,12 @@
 sources = files('rte_metrics.c')
 headers = files('rte_metrics.h')
 
-jansson = dependency('jansson', required: false, method: 'pkg-config')
-if jansson.found()
+if dpdk_conf.has('RTE_HAVE_JANSSON')
 	ext_deps += jansson
-	sources += files('rte_metrics_telemetry.c')
-	headers += files('rte_metrics_telemetry.h')
-	deps += ['ethdev', 'telemetry']
-	includes += include_directories('../librte_telemetry')
 endif
+
+sources += files('rte_metrics_telemetry.c')
+headers += files('rte_metrics_telemetry.h')
+
+deps += ['ethdev', 'telemetry']
+includes += include_directories('../librte_telemetry')
diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index b8ee56ef0..225d9ebce 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -2,7 +2,9 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
+#ifdef RTE_HAVE_JANSSON
 #include <jansson.h>
+#endif
 
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
@@ -13,6 +15,7 @@
 #include "rte_metrics.h"
 #include "rte_metrics_telemetry.h"
 
+#ifdef RTE_HAVE_JANSSON
 struct telemetry_metrics_data tel_met_data;
 
 int metrics_log_level;
@@ -70,10 +73,12 @@ rte_metrics_tel_reg_port_ethdev_to_metrics(uint16_t port_id)
 	free(xstats_names);
 	return ret;
 }
+#endif
 
 int32_t
 rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
 {
+#ifdef JANSSON
 	struct driver_index {
 		const void *dev_ops;
 		int reg_index;
@@ -110,8 +115,15 @@ rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
 	}
 	*metrics_register_done = 1;
 	return 0;
+#else
+	RTE_SET_USED(metrics_register_done);
+	RTE_SET_USED(reg_index_list);
+
+	return -ENOTSUP;
+#endif
 }
 
+#ifdef RTE_HAVE_JANSSON
 static int32_t
 rte_metrics_tel_update_metrics_ethdev(uint16_t port_id, int reg_start_index)
 {
@@ -224,11 +236,13 @@ rte_metrics_tel_format_port(uint32_t pid, json_t *ports,
 	free(names);
 	return ret;
 }
+#endif
 
 int32_t
 rte_metrics_tel_encode_json_format(struct telemetry_encode_param *ep,
 		char **json_buffer)
 {
+#ifdef JANSSON
 	json_t *root, *ports;
 	int ret, i;
 
@@ -276,12 +290,19 @@ rte_metrics_tel_encode_json_format(struct telemetry_encode_param *ep,
 	*json_buffer = json_dumps(root, JSON_INDENT(2));
 	json_decref(root);
 	return 0;
+#else
+	RTE_SET_USED(ep);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+#endif
 }
 
 int32_t
 rte_metrics_tel_get_ports_stats_json(struct telemetry_encode_param *ep,
 		int *reg_index, char **json_buffer)
 {
+#ifdef RTE_HAVE_JANSSON
 	int ret, i;
 	uint32_t port_id;
 
@@ -306,11 +327,19 @@ rte_metrics_tel_get_ports_stats_json(struct telemetry_encode_param *ep,
 		return ret;
 	}
 	return 0;
+#else
+	RTE_SET_USED(ep);
+	RTE_SET_USED(reg_index);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+#endif
 }
 
 int32_t
 rte_metrics_tel_get_port_stats_ids(struct telemetry_encode_param *ep)
 {
+#ifdef RTE_HAVE_JANSSON
 	int p, num_port_ids = 0;
 
 	RTE_ETH_FOREACH_DEV(p) {
@@ -327,8 +356,14 @@ rte_metrics_tel_get_port_stats_ids(struct telemetry_encode_param *ep)
 	ep->pp.num_metric_ids = 0;
 	ep->type = PORT_STATS;
 	return 0;
+#else
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+#endif
 }
 
+#ifdef RTE_HAVE_JANSSON
 static int32_t
 rte_metrics_tel_stat_names_to_ids(const char * const *stat_names,
 	uint32_t *stat_ids, int num_stat_names)
@@ -373,10 +408,12 @@ rte_metrics_tel_stat_names_to_ids(const char * const *stat_names,
 	free(names);
 	return 0;
 }
+#endif
 
 int32_t
 rte_metrics_tel_extract_data(struct telemetry_encode_param *ep, json_t *data)
 {
+#ifdef RTE_HAVE_JANSSON
 	int ret;
 	json_t *port_ids_json = json_object_get(data, "ports");
 	json_t *stat_names_json = json_object_get(data, "stats");
@@ -420,8 +457,23 @@ rte_metrics_tel_extract_data(struct telemetry_encode_param *ep, json_t *data)
 
 	ep->type = PORT_STATS;
 	return 0;
+#else
+	RTE_SET_USED(ep);
+	RTE_SET_USED(data);
+
+	return -ENOTSUP;
+#endif
+}
+
+int32_t
+rte_metrics_tel_get_global_stats(struct telemetry_encode_param* ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
 }
 
+#ifdef RTE_HAVE_JANSSON
 static int
 rte_metrics_tel_initial_metrics_setup(void)
 {
@@ -541,3 +593,4 @@ RTE_INIT(metrics_ctor)
 			handle_ports_stats_values_by_name);
 #endif
 }
+#endif
diff --git a/lib/librte_metrics/rte_metrics_telemetry.h b/lib/librte_metrics/rte_metrics_telemetry.h
index 5dbb32ca0..efc6b9a9e 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.h
+++ b/lib/librte_metrics/rte_metrics_telemetry.h
@@ -2,7 +2,7 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#ifdef RTE_LIB_TELEMETRY
+#ifdef RTE_HAVE_JANSSON
 #include <jansson.h>
 #else
 #define json_t void *
-- 
2.30.0.vfs.0.2


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

* Re: [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions
  2021-02-22 21:25 [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions Jie
@ 2021-02-22 22:24 ` Dmitry Kozlyuk
  2021-02-23 10:03   ` Bruce Richardson
  2021-02-23 23:01 ` [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports " Jie Zhou
  1 sibling, 1 reply; 9+ messages in thread
From: Dmitry Kozlyuk @ 2021-02-22 22:24 UTC (permalink / raw)
  To: Jie; +Cc: dev, roretzla, Bruce Richardson

+ Bruce

On Mon, 22 Feb 2021 13:25:02 -0800, Jie wrote:
[...]
> diff --git a/config/meson.build b/config/meson.build
> index 3cf560b8a..892bd9677 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -292,6 +292,11 @@ if is_freebsd
>  	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
>  endif
>  
> +jansson = dependency('jansson', required: false, method: 'pkg-config')
> +if jansson.found()
> +	dpdk_conf.set('RTE_HAVE_JANSSON', 1)
> +endif

DPDK seems to prefer "HAS" for such macros.

Why not do this in lib/librte_telemetry/meson.build?

[...]
> --- a/lib/librte_metrics/meson.build
> +++ b/lib/librte_metrics/meson.build
> @@ -4,11 +4,12 @@
>  sources = files('rte_metrics.c')
>  headers = files('rte_metrics.h')
>  
> -jansson = dependency('jansson', required: false, method: 'pkg-config')
> -if jansson.found()
> +if dpdk_conf.has('RTE_HAVE_JANSSON')
>  	ext_deps += jansson
> -	sources += files('rte_metrics_telemetry.c')
> -	headers += files('rte_metrics_telemetry.h')
> -	deps += ['ethdev', 'telemetry']
> -	includes += include_directories('../librte_telemetry')
>  endif
> +
> +sources += files('rte_metrics_telemetry.c')
> +headers += files('rte_metrics_telemetry.h')

Can be merged with definitions above.

[...]
>  int32_t
>  rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
>  {
> +#ifdef JANSSON

Why not use RTE_HAS_JANSSON everywhere? (One more occurrence below.)


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

* Re: [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions
  2021-02-22 22:24 ` Dmitry Kozlyuk
@ 2021-02-23 10:03   ` Bruce Richardson
  0 siblings, 0 replies; 9+ messages in thread
From: Bruce Richardson @ 2021-02-23 10:03 UTC (permalink / raw)
  To: Dmitry Kozlyuk; +Cc: Jie, dev, roretzla

On Tue, Feb 23, 2021 at 01:24:15AM +0300, Dmitry Kozlyuk wrote:
> + Bruce
> 
> On Mon, 22 Feb 2021 13:25:02 -0800, Jie wrote:
> [...]
> > diff --git a/config/meson.build b/config/meson.build
> > index 3cf560b8a..892bd9677 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -292,6 +292,11 @@ if is_freebsd
> >  	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
> >  endif
> >  
> > +jansson = dependency('jansson', required: false, method: 'pkg-config')
> > +if jansson.found()
> > +	dpdk_conf.set('RTE_HAVE_JANSSON', 1)
> > +endif
> 
> DPDK seems to prefer "HAS" for such macros.
> 
> Why not do this in lib/librte_telemetry/meson.build?
> 
> [...]
> > --- a/lib/librte_metrics/meson.build
> > +++ b/lib/librte_metrics/meson.build
> > @@ -4,11 +4,12 @@
> >  sources = files('rte_metrics.c')
> >  headers = files('rte_metrics.h')
> >  
> > -jansson = dependency('jansson', required: false, method: 'pkg-config')
> > -if jansson.found()
> > +if dpdk_conf.has('RTE_HAVE_JANSSON')
> >  	ext_deps += jansson
> > -	sources += files('rte_metrics_telemetry.c')
> > -	headers += files('rte_metrics_telemetry.h')
> > -	deps += ['ethdev', 'telemetry']
> > -	includes += include_directories('../librte_telemetry')
> >  endif
> > +
> > +sources += files('rte_metrics_telemetry.c')
> > +headers += files('rte_metrics_telemetry.h')
> 
> Can be merged with definitions above.
> 
> [...]
> >  int32_t
> >  rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
> >  {
> > +#ifdef JANSSON
> 
> Why not use RTE_HAS_JANSSON everywhere? (One more occurrence below.)
> 

+1 for this suggestion.

Also, since this is essentially stubbing out the functions in this file,
can you reduce the amount of ifdefs by putting either at the start or the
end the full set of stubs, leaving just the one ifdef block covering the
whole file.

/Bruce

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

* [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
  2021-02-22 21:25 [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions Jie
  2021-02-22 22:24 ` Dmitry Kozlyuk
@ 2021-02-23 23:01 ` Jie Zhou
  2021-02-24 10:36   ` Bruce Richardson
  2021-02-24 18:46   ` [dpdk-dev] [PATCH v3] " Jie Zhou
  1 sibling, 2 replies; 9+ messages in thread
From: Jie Zhou @ 2021-02-23 23:01 UTC (permalink / raw)
  To: dev; +Cc: dmitry.kozliuk, bruce.richardson

From: Jie Zhou <jizh@microsoft.com>

This patch allows the same set of rte_metrics_tel_* functions to be
exported no matter JANSSON is available or not, by doing following:
1.	Leverage dpdk_conf to set configuration flag RTE_HAVE_JANSSON
when Jansson dependency is found.
2.	In rte_metrics_telemetry.c, leverage RTE_HAVE_JANSSON to handle the
case when JANSSON is not available by adding stubs for all the instances.
3.	In meson.build, per dpdk\doc\guides\rel_notes\release_20_05.rst,
it is claimed that "Telemetry library is no longer dependent on the
external Jansson library, which allows Telemetry be enabled by default.",
thus make the deps and includes of Telemetry as not conditional anymore.

V2 changes:
    Address comments from Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> and
    Bruce Richardson <bruce.richardson@intel.com>:
        - Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build
        - Reduce #ifdef RTE_HAS_JANSSON blocks

Signed-off-by: Jie Zhou <jizh@microsoft.com>
---
 lib/librte_metrics/meson.build             | 12 ++---
 lib/librte_metrics/rte_metrics_telemetry.c | 60 +++++++++++++++++++++-
 lib/librte_metrics/rte_metrics_telemetry.h |  2 +-
 3 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/lib/librte_metrics/meson.build b/lib/librte_metrics/meson.build
index 28a8cc115..e543e4cdd 100644
--- a/lib/librte_metrics/meson.build
+++ b/lib/librte_metrics/meson.build
@@ -1,14 +1,14 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-sources = files('rte_metrics.c')
-headers = files('rte_metrics.h')
+sources = files('rte_metrics.c', 'rte_metrics_telemetry.c')
+headers = files('rte_metrics.h', 'rte_metrics_telemetry.h')
 
 jansson = dependency('jansson', required: false, method: 'pkg-config')
 if jansson.found()
+	dpdk_conf.set('RTE_HAS_JANSSON', 1)
 	ext_deps += jansson
-	sources += files('rte_metrics_telemetry.c')
-	headers += files('rte_metrics_telemetry.h')
-	deps += ['ethdev', 'telemetry']
-	includes += include_directories('../librte_telemetry')
 endif
+
+deps += ['ethdev', 'telemetry']
+includes += include_directories('../librte_telemetry')
diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index b8ee56ef0..305ee271f 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -2,8 +2,6 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#include <jansson.h>
-
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TELEMETRY
@@ -13,6 +11,7 @@
 #include "rte_metrics.h"
 #include "rte_metrics_telemetry.h"
 
+#ifdef RTE_HAS_JANSSON
 struct telemetry_metrics_data tel_met_data;
 
 int metrics_log_level;
@@ -541,3 +540,60 @@ RTE_INIT(metrics_ctor)
 			handle_ports_stats_values_by_name);
 #endif
 }
+#else
+int32_t
+rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
+{
+	RTE_SET_USED(metrics_register_done);
+	RTE_SET_USED(reg_index_list);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_encode_json_format(struct telemetry_encode_param *ep,
+	char **json_buffer)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_ports_stats_json(struct telemetry_encode_param *ep,
+	int *reg_index, char **json_buffer)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(reg_index);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_port_stats_ids(struct telemetry_encode_param *ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_extract_data(struct telemetry_encode_param *ep, json_t *data)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(data);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_global_stats(struct telemetry_encode_param *ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+}
+
+#endif
diff --git a/lib/librte_metrics/rte_metrics_telemetry.h b/lib/librte_metrics/rte_metrics_telemetry.h
index 5dbb32ca0..2b6eb1ccc 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.h
+++ b/lib/librte_metrics/rte_metrics_telemetry.h
@@ -2,7 +2,7 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#ifdef RTE_LIB_TELEMETRY
+#ifdef RTE_HAS_JANSSON
 #include <jansson.h>
 #else
 #define json_t void *
-- 
2.30.0.vfs.0.2


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

* Re: [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
  2021-02-23 23:01 ` [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports " Jie Zhou
@ 2021-02-24 10:36   ` Bruce Richardson
  2021-02-24 18:46   ` [dpdk-dev] [PATCH v3] " Jie Zhou
  1 sibling, 0 replies; 9+ messages in thread
From: Bruce Richardson @ 2021-02-24 10:36 UTC (permalink / raw)
  To: Jie Zhou; +Cc: dev, dmitry.kozliuk

On Tue, Feb 23, 2021 at 03:01:12PM -0800, Jie Zhou wrote:
> From: Jie Zhou <jizh@microsoft.com>
> 
> This patch allows the same set of rte_metrics_tel_* functions to be
> exported no matter JANSSON is available or not, by doing following:
> 1.	Leverage dpdk_conf to set configuration flag RTE_HAVE_JANSSON
> when Jansson dependency is found.
> 2.	In rte_metrics_telemetry.c, leverage RTE_HAVE_JANSSON to handle the
> case when JANSSON is not available by adding stubs for all the instances.
> 3.	In meson.build, per dpdk\doc\guides\rel_notes\release_20_05.rst,
> it is claimed that "Telemetry library is no longer dependent on the
> external Jansson library, which allows Telemetry be enabled by default.",
> thus make the deps and includes of Telemetry as not conditional anymore.
> 
> V2 changes:
>     Address comments from Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> and
>     Bruce Richardson <bruce.richardson@intel.com>:
>         - Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build
>         - Reduce #ifdef RTE_HAS_JANSSON blocks
> 
> Signed-off-by: Jie Zhou <jizh@microsoft.com>

One small nit below, but otherwise this looks better.

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

> ---
>  lib/librte_metrics/meson.build             | 12 ++---
>  lib/librte_metrics/rte_metrics_telemetry.c | 60 +++++++++++++++++++++-
>  lib/librte_metrics/rte_metrics_telemetry.h |  2 +-
>  3 files changed, 65 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_metrics/meson.build b/lib/librte_metrics/meson.build
> index 28a8cc115..e543e4cdd 100644
> --- a/lib/librte_metrics/meson.build
> +++ b/lib/librte_metrics/meson.build
> @@ -1,14 +1,14 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017 Intel Corporation
>  
> -sources = files('rte_metrics.c')
> -headers = files('rte_metrics.h')
> +sources = files('rte_metrics.c', 'rte_metrics_telemetry.c')
> +headers = files('rte_metrics.h', 'rte_metrics_telemetry.h')
>  
>  jansson = dependency('jansson', required: false, method: 'pkg-config')
>  if jansson.found()
> +	dpdk_conf.set('RTE_HAS_JANSSON', 1)
>  	ext_deps += jansson
> -	sources += files('rte_metrics_telemetry.c')
> -	headers += files('rte_metrics_telemetry.h')
> -	deps += ['ethdev', 'telemetry']
> -	includes += include_directories('../librte_telemetry')
>  endif
> +
> +deps += ['ethdev', 'telemetry']
> +includes += include_directories('../librte_telemetry')

This includes line should not be necessary because the dependency implies
that the relevant header paths are added.

> diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
> index b8ee56ef0..305ee271f 100644
> --- a/lib/librte_metrics/rte_metrics_telemetry.c
> +++ b/lib/librte_metrics/rte_metrics_telemetry.c
> @@ -2,8 +2,6 @@
>   * Copyright(c) 2020 Intel Corporation
>   */
>  
> -#include <jansson.h>
> -
>  #include <rte_ethdev.h>
>  #include <rte_string_fns.h>
>  #ifdef RTE_LIB_TELEMETRY
> @@ -13,6 +11,7 @@
>  #include "rte_metrics.h"
>  #include "rte_metrics_telemetry.h"
>  
> +#ifdef RTE_HAS_JANSSON
>  struct telemetry_metrics_data tel_met_data;
>  
>  int metrics_log_level;
> @@ -541,3 +540,60 @@ RTE_INIT(metrics_ctor)
>  			handle_ports_stats_values_by_name);
>  #endif
>  }
> +#else
> +int32_t
> +rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
> +{
> +	RTE_SET_USED(metrics_register_done);
> +	RTE_SET_USED(reg_index_list);
> +
> +	return -ENOTSUP;
> +}
> +
> +int32_t
> +rte_metrics_tel_encode_json_format(struct telemetry_encode_param *ep,
> +	char **json_buffer)
> +{
> +	RTE_SET_USED(ep);
> +	RTE_SET_USED(json_buffer);
> +
> +	return -ENOTSUP;
> +}
> +
> +int32_t
> +rte_metrics_tel_get_ports_stats_json(struct telemetry_encode_param *ep,
> +	int *reg_index, char **json_buffer)
> +{
> +	RTE_SET_USED(ep);
> +	RTE_SET_USED(reg_index);
> +	RTE_SET_USED(json_buffer);
> +
> +	return -ENOTSUP;
> +}
> +
> +int32_t
> +rte_metrics_tel_get_port_stats_ids(struct telemetry_encode_param *ep)
> +{
> +	RTE_SET_USED(ep);
> +
> +	return -ENOTSUP;
> +}
> +
> +int32_t
> +rte_metrics_tel_extract_data(struct telemetry_encode_param *ep, json_t *data)
> +{
> +	RTE_SET_USED(ep);
> +	RTE_SET_USED(data);
> +
> +	return -ENOTSUP;
> +}
> +
> +int32_t
> +rte_metrics_tel_get_global_stats(struct telemetry_encode_param *ep)
> +{
> +	RTE_SET_USED(ep);
> +
> +	return -ENOTSUP;
> +}
> +
> +#endif
> diff --git a/lib/librte_metrics/rte_metrics_telemetry.h b/lib/librte_metrics/rte_metrics_telemetry.h
> index 5dbb32ca0..2b6eb1ccc 100644
> --- a/lib/librte_metrics/rte_metrics_telemetry.h
> +++ b/lib/librte_metrics/rte_metrics_telemetry.h
> @@ -2,7 +2,7 @@
>   * Copyright(c) 2020 Intel Corporation
>   */
>  
> -#ifdef RTE_LIB_TELEMETRY
> +#ifdef RTE_HAS_JANSSON
>  #include <jansson.h>
>  #else
>  #define json_t void *
> -- 
> 2.30.0.vfs.0.2
> 

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

* [dpdk-dev] [PATCH v3] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
  2021-02-23 23:01 ` [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports " Jie Zhou
  2021-02-24 10:36   ` Bruce Richardson
@ 2021-02-24 18:46   ` Jie Zhou
  2021-03-06 20:18     ` Dmitry Kozlyuk
  2021-03-08 18:05     ` [dpdk-dev] [PATCH v4] " Jie Zhou
  1 sibling, 2 replies; 9+ messages in thread
From: Jie Zhou @ 2021-02-24 18:46 UTC (permalink / raw)
  To: dev; +Cc: dmitry.kozliuk, bruce.richardson

From: Jie Zhou <jizh@microsoft.com>

This patch allows the same set of rte_metrics_tel_* functions to be
exported no matter JANSSON is available or not, by doing following:
1.	Leverage dpdk_conf to set configuration flag RTE_HAVE_JANSSON
when Jansson dependency is found.
2.	In rte_metrics_telemetry.c, leverage RTE_HAVE_JANSSON to handle the
case when JANSSON is not available by adding stubs for all the instances.
3.	In meson.build, per dpdk\doc\guides\rel_notes\release_20_05.rst,
it is claimed that "Telemetry library is no longer dependent on the
external Jansson library, which allows Telemetry be enabled by default.",
thus make the deps and includes of Telemetry as not conditional anymore.

V2 changes:
    Address comments from Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> and
    Bruce Richardson <bruce.richardson@intel.com>:
        - Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build
        - Reduce #ifdef RTE_HAS_JANSSON blocks

V3 changes:
    Address comment from Bruce Richardson <bruce.richardson@intel.com>:
        - Remove redundant includes line on librte_telemetry

Signed-off-by: Jie Zhou <jizh@microsoft.com>
Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
---
 lib/librte_metrics/meson.build             | 11 ++--
 lib/librte_metrics/rte_metrics_telemetry.c | 60 +++++++++++++++++++++-
 lib/librte_metrics/rte_metrics_telemetry.h |  2 +-
 3 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/lib/librte_metrics/meson.build b/lib/librte_metrics/meson.build
index 28a8cc115..d5be6a214 100644
--- a/lib/librte_metrics/meson.build
+++ b/lib/librte_metrics/meson.build
@@ -1,14 +1,13 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-sources = files('rte_metrics.c')
-headers = files('rte_metrics.h')
+sources = files('rte_metrics.c', 'rte_metrics_telemetry.c')
+headers = files('rte_metrics.h', 'rte_metrics_telemetry.h')
 
 jansson = dependency('jansson', required: false, method: 'pkg-config')
 if jansson.found()
+	dpdk_conf.set('RTE_HAS_JANSSON', 1)
 	ext_deps += jansson
-	sources += files('rte_metrics_telemetry.c')
-	headers += files('rte_metrics_telemetry.h')
-	deps += ['ethdev', 'telemetry']
-	includes += include_directories('../librte_telemetry')
 endif
+
+deps += ['ethdev', 'telemetry']
diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index b8ee56ef0..305ee271f 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -2,8 +2,6 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#include <jansson.h>
-
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TELEMETRY
@@ -13,6 +11,7 @@
 #include "rte_metrics.h"
 #include "rte_metrics_telemetry.h"
 
+#ifdef RTE_HAS_JANSSON
 struct telemetry_metrics_data tel_met_data;
 
 int metrics_log_level;
@@ -541,3 +540,60 @@ RTE_INIT(metrics_ctor)
 			handle_ports_stats_values_by_name);
 #endif
 }
+#else
+int32_t
+rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
+{
+	RTE_SET_USED(metrics_register_done);
+	RTE_SET_USED(reg_index_list);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_encode_json_format(struct telemetry_encode_param *ep,
+	char **json_buffer)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_ports_stats_json(struct telemetry_encode_param *ep,
+	int *reg_index, char **json_buffer)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(reg_index);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_port_stats_ids(struct telemetry_encode_param *ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_extract_data(struct telemetry_encode_param *ep, json_t *data)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(data);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_global_stats(struct telemetry_encode_param *ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+}
+
+#endif
diff --git a/lib/librte_metrics/rte_metrics_telemetry.h b/lib/librte_metrics/rte_metrics_telemetry.h
index 5dbb32ca0..2b6eb1ccc 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.h
+++ b/lib/librte_metrics/rte_metrics_telemetry.h
@@ -2,7 +2,7 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#ifdef RTE_LIB_TELEMETRY
+#ifdef RTE_HAS_JANSSON
 #include <jansson.h>
 #else
 #define json_t void *
-- 
2.30.0.vfs.0.2


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

* Re: [dpdk-dev] [PATCH v3] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
  2021-02-24 18:46   ` [dpdk-dev] [PATCH v3] " Jie Zhou
@ 2021-03-06 20:18     ` Dmitry Kozlyuk
  2021-03-08 18:05     ` [dpdk-dev] [PATCH v4] " Jie Zhou
  1 sibling, 0 replies; 9+ messages in thread
From: Dmitry Kozlyuk @ 2021-03-06 20:18 UTC (permalink / raw)
  To: Jie Zhou; +Cc: dev, bruce.richardson

2021-02-24 10:46 (UTC-0800), Jie Zhou:
[...]
> V2 changes:
>     Address comments from Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> and
>     Bruce Richardson <bruce.richardson@intel.com>:
>         - Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build
>         - Reduce #ifdef RTE_HAS_JANSSON blocks
> 
> V3 changes:
>     Address comment from Bruce Richardson <bruce.richardson@intel.com>:
>         - Remove redundant includes line on librte_telemetry

Nit: change log should be below "---" so that it's not included in commit.

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

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

* [dpdk-dev] [PATCH v4] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
  2021-02-24 18:46   ` [dpdk-dev] [PATCH v3] " Jie Zhou
  2021-03-06 20:18     ` Dmitry Kozlyuk
@ 2021-03-08 18:05     ` Jie Zhou
  2021-03-16  9:07       ` Thomas Monjalon
  1 sibling, 1 reply; 9+ messages in thread
From: Jie Zhou @ 2021-03-08 18:05 UTC (permalink / raw)
  To: dev; +Cc: dmitry.kozliuk, bruce.richardson

From: Jie Zhou <jizh@microsoft.com>

This patch allows the same set of rte_metrics_tel_* functions to be
exported no matter JANSSON is available or not, by doing following:
1.	Leverage dpdk_conf to set configuration flag RTE_HAVE_JANSSON
when Jansson dependency is found.
2.	In rte_metrics_telemetry.c, leverage RTE_HAVE_JANSSON to handle the
case when JANSSON is not available by adding stubs for all the instances.
3.	In meson.build, per dpdk\doc\guides\rel_notes\release_20_05.rst,
it is claimed that "Telemetry library is no longer dependent on the
external Jansson library, which allows Telemetry be enabled by default.",
thus make the deps and includes of Telemetry as not conditional anymore.

Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

---

V2 changes:
    Address comments from Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> and
    Bruce Richardson <bruce.richardson@intel.com>:
        - Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build.
        - Reduce #ifdef RTE_HAS_JANSSON blocks.

V3 changes:
    Address comment from Bruce Richardson <bruce.richardson@intel.com>:
        - Remove redundant includes line on librte_telemetry.

V4 changes:
    Carry forward Acked-by from previous versions.
    Address comments from Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>:
        - Move change log below "---" so that it's not included in commit.
---
 lib/librte_metrics/meson.build             | 11 ++--
 lib/librte_metrics/rte_metrics_telemetry.c | 60 +++++++++++++++++++++-
 lib/librte_metrics/rte_metrics_telemetry.h |  2 +-
 3 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/lib/librte_metrics/meson.build b/lib/librte_metrics/meson.build
index 28a8cc115..d5be6a214 100644
--- a/lib/librte_metrics/meson.build
+++ b/lib/librte_metrics/meson.build
@@ -1,14 +1,13 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-sources = files('rte_metrics.c')
-headers = files('rte_metrics.h')
+sources = files('rte_metrics.c', 'rte_metrics_telemetry.c')
+headers = files('rte_metrics.h', 'rte_metrics_telemetry.h')
 
 jansson = dependency('jansson', required: false, method: 'pkg-config')
 if jansson.found()
+	dpdk_conf.set('RTE_HAS_JANSSON', 1)
 	ext_deps += jansson
-	sources += files('rte_metrics_telemetry.c')
-	headers += files('rte_metrics_telemetry.h')
-	deps += ['ethdev', 'telemetry']
-	includes += include_directories('../librte_telemetry')
 endif
+
+deps += ['ethdev', 'telemetry']
diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index b8ee56ef0..305ee271f 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -2,8 +2,6 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#include <jansson.h>
-
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TELEMETRY
@@ -13,6 +11,7 @@
 #include "rte_metrics.h"
 #include "rte_metrics_telemetry.h"
 
+#ifdef RTE_HAS_JANSSON
 struct telemetry_metrics_data tel_met_data;
 
 int metrics_log_level;
@@ -541,3 +540,60 @@ RTE_INIT(metrics_ctor)
 			handle_ports_stats_values_by_name);
 #endif
 }
+#else
+int32_t
+rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list)
+{
+	RTE_SET_USED(metrics_register_done);
+	RTE_SET_USED(reg_index_list);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_encode_json_format(struct telemetry_encode_param *ep,
+	char **json_buffer)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_ports_stats_json(struct telemetry_encode_param *ep,
+	int *reg_index, char **json_buffer)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(reg_index);
+	RTE_SET_USED(json_buffer);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_port_stats_ids(struct telemetry_encode_param *ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_extract_data(struct telemetry_encode_param *ep, json_t *data)
+{
+	RTE_SET_USED(ep);
+	RTE_SET_USED(data);
+
+	return -ENOTSUP;
+}
+
+int32_t
+rte_metrics_tel_get_global_stats(struct telemetry_encode_param *ep)
+{
+	RTE_SET_USED(ep);
+
+	return -ENOTSUP;
+}
+
+#endif
diff --git a/lib/librte_metrics/rte_metrics_telemetry.h b/lib/librte_metrics/rte_metrics_telemetry.h
index 5dbb32ca0..2b6eb1ccc 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.h
+++ b/lib/librte_metrics/rte_metrics_telemetry.h
@@ -2,7 +2,7 @@
  * Copyright(c) 2020 Intel Corporation
  */
 
-#ifdef RTE_LIB_TELEMETRY
+#ifdef RTE_HAS_JANSSON
 #include <jansson.h>
 #else
 #define json_t void *
-- 
2.30.0.vfs.0.2


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

* Re: [dpdk-dev] [PATCH v4] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
  2021-03-08 18:05     ` [dpdk-dev] [PATCH v4] " Jie Zhou
@ 2021-03-16  9:07       ` Thomas Monjalon
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2021-03-16  9:07 UTC (permalink / raw)
  To: Jie Zhou; +Cc: dev, dmitry.kozliuk, bruce.richardson

08/03/2021 19:05, Jie Zhou:
> From: Jie Zhou <jizh@microsoft.com>
> 
> This patch allows the same set of rte_metrics_tel_* functions to be
> exported no matter JANSSON is available or not, by doing following:
> 1.	Leverage dpdk_conf to set configuration flag RTE_HAVE_JANSSON

Changed to RTE_HAS_JANSSON.

> when Jansson dependency is found.
> 2.	In rte_metrics_telemetry.c, leverage RTE_HAVE_JANSSON to handle the
> case when JANSSON is not available by adding stubs for all the instances.
> 3.	In meson.build, per dpdk\doc\guides\rel_notes\release_20_05.rst,

Changed to use / instead of \

> it is claimed that "Telemetry library is no longer dependent on the
> external Jansson library, which allows Telemetry be enabled by default.",
> thus make the deps and includes of Telemetry as not conditional anymore.
> 
> Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>

This email address is not the same as above.
Changed to be the same.

> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

> +#ifdef RTE_HAS_JANSSON
>  struct telemetry_metrics_data tel_met_data;

Added space and comments around #ifdef and #else.

Applied with minor changes, thanks.



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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22 21:25 [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions Jie
2021-02-22 22:24 ` Dmitry Kozlyuk
2021-02-23 10:03   ` Bruce Richardson
2021-02-23 23:01 ` [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports " Jie Zhou
2021-02-24 10:36   ` Bruce Richardson
2021-02-24 18:46   ` [dpdk-dev] [PATCH v3] " Jie Zhou
2021-03-06 20:18     ` Dmitry Kozlyuk
2021-03-08 18:05     ` [dpdk-dev] [PATCH v4] " Jie Zhou
2021-03-16  9:07       ` 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).