netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: netdev@vger.kernel.org, Alexander Aring <aring@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>, Jakub Kicinski <kubakici@wp.pl>
Subject: Re: [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message
Date: Sun, 18 Mar 2018 22:27:00 -0600	[thread overview]
Message-ID: <cea2b3c4-c5ae-878a-21c4-57ada02aa2f3@gmail.com> (raw)
In-Reply-To: <20180318181922.GG9345@localhost.localdomain>

On 3/18/18 12:19 PM, Marcelo Ricardo Leitner wrote:
> On Sun, Mar 18, 2018 at 10:11:20AM -0600, David Ahern wrote:
>> On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
>>> Currently extack can carry only a single message, which is usually the
>>> error message.
>>>
>>> This imposes a limitation on a more verbose error reporting. For
>>> example, it's not able to carry warning messages together with the error
>>> message, or 2 warning messages.
>>
>>
>> The only means for userspace to separate an error message from info or
>> warnings is the error in nlmsgerr. If it is non-0, any extack message is
>> considered an error else it is a warning.
> 
> I don't see your point here.
> 
> The proposed patch extends what you said to:
> - include warnings on error reports
> - allow more than 1 message
> 
> With the proposed patch, if nlmsgerr is 0 all messages are considered
> as warnings. If it's non-zero, some may be marked as warnings.

It's the 'some' that I was referring to, but ...


>>> +#define NL_SET_ERR_MSG(extack, msg)	NL_SET_MSG(extack, msg)
>>> +#define NL_SET_WARN_MSG(extack, msg)	NL_SET_MSG(extack, KERN_WARNING msg)
>>> +
>>>  #define NL_SET_ERR_MSG_MOD(extack, msg)			\
>>>  	NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " msg)
>>> +#define NL_SET_WARN_MSG_MOD(extack, msg)		\
>>> +	NL_SET_WARN_MSG((extack), KBUILD_MODNAME ": " msg)
>>> +
>>
>> Adding separate macros for error versus warning is confusing since from
>> an extack perspective a message is a message and there is no uapi to
>> separate them.
> 
> Are you saying the markings at beginning of the messages are not
> possible? If that's the case, we probably can think of something else,
> as I see value in being able to deliver warnings together with errors.

... I did miss the KERN_WARNIN above. That means that warning messages
are prefixed by 0x1 (KERN_SOH) and "4" (warning loglevel). There will be
cases missed by iproute2 as current versions won't catch the 2 new
characters.

Converting code to be able to continue generating error messages yet
ultimately fail seems overly complex to me. I get the intent of
returning as much info as possible, but most of that feels (e.g., in the
mlx5 example you referenced) like someone learning how to do something
the first time in which case 1 at a time errors seems reasonable - in
fact might drive home some lessons. ;-)

  reply	other threads:[~2018-03-19  4:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-16 19:23 [PATCH RFC 0/2] Add support for warnings to extack Marcelo Ricardo Leitner
2018-03-16 19:23 ` [PATCH RFC iproute2] libnetlink: allow reading more than one message from extack Marcelo Ricardo Leitner
2018-03-19 16:09   ` Stephen Hemminger
2018-03-19 17:14     ` Marcelo Ricardo Leitner
2018-03-16 19:23 ` [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message Marcelo Ricardo Leitner
2018-03-18 16:11   ` David Ahern
2018-03-18 18:19     ` Marcelo Ricardo Leitner
2018-03-19  4:27       ` David Ahern [this message]
2018-03-19 15:34         ` Marcelo Ricardo Leitner
2018-03-16 19:23 ` [PATCH RFC 2/2] sched: pass extack through even if skip_sw is not set Marcelo Ricardo Leitner
2018-03-16 19:27 ` [PATCH RFC 0/2] Add support for warnings to extack Marcelo Ricardo Leitner
2018-03-16 22:05 ` Jakub Kicinski
2018-03-18 17:38   ` Marcelo Ricardo Leitner
2018-03-18 16:11 ` David Ahern
2018-03-18 18:20   ` Marcelo Ricardo Leitner
2018-03-18 18:36   ` Marcelo Ricardo Leitner

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=cea2b3c4-c5ae-878a-21c4-57ada02aa2f3@gmail.com \
    --to=dsahern@gmail.com \
    --cc=aring@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kubakici@wp.pl \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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).