netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch.org>
To: David Ahern <dsa@cumulusnetworks.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	roopa@cumulusnetworks.com, shm@cumulusnetworks.com,
	jiri@mellanox.com, idosch@mellanox.com,
	jakub.kicinski@netronome.com
Subject: Re: [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno
Date: Sun, 25 Mar 2018 18:37:07 +0300	[thread overview]
Message-ID: <20180325153707.GA4340@splinter> (raw)
In-Reply-To: <e2115470-778a-9923-35a3-04e5d9c8af91@cumulusnetworks.com>

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 <dsa@cumulusnetworks.com>
> >> ---
> >>  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.

  reply	other threads:[~2018-03-25 15:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-22 22:57 [PATCH RFC net-next 0/7] net: Allow FIB notifiers to fail add and replace David Ahern
2018-03-22 22:57 ` [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno David Ahern
2018-03-25  8:16   ` Ido Schimmel
2018-03-25 14:00     ` David Ahern
2018-03-25 15:37       ` Ido Schimmel [this message]
2018-03-22 22:57 ` [PATCH RFC net-next 2/7] net: Move call_fib_rule_notifiers up in fib_nl_newrule David Ahern
2018-03-22 22:57 ` [PATCH RFC net-next 3/7] net/ipv4: Move call_fib_entry_notifiers up for new routes David Ahern
2018-03-22 22:57 ` [PATCH RFC net-next 4/7] net/ipv4: Allow notifier to fail route repolace David Ahern
2018-03-22 22:57 ` [PATCH RFC net-next 5/7] net/ipv6: Move call_fib6_entry_notifiers up for route adds David Ahern
2018-03-22 22:57 ` [PATCH RFC net-next 6/7] devlink: Export methods to get and set namespace David Ahern
2018-03-22 22:57 ` [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink David Ahern
2018-03-23  6:50   ` Jiri Pirko
2018-03-23 14:31     ` David Ahern
2018-03-23 15:01       ` Jiri Pirko
2018-03-23 15:03         ` David Ahern
2018-03-23 15:05           ` Jiri Pirko
2018-03-23 15:13             ` David Ahern
2018-03-24  7:26               ` Jiri Pirko
2018-03-24 15:05                 ` David Ahern
2018-03-24 16:02                   ` Jiri Pirko
2018-03-25 14:24                     ` David Ahern
2018-03-26 14:33                       ` Jiri Pirko
2018-03-24  3:47   ` Jakub Kicinski
2018-03-24 15:02     ` David Ahern
2018-03-25  6:35       ` Jakub Kicinski
2018-03-25 14:27         ` David Ahern
2018-03-25 19:53           ` Jakub Kicinski

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=20180325153707.GA4340@splinter \
    --to=idosch@idosch.org \
    --cc=davem@davemloft.net \
    --cc=dsa@cumulusnetworks.com \
    --cc=idosch@mellanox.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jiri@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    --cc=shm@cumulusnetworks.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 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).