log: add API to check if a logtype can log in a given level

Message ID 20200303132512.4138303-1-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series log: add API to check if a logtype can log in a given level |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit March 3, 2020, 1:25 p.m. UTC
  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(-)
  

Comments

Stephen Hemminger March 3, 2020, 4:02 p.m. UTC | #1
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?
  
Ferruh Yigit March 3, 2020, 4:15 p.m. UTC | #2
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.
  

Patch

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;