From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A1AD5A0547; Mon, 8 Feb 2021 15:09:45 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E11E24014E; Mon, 8 Feb 2021 15:09:44 +0100 (CET) Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) by mails.dpdk.org (Postfix) with ESMTP id BA2761606BC for ; Mon, 8 Feb 2021 07:58:55 +0100 (CET) Received: by mail-pl1-f172.google.com with SMTP id y10so7303982plk.7 for ; Sun, 07 Feb 2021 22:58:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=NR9It4/EQe5CyKrmOLFAyQQRtx2aBGDGEv641hvU/Vc=; b=BiIrXsFmgaVV7DvkhxkezxMGqJapj6vgCiwYgWFTA6w3cXLXkLCGkKCG63wDOeu/Mb hYBKVthSbVcYgdjyE6HT0vW1tP39v8U73xTcjTjgyHMB83OrNGrwXHMAM4diYZTrIIS1 vZe5iF2yN2y0/5M4KWOYoem9uln1pl02/edOy9ApwgzwU6p7k0GhT9IbDpW+PLqRksrH AyVGE6rNNPgu8foM1uEaAc3AvCfMnv/45D7ZNdgCRZ6njtR2Pg+CmtXscsKGCGVvb1UA oywD4U7PN6c5BRLeOY24mOCz087Y9OukS0cwiiEgQDyXvZEKR8zjvjSH0FdM9utxtRl6 BxyQ== 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:content-transfer-encoding; bh=NR9It4/EQe5CyKrmOLFAyQQRtx2aBGDGEv641hvU/Vc=; b=noXeiWoKl3kXkOjLrOg6xaFPUhMAHOI4N+lV9uMOKOZmjVe2fe1BlZNEK6wm9C17nU dQDRaY+1IfFQAeaa2pXxwfvoj1UxxYz0z+3l/Dthwx9KgAmaCZdQnMHCXcazl6h5ZMkw x57SK/VVY04VL4GksmZ6BRMRmod1vL1qZlEWU6QdcFXNV3jZzLfQObXHGJBd7sg1J0L6 qoD64tP/y4zeyfErj1SLHdbsZGFuUCfdWAzlSmnppOH79Epd9/Wdzek1S6lI9eOjhkXT 6CHJ7RwZ1BN4a/uAIW1hesWn7Z48WjwExERSSvrDpuod2g8bpitkTEhI1neaSNYO4vCw gbhA== X-Gm-Message-State: AOAM530yc+Klb3lkaNCYwj936TvJd79iIOHu2Odw79FD1m31rJzoGcfW qPN6BSFlq7yq0BUyzzAzRgohhyNxYIgvGrTTHRM= X-Google-Smtp-Source: ABdhPJxbnMV8x9w4orEwmzHNQMW7oBUdb2ly4Fe3BpaIIZC5z8OpmvD5xUTvC9qyRnzeXRGyi03LydjvVOJNE2laus0= X-Received: by 2002:a17:90a:fd0f:: with SMTP id cv15mr15970233pjb.36.1612767534740; Sun, 07 Feb 2021 22:58:54 -0800 (PST) MIME-Version: 1.0 References: <20210205112433.1681853-1-fengli@smartx.com> <20210205174204.1878089-1-fengli@smartx.com> <20210205223207.7fbf82ba@sovereign> In-Reply-To: <20210205223207.7fbf82ba@sovereign> From: Feng Li Date: Mon, 8 Feb 2021 14:58:29 +0800 Message-ID: To: Dmitry Kozlyuk Cc: Li Feng , dev , David Marchand , Narcisa Ana Maria Vasile , Dmitry Malloy , Pallavi Kadam Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Mon, 08 Feb 2021 15:09:42 +0100 Subject: Re: [dpdk-dev] [PATCH v2] log: support custom log function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" Thanks for your comments. Dmitry Kozlyuk =E4=BA=8E2021=E5=B9=B42=E6=9C=886= =E6=97=A5=E5=91=A8=E5=85=AD =E4=B8=8A=E5=8D=883:32=E5=86=99=E9=81=93=EF=BC= =9A > > On Sat, 6 Feb 2021 01:42:04 +0800, Li Feng wrote: > > Currently, the dpdk log is out to stdout/stderr and syslog. > > The rte_openlog_stream could set an external FILE* stream, but it asks = the > > consumer to give it a FILE* pointer. > > For C++ or other languages, it's hard to get a libc FILE*. > > > > Support to set a hook method is another choice for this scenario. > > It's "DPDK"; and "hard" sounds vague. I'd state the real issue such that > API to change DPDK log sink mandates the (direct) use of libc in consumer > application. To invoke arbitrary function, consumers have to use faciliti= es > that are non-standard (fopencookie) or OS-specific (pipes). > > > > > Change-Id: I3b2419cc2fe5256205daa8077fd8862a8e58fb6c > > What is this? > It's Gerrit's Change-ID, I will remove it. > > Signed-off-by: Li Feng > > --- > > v2: simplify the code. > > > > lib/librte_eal/include/rte_eal.h | 16 ++++++++++++++++ > > lib/librte_eal/linux/eal_log.c | 10 ++++++++++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/= rte_eal.h > > index eaf6469e5..bd6cf554b 100644 > > --- a/lib/librte_eal/include/rte_eal.h > > +++ b/lib/librte_eal/include/rte_eal.h > > @@ -501,6 +501,22 @@ rte_eal_mbuf_user_pool_ops(void); > > const char * > > rte_eal_get_runtime_dir(void); > > > > +/** > > + * Usage function typedef used by the application usage function. > > It's unrelated to the following typedef purpose, is it? It's borrowed from the front typedef sentence. > > > + * > > + * Use this function typedef to define a logger formatter. > > + */ > > +typedef cookie_write_function_t rte_log_func_t; > > "cookie_write_function_t" is not standard C, please write this type > explicitly. POSIX reserves "_t" suffix, "rte_" prefix is enough. Fix to this? typedef cookie_write_function_t rte_cookie_write_function; > > > + > > +/** > > + * Set a customized logger. > > + * > > + * @param logf > > + * The customized logger function. > > + */ > > +void > > +rte_eal_set_log_func(rte_log_func_t *logf); > > + > > How about rte_log_sink_set() and a companion rte_log_sink_get() or return= ing > the previous value when setting a new one? Also, like in fopencookie(), a > cookie argument is needed. void rte_log_sink_set(rte_cookie_write_function* cookie_write); rte_cookie_write_function* rte_log_sink_get(); Right? > > New functions must be marker __rte_experimental and added to .map and .de= f > files. Please use devtools/checkpatches.sh that will run checks like that= for > your commits. > I have run the `devtools/checkpatches.sh`, it reports valid, so weird. I'll fix it. > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/librte_eal/linux/eal_log.c b/lib/librte_eal/linux/eal_= log.c > > index 43c8460bf..c0a7a12ab 100644 > > --- a/lib/librte_eal/linux/eal_log.c > > +++ b/lib/librte_eal/linux/eal_log.c > > @@ -60,3 +60,13 @@ rte_eal_log_init(const char *id, int facility) > > > > return 0; > > } > > + > > +/* > > + * Set the customized logger, it will override the default action, whi= ch is > > + * writing to syslog and stdout. > > + */ > > +void > > +rte_eal_set_log_func(rte_log_func_t *logf) > > +{ > > + console_log_func.write =3D logf; > > +} >