From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f173.google.com ([209.85.223.173]:32820 "EHLO mail-io0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755170AbeCSE1E (ORCPT ); Mon, 19 Mar 2018 00:27:04 -0400 Received: by mail-io0-f173.google.com with SMTP id l3so2484056iog.0 for ; Sun, 18 Mar 2018 21:27:04 -0700 (PDT) Subject: Re: [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message To: Marcelo Ricardo Leitner Cc: netdev@vger.kernel.org, Alexander Aring , Jiri Pirko , Jakub Kicinski References: <673abaddb26351826ca454f46d1271f1f4814c56.1521226621.git.marcelo.leitner@gmail.com> <20180318181922.GG9345@localhost.localdomain> From: David Ahern Message-ID: Date: Sun, 18 Mar 2018 22:27:00 -0600 MIME-Version: 1.0 In-Reply-To: <20180318181922.GG9345@localhost.localdomain> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: 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. ;-)