From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt0-f182.google.com ([209.85.216.182]:35041 "EHLO mail-qt0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932814AbeCSPew (ORCPT ); Mon, 19 Mar 2018 11:34:52 -0400 Received: by mail-qt0-f182.google.com with SMTP id s2so3275775qti.2 for ; Mon, 19 Mar 2018 08:34:52 -0700 (PDT) Date: Mon, 19 Mar 2018 12:34:47 -0300 From: Marcelo Ricardo Leitner To: David Ahern Cc: netdev@vger.kernel.org, Alexander Aring , Jiri Pirko , Jakub Kicinski Subject: Re: [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message Message-ID: <20180319153447.GK9345@localhost.localdomain> References: <673abaddb26351826ca454f46d1271f1f4814c56.1521226621.git.marcelo.leitner@gmail.com> <20180318181922.GG9345@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Sun, Mar 18, 2018 at 10:27:00PM -0600, David Ahern wrote: > 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. The first one is not printable, so it would print a weird '4' at the beginning of the message. But: only if it didn't have any error message later, because old iproute will display only the last message (and error messages are not tagged). > > 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. ;-) That is true. Yep, I'm still lacking a real user for it. Maybe with the patchset split it will come up. M.