From: David Marchand <david.marchand@redhat.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org, thomas@monjalon.net, ferruh.yigit@amd.com,
stephen@networkplumber.org,
"Morten Brørup" <mb@smartsharesystems.com>,
"Luca Boccassi" <bluca@debian.org>,
"Kevin Traynor" <ktraynor@redhat.com>,
"Xueming(Steven) Li" <xuemingl@nvidia.com>
Subject: Re: [RFC 0/3] Detect superfluous newline in logs
Date: Fri, 17 Nov 2023 15:21:57 +0100 [thread overview]
Message-ID: <CAJFAV8w5xeG7ONY_LX0QgyraBDFV7Lg7d7jJ8UQP-5JzcyiNJg@mail.gmail.com> (raw)
In-Reply-To: <ZVd0fw8jCmgWDaJ1@bricha3-MOBL.ger.corp.intel.com>
On Fri, Nov 17, 2023 at 3:11 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Nov 17, 2023 at 02:48:25PM +0100, David Marchand wrote:
> > On Fri, Nov 17, 2023 at 2:27 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Fri, Nov 17, 2023 at 02:18:21PM +0100, David Marchand wrote:
> > > > Getting readable and consistent logs is important when running a DPDK
> > > > application, especially when troubleshooting.
> > > > A common issue with logs is when a DPDK change do not add (or on the
> > > > contrary add too many \n) in the format string.
> > > >
> > > > This issue would only get noticed when actually hitting this log (which
> > > > may be something difficult to do).
> > > >
> > > > This series proposes to introduce a new RTE_LOG helper that is
> > > > responsible for logging a one line message and spews a build error (with
> > > > gcc) if any \n is part of the format string.
> > > >
> > > >
> > > > Note:
> > > > - the first patch is intentionnally sent as a single block: splitting it
> > > > into per library commits with correct Fixes: tags is a tedious work.
> > > > I would split it for a non RFC series. For now, it is enough to show
> > > > case the idea.
> > > > - the last patch shows how an existing log macro is converted,
> > > >
> > > >
> > > very nice. I definitely think this should be implemented for 24.03
> >
> > I am still wondering how this idea should evolve...
> >
> > Some points I have in mind but for which I am not sure what is the best.
> >
> > Some log helpers were exposed to applications and had no explicit
> > requirement wrt \n.
> > Applications may have used those helpers with multiline messages.
> > So maybe existing *exposed* helpers should be left untouched... and a
> > new helper would need to be introduced.
>
> And the existing helpers deprecated. Internal log helpers should not be
> exposed to apps.
I agree.
>
> > IOW with an example, cryptodev (missing a RTE_ prefix) CDEV_LOG_ERR
> > macro is publicly exposed.
> > A CDEV_LOG_LINE_ERR may be needed to avoid breaking external users.
> >
>
> RTE_CDEV_LOG_ERR should be available, though, right? :-)
Err, yes, but if we make it private, we don't need the RTE_ prefix ? :-)
>
> >
> > There are a lot of other log macros that let it to the callers to add
> > a trailing \n.
> > Should we convert them?
>
> I would think that, ideally, yes we should.
>
> > Converting the *whole* DPDK code to the new helper (with some
> > exceptions for people who like multiline logs..) would be nice to
> > close this topic once and for all.
> > But it would likely be a nightmare for later fixes that contain logs
> > and which could introduce regressions in such logs if the backport
> > does not take care of re-adding a \n.
>
> Good point. I wonder how many backports add new logs, or modify existing
> ones? Are there any automated checks, or a checklist for backports to
Touching logs in backport happens often afaics.
(to simplify I added a vXX.11.0 tag pointing at vXX.11)
$ for i in $(seq 0 4); do echo v21.11.${i}..v21.11.$((i + 1)) $(git
diff v21.11.${i}..v21.11.$((i + 1)) | grep -c ^\+.*LOG); done
v21.11.0..v21.11.1 122
v21.11.1..v21.11.2 40
v21.11.2..v21.11.3 43
v21.11.3..v21.11.4 30
v21.11.4..v21.11.5 24
$ for i in $(seq 0 2); do echo v22.11.${i}..v22.11.$((i + 1)) $(git
diff v22.11.${i}..v22.11.$((i + 1)) | grep -c ^\+.*LOG); done
v22.11.0..v22.11.1 0
v22.11.1..v22.11.2 37
v22.11.2..v22.11.3 106
I don't think there are checks on this topic (and to be fair, I don't
see which check could be done).
> enable us to catch these sort of things?
> Naming could be a problem, but we could look to move over existing logs by
> using new log macro names, which should help us to catch these. Even if the
> log names are a bit awkward, if they are kept internal-only, it shouldn't
> matter much. [Plus we would always be free to do a mass-rename back to the
> original name in future, once the backport issues are reduced].
This is the way I had in mind.
--
David Marchand
next prev parent reply other threads:[~2023-11-17 14:22 UTC|newest]
Thread overview: 122+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-17 13:18 David Marchand
2023-11-17 13:18 ` [RFC 1/3] lib: remove redundant newline from logs David Marchand
2023-11-17 13:18 ` [RFC 2/3] log: add a per line log helper David Marchand
2023-11-17 13:18 ` [RFC 3/3] lib: use per line logging David Marchand
2023-11-17 13:27 ` [RFC 0/3] Detect superfluous newline in logs Bruce Richardson
2023-11-17 13:48 ` David Marchand
2023-11-17 14:11 ` Bruce Richardson
2023-11-17 14:21 ` David Marchand [this message]
2023-11-17 13:47 ` Morten Brørup
2023-11-17 14:09 ` David Marchand
2023-11-17 14:17 ` Morten Brørup
2023-12-08 14:59 ` [RFC v2 00/14] " David Marchand
2023-12-08 14:59 ` [RFC v2 01/14] hash: remove some dead code David Marchand
2023-12-08 16:53 ` Stephen Hemminger
2023-12-08 20:46 ` Tyler Retzlaff
2023-12-08 14:59 ` [RFC v2 02/14] regexdev: fix logtype register David Marchand
2023-12-08 16:58 ` Stephen Hemminger
2023-12-08 20:46 ` Tyler Retzlaff
2023-12-14 10:11 ` Ori Kam
2023-12-08 14:59 ` [RFC v2 03/14] lib: use dedicated logtypes David Marchand
2023-12-08 17:00 ` Stephen Hemminger
2023-12-08 20:49 ` Tyler Retzlaff
2023-12-16 9:47 ` Andrew Rybchenko
2023-12-08 14:59 ` [RFC v2 04/14] lib: add newline in logs David Marchand
2023-12-08 17:01 ` Stephen Hemminger
2023-12-11 12:38 ` David Marchand
2023-12-08 20:50 ` Tyler Retzlaff
2023-12-16 9:43 ` Andrew Rybchenko
2023-12-08 14:59 ` [RFC v2 05/14] lib: remove redundant newline from logs David Marchand
2023-12-08 17:02 ` Stephen Hemminger
2023-12-09 7:15 ` fengchengwen
2023-12-11 8:48 ` Mattias Rönnblom
2023-12-08 14:59 ` [RFC v2 06/14] eal/linux: remove log paraphrasing the doc David Marchand
2023-12-08 17:03 ` Stephen Hemminger
2023-12-08 20:54 ` Tyler Retzlaff
2023-12-08 14:59 ` [RFC v2 07/14] bpf: remove log level in internal helper David Marchand
2023-12-08 17:04 ` Stephen Hemminger
2023-12-08 21:02 ` Tyler Retzlaff
2023-12-08 14:59 ` [RFC v2 08/14] lib: simplify multilines log messages David Marchand
2023-12-08 17:05 ` Stephen Hemminger
2023-12-16 9:26 ` Andrew Rybchenko
2023-12-08 21:03 ` Tyler Retzlaff
2023-12-08 14:59 ` [RFC v2 09/14] rcu: introduce a logging helper David Marchand
2023-12-08 17:08 ` Stephen Hemminger
2023-12-08 18:26 ` Honnappa Nagarahalli
2023-12-08 21:27 ` Tyler Retzlaff
2023-12-12 15:00 ` David Marchand
2023-12-12 19:11 ` Tyler Retzlaff
2023-12-18 9:37 ` David Marchand
2023-12-18 19:52 ` Tyler Retzlaff
2023-12-08 14:59 ` [RFC v2 10/14] vhost: improve log for memory dumping configuration David Marchand
2023-12-08 17:14 ` Stephen Hemminger
2023-12-08 14:59 ` [RFC v2 11/14] log: add a per line log helper David Marchand
2023-12-08 17:15 ` Stephen Hemminger
2023-12-09 7:21 ` fengchengwen
2023-12-08 14:59 ` [RFC v2 12/14] lib: convert to per line logging David Marchand
2023-12-08 17:16 ` Stephen Hemminger
2023-12-11 12:34 ` David Marchand
2023-12-16 9:30 ` Andrew Rybchenko
2023-12-08 14:59 ` [RFC v2 13/14] lib: replace logging helpers David Marchand
2023-12-08 17:18 ` Stephen Hemminger
2023-12-11 12:36 ` David Marchand
2023-12-16 9:42 ` Andrew Rybchenko
2023-12-08 14:59 ` [RFC v2 14/14] lib: use per line logging in helpers David Marchand
2023-12-09 7:19 ` fengchengwen
2023-12-16 9:41 ` Andrew Rybchenko
2023-12-18 9:27 ` [PATCH v3 00/14] Detect superfluous newline in logs David Marchand
2023-12-18 9:27 ` [PATCH v3 01/14] hash: remove some dead code David Marchand
2023-12-18 9:27 ` [PATCH v3 02/14] regexdev: fix logtype register David Marchand
2023-12-18 9:27 ` [PATCH v3 03/14] lib: use dedicated logtypes and macros David Marchand
2023-12-18 9:27 ` [PATCH v3 04/14] lib: add newline in logs David Marchand
2023-12-18 9:27 ` [PATCH v3 05/14] lib: remove redundant newline from logs David Marchand
2023-12-18 9:27 ` [PATCH v3 06/14] eal/linux: remove log paraphrasing the doc David Marchand
2023-12-18 9:27 ` [PATCH v3 07/14] bpf: remove log level in internal helper David Marchand
2023-12-18 9:27 ` [PATCH v3 08/14] lib: simplify multilines log messages David Marchand
2023-12-18 10:05 ` Andrew Rybchenko
2023-12-18 9:27 ` [PATCH v3 09/14] rcu: introduce a logging helper David Marchand
2023-12-18 9:27 ` [PATCH v3 10/14] vhost: improve log for memory dumping configuration David Marchand
2023-12-18 9:27 ` [PATCH v3 11/14] log: add a per line log helper David Marchand
2023-12-18 9:27 ` [PATCH v3 12/14] lib: convert to per line logging David Marchand
2023-12-18 9:27 ` [PATCH v3 13/14] lib: replace logging helpers David Marchand
2023-12-18 9:27 ` [PATCH v3 14/14] lib: use per line logging in helpers David Marchand
2023-12-18 14:37 ` [PATCH v4 00/14] Detect superfluous newline in logs David Marchand
2023-12-18 14:37 ` [PATCH v4 01/14] hash: remove some dead code David Marchand
2023-12-18 14:37 ` [PATCH v4 02/14] regexdev: fix logtype register David Marchand
2023-12-18 16:46 ` Stephen Hemminger
2023-12-18 14:37 ` [PATCH v4 03/14] lib: use dedicated logtypes and macros David Marchand
2023-12-18 14:37 ` [PATCH v4 04/14] lib: add newline in logs David Marchand
2023-12-18 14:37 ` [PATCH v4 05/14] lib: remove redundant newline from logs David Marchand
2023-12-18 14:37 ` [PATCH v4 06/14] eal/linux: remove log paraphrasing the doc David Marchand
2023-12-18 14:37 ` [PATCH v4 07/14] bpf: remove log level in internal helper David Marchand
2023-12-18 14:37 ` [PATCH v4 08/14] lib: simplify multilines log messages David Marchand
2023-12-18 14:37 ` [PATCH v4 09/14] rcu: introduce a logging helper David Marchand
2023-12-18 14:37 ` [PATCH v4 10/14] vhost: improve log for memory dumping configuration David Marchand
2023-12-18 14:38 ` [PATCH v4 11/14] log: add a per line log helper David Marchand
2023-12-19 15:45 ` Thomas Monjalon
2023-12-19 17:16 ` Stephen Hemminger
2023-12-20 8:26 ` David Marchand
2023-12-18 14:38 ` [PATCH v4 12/14] lib: convert to per line logging David Marchand
2023-12-20 13:46 ` Thomas Monjalon
2023-12-20 14:00 ` David Marchand
2023-12-18 14:38 ` [PATCH v4 13/14] lib: replace logging helpers David Marchand
2023-12-18 14:38 ` [PATCH v4 14/14] lib: use per line logging in helpers David Marchand
2023-12-20 15:35 ` [PATCH v5 00/13] Detect superfluous newline in logs David Marchand
2023-12-20 15:35 ` [PATCH v5 01/13] hash: remove some dead code David Marchand
2023-12-21 5:58 ` Ruifeng Wang
2023-12-21 6:26 ` Ruifeng Wang
2023-12-20 15:35 ` [PATCH v5 02/13] regexdev: fix logtype register David Marchand
2023-12-20 15:35 ` [PATCH v5 03/13] lib: use dedicated logtypes and macros David Marchand
2023-12-20 15:35 ` [PATCH v5 04/13] lib: add newline in logs David Marchand
2023-12-20 15:35 ` [PATCH v5 05/13] lib: remove redundant newline from logs David Marchand
2023-12-20 15:35 ` [PATCH v5 06/13] eal/linux: remove log paraphrasing the doc David Marchand
2023-12-20 15:36 ` [PATCH v5 07/13] bpf: remove log level in internal helper David Marchand
2023-12-20 15:36 ` [PATCH v5 08/13] lib: simplify multilines log messages David Marchand
2023-12-20 15:36 ` [PATCH v5 09/13] lib: add more logging helpers David Marchand
2023-12-20 15:36 ` [PATCH v5 10/13] vhost: improve log for memory dumping configuration David Marchand
2023-12-20 15:36 ` [PATCH v5 11/13] log: add a per line log helper David Marchand
2023-12-20 15:42 ` David Marchand
2023-12-20 15:36 ` [PATCH v5 12/13] lib: replace logging helpers David Marchand
2023-12-20 15:36 ` [PATCH v5 13/13] lib: use per line logging in helpers David Marchand
2023-12-21 9:31 ` [PATCH v5 00/13] Detect superfluous newline in logs David Marchand
2023-12-21 16:32 ` Stephen Hemminger
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=CAJFAV8w5xeG7ONY_LX0QgyraBDFV7Lg7d7jJ8UQP-5JzcyiNJg@mail.gmail.com \
--to=david.marchand@redhat.com \
--cc=bluca@debian.org \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=ktraynor@redhat.com \
--cc=mb@smartsharesystems.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
--cc=xuemingl@nvidia.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).