From: David Marchand <david.marchand@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org, Olivier Matz <olivier.matz@6wind.com>
Subject: Re: [PATCH] mbuf: replace RTE_LOGTYPE_MBUF with dynamic type
Date: Tue, 7 Feb 2023 09:34:13 +0100 [thread overview]
Message-ID: <CAJFAV8xCnyyi1FF1oAjvSzRLbrdvT+VYrUPrPfKUFVrEfLrs_g@mail.gmail.com> (raw)
In-Reply-To: <20230206173941.783305-1-stephen@networkplumber.org>
On Mon, Feb 6, 2023 at 6:39 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Introduce a new dynamic logtype for mbuf related messages.
> Since this is used in multiple files put one macro in mbuf_log.h
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/mbuf/mbuf_log.h | 10 ++++++++++
> lib/mbuf/rte_mbuf.c | 20 ++++++++++++--------
> lib/mbuf/rte_mbuf_dyn.c | 14 +++++++-------
> lib/mbuf/rte_mbuf_pool_ops.c | 5 +++--
> 4 files changed, 32 insertions(+), 17 deletions(-)
> create mode 100644 lib/mbuf/mbuf_log.h
>
> diff --git a/lib/mbuf/mbuf_log.h b/lib/mbuf/mbuf_log.h
> new file mode 100644
> index 000000000000..fe97f338c9b7
> --- /dev/null
> +++ b/lib/mbuf/mbuf_log.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2014 Intel Corporation.
> + * Copyright 2014 6WIND S.A.
> + */
> +
> +extern int mbuf_logtype;
> +
> +#define MBUF_LOG(level, fmt, args...) \
> + rte_log(RTE_LOG_ ## level, mbuf_logtype, \
> + "%s(): " fmt "\n", __func__, ##args)
> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> index cfd8062f1e6a..64847afbf931 100644
> --- a/lib/mbuf/rte_mbuf.c
> +++ b/lib/mbuf/rte_mbuf.c
> @@ -20,6 +20,8 @@
> #include <rte_errno.h>
> #include <rte_memcpy.h>
>
> +#include "mbuf_log.h"
> +
> /*
> * pktmbuf pool constructor, given as a callback function to
> * rte_mempool_create(), or called directly if using
> @@ -227,8 +229,8 @@ rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
> int ret;
>
> if (RTE_ALIGN(priv_size, RTE_MBUF_PRIV_ALIGN) != priv_size) {
> - RTE_LOG(ERR, MBUF, "mbuf priv_size=%u is not aligned\n",
> - priv_size);
> + MBUF_LOG(ERR, "mbuf priv_size=%u is not aligned",
> + priv_size);
> rte_errno = EINVAL;
> return NULL;
> }
> @@ -247,7 +249,7 @@ rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
> mp_ops_name = rte_mbuf_best_mempool_ops();
> ret = rte_mempool_set_ops_byname(mp, mp_ops_name, NULL);
> if (ret != 0) {
> - RTE_LOG(ERR, MBUF, "error setting mempool handler\n");
> + MBUF_LOG(ERR, "error setting mempool handler");
> rte_mempool_free(mp);
> rte_errno = -ret;
> return NULL;
> @@ -293,7 +295,7 @@ rte_pktmbuf_pool_create_extbuf(const char *name, unsigned int n,
> int ret;
>
> if (RTE_ALIGN(priv_size, RTE_MBUF_PRIV_ALIGN) != priv_size) {
> - RTE_LOG(ERR, MBUF, "mbuf priv_size=%u is not aligned\n",
> + MBUF_LOG(ERR, "mbuf priv_size=%u is not aligned",
> priv_size);
> rte_errno = EINVAL;
> return NULL;
> @@ -303,12 +305,12 @@ rte_pktmbuf_pool_create_extbuf(const char *name, unsigned int n,
> const struct rte_pktmbuf_extmem *extm = ext_mem + i;
>
> if (!extm->elt_size || !extm->buf_len || !extm->buf_ptr) {
> - RTE_LOG(ERR, MBUF, "invalid extmem descriptor\n");
> + MBUF_LOG(ERR, "invalid extmem descriptor");
> rte_errno = EINVAL;
> return NULL;
> }
> if (data_room_size > extm->elt_size) {
> - RTE_LOG(ERR, MBUF, "ext elt_size=%u is too small\n",
> + MBUF_LOG(ERR, "ext elt_size=%u is too small",
> priv_size);
> rte_errno = EINVAL;
> return NULL;
> @@ -317,7 +319,7 @@ rte_pktmbuf_pool_create_extbuf(const char *name, unsigned int n,
> }
> /* Check whether enough external memory provided. */
> if (n_elts < n) {
> - RTE_LOG(ERR, MBUF, "not enough extmem\n");
> + MBUF_LOG(ERR, "not enough extmem");
> rte_errno = ENOMEM;
> return NULL;
> }
> @@ -338,7 +340,7 @@ rte_pktmbuf_pool_create_extbuf(const char *name, unsigned int n,
> mp_ops_name = rte_mbuf_best_mempool_ops();
> ret = rte_mempool_set_ops_byname(mp, mp_ops_name, NULL);
> if (ret != 0) {
> - RTE_LOG(ERR, MBUF, "error setting mempool handler\n");
> + MBUF_LOG(ERR, "error setting mempool handler");
> rte_mempool_free(mp);
> rte_errno = -ret;
> return NULL;
> @@ -936,3 +938,5 @@ rte_get_tx_ol_flag_list(uint64_t mask, char *buf, size_t buflen)
>
> return 0;
> }
> +
> +RTE_LOG_REGISTER(mbuf_logtype, "lib/mbuf", INFO);
?!
Why do we need a new naming convention?
Please use RTE_LOG_REGISTER_DEFAULT.
And we should invalidate/deprecate the use of RTE_LOGTYPE_MBUF + avoid
it is registered statically.
lib/eal/common/eal_common_log.c: {RTE_LOGTYPE_MBUF, "lib.mbuf"},
lib/eal/include/rte_log.h:#define RTE_LOGTYPE_MBUF 16 /**< Log
related to mbuf. */
These comments apply to the EFD patch too.
--
David Marchand
next prev parent reply other threads:[~2023-02-07 8:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-06 17:39 Stephen Hemminger
2023-02-07 8:24 ` Morten Brørup
2023-02-07 8:34 ` David Marchand [this message]
2023-02-07 16:19 ` Stephen Hemminger
2023-02-07 16:41 ` David Marchand
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=CAJFAV8xCnyyi1FF1oAjvSzRLbrdvT+VYrUPrPfKUFVrEfLrs_g@mail.gmail.com \
--to=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=olivier.matz@6wind.com \
--cc=stephen@networkplumber.org \
/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).