From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ido Schimmel Subject: Re: [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno Date: Sun, 25 Mar 2018 18:37:07 +0300 Message-ID: <20180325153707.GA4340@splinter> References: <20180322225757.10377-1-dsa@cumulusnetworks.com> <20180322225757.10377-2-dsa@cumulusnetworks.com> <20180325081632.GA11327@splinter> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, davem@davemloft.net, roopa@cumulusnetworks.com, shm@cumulusnetworks.com, jiri@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com To: David Ahern Return-path: Received: from out5-smtp.messagingengine.com ([66.111.4.29]:57979 "EHLO out5-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753337AbeCYPhK (ORCPT ); Sun, 25 Mar 2018 11:37:10 -0400 Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Sun, Mar 25, 2018 at 08:00:19AM -0600, David Ahern wrote: > On 3/25/18 2:16 AM, Ido Schimmel wrote: > > On Thu, Mar 22, 2018 at 03:57:51PM -0700, David Ahern wrote: > >> Notifier handlers use notifier_from_errno to convert any potential error > >> to an encoded format. As a consequence the other side, call_fib_notifiers > >> in this case, needs to use notifier_to_errno to return the error from > >> the handler back to its caller. > >> > >> Signed-off-by: David Ahern > >> --- > >> net/core/fib_notifier.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c > >> index 5ace0705a3f9..14ba52ebe8c9 100644 > >> --- a/net/core/fib_notifier.c > >> +++ b/net/core/fib_notifier.c > >> @@ -21,8 +21,11 @@ EXPORT_SYMBOL(call_fib_notifier); > >> int call_fib_notifiers(struct net *net, enum fib_event_type event_type, > >> struct fib_notifier_info *info) > > > > There's another (less interesting case) - call_fib_notifier(), which is > > used to dump the FIB tables for the caller registering to the > > notification chain. > > > > For example, if you have a non-default FIB rule in the system and you > > modprobe mlxsw, you'll get a silent failure and routes will not be > > offloaded. On the other hand, I'm not sure we want to fail the module > > loading in such cases. > > right. In normal cases the driver is loaded to create the netdevices > long before any networking config is done. So it seems to me the use > case you refer to, some user would have go out of there way to create a > situation where they install config that is not supported by the driver. Yes. > > A possible solution is to have the driver emit a warning via extack for > > each route/rule being notified after the abort mechanism was triggered. > > extack is not available on module load. I'm aware. I meant that during module load we'll trigger the abort mechanism (due to an unsupported FIB rule for example), then when user configures additional routes and extack is available we'll emit a warning. > Per past discussions, something you suggested, we need a message for > "out-of-line" cases like this where a driver notifies userspace of a > problem. That's another possibility. We can implement both options to make it perfectly clear to users and daemons what's going on.