[v2] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions

Message ID 1614121272-26776-1-git-send-email-jizh@linux.microsoft.com (mailing list archive)
State Superseded, archived
Headers
Series [v2] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-mellanox-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Jie Zhou Feb. 23, 2021, 11:01 p.m. UTC
  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(-)
  

Comments

Bruce Richardson Feb. 24, 2021, 10:36 a.m. UTC | #1
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
>
  

Patch

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 *