| Message ID | d75d345a-0571-81cb-63c8-f06ada45f252@cumulusnetworks.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Tue, Sep 06, 2016 at 05:11:11PM CEST, dsa@cumulusnetworks.com wrote: >On 9/6/16 8:44 AM, Jiri Pirko wrote: >> Tue, Sep 06, 2016 at 04:32:12PM CEST, dsa@cumulusnetworks.com wrote: >>> On 9/6/16 6:01 AM, Jiri Pirko wrote: >>>> From: Jiri Pirko <jiri@mellanox.com> >>>> >>>> This allows to pass information about added/deleted fib entries to >>>> whoever is interested. This is done in a very similar way as devinet >>>> notifies address additions/removals. >>>> >>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com> >>>> --- >>>> include/net/ip_fib.h | 19 +++++++++++++++++++ >>>> net/ipv4/fib_trie.c | 43 +++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 62 insertions(+) >>>> >>> >>> The notifier infrastructure should be generalized for use with IPv4 and IPv6. While the data will be family based, the infra can be generic. >>> >> >> Yeah, that I thought about as well. Thing is, ipv6 notifier has to be >> atomic. That is the reason we have: >> inetaddr_chain and register_inetaddr_notifier (blocking notifier) >> inet6addr_chain and register_inet6addr_notifier (atomic notifier) >> > >Why is IPv6 atomic? Looking at code paths for adding addresses seems like all of the locks are dropped before the notifier is called and adding and deleting ipv6 addresses does not show a hit with this WARN_ON: Maybe historic reasons. Would be good to unite the notifiers then. I'll look at it. > > >diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c >index bfa941fc1165..4f9f964d95e5 100644 >--- a/net/ipv6/addrconf_core.c >+++ b/net/ipv6/addrconf_core.c >@@ -103,6 +103,7 @@ EXPORT_SYMBOL(unregister_inet6addr_notifier); > > int inet6addr_notifier_call_chain(unsigned long val, void *v) > { >+WARN_ON(in_atomic()); > return atomic_notifier_call_chain(&inet6addr_chain, val, v); > } > EXPORT_SYMBOL(inet6addr_notifier_call_chain);
On Tue, Sep 6, 2016, at 17:49, Jiri Pirko wrote: > Tue, Sep 06, 2016 at 05:11:11PM CEST, dsa@cumulusnetworks.com wrote: > >On 9/6/16 8:44 AM, Jiri Pirko wrote: > >> Tue, Sep 06, 2016 at 04:32:12PM CEST, dsa@cumulusnetworks.com wrote: > >>> On 9/6/16 6:01 AM, Jiri Pirko wrote: > >>>> From: Jiri Pirko <jiri@mellanox.com> > >>>> > >>>> This allows to pass information about added/deleted fib entries to > >>>> whoever is interested. This is done in a very similar way as devinet > >>>> notifies address additions/removals. > >>>> > >>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com> > >>>> --- > >>>> include/net/ip_fib.h | 19 +++++++++++++++++++ > >>>> net/ipv4/fib_trie.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > >>>> 2 files changed, 62 insertions(+) > >>>> > >>> > >>> The notifier infrastructure should be generalized for use with IPv4 and IPv6. While the data will be family based, the infra can be generic. > >>> > >> > >> Yeah, that I thought about as well. Thing is, ipv6 notifier has to be > >> atomic. That is the reason we have: > >> inetaddr_chain and register_inetaddr_notifier (blocking notifier) > >> inet6addr_chain and register_inet6addr_notifier (atomic notifier) > >> > > > >Why is IPv6 atomic? Looking at code paths for adding addresses seems like all of the locks are dropped before the notifier is called and adding and deleting ipv6 addresses does not show a hit with this WARN_ON: > > > Maybe historic reasons. Would be good to unite the notifiers then. I'll > look at it. We add IPs and routes from bottom half layer because of neighbour discovery router advertisements. They need to run in atomic context without sleeping. Bye, Hannes
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c index bfa941fc1165..4f9f964d95e5 100644 --- a/net/ipv6/addrconf_core.c +++ b/net/ipv6/addrconf_core.c @@ -103,6 +103,7 @@ EXPORT_SYMBOL(unregister_inet6addr_notifier); int inet6addr_notifier_call_chain(unsigned long val, void *v) { +WARN_ON(in_atomic()); return atomic_notifier_call_chain(&inet6addr_chain, val, v); } EXPORT_SYMBOL(inet6addr_notifier_call_chain);