Re: [PATCH net-next 0/7] rtnetlink: allow to run selected handlers without rtnl

[David Miller]

From: Florian Westphal 
Date: Wed, 9 Aug 2017 10:19:28 +0200

> Would you accept a v2 if i don't touch ipv6 routes for the time being?
> 
> I would then audit those again.  At the very least inet6_rtm_getroute should
> be able to work without rtnl lock (i.e., use a different lock if
> needed to protect vs. concurrent modifications).

Generally speaking 'get' operations should be fine.

Re: [PATCH net-next 0/7] rtnetlink: allow to run selected handlers without rtnl

[David Miller]

From: Florian Westphal 
Date: Wed, 9 Aug 2017 10:19:28 +0200

> David Miller  wrote:
>> From: Florian Westphal 
>> Date: Tue,  8 Aug 2017 18:02:29 +0200
>> 
>> > Unfortunately RTNL mutex is a performance issue, e.g. a cpu adding
>> > an ip address prevents other cpus from seemingly unrelated tasks
>> > such as dumping tc classifiers.
>> 
>> It is related if somehow the TC entries refer to IP addresses.
>>
>> Someone could create something like that.
> 
> Actually I am not following.  Why would read-only accesses need rtnl
> locking wrt. any other operation (provided of course rtnl lock doesn't
> protect the data structure)?

If the validity of change X depends upon another value Y meeting some
criteria, we test Y and then must be sure that Y doesn't change while
we go about making the change to X.

For example if I need to make sure a path to an IPV4 address exists
configured to an interface when adding a route, I must be sure that
someone can't remove that IPV4 address while I'm adding the route,
after I've checked that it does in fact exist.

This is the kind of stuff that the RTNL mutex ensures.

Re: [PATCH net-next 0/7] rtnetlink: allow to run selected handlers without rtnl

[Florian Westphal]

David Miller  wrote:
> From: Florian Westphal 
> Date: Tue,  8 Aug 2017 18:02:29 +0200
> 
> > Unfortunately RTNL mutex is a performance issue, e.g. a cpu adding
> > an ip address prevents other cpus from seemingly unrelated tasks
> > such as dumping tc classifiers.
> 
> It is related if somehow the TC entries refer to IP addresses.
>
> Someone could create something like that.

Actually I am not following.  Why would read-only accesses need rtnl
locking wrt. any other operation (provided of course rtnl lock doesn't
protect the data structure)?

> > Initial no-rtnl spots are ip6 fib add/del and netns new/getid.
> 
> I could see the netns stuff being ok, but IPv6 route add/del I'm
> not so sure of.

[..]

> There really is a hierachy of these dependencies.  Device state, up
> to neighbour table state, up to protocol address state, up to routes,
>up to FIB tables, etc. etc. etc.
> 
>I'd really like to make this operate more freely, but this is an
>extremely delicate area which has been bottled up like this for
>two decades so good luck :-)

Would you accept a v2 if i don't touch ipv6 routes for the time being?

I would then audit those again.  At the very least inet6_rtm_getroute should
be able to work without rtnl lock (i.e., use a different lock if
needed to protect vs. concurrent modifications).

Re: [PATCH net-next 0/7] rtnetlink: allow to run selected handlers without rtnl

[David Miller]

From: Florian Westphal 
Date: Tue,  8 Aug 2017 18:02:29 +0200

> Unfortunately RTNL mutex is a performance issue, e.g. a cpu adding
> an ip address prevents other cpus from seemingly unrelated tasks
> such as dumping tc classifiers.

It is related if somehow the TC entries refer to IP addresses.

Someone could create something like that.

> Initial no-rtnl spots are ip6 fib add/del and netns new/getid.

I could see the netns stuff being ok, but IPv6 route add/del I'm
not so sure of.

Because of things like nexthops etc. there are dependencies on
other configuration things.

That's the whole reason we have this unfortunate global
synchronization point.  If I'm changing some aspect of network
configuration, I know I can atomically test any piece of networking
configuration state.

If I test a network address to make sure I can properly reacy X and
use X as a nexthop in the route I'm adding, it will be there
throughout the entire operation.

There really is a hierachy of these dependencies.  Device state, up
to neighbour table state, up to protocol address state, up to routes,
up to FIB tables, etc. etc. etc.

I'd really like to make this operate more freely, but this is an
extremely delicate area which has been bottled up like this for
two decades so good luck :-)

[PATCH net-next 0/7] rtnetlink: allow to run selected handlers without rtnl

[Florian Westphal]

The RTNL mutex is used to serialize both rtnetlink calls and dump requests.
Its also used to protect other things such as the list of current netns.

Unfortunately RTNL mutex is a performance issue, e.g. a cpu adding an
ip address prevents other cpus from seemingly unrelated tasks such as
dumping tc classifiers.

This patch set attempts to add basic infrastructure to start pushing the
rtnl lock down to those places that need it, or even elide it entirely in
some cases.

Subsystems can now indicate that their doit() callback can run without
RTNL mutex.  Such callbacks can then run in parallel.

This will obviously need a lot of followup work; all current users need
to be audited/changed to benefit from this.

Initial no-rtnl spots are ip6 fib add/del and netns new/getid.

Dumps are another problem entirely, see
commit 2907c35ff64708065 ("net: hold rtnl again in dump callbacks"),
this patchset doesn't touch them.