DPDK patches and discussions
 help / color / mirror / Atom feed
From: Feng Li <lifeng1519@gmail.com>
To: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Cc: Li Feng <fengli@smartx.com>, dev <dev@dpdk.org>,
	 David Marchand <david.marchand@redhat.com>,
	 Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>,
	Dmitry Malloy <dmitrym@microsoft.com>,
	 Pallavi Kadam <pallavi.kadam@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] log: support custom log function
Date: Wed, 10 Feb 2021 11:59:15 +0800	[thread overview]
Message-ID: <CAEK8JBDLf3BQXhHvZGAvxFwXiYD7xqJURZPxDhYovr5vVieaQQ@mail.gmail.com> (raw)
In-Reply-To: <20210209014033.30b55ae2@sovereign>

On Tue, Feb 9, 2021 at 6:40 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> On Mon, 8 Feb 2021 14:58:29 +0800, Feng Li wrote:
> > > > +/**
> > > > + * 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.
>
> Doesn't make much sense here anyway.
Acked.

>
> > > > + *
> > > > + * 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;
>
> You cannot expose "cookie_write_function_t" in public API, because it is not
> portable. You have to write out the type in full.
> I'd replace "cookie" with "log".
Acked.

>
> > void rte_log_sink_set(rte_cookie_write_function* cookie_write);
> > rte_cookie_write_function* rte_log_sink_get();
> > Right?
>
> OK.
>
> > > > +void
> > > > +rte_eal_set_log_func(rte_log_func_t *logf)
> > > > +{
> > > > +    console_log_func.write = logf;
> > > > +}
>
> Is this correct? AFAIK, updating the member has no effect, unless you call
> fopencookie() with an updated structure.
The call order is:
rte_eal_set_log_func(logf);
...
rte_eal_init();

Because the rte_eal_init will call rte_eal_log_init, so set the struct
before rte_eal_init.
And the logs in `rte_eal_init` will also be redirected.

>
> Your new callbacks don't seem to be integrated with DPDK logging system for
> all platforms ("eal_common_log.c", "windows/eal_log.c"). If the new API and
> rte_openlog_stream() cancel effect of each other, this should be documented.
>

Looks like `rte_eal_log_init` is defined in Linux and Windows, and
Windows only redirects
the log to stderr. So Windows doesn't support this feature.

rte_openlog_stream has a higher priority than rte_eal_set_log_func.
If it isn't called, then 'rte_eal_set_log_func' call will work.
I will update the comments of this API.

Thanks.

  reply	other threads:[~2021-02-11 23:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-05 11:24 [dpdk-dev] [PATCH] " Li Feng
2021-02-05 11:31 ` Feng Li
2021-02-05 11:55 ` David Marchand
2021-02-05 12:22   ` Feng Li
2021-02-05 14:08     ` Dmitry Kozlyuk
2021-02-05 16:39     ` Stephen Hemminger
2021-02-05 16:10   ` Stephen Hemminger
2021-02-05 16:54     ` Feng Li
2021-02-05 17:06       ` Feng Li
2021-02-05 17:42 ` [dpdk-dev] [PATCH v2] " Li Feng
2021-02-05 17:47   ` Feng Li
2021-02-05 19:32   ` Dmitry Kozlyuk
2021-02-08  6:58     ` Feng Li
2021-02-08 22:40       ` Dmitry Kozlyuk
2021-02-10  3:59         ` Feng Li [this message]
2021-02-10  5:20 ` [dpdk-dev] [PATCH v3] " Li Feng
2021-02-18  2:55   ` Feng Li
2023-06-15 16:12   ` Stephen Hemminger
2021-02-18  6:12 ` [dpdk-dev] [PATCH v4] " Li Feng
2021-02-19  8:11   ` Dmitry Kozlyuk
2021-02-23 11:22     ` Feng Li

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=CAEK8JBDLf3BQXhHvZGAvxFwXiYD7xqJURZPxDhYovr5vVieaQQ@mail.gmail.com \
    --to=lifeng1519@gmail.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.kozliuk@gmail.com \
    --cc=dmitrym@microsoft.com \
    --cc=fengli@smartx.com \
    --cc=navasile@linux.microsoft.com \
    --cc=pallavi.kadam@intel.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).