From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 39A5F41C2C;
	Tue,  7 Feb 2023 09:34:29 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id F2288427F2;
	Tue,  7 Feb 2023 09:34:28 +0100 (CET)
Received: from us-smtp-delivery-124.mimecast.com
 (us-smtp-delivery-124.mimecast.com [170.10.129.124])
 by mails.dpdk.org (Postfix) with ESMTP id 2EBE6427E9
 for <dev@dpdk.org>; Tue,  7 Feb 2023 09:34:28 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1675758867;
 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:
 in-reply-to:in-reply-to:references:references;
 bh=EqV4B2mhPPEy+aErgRAincJwBd6llgV2Y2ND/IPvjmk=;
 b=ReB+HulaDG8k7hYSARn2bejXjDKi4D/wcWeZgox04I3GdPm4SxeA1YG+KDBCqZK5E4TGNh
 pWqIDo21WThOS106HZeZPWEUhAZ7DfQXOCCH/64DikNrZE/tUdE9hddWR7W/GZjjcliqTp
 oYRQ6KIF90HGVAxOltHfNMkvxiH8GQY=
Received: from mail-pj1-f69.google.com (mail-pj1-f69.google.com
 [209.85.216.69]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id
 us-mta-241-lXb6d2GzNjGYaLDu6Vc_xg-1; Tue, 07 Feb 2023 03:34:26 -0500
X-MC-Unique: lXb6d2GzNjGYaLDu6Vc_xg-1
Received: by mail-pj1-f69.google.com with SMTP id
 qb3-20020a17090b280300b002307b2547b5so573473pjb.1
 for <dev@dpdk.org>; Tue, 07 Feb 2023 00:34:26 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20210112;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id
 :reply-to;
 bh=EqV4B2mhPPEy+aErgRAincJwBd6llgV2Y2ND/IPvjmk=;
 b=FGV3BdqxGnRLC2455qsCGHLKsDVbs9lCXAhjbCLdksbQLXUWrUhDtnqJz2fQ/xihx2
 99fSKLIEoCFzaSiPvjA7sXb4xBMGtSBLwbD7875nTjbCiE6iYPJIrcQxoPcGoyAmgS3H
 Uwo5GrjYvUB6a9xw90o85aYOJcEzyx9U92ZV1UJJCtmHPM3NcrTKhv4bDsJSpLkdTRDb
 YA096r3iGJDuKvQDJXPmKUBCcySY3yWyAA1xnKj+44quS7GMFK+9aU8IEyNRaXJMDtTm
 FHc30mfMEK1yfzGD0GzluFm397JdzsSxEycTHx+Boq7OHxzLQwRLzvsh9xycMKScuvy8
 AynQ==
X-Gm-Message-State: AO0yUKXEtx3APNjAqxNHFoqy5otbLXOE7AHwngDgnBciblniZJYHewhD
 klg9CsO+tb9BBc9J3iR7O0XGU60BDZyzBczZvbllz9K+bawVoYGFMEE7MlQRe4DCKfINB9ILV8q
 qKA8zB96wC3uXsqvSZb+VPIKwqWzXvA==
X-Received: by 2002:a17:903:41c9:b0:199:642:1c2c with SMTP id
 u9-20020a17090341c900b0019906421c2cmr477116ple.33.1675758865356; 
 Tue, 07 Feb 2023 00:34:25 -0800 (PST)
X-Google-Smtp-Source: AK7set+yFhNOH0RdB8GeUzo6Qu3a4atVHG1bJpwiabkocamv2Lsbp9FB0zEf4EXHCnLWt9CHpSlTYhSurDIWLGzriZY=
X-Received: by 2002:a17:903:41c9:b0:199:642:1c2c with SMTP id
 u9-20020a17090341c900b0019906421c2cmr477112ple.33.1675758865015; Tue, 07 Feb
 2023 00:34:25 -0800 (PST)
MIME-Version: 1.0
References: <20230206173941.783305-1-stephen@networkplumber.org>
In-Reply-To: <20230206173941.783305-1-stephen@networkplumber.org>
From: David Marchand <david.marchand@redhat.com>
Date: Tue, 7 Feb 2023 09:34:13 +0100
Message-ID: <CAJFAV8xCnyyi1FF1oAjvSzRLbrdvT+VYrUPrPfKUFVrEfLrs_g@mail.gmail.com>
Subject: Re: [PATCH] mbuf: replace RTE_LOGTYPE_MBUF with dynamic type
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org, Olivier Matz <olivier.matz@6wind.com>
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.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