From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2025BA056B; Thu, 12 Mar 2020 14:41:42 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C09142BE3; Thu, 12 Mar 2020 14:41:40 +0100 (CET) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 54F0D2BAA for ; Thu, 12 Mar 2020 14:41:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1584020498; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eS/oFwraD2RGOI9LX5TDVNus/VBZ9aaVOTsPmrFnB2w=; b=IHdZDZGMHTBD5wlS0Hyp57jDYbERkUhoo1M/s0BWIMz2vfjTPEpuurgpdzf/Q5mzr1O1Aj Rc9+BdpHVVtwyTL5nQcJcedIkinOBniX1EncFTCLngmL5/vxtofZ/oSAXt6gnTcb2Uyagm fDMq9wJByiQiUEBYfs5r5gug68bK+8o= Received: from mail-vk1-f199.google.com (mail-vk1-f199.google.com [209.85.221.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-373-uZ-4h_tSM_2f2RP0feaQrg-1; Thu, 12 Mar 2020 09:41:34 -0400 X-MC-Unique: uZ-4h_tSM_2f2RP0feaQrg-1 Received: by mail-vk1-f199.google.com with SMTP id n8so2331265vke.13 for ; Thu, 12 Mar 2020 06:41:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8BTf/gbjak4KAudEtx7Gxp+gKWsLUoHfpZfuBulqyLs=; b=Vy42tYBk4KHcAQQrp4USYPUPPfGa42pfpGP81dXqKSwp/mQ0PMucRaNhMEkNPKwSw6 L7EfGq8FTcMg33mqFgQOZF4rqJ6eztWKgEAeyaKtNdqpju8JOOjY1YgQi3RAMXCDBh18 z7NlbgX53Snfy3TdtnlZ5NiBBuj0/sBM/DUghowtFtkJgUEjmdeF0UmOG3TTPl7j/TbY U4Cf7cmiDbSkE/UhatNrNOwXvu/K8notjYY3e7gMwlTmrL839Sxa6Go6V8vnFte0wcUj H0iVJqaPXko+2xXP9oCfMQ0wovgACuMVwjhP5U5U7EKQZNgqd9JlmnRpePb8ZHDJtJQZ Ukog== X-Gm-Message-State: ANhLgQ2KqrU+GQ/R//EyxjAFWw96lJF09LCCvOskIanWblFV+OLtdtIk 6ziblzH5RIfLbCnhHqFsfZzLD9S9Zr1JaVAQPVepxae89k3FpFwBWb+XZsIbK0nM3b9prQoo8FV LikF8ZBdnz6SeyroaNzs= X-Received: by 2002:a67:69d8:: with SMTP id e207mr5639320vsc.141.1584020494292; Thu, 12 Mar 2020 06:41:34 -0700 (PDT) X-Google-Smtp-Source: ADFU+vuA76V9+4byIf/ZM5ZI/dg/uEt5Xl9mtZHzp1li57aq6JeAzK2/XhulFRhaJv3Yvf3WmUPYANugDf6BxvI867o= X-Received: by 2002:a67:69d8:: with SMTP id e207mr5639290vsc.141.1584020493961; Thu, 12 Mar 2020 06:41:33 -0700 (PDT) MIME-Version: 1.0 References: <20200303132512.4138303-1-ferruh.yigit@intel.com> <20200303181817.3448334-1-ferruh.yigit@intel.com> In-Reply-To: <20200303181817.3448334-1-ferruh.yigit@intel.com> From: David Marchand Date: Thu, 12 Mar 2020 14:41:15 +0100 Message-ID: To: Ferruh Yigit , Olivier Matz Cc: Hemant Agrawal , Sachin Saxena , Fiona Trahe , John Griffin , Deepak Kumar Jain , John Daley , Hyong Youb Kim , Alfredo Cardigliano , Matan Azrad , Shahaf Shuler , Viacheslav Ovsiienko , Bernard Iremonger , dev , Thomas Monjalon , Stephen Hemminger X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v2] log: add API to check if a logtype can log in a given level X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Mar 3, 2020 at 7:18 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 > --- > Cc: David Marchand > Cc: Thomas Monjalon > Cc: Stephen Hemminger > > 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 =3D rte_log_get_global_level(); > - local_log_level =3D rte_log_get_level(dpaa2_logtype_bus); > - if (global_log_level =3D=3D RTE_LOG_DEBUG || > - local_log_level =3D=3D 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_f= low.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_os= dep.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) =3D=3D 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/comm= on/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 =3D 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/com= mon/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 > #include > #include > +#include > #include > > #include > @@ -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_fl= ow_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 =3D malloc(sizeof(struct rte_flow_classify_rule)); > if (!rule) > @@ -466,9 +465,7 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classif= ier *cls) > cls->ntuple_filter.dst_port_mask; > rule->rules.u.ipv4_5tuple.dst_port =3D cls->ntuple_filter.dst_por= t; > > - log_level =3D rte_log_get_level(librte_flow_classify_logtype); > - > - if (log_level =3D=3D 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_classif= ier *cls) > &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4], > NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field)); > > - if (log_level =3D=3D 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 > --=20 David marchand