[dpdk-dev] [PATCH] log: add API to check if a logtype can log in a given level

[dpdk-dev] [PATCH] log: add API to check if a logtype can log in a given level

[Ferruh Yigit]

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>
---
 drivers/bus/fslmc/fslmc_bus.c                |  7 +------
 drivers/common/qat/qat_logs.c                |  7 ++-----
 drivers/net/enic/enic_fm_flow.c              |  2 +-
 drivers/net/mlx5/mlx5_mr.c                   |  2 +-
 lib/librte_eal/common/eal_common_log.c       | 18 ++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h      | 13 +++++++++++++
 lib/librte_eal/rte_eal_version.map           |  3 +++
 lib/librte_flow_classify/rte_flow_classify.c |  7 ++-----
 8 files changed, 41 insertions(+), 18 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)) {
 		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/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..5b91ae31b 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;
 }
 
+int
+rte_log_can_log(uint32_t logtype, uint32_t level)
+{
+	int log_level;
+
+	if (level > rte_log_get_global_level())
+		return 0;
+
+	log_level = rte_log_get_level(logtype);
+	if (log_level < 0)
+		return 0;
+
+	if (level > (uint32_t)log_level)
+		return 0;
+
+	return 1;
+}
+
 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..56e8952e5 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -143,6 +143,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 '1' if log can be printed and '0' if it can't.
+ */
+__rte_experimental
+int 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;
-- 
2.24.1

Re: [dpdk-dev] [PATCH] log: add API to check if a logtype can log in a given level

[Stephen Hemminger]

On Tue,  3 Mar 2020 13:25:12 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

>  
> +int
> +rte_log_can_log(uint32_t logtype, uint32_t level)
> +{
> +	int log_level;
> +
> +	if (level > rte_log_get_global_level())
> +		return 0;
> +
> +	log_level = rte_log_get_level(logtype);
> +	if (log_level < 0)
> +		return 0;
> +
> +	if (level > (uint32_t)log_level)
> +		return 0;
> +
> +	return 1;
> +}

Why not use a boolean (stdbool) for return value?

Re: [dpdk-dev] [PATCH] log: add API to check if a logtype can log in a given level

[Ferruh Yigit]

On 3/3/2020 4:02 PM, Stephen Hemminger wrote:
> On Tue,  3 Mar 2020 13:25:12 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>>  
>> +int
>> +rte_log_can_log(uint32_t logtype, uint32_t level)
>> +{
>> +	int log_level;
>> +
>> +	if (level > rte_log_get_global_level())
>> +		return 0;
>> +
>> +	log_level = rte_log_get_level(logtype);
>> +	if (log_level < 0)
>> +		return 0;
>> +
>> +	if (level > (uint32_t)log_level)
>> +		return 0;
>> +
>> +	return 1;
>> +}
> 
> Why not use a boolean (stdbool) for return value?
> 

No specific reason, but agree functions suits the bool, I will send a v2.

[dpdk-dev] [PATCH v2] log: add API to check if a logtype can log in a given level

[Ferruh Yigit]

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)) {
 		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;
-- 
2.24.1

Re: [dpdk-dev] [PATCH v2] log: add API to check if a logtype can log in a given level

[Hyong Youb Kim (hyonkim)]

> -----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

Re: [dpdk-dev] [PATCH v2] log: add API to check if a logtype can log in a given level

[Hemant Agrawal (OSS)]

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Re: [dpdk-dev] [PATCH v2] log: add API to check if a logtype can log in a given level

[David Marchand]

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
>


-- 
David marchand

Re: [dpdk-dev] [PATCH v2] log: add API to check if a logtype can log in a given level

[Ferruh Yigit]

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
>>
> 
> 

[dpdk-dev] [PATCH v3] log: add API to check if a logtype can log in a given level

[Ferruh Yigit]

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>
Acked-by: Hyong Youb Kim <hyonkim@cisco.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.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.

v3:
* Update function comment
* Use new function in `rte_vlog()`
---
 drivers/bus/fslmc/fslmc_bus.c                |  7 +------
 drivers/common/qat/qat_logs.c                |  7 ++-----
 drivers/net/enic/enic_fm_flow.c              |  2 +-
 drivers/net/mlx5/mlx5_mr.c                   |  2 +-
 lib/librte_eal/common/eal_common_log.c       | 22 +++++++++++++++++---
 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 ++-----
 8 files changed, 43 insertions(+), 21 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)) {
 		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/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..7647a916e 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)
 {
@@ -417,11 +435,9 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 	FILE *f = rte_log_get_stream();
 	int ret;
 
-	if (level > rte_logs.level)
-		return 0;
 	if (logtype >= rte_logs.dynamic_types_len)
 		return -1;
-	if (level > rte_logs.dynamic_types[logtype].loglevel)
+	if (!rte_log_can_log(logtype, level))
 		return 0;
 
 	/* save loglevel and logtype in a global per-lcore variable */
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 1bb0e6694..7f5cdbb39 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);
 
+/**
+ * For a given `logtype`, check if a log with `loglevel` can be printed.
+ *
+ * @param logtype
+ *   The log type identifier
+ * @param loglevel
+ *   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 loglevel);
+
 /**
  * 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;
-- 
2.24.1

Re: [dpdk-dev] [PATCH v3] log: add API to check if a logtype can log in a given level

[Andrzej Ostruszka]

On 3/13/20 3:51 PM, Ferruh Yigit 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>
> Acked-by: Hyong Youb Kim <hyonkim@cisco.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.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.
> 
> v3:
> * Update function comment
> * Use new function in `rte_vlog()`
> ---
>  drivers/bus/fslmc/fslmc_bus.c                |  7 +------
>  drivers/common/qat/qat_logs.c                |  7 ++-----
>  drivers/net/enic/enic_fm_flow.c              |  2 +-
>  drivers/net/mlx5/mlx5_mr.c                   |  2 +-
>  lib/librte_eal/common/eal_common_log.c       | 22 +++++++++++++++++---
>  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 ++-----
>  8 files changed, 43 insertions(+), 21 deletions(-)

Ferruh

I like this and would like use it for the next round of my patches.  Is
there any timeline for that?

With regards
Andrzej Ostruszka

Reviewed-by: Andrzej Ostruszka <aostruszka@marvell.com>

Re: [dpdk-dev] [PATCH v3] log: add API to check if a logtype can log in a given level

[David Marchand]

On Fri, Mar 13, 2020 at 3:52 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>
> Acked-by: Hyong Youb Kim <hyonkim@cisco.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Reviewed-by: Andrzej Ostruszka <aostruszka@marvell.com>

lgtm
Acked-by: David Marchand <david.marchand@redhat.com>


Applied, thanks.

-- 
David Marchand