DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: longli@linuxonhyperv.com, Thomas Monjalon <thomas@monjalon.net>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	David Marchand <david.marchand@redhat.com>
Cc: dev@dpdk.org, Ajay Sharma <sharmaajay@microsoft.com>,
	Long Li <longli@microsoft.com>,
	stable@dpdk.org, Luca Boccassi <bluca@debian.org>,
	Qi Z Zhang <qi.z.zhang@intel.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
	Olivier Matz <olivier.matz@6wind.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Subject: Re: [PATCH] net/mana: use RTE_LOG_DP for logs on datapath
Date: Thu, 23 Feb 2023 14:07:25 +0000	[thread overview]
Message-ID: <a2311c41-db09-8866-9a97-e5074620d196@amd.com> (raw)
In-Reply-To: <1677012145-3559-1-git-send-email-longli@linuxonhyperv.com>

On 2/21/2023 8:42 PM, longli@linuxonhyperv.com wrote:
> diff --git a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c
> index 300bf27cc1..a45b5e289c 100644
> --- a/drivers/net/mana/tx.c
> +++ b/drivers/net/mana/tx.c
> @@ -183,17 +183,17 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>  			(struct mana_tx_comp_oob *)&comp.completion_data[0];
>  
>  		if (oob->cqe_hdr.cqe_type != CQE_TX_OKAY) {
> -			DRV_LOG(ERR,
> -				"mana_tx_comp_oob cqe_type %u vendor_err %u",
> -				oob->cqe_hdr.cqe_type, oob->cqe_hdr.vendor_err);
> +			DP_LOG(ERR,
> +			       "mana_tx_comp_oob cqe_type %u vendor_err %u",
> +			       oob->cqe_hdr.cqe_type, oob->cqe_hdr.vendor_err);
>  			txq->stats.errors++;
>  		} else {
> -			DRV_LOG(DEBUG, "mana_tx_comp_oob CQE_TX_OKAY");
> +			DP_LOG(DEBUG, "mana_tx_comp_oob CQE_TX_OKAY");
>  			txq->stats.packets++;
>  		}
>  
>  		if (!desc->pkt) {
> -			DRV_LOG(ERR, "mana_txq_desc has a NULL pkt");
> +			DP_LOG(ERR, "mana_txq_desc has a NULL pkt");
>  		} else {
>  			txq->stats.bytes += desc->pkt->data_len;
>  			rte_pktmbuf_free(desc->pkt);


I have some doubts about data path logging, let me use above example to
discuss this, these are not specific to this driver. TLDR; jump [1].

Above 'ERR' log will leak into the production code, since
'RTE_LOG_DP_LEVEL' value is 'RTE_LOG_INFO' by default.

Question is who will benefit from these error logs, like descriptor
having null packet etc..

I can think of following users for the dpdk logs:
a) DPDK library/driver developer
b) DPDK application developer
c) Who deploys/maintains DPDK application
d) DPDK end user

Unless DPDK packagers or who deploys DPDK application updates
'RTE_LOG_DP_LEVEL', all above parties will observe these datapath logs
but I assume intended user of it is only (a).

Overall I am not sure if anyone is interested in driver datapath logs
other than driver developers themselves.

For datapath logging I think there are two concerns,
1) It should not eat *any* cycles unless explicitly enabled
2) Capability of enable/disable them because of massive amount of log it
can generate


Currently there are two existing approaches for driver datapath logging:
i)  Controlled by 'RTE_ETHDEV_DEBUG_RX/TX' compile time flag,
    when enabled 'rte_log()' is used with Rx/Tx specific log type.
ii) 'RTE_LOG_DP' ', compile time control per logtype via
    'RTE_LOG_DP_LEVEL',
     when enabled 'rte_log()' is used with PMD logtype.


In (ii), need to re-compile code when you need to increase the log
verbosity, and it leaks to production code as mentioned above.

For (i), developer compiles once enabling debug, later can fine grain
log level dynamically. This is more DPDK developer focused approach.


[1]
According above, what do you think to retire 'RTE_LOG_DP', (at least
within ethdev datapath), and chose (i) as preferred datapath logging?








  reply	other threads:[~2023-02-23 14:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-21 20:42 longli
2023-02-23 14:07 ` Ferruh Yigit [this message]
2023-02-23 18:09   ` Stephen Hemminger
2023-02-24 17:51     ` Stephen Hemminger
2023-03-03  2:16       ` Long Li
2023-03-03 13:54         ` Ferruh Yigit
2023-03-03 19:04           ` Long Li
2023-03-04  1:15             ` Ferruh Yigit
2023-04-28  1:29               ` Long Li
2023-04-28 10:23                 ` Ferruh Yigit
2023-04-28 17:52                   ` Long Li
2023-03-04  0:16           ` Stephen Hemminger
2023-05-02 15:45 ` 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=a2311c41-db09-8866-9a97-e5074620d196@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=bluca@debian.org \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=konstantin.v.ananyev@yandex.ru \
    --cc=longli@linuxonhyperv.com \
    --cc=longli@microsoft.com \
    --cc=olivier.matz@6wind.com \
    --cc=qi.z.zhang@intel.com \
    --cc=sharmaajay@microsoft.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).