rte_metrics: unconditionally export rte_metrics_tel_xxx functions
Checks
Commit Message
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(-)
Comments
+ 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.)
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
@@ -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')
@@ -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')
@@ -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
@@ -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 *