| Message ID | 20200303181817.3448334-1-ferruh.yigit@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Marchand |
| Headers | show |
| Series | [v2] log: add API to check if a logtype can log in a given level | expand |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | success | Compilation OK |
| ci/travis-robot | success | Travis build: passed |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/iol-testing | success | Testing PASS |
| ci/checkpatch | success | coding style OK |
> -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > Sent: Wednesday, March 4, 2020 3:18 AM > To: Hemant Agrawal <hemant.agrawal@nxp.com>; Sachin Saxena > <sachin.saxena@nxp.com>; Fiona Trahe <fiona.trahe@intel.com>; John > Griffin <john.griffin@intel.com>; Deepak Kumar Jain > <deepak.k.jain@intel.com>; John Daley (johndale) <johndale@cisco.com>; > Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>; Alfredo Cardigliano > <cardigliano@ntop.org>; Matan Azrad <matan@mellanox.com>; Shahaf > Shuler <shahafs@mellanox.com>; Viacheslav Ovsiienko > <viacheslavo@mellanox.com>; Bernard Iremonger > <bernard.iremonger@intel.com> > Cc: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@intel.com>; David Marchand > <david.marchand@redhat.com>; Thomas Monjalon > <thomas@monjalon.net>; Stephen Hemminger > <stephen@networkplumber.org> > Subject: [PATCH v2] log: add API to check if a logtype can log in a given level > > This is a helper function in case components would like to do more work > than just logging a message based on log level, like for example > collecting some stats if the log type is DEBUG etc.. > > A few existing relevant usage converted to this new API. > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > --- > Cc: David Marchand <david.marchand@redhat.com> > Cc: Thomas Monjalon <thomas@monjalon.net> > Cc: Stephen Hemminger <stephen@networkplumber.org> > > v2: > * Convert API return type to 'bool'. Removed custom definitions from > 'ionic' PMD for this. > --- > drivers/bus/fslmc/fslmc_bus.c | 7 +------ > drivers/common/qat/qat_logs.c | 7 ++----- > drivers/net/enic/enic_fm_flow.c | 2 +- > drivers/net/ionic/ionic_osdep.h | 6 ------ > drivers/net/mlx5/mlx5_mr.c | 2 +- > lib/librte_eal/common/eal_common_log.c | 18 ++++++++++++++++++ > lib/librte_eal/common/include/rte_log.h | 14 ++++++++++++++ > lib/librte_eal/rte_eal_version.map | 3 +++ > lib/librte_flow_classify/rte_flow_classify.c | 7 ++----- > 9 files changed, 42 insertions(+), 24 deletions(-) > [...] > diff --git a/drivers/net/enic/enic_fm_flow.c > b/drivers/net/enic/enic_fm_flow.c > index c0ddfe9ba..d815f369e 100644 > --- a/drivers/net/enic/enic_fm_flow.c > +++ b/drivers/net/enic/enic_fm_flow.c > @@ -1504,7 +1504,7 @@ enic_fm_dump_tcam_entry(const struct > fm_tcam_match_entry *fm_match, > const struct fm_action *fm_action, > uint8_t ingress) > { > - if (rte_log_get_level(enic_pmd_logtype) < (int)RTE_LOG_DEBUG) > + if (!rte_log_can_log(enic_pmd_logtype, RTE_LOG_DEBUG)) > return; > enic_fm_dump_tcam_match(fm_match, ingress); > enic_fm_dump_tcam_actions(fm_action); For enic, Acked-by: Hyong Youb Kim <hyonkim@cisco.com> Thanks.. -Hyong
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
On Tue, Mar 3, 2020 at 7:18 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote: > > This is a helper function in case components would like to do more work > than just logging a message based on log level, like for example > collecting some stats if the log type is DEBUG etc.. > > A few existing relevant usage converted to this new API. > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > --- > Cc: David Marchand <david.marchand@redhat.com> > Cc: Thomas Monjalon <thomas@monjalon.net> > Cc: Stephen Hemminger <stephen@networkplumber.org> > > v2: > * Convert API return type to 'bool'. Removed custom definitions from > 'ionic' PMD for this. > --- > drivers/bus/fslmc/fslmc_bus.c | 7 +------ > drivers/common/qat/qat_logs.c | 7 ++----- > drivers/net/enic/enic_fm_flow.c | 2 +- > drivers/net/ionic/ionic_osdep.h | 6 ------ > drivers/net/mlx5/mlx5_mr.c | 2 +- > lib/librte_eal/common/eal_common_log.c | 18 ++++++++++++++++++ > lib/librte_eal/common/include/rte_log.h | 14 ++++++++++++++ > lib/librte_eal/rte_eal_version.map | 3 +++ > lib/librte_flow_classify/rte_flow_classify.c | 7 ++----- > 9 files changed, 42 insertions(+), 24 deletions(-) > > diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c > index b3e964aa9..afbd82e8d 100644 > --- a/drivers/bus/fslmc/fslmc_bus.c > +++ b/drivers/bus/fslmc/fslmc_bus.c > @@ -115,14 +115,9 @@ static void > dump_device_list(void) > { > struct rte_dpaa2_device *dev; > - uint32_t global_log_level; > - int local_log_level; > > /* Only if the log level has been set to Debugging, print list */ > - global_log_level = rte_log_get_global_level(); > - local_log_level = rte_log_get_level(dpaa2_logtype_bus); > - if (global_log_level == RTE_LOG_DEBUG || > - local_log_level == RTE_LOG_DEBUG) { > + if (rte_log_can_log(dpaa2_logtype_bus, RTE_LOG_DEBUG)) { Here, this is a change in behavior, but it aligns this driver to the rest of dpdk, and we got a ack from Hemant. > DPAA2_BUS_LOG(DEBUG, "List of devices scanned on bus:"); > TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) { > DPAA2_BUS_LOG(DEBUG, "\t\t%s", dev->device.name); > diff --git a/drivers/common/qat/qat_logs.c b/drivers/common/qat/qat_logs.c > index f97aba19d..dfd0cbe5d 100644 > --- a/drivers/common/qat/qat_logs.c > +++ b/drivers/common/qat/qat_logs.c > @@ -14,12 +14,9 @@ int > qat_hexdump_log(uint32_t level, uint32_t logtype, const char *title, > const void *buf, unsigned int len) > { > - if (level > rte_log_get_global_level()) > - return 0; > - if (level > (uint32_t)(rte_log_get_level(logtype))) > - return 0; > + if (rte_log_can_log(logtype, level)) > + rte_hexdump(rte_log_get_stream(), title, buf, len); > > - rte_hexdump(rte_log_get_stream(), title, buf, len); > return 0; > } > > diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c > index c0ddfe9ba..d815f369e 100644 > --- a/drivers/net/enic/enic_fm_flow.c > +++ b/drivers/net/enic/enic_fm_flow.c > @@ -1504,7 +1504,7 @@ enic_fm_dump_tcam_entry(const struct fm_tcam_match_entry *fm_match, > const struct fm_action *fm_action, > uint8_t ingress) > { > - if (rte_log_get_level(enic_pmd_logtype) < (int)RTE_LOG_DEBUG) > + if (!rte_log_can_log(enic_pmd_logtype, RTE_LOG_DEBUG)) > return; > enic_fm_dump_tcam_match(fm_match, ingress); > enic_fm_dump_tcam_actions(fm_action); > diff --git a/drivers/net/ionic/ionic_osdep.h b/drivers/net/ionic/ionic_osdep.h > index ecdbc24e6..e04bb8f65 100644 > --- a/drivers/net/ionic/ionic_osdep.h > +++ b/drivers/net/ionic/ionic_osdep.h > @@ -44,12 +44,6 @@ typedef uint16_t __le16; > typedef uint32_t __le32; > typedef uint64_t __le64; > > -#ifndef __cplusplus > -typedef uint8_t bool; > -#define false 0 > -#define true 1 > -#endif > - > static inline uint32_t div_round_up(uint32_t n, uint32_t d) > { > return (n + d - 1) / d; > diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c > index cb97c876e..6aa578646 100644 > --- a/drivers/net/mlx5/mlx5_mr.c > +++ b/drivers/net/mlx5/mlx5_mr.c > @@ -1597,7 +1597,7 @@ mlx5_mr_release(struct mlx5_ibv_shared *sh) > { > struct mlx5_mr *mr_next; > > - if (rte_log_get_level(mlx5_logtype) == RTE_LOG_DEBUG) > + if (rte_log_can_log(mlx5_logtype, RTE_LOG_DEBUG)) > mlx5_mr_dump_dev(sh); > rte_rwlock_write_lock(&sh->mr.rwlock); > /* Detach from MR list and move to free list. */ > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c > index c0efd5214..679e7326f 100644 > --- a/lib/librte_eal/common/eal_common_log.c > +++ b/lib/librte_eal/common/eal_common_log.c > @@ -112,6 +112,24 @@ rte_log_get_level(uint32_t type) > return rte_logs.dynamic_types[type].loglevel; > } > > +bool > +rte_log_can_log(uint32_t logtype, uint32_t level) > +{ > + int log_level; > + > + if (level > rte_log_get_global_level()) > + return false; > + > + log_level = rte_log_get_level(logtype); > + if (log_level < 0) > + return false; > + > + if (level > (uint32_t)log_level) > + return false; > + > + return true; > +} > + > int > rte_log_set_level(uint32_t type, uint32_t level) > { > diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h > index 1bb0e6694..2d1bae5c4 100644 > --- a/lib/librte_eal/common/include/rte_log.h > +++ b/lib/librte_eal/common/include/rte_log.h > @@ -20,6 +20,7 @@ extern "C" { > #include <stdint.h> > #include <stdio.h> > #include <stdarg.h> > +#include <stdbool.h> > #include <sys/queue.h> > > #include <rte_common.h> > @@ -143,6 +144,19 @@ uint32_t rte_log_get_global_level(void); > */ > int rte_log_get_level(uint32_t logtype); > > +/** > + * Check if a given `level` can be printed by a given `logtype` The description is a bit odd to me. A "level" is not printed. A "logtype" does not print. > + * > + * @param logtype > + * The log type identifier > + * @param level > + * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8). > + * @return > + * Returns 'true' if log can be printed and 'false' if it can't. > + */ > +__rte_experimental > +bool rte_log_can_log(uint32_t logtype, uint32_t level); > + Would it make sense to actually change rte_log_get_level() to always take the global level into account? Then we would not introduce a new API, but this means deferring this patch to 20.11. If we go with this helper, we can replace the check we have in rte_vlog: https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/eal_common_log.c#n420 > /** > * Set the log level for a given type based on shell pattern. > * > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map > index 6cf507068..f9ede5b41 100644 > --- a/lib/librte_eal/rte_eal_version.map > +++ b/lib/librte_eal/rte_eal_version.map > @@ -335,4 +335,7 @@ EXPERIMENTAL { > > # added in 20.02 > rte_thread_is_intr; > + > + # added in 20.05 > + rte_log_can_log; > }; > diff --git a/lib/librte_flow_classify/rte_flow_classify.c b/lib/librte_flow_classify/rte_flow_classify.c > index 5ff585803..6022064d8 100644 > --- a/lib/librte_flow_classify/rte_flow_classify.c > +++ b/lib/librte_flow_classify/rte_flow_classify.c > @@ -417,7 +417,6 @@ static struct rte_flow_classify_rule * > allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) > { > struct rte_flow_classify_rule *rule; > - int log_level; > > rule = malloc(sizeof(struct rte_flow_classify_rule)); > if (!rule) > @@ -466,9 +465,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) > cls->ntuple_filter.dst_port_mask; > rule->rules.u.ipv4_5tuple.dst_port = cls->ntuple_filter.dst_port; > > - log_level = rte_log_get_level(librte_flow_classify_logtype); > - > - if (log_level == RTE_LOG_DEBUG) > + if (rte_log_can_log(librte_flow_classify_logtype, RTE_LOG_DEBUG)) Idem, behavior change, but looks sane. > print_acl_ipv4_key_add(&rule->u.key.key_add); > > /* key delete values */ > @@ -476,7 +473,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) > &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4], > NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field)); > > - if (log_level == RTE_LOG_DEBUG) > + if (rte_log_can_log(librte_flow_classify_logtype, RTE_LOG_DEBUG)) > print_acl_ipv4_key_delete(&rule->u.key.key_del); > > return rule; > -- > 2.24.1 >
On 3/12/2020 1:41 PM, David Marchand wrote: > On Tue, Mar 3, 2020 at 7:18 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote: >> >> This is a helper function in case components would like to do more work >> than just logging a message based on log level, like for example >> collecting some stats if the log type is DEBUG etc.. >> >> A few existing relevant usage converted to this new API. >> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> >> --- >> Cc: David Marchand <david.marchand@redhat.com> >> Cc: Thomas Monjalon <thomas@monjalon.net> >> Cc: Stephen Hemminger <stephen@networkplumber.org> >> >> v2: >> * Convert API return type to 'bool'. Removed custom definitions from >> 'ionic' PMD for this. >> --- >> drivers/bus/fslmc/fslmc_bus.c | 7 +------ >> drivers/common/qat/qat_logs.c | 7 ++----- >> drivers/net/enic/enic_fm_flow.c | 2 +- >> drivers/net/ionic/ionic_osdep.h | 6 ------ >> drivers/net/mlx5/mlx5_mr.c | 2 +- >> lib/librte_eal/common/eal_common_log.c | 18 ++++++++++++++++++ >> lib/librte_eal/common/include/rte_log.h | 14 ++++++++++++++ >> lib/librte_eal/rte_eal_version.map | 3 +++ >> lib/librte_flow_classify/rte_flow_classify.c | 7 ++----- >> 9 files changed, 42 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c >> index b3e964aa9..afbd82e8d 100644 >> --- a/drivers/bus/fslmc/fslmc_bus.c >> +++ b/drivers/bus/fslmc/fslmc_bus.c >> @@ -115,14 +115,9 @@ static void >> dump_device_list(void) >> { >> struct rte_dpaa2_device *dev; >> - uint32_t global_log_level; >> - int local_log_level; >> >> /* Only if the log level has been set to Debugging, print list */ >> - global_log_level = rte_log_get_global_level(); >> - local_log_level = rte_log_get_level(dpaa2_logtype_bus); >> - if (global_log_level == RTE_LOG_DEBUG || >> - local_log_level == RTE_LOG_DEBUG) { >> + if (rte_log_can_log(dpaa2_logtype_bus, RTE_LOG_DEBUG)) { > > Here, this is a change in behavior, but it aligns this driver to the > rest of dpdk, and we got a ack from Hemant. > > >> DPAA2_BUS_LOG(DEBUG, "List of devices scanned on bus:"); >> TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) { >> DPAA2_BUS_LOG(DEBUG, "\t\t%s", dev->device.name); >> diff --git a/drivers/common/qat/qat_logs.c b/drivers/common/qat/qat_logs.c >> index f97aba19d..dfd0cbe5d 100644 >> --- a/drivers/common/qat/qat_logs.c >> +++ b/drivers/common/qat/qat_logs.c >> @@ -14,12 +14,9 @@ int >> qat_hexdump_log(uint32_t level, uint32_t logtype, const char *title, >> const void *buf, unsigned int len) >> { >> - if (level > rte_log_get_global_level()) >> - return 0; >> - if (level > (uint32_t)(rte_log_get_level(logtype))) >> - return 0; >> + if (rte_log_can_log(logtype, level)) >> + rte_hexdump(rte_log_get_stream(), title, buf, len); >> >> - rte_hexdump(rte_log_get_stream(), title, buf, len); >> return 0; >> } >> >> diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c >> index c0ddfe9ba..d815f369e 100644 >> --- a/drivers/net/enic/enic_fm_flow.c >> +++ b/drivers/net/enic/enic_fm_flow.c >> @@ -1504,7 +1504,7 @@ enic_fm_dump_tcam_entry(const struct fm_tcam_match_entry *fm_match, >> const struct fm_action *fm_action, >> uint8_t ingress) >> { >> - if (rte_log_get_level(enic_pmd_logtype) < (int)RTE_LOG_DEBUG) >> + if (!rte_log_can_log(enic_pmd_logtype, RTE_LOG_DEBUG)) >> return; >> enic_fm_dump_tcam_match(fm_match, ingress); >> enic_fm_dump_tcam_actions(fm_action); >> diff --git a/drivers/net/ionic/ionic_osdep.h b/drivers/net/ionic/ionic_osdep.h >> index ecdbc24e6..e04bb8f65 100644 >> --- a/drivers/net/ionic/ionic_osdep.h >> +++ b/drivers/net/ionic/ionic_osdep.h >> @@ -44,12 +44,6 @@ typedef uint16_t __le16; >> typedef uint32_t __le32; >> typedef uint64_t __le64; >> >> -#ifndef __cplusplus >> -typedef uint8_t bool; >> -#define false 0 >> -#define true 1 >> -#endif >> - >> static inline uint32_t div_round_up(uint32_t n, uint32_t d) >> { >> return (n + d - 1) / d; >> diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c >> index cb97c876e..6aa578646 100644 >> --- a/drivers/net/mlx5/mlx5_mr.c >> +++ b/drivers/net/mlx5/mlx5_mr.c >> @@ -1597,7 +1597,7 @@ mlx5_mr_release(struct mlx5_ibv_shared *sh) >> { >> struct mlx5_mr *mr_next; >> >> - if (rte_log_get_level(mlx5_logtype) == RTE_LOG_DEBUG) >> + if (rte_log_can_log(mlx5_logtype, RTE_LOG_DEBUG)) >> mlx5_mr_dump_dev(sh); >> rte_rwlock_write_lock(&sh->mr.rwlock); >> /* Detach from MR list and move to free list. */ >> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c >> index c0efd5214..679e7326f 100644 >> --- a/lib/librte_eal/common/eal_common_log.c >> +++ b/lib/librte_eal/common/eal_common_log.c >> @@ -112,6 +112,24 @@ rte_log_get_level(uint32_t type) >> return rte_logs.dynamic_types[type].loglevel; >> } >> >> +bool >> +rte_log_can_log(uint32_t logtype, uint32_t level) >> +{ >> + int log_level; >> + >> + if (level > rte_log_get_global_level()) >> + return false; >> + >> + log_level = rte_log_get_level(logtype); >> + if (log_level < 0) >> + return false; >> + >> + if (level > (uint32_t)log_level) >> + return false; >> + >> + return true; >> +} >> + >> int >> rte_log_set_level(uint32_t type, uint32_t level) >> { >> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h >> index 1bb0e6694..2d1bae5c4 100644 >> --- a/lib/librte_eal/common/include/rte_log.h >> +++ b/lib/librte_eal/common/include/rte_log.h >> @@ -20,6 +20,7 @@ extern "C" { >> #include <stdint.h> >> #include <stdio.h> >> #include <stdarg.h> >> +#include <stdbool.h> >> #include <sys/queue.h> >> >> #include <rte_common.h> >> @@ -143,6 +144,19 @@ uint32_t rte_log_get_global_level(void); >> */ >> int rte_log_get_level(uint32_t logtype); >> >> +/** >> + * Check if a given `level` can be printed by a given `logtype` > > The description is a bit odd to me. > A "level" is not printed. > A "logtype" does not print. Agree, what about following, is it more clear: " For a given `logtype`, check if log with `loglevel` will be printed or not. " > > >> + * >> + * @param logtype >> + * The log type identifier >> + * @param level >> + * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8). >> + * @return >> + * Returns 'true' if log can be printed and 'false' if it can't. >> + */ >> +__rte_experimental >> +bool rte_log_can_log(uint32_t logtype, uint32_t level); >> + > > Would it make sense to actually change rte_log_get_level() to always > take the global level into account? Not sure, it can make 'rte_log_get_level()' confusing > Then we would not introduce a new API, but this means deferring this > patch to 20.11. Apart from global level check, I think it is good to have this helper instead of applications getting log level and comparing themselves, this API simplifies it. > > > If we go with this helper, we can replace the check we have in rte_vlog: > https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/eal_common_log.c#n420 Right, why not, I can update in next version. > > >> /** >> * Set the log level for a given type based on shell pattern. >> * >> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map >> index 6cf507068..f9ede5b41 100644 >> --- a/lib/librte_eal/rte_eal_version.map >> +++ b/lib/librte_eal/rte_eal_version.map >> @@ -335,4 +335,7 @@ EXPERIMENTAL { >> >> # added in 20.02 >> rte_thread_is_intr; >> + >> + # added in 20.05 >> + rte_log_can_log; >> }; >> diff --git a/lib/librte_flow_classify/rte_flow_classify.c b/lib/librte_flow_classify/rte_flow_classify.c >> index 5ff585803..6022064d8 100644 >> --- a/lib/librte_flow_classify/rte_flow_classify.c >> +++ b/lib/librte_flow_classify/rte_flow_classify.c >> @@ -417,7 +417,6 @@ static struct rte_flow_classify_rule * >> allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) >> { >> struct rte_flow_classify_rule *rule; >> - int log_level; >> >> rule = malloc(sizeof(struct rte_flow_classify_rule)); >> if (!rule) >> @@ -466,9 +465,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) >> cls->ntuple_filter.dst_port_mask; >> rule->rules.u.ipv4_5tuple.dst_port = cls->ntuple_filter.dst_port; >> >> - log_level = rte_log_get_level(librte_flow_classify_logtype); >> - >> - if (log_level == RTE_LOG_DEBUG) >> + if (rte_log_can_log(librte_flow_classify_logtype, RTE_LOG_DEBUG)) > > Idem, behavior change, but looks sane. > > >> print_acl_ipv4_key_add(&rule->u.key.key_add); >> >> /* key delete values */ >> @@ -476,7 +473,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) >> &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4], >> NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field)); >> >> - if (log_level == RTE_LOG_DEBUG) >> + if (rte_log_can_log(librte_flow_classify_logtype, RTE_LOG_DEBUG)) >> print_acl_ipv4_key_delete(&rule->u.key.key_del); >> >> return rule; >> -- >> 2.24.1 >> > >
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c index b3e964aa9..afbd82e8d 100644 --- a/drivers/bus/fslmc/fslmc_bus.c +++ b/drivers/bus/fslmc/fslmc_bus.c @@ -115,14 +115,9 @@ static void dump_device_list(void) { struct rte_dpaa2_device *dev; - uint32_t global_log_level; - int local_log_level; /* Only if the log level has been set to Debugging, print list */ - global_log_level = rte_log_get_global_level(); - local_log_level = rte_log_get_level(dpaa2_logtype_bus); - if (global_log_level == RTE_LOG_DEBUG || - local_log_level == RTE_LOG_DEBUG) { + if (rte_log_can_log(dpaa2_logtype_bus, RTE_LOG_DEBUG)) { DPAA2_BUS_LOG(DEBUG, "List of devices scanned on bus:"); TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) { DPAA2_BUS_LOG(DEBUG, "\t\t%s", dev->device.name); diff --git a/drivers/common/qat/qat_logs.c b/drivers/common/qat/qat_logs.c index f97aba19d..dfd0cbe5d 100644 --- a/drivers/common/qat/qat_logs.c +++ b/drivers/common/qat/qat_logs.c @@ -14,12 +14,9 @@ int qat_hexdump_log(uint32_t level, uint32_t logtype, const char *title, const void *buf, unsigned int len) { - if (level > rte_log_get_global_level()) - return 0; - if (level > (uint32_t)(rte_log_get_level(logtype))) - return 0; + if (rte_log_can_log(logtype, level)) + rte_hexdump(rte_log_get_stream(), title, buf, len); - rte_hexdump(rte_log_get_stream(), title, buf, len); return 0; } diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c index c0ddfe9ba..d815f369e 100644 --- a/drivers/net/enic/enic_fm_flow.c +++ b/drivers/net/enic/enic_fm_flow.c @@ -1504,7 +1504,7 @@ enic_fm_dump_tcam_entry(const struct fm_tcam_match_entry *fm_match, const struct fm_action *fm_action, uint8_t ingress) { - if (rte_log_get_level(enic_pmd_logtype) < (int)RTE_LOG_DEBUG) + if (!rte_log_can_log(enic_pmd_logtype, RTE_LOG_DEBUG)) return; enic_fm_dump_tcam_match(fm_match, ingress); enic_fm_dump_tcam_actions(fm_action); diff --git a/drivers/net/ionic/ionic_osdep.h b/drivers/net/ionic/ionic_osdep.h index ecdbc24e6..e04bb8f65 100644 --- a/drivers/net/ionic/ionic_osdep.h +++ b/drivers/net/ionic/ionic_osdep.h @@ -44,12 +44,6 @@ typedef uint16_t __le16; typedef uint32_t __le32; typedef uint64_t __le64; -#ifndef __cplusplus -typedef uint8_t bool; -#define false 0 -#define true 1 -#endif - static inline uint32_t div_round_up(uint32_t n, uint32_t d) { return (n + d - 1) / d; diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c index cb97c876e..6aa578646 100644 --- a/drivers/net/mlx5/mlx5_mr.c +++ b/drivers/net/mlx5/mlx5_mr.c @@ -1597,7 +1597,7 @@ mlx5_mr_release(struct mlx5_ibv_shared *sh) { struct mlx5_mr *mr_next; - if (rte_log_get_level(mlx5_logtype) == RTE_LOG_DEBUG) + if (rte_log_can_log(mlx5_logtype, RTE_LOG_DEBUG)) mlx5_mr_dump_dev(sh); rte_rwlock_write_lock(&sh->mr.rwlock); /* Detach from MR list and move to free list. */ diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c index c0efd5214..679e7326f 100644 --- a/lib/librte_eal/common/eal_common_log.c +++ b/lib/librte_eal/common/eal_common_log.c @@ -112,6 +112,24 @@ rte_log_get_level(uint32_t type) return rte_logs.dynamic_types[type].loglevel; } +bool +rte_log_can_log(uint32_t logtype, uint32_t level) +{ + int log_level; + + if (level > rte_log_get_global_level()) + return false; + + log_level = rte_log_get_level(logtype); + if (log_level < 0) + return false; + + if (level > (uint32_t)log_level) + return false; + + return true; +} + int rte_log_set_level(uint32_t type, uint32_t level) { diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h index 1bb0e6694..2d1bae5c4 100644 --- a/lib/librte_eal/common/include/rte_log.h +++ b/lib/librte_eal/common/include/rte_log.h @@ -20,6 +20,7 @@ extern "C" { #include <stdint.h> #include <stdio.h> #include <stdarg.h> +#include <stdbool.h> #include <sys/queue.h> #include <rte_common.h> @@ -143,6 +144,19 @@ uint32_t rte_log_get_global_level(void); */ int rte_log_get_level(uint32_t logtype); +/** + * Check if a given `level` can be printed by a given `logtype` + * + * @param logtype + * The log type identifier + * @param level + * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8). + * @return + * Returns 'true' if log can be printed and 'false' if it can't. + */ +__rte_experimental +bool rte_log_can_log(uint32_t logtype, uint32_t level); + /** * Set the log level for a given type based on shell pattern. * diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map index 6cf507068..f9ede5b41 100644 --- a/lib/librte_eal/rte_eal_version.map +++ b/lib/librte_eal/rte_eal_version.map @@ -335,4 +335,7 @@ EXPERIMENTAL { # added in 20.02 rte_thread_is_intr; + + # added in 20.05 + rte_log_can_log; }; diff --git a/lib/librte_flow_classify/rte_flow_classify.c b/lib/librte_flow_classify/rte_flow_classify.c index 5ff585803..6022064d8 100644 --- a/lib/librte_flow_classify/rte_flow_classify.c +++ b/lib/librte_flow_classify/rte_flow_classify.c @@ -417,7 +417,6 @@ static struct rte_flow_classify_rule * allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) { struct rte_flow_classify_rule *rule; - int log_level; rule = malloc(sizeof(struct rte_flow_classify_rule)); if (!rule) @@ -466,9 +465,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) cls->ntuple_filter.dst_port_mask; rule->rules.u.ipv4_5tuple.dst_port = cls->ntuple_filter.dst_port; - log_level = rte_log_get_level(librte_flow_classify_logtype); - - if (log_level == RTE_LOG_DEBUG) + if (rte_log_can_log(librte_flow_classify_logtype, RTE_LOG_DEBUG)) print_acl_ipv4_key_add(&rule->u.key.key_add); /* key delete values */ @@ -476,7 +473,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls) &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4], NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field)); - if (log_level == RTE_LOG_DEBUG) + if (rte_log_can_log(librte_flow_classify_logtype, RTE_LOG_DEBUG)) print_acl_ipv4_key_delete(&rule->u.key.key_del); return rule;
This is a helper function in case components would like to do more work than just logging a message based on log level, like for example collecting some stats if the log type is DEBUG etc.. A few existing relevant usage converted to this new API. Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: David Marchand <david.marchand@redhat.com> Cc: Thomas Monjalon <thomas@monjalon.net> Cc: Stephen Hemminger <stephen@networkplumber.org> v2: * Convert API return type to 'bool'. Removed custom definitions from 'ionic' PMD for this. --- drivers/bus/fslmc/fslmc_bus.c | 7 +------ drivers/common/qat/qat_logs.c | 7 ++----- drivers/net/enic/enic_fm_flow.c | 2 +- drivers/net/ionic/ionic_osdep.h | 6 ------ drivers/net/mlx5/mlx5_mr.c | 2 +- lib/librte_eal/common/eal_common_log.c | 18 ++++++++++++++++++ lib/librte_eal/common/include/rte_log.h | 14 ++++++++++++++ lib/librte_eal/rte_eal_version.map | 3 +++ lib/librte_flow_classify/rte_flow_classify.c | 7 ++----- 9 files changed, 42 insertions(+), 24 deletions(-)