rte_metrics: unconditionally export rte_metrics_tel_xxx functions
PATCH 1/1 rte_metrics: unconditionally export rte_metrics_tel_xxx functions
From: Jie Zhou <[email protected]>
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.
--- 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(-)
config/meson.build +5 -0
@@ -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')
lib/librte_metrics/meson.build +7 -6
@@ -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')
lib/librte_metrics/rte_metrics_telemetry.c +53 -0
147 lines not on this page · raw message
lib/librte_metrics/rte_metrics_telemetry.h +1 -1
@@ -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 *
Signature · 1 line
2.30.0.vfs.0.2
+ Bruce
On Mon, 22 Feb 2021 13:25:02 -0800, Jie wrote:
[...]
config/meson.build if is_freebsd · line 298
Quoted 12 lines
config/meson.build +4
-0@@ -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?
[...]
Quoted 8 lines
lib/librte_metrics/meson.build +0
-2@@ -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()
lib/librte_metrics/meson.build line 12
Quoted 10 lines
lib/librte_metrics/meson.build +4
-4+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.
[...]
Quoted 4 lines
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.)
Quoted 43 lines · Dmitry Kozlyuk
+ Bruce
On Mon, 22 Feb 2021 13:25:02 -0800, Jie wrote:
[...]config/meson.build if is_freebsd · line 298
Quoted 12 lines
config/meson.build +4
-0@@ -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) +endifDPDK seems to prefer "HAS" for such macros.
Why not do this in
lib/librte_telemetry/meson.build?[...]
Quoted 8 lines
lib/librte_metrics/meson.build +0
-2@@ -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()lib/librte_metrics/meson.build line 12
Quoted 10 lines
lib/librte_metrics/meson.build +4
-4+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.
Quoted 8 lines
[...]
Quoted 4 lines
int32_t
rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list){
+#ifdef JANSSONWhy not use
RTE_HAS_JANSSONeverywhere? (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
PATCH v2 1/1 rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
In reply to JieFrom: Jie Zhou <[email protected]>
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 <[email protected]> and Bruce Richardson <[email protected]>:
- Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build
- Reduce #ifdef RTE_HAS_JANSSON blocks
--- 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(-)
lib/librte_metrics/meson.build +6 -6
@@ -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')
lib/librte_metrics/rte_metrics_telemetry.c +58 -2
75 lines not on this page · raw message
lib/librte_metrics/rte_metrics_telemetry.h +1 -1
@@ -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 *
Signature · 1 line
2.30.0.vfs.0.2
Re: [dpdk-dev] [PATCH v2] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
Quoted 13 lines · Jie Zhou
From: Jie Zhou <[email protected]>
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. Leveragedpdk_confto set configuration flagRTE_HAVE_JANSSON
when Jansson dependency is found.
2. Inrte_metrics_telemetry.c, leverageRTE_HAVE_JANSSONto 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.
Quoted 7 lines
V2 changes:
Address comments from Dmitry Kozlyuk <[email protected]> and Bruce Richardson <[email protected]>:
- Set dpdk.confRTE_HAS_JANSSONin metrics meson.build
- Reduce #ifdefRTE_HAS_JANSSONblocksSigned-off-by Jie Zhou <[email protected]>
One small nit below, but otherwise this looks better.
Quoted 21 lines
--- 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(-)lib/librte_metrics/meson.build +2
-2@@ -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()
lib/librte_metrics/meson.build line 14
Quoted 10 lines
lib/librte_metrics/meson.build +4
-4+ 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.
Quoted 98 lines
lib/librte_metrics/rte_metrics_telemetry.c +58
-2@@ -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 lib/librte_metrics/rte_metrics_telemetry.h +1
-1@@ -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 *Signature · 1 line
2.30.0.vfs.0.2
PATCH v3 1/1 rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
In reply to Jie ZhouFrom: Jie Zhou <[email protected]>
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 <[email protected]> and Bruce Richardson <[email protected]>:
- Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build
- Reduce #ifdef RTE_HAS_JANSSON blocks
V3 changes:
Address comment from Bruce Richardson <[email protected]>:
- Remove redundant includes line on librte_telemetry
--- 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(-)
lib/librte_metrics/meson.build +5 -6
@@ -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']
lib/librte_metrics/rte_metrics_telemetry.c +58 -2
75 lines not on this page · raw message
lib/librte_metrics/rte_metrics_telemetry.h +1 -1
@@ -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 *
Signature · 1 line
2.30.0.vfs.0.2
Re: [dpdk-dev] [PATCH v3] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
2021-02-24 10:46 (UTC-0800), Jie Zhou:
[...]
Quoted 9 lines
V2 changes:
Address comments from Dmitry Kozlyuk <[email protected]> and Bruce Richardson <[email protected]>:
- Set dpdk.confRTE_HAS_JANSSONin metrics meson.build
- Reduce #ifdefRTE_HAS_JANSSONblocksV3 changes:
Address comment from Bruce Richardson <[email protected]>:
- Remove redundant includes line onlibrte_telemetry
Nit: change log should be below "---" so that it's not included in commit.
PATCH v4 1/1 rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
In reply to Jie ZhouFrom: Jie Zhou <[email protected]>
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 <[email protected]> and Bruce Richardson <[email protected]>:
- Set dpdk.conf RTE_HAS_JANSSON in metrics meson.build.
- Reduce #ifdef RTE_HAS_JANSSON blocks.
V3 changes:
Address comment from Bruce Richardson <[email protected]>:
- Remove redundant includes line on librte_telemetry.
V4 changes:
Carry forward Acked-by from previous versions.
Address comments from Dmitry Kozlyuk <[email protected]>:
- 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(-)
lib/librte_metrics/meson.build +5 -6
@@ -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']
lib/librte_metrics/rte_metrics_telemetry.c +58 -2
75 lines not on this page · raw message
lib/librte_metrics/rte_metrics_telemetry.h +1 -1
@@ -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 *
Signature · 1 line
2.30.0.vfs.0.2
Re: [dpdk-dev] [PATCH v4] rte_metrics: unconditionally exports rte_metrics_tel_xxx functions
08/03/2021 19:05, Jie Zhou:
Quoted 5 lines
From: Jie Zhou <[email protected]>
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. Leveragedpdk_confto set configuration flagRTE_HAVE_JANSSON
Changed to RTE_HAS_JANSSON.
Quoted 4 lines
when Jansson dependency is found.
2. Inrte_metrics_telemetry.c, leverageRTE_HAVE_JANSSONto 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 \
Quoted 5 lines
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 <[email protected]>
This email address is not the same as above.
Changed to be the same.
Quoted 5 lines
Acked-by Bruce Richardson <[email protected]>Acked-by Dmitry Kozlyuk <[email protected]>+#ifdef
RTE_HAS_JANSSON
structtelemetry_metrics_datatel_met_data;
Added space and comments around #ifdef and #else.
Applied with minor changes, thanks.