From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Chaoyong He <chaoyong.he@corigine.com>, dev@dpdk.org
Cc: oss-drivers@corigine.com, niklas.soderlund@corigine.com
Subject: Re: [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type
Date: Fri, 17 Feb 2023 13:59:47 +0000 [thread overview]
Message-ID: <f8fc36a5-ba36-fd6a-4a08-9f22ea335898@amd.com> (raw)
In-Reply-To: <20230217024539.16514-3-chaoyong.he@corigine.com>
On 2/17/2023 2:45 AM, Chaoyong He wrote:
> Register the own RX/TX debug log level type, and get rid of the
> usage of RTE_LOGTYPE_*. Then we can control the log by a independent
> switch.
>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> ---
> drivers/net/nfp/nfp_logs.c | 10 ++++++++++
> drivers/net/nfp/nfp_logs.h | 8 ++++++--
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_logs.c b/drivers/net/nfp/nfp_logs.c
> index 48c42fe53f..cd58bcee43 100644
> --- a/drivers/net/nfp/nfp_logs.c
> +++ b/drivers/net/nfp/nfp_logs.c
> @@ -5,6 +5,16 @@
>
> #include "nfp_logs.h"
>
> +#include <rte_ethdev.h>
> +
> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE);
> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE);
> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE);
> +
> +#ifdef RTE_ETHDEV_DEBUG_RX
> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_rx, rx, DEBUG)
> +#endif
> +
> +#ifdef RTE_ETHDEV_DEBUG_TX
> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_tx, tx, DEBUG)
> +#endif
> diff --git a/drivers/net/nfp/nfp_logs.h b/drivers/net/nfp/nfp_logs.h
> index b7632ee72c..315a57811c 100644
> --- a/drivers/net/nfp/nfp_logs.h
> +++ b/drivers/net/nfp/nfp_logs.h
> @@ -15,15 +15,19 @@ extern int nfp_logtype_init;
> #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
>
> #ifdef RTE_ETHDEV_DEBUG_RX
> +extern int nfp_logtype_rx;
> #define PMD_RX_LOG(level, fmt, args...) \
> - RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args)
> + rte_log(RTE_LOG_ ## level, nfp_logtype_rx, \
> + "%s(): " fmt "\n", __func__, ## args)
> #else
> #define PMD_RX_LOG(level, fmt, args...) do { } while (0)
> #endif
>
> #ifdef RTE_ETHDEV_DEBUG_TX
> +extern int nfp_logtype_tx;
> #define PMD_TX_LOG(level, fmt, args...) \
> - RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args)
> + rte_log(RTE_LOG_ ## level, nfp_logtype_tx, \
> + "%s(): " fmt "\n", __func__, ## args)
> #else
> #define PMD_TX_LOG(level, fmt, args...) do { } while (0)
> #endif
Intention is to replace 'RTE_LOG_DP' with 'PMD_RX_LOG'/'PMD_TX_LOG', but
these are not exactly same (although difference is minor).
When 'RTE_ETHDEV_DEBUG_RX' is set, ethdev layer also adds some
additional load, although I believe that will small comparing to logging
in driver.
If 'RTE_LOG_DP' used, the ethdev layer cost can be removed.
With 'RTE_LOG_DP', log level more verbose than requested won't cause any
performance impact. Like if ERR level requested, INFO, DEBUG etc logs
will be compiled out and won't cause any performance impact.
But with 'RTE_ETHDEV_DEBUG_RX', even log level only request ERR, all
logging will add cost of at least an if branch (checking log level).
For many cases I am not sure these differences matters, and already many
drivers directly uses 'RTE_ETHDEV_DEBUG_RX' as done here. So you may
prefer to keep as it is.
But if there is a desire for this fine grain approach, it is possible to
add a version of 'RTE_LOG_DP' macro that accepts dynamic log type
(instead of static RTE_LOGTYPE_# type),
what do you think?
next prev parent reply other threads:[~2023-02-17 14:00 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-17 2:45 [PATCH 0/3] refactor the nfp log subsystem Chaoyong He
2023-02-17 2:45 ` [PATCH 1/3] net/nfp: add the log source file Chaoyong He
2023-02-17 2:45 ` [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type Chaoyong He
2023-02-17 13:59 ` Ferruh Yigit [this message]
2023-02-20 1:36 ` Chaoyong He
2023-02-20 10:09 ` Ferruh Yigit
2023-02-20 16:16 ` Stephen Hemminger
2023-02-20 16:32 ` Ferruh Yigit
2023-02-17 2:45 ` [PATCH 3/3] net/nfp: get rid of the usage of RTE log macro Chaoyong He
2023-02-20 12:03 ` [PATCH 0/3] refactor the nfp log subsystem Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f8fc36a5-ba36-fd6a-4a08-9f22ea335898@amd.com \
--to=ferruh.yigit@amd.com \
--cc=chaoyong.he@corigine.com \
--cc=dev@dpdk.org \
--cc=niklas.soderlund@corigine.com \
--cc=oss-drivers@corigine.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).