All of lore.kernel.org
 help / color / mirror / Atom feed
From: Quentin Monnet <quentin.monnet@netronome.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	bpf@vger.kernel.org, Networking <netdev@vger.kernel.org>,
	oss-drivers@netronome.com, Yonghong Song <yhs@fb.com>
Subject: Re: [PATCH bpf-next v2 1/3] tools: bpftool: add -d option to get debug output from libbpf
Date: Fri, 24 May 2019 10:46:55 +0100	[thread overview]
Message-ID: <c720f3ce-059b-b47a-a0de-5e360b590a30@netronome.com> (raw)
In-Reply-To: <CAEf4BzbyE8w1wLN33OfUgu8qGqRbxE5LbXFniucyqW4mH7mQFw@mail.gmail.com>

2019-05-23 13:57 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, May 23, 2019 at 1:44 PM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
>>
>> On Thu, 23 May 2019 09:20:52 -0700, Andrii Nakryiko wrote:
>>> On Thu, May 23, 2019 at 3:54 AM Quentin Monnet wrote:
>>>>
>>>> libbpf has three levels of priority for output messages: warn, info,
>>>> debug. By default, debug output is not printed to the console.
>>>>
>>>> Add a new "--debug" (short name: "-d") option to bpftool to print libbpf
>>>> logs for all three levels.
>>>>
>>>> Internally, we simply use the function provided by libbpf to replace the
>>>> default printing function by one that prints logs regardless of their
>>>> level.
>>>>
>>>> v2:
>>>> - Remove the possibility to select the log-levels to use (v1 offered a
>>>>   combination of "warn", "info" and "debug").
>>>> - Rename option and offer a short name: -d|--debug.
>>>
>>> Such and option in CLI tools is usually called -v|--verbose, I'm
>>> wondering if it might be a better name choice?
>>>
>>> Btw, some tools also use -v, -vv and -vvv to define different levels
>>> of verbosity, which is something we can consider in the future, as
>>> it's backwards compatible.
>>
>> That was my weak suggestion.  Sometimes -v is used for version, e.g.
>> GCC.  -d is sometimes used for debug, e.g. man, iproute2 uses it as
>> short for "detailed".  If the consensus is that -v is better I don't
>> really mind.
> 
> It's minor, so I'm not insisting at all, just wasn't sure it was
> brought up. bpftool is sufficiently different in its conventions from
> other modern CLIs anyways.
> 
> As for -v for version. It seems like the trend is to use -V|--version
> for version, and -v|--verbose for verbosity. I've also seen some tools
> option for `cli version` (subcommand) for version. Anyway, no strong
> preferences from me either.
> 

For the record, bpftool has both "bpftool -V" and "bpftool version" to
dump the version number.

This leaves us with "-v" free to do something like "--verbose", but just
as Jakub said we wanted to limit the risks of confusion... I don't mind
changing, but since nobody has expressed a strong opinion on the matter,
I'll stick to "-d|--debug" for now.

Thanks Andrii for the review!
Quentin

  reply	other threads:[~2019-05-24  9:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23 10:54 [PATCH bpf-next v2 0/3] tools: bpftool: add an option for debug output from libbpf and verifier Quentin Monnet
2019-05-23 10:54 ` [PATCH bpf-next v2 1/3] tools: bpftool: add -d option to get debug output from libbpf Quentin Monnet
2019-05-23 16:20   ` Andrii Nakryiko
2019-05-23 20:44     ` Jakub Kicinski
2019-05-23 20:57       ` Andrii Nakryiko
2019-05-24  9:46         ` Quentin Monnet [this message]
2019-05-23 10:54 ` [PATCH bpf-next v2 2/3] libbpf: add bpf_object__load_xattr() API function to pass log_level Quentin Monnet
2019-05-23 16:19   ` Yonghong Song
2019-05-23 16:29     ` Yonghong Song
2019-05-24  9:47       ` Quentin Monnet
2019-05-23 10:54 ` [PATCH bpf-next v2 3/3] tools: bpftool: make -d option print debug output from verifier Quentin Monnet
2019-05-23 16:38   ` Yonghong Song
2019-05-24  9:47     ` Quentin Monnet

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=c720f3ce-059b-b47a-a0de-5e360b590a30@netronome.com \
    --to=quentin.monnet@netronome.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@netronome.com \
    --cc=yhs@fb.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.