[PATCH net-next 0/4] batch calls to fib_flush and arp_ifdown

[Salam Noureddine]

fib_flush walks the whole fib in a net_namespace and is called for
each net_device being closed or unregistered. This can be very expensive
when dealing with 100k or more routes in the fib and removal of a lot
of interfaces. These four patches deal with this issue by calling fib_flush
just once for each net namespace and introduce a new function arp_ifdown_all
that does a similar optimization for the neighbour table.

I got the following benchmark results on one of our switches.
Without this patch, deleting 1k interfaces with 100k routes in the fib held
the rtnl_lock for 13 seconds. With the patch, rtnl_lock hold time went down
to 5 seconds. The gain is even more pronounced with 512k routes in the FIB.
In this case, without the patch, rtnl_lock was held for 36 seconds and with
the patch it was held for 5.5 seconds.

Salam Noureddine (4):
  net: add event_list to struct net and provide utility functions
  net: dev: add batching to net_device notifiers
  net: core: introduce neigh_ifdown_all for all down interfaces
  net: fib: avoid calling fib_flush for each device when doing batch
close and unregister

 include/linux/netdevice.h   |  2 ++
 include/net/arp.h   |  1 +
 include/net/neighbour.h |  1 +
 include/net/net_namespace.h | 22 ++
 include/net/netns/ipv4.h|  1 +
 net/core/dev.c  | 39 ---
 net/core/neighbour.c| 38 +++---
 net/ipv4/arp.c  |  4 
 net/ipv4/fib_frontend.c | 16 ++--
 9 files changed, 112 insertions(+), 12 deletions(-)

-- 
1.8.1.4

[PATCH net-next 0/4] batch calls to fib_flush and arp_ifdown

[Salam Noureddine]

fib_flush walks the whole fib in a net_namespace and is called for
each net_device being closed or unregistered. This can be very expensive
when dealing with 100k or more routes in the fib and removal of a lot
of interfaces. These four patches deal with this issue by calling fib_flush
just once for each net namespace and introduce a new function arp_ifdown_all
that does a similar optimization for the neighbour table.

Salam Noureddine (4):
  net: add event_list to struct net and provide utility functions
  net: dev: add batching to net_device notifiers
  net: core: introduce neigh_ifdown_all for all down interfaces
  net: fib: avoid calling fib_flush for each device when doing batch
close and unregister

 include/linux/netdevice.h   |  2 ++
 include/net/arp.h   |  1 +
 include/net/neighbour.h |  1 +
 include/net/net_namespace.h | 22 ++
 include/net/netns/ipv4.h|  1 +
 net/core/dev.c  | 39 ---
 net/core/neighbour.c| 38 +++---
 net/ipv4/arp.c  |  4 
 net/ipv4/fib_frontend.c | 16 ++--
 9 files changed, 112 insertions(+), 12 deletions(-)

-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 0/4] batch calls to fib_flush and arp_ifdown

[Eric W. Biederman]

Salam Noureddine  writes:

> fib_flush walks the whole fib in a net_namespace and is called for
> each net_device being closed or unregistered. This can be very expensive
> when dealing with 100k or more routes in the fib and removal of a lot
> of interfaces. These four patches deal with this issue by calling fib_flush
> just once for each net namespace and introduce a new function arp_ifdown_all
> that does a similar optimization for the neighbour table.

Two things would be very valuable with this patchset.

Some numbers on how much your changes have improved the code in the case
you care about.  I suspect the improvements are not subtle so this
should not be hard.

Can you please provide a justification for event_list.  Just skimming
through it appears that event_list because a duplicate of the list of
batched network devices that are passed to dev_close_many and friends.
If the list is actually a duplicate it appears foolish to create it.

Eric

> Salam Noureddine (4):
>   net: add event_list to struct net and provide utility functions
>   net: dev: add batching to net_device notifiers
>   net: core: introduce neigh_ifdown_all for all down interfaces
>   net: fib: avoid calling fib_flush for each device when doing batch
> close and unregister
>
>  include/linux/netdevice.h   |  2 ++
>  include/net/arp.h   |  1 +
>  include/net/neighbour.h |  1 +
>  include/net/net_namespace.h | 22 ++
>  include/net/netns/ipv4.h|  1 +
>  net/core/dev.c  | 39 ---
>  net/core/neighbour.c| 38 +++---
>  net/ipv4/arp.c  |  4 
>  net/ipv4/fib_frontend.c | 16 ++--
>  9 files changed, 112 insertions(+), 12 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 0/4] batch calls to fib_flush and arp_ifdown

[Salam Noureddine]

On Mon, Jan 4, 2016 at 4:35 PM, Eric W. Biederman  wrote:

> Two things would be very valuable with this patchset.
>
> Some numbers on how much your changes have improved the code in the case
> you care about.  I suspect the improvements are not subtle so this
> should not be hard.
>
> Can you please provide a justification for event_list.  Just skimming
> through it appears that event_list because a duplicate of the list of
> batched network devices that are passed to dev_close_many and friends.
> If the list is actually a duplicate it appears foolish to create it.
>
> Eric

The performance test I ran tries to unregister 1000 dummy interfaces
with 512K routes in the fib.
Without the patch I could unregister 35 interfaces per second and with
the patch it jumped to 620
interfaces per second. 512K is a lot of routes but I am assuming we
would get a good improvement
even with 100K routes in the fib.

I am using event_list to put all the net namespaces in the current
net_device batch on a list and only
call the NETDEV_UNREGISTER_BATCH on those namespaces. It would be
possible to just call the
notifier for NETDEV_DOWN/UNREGISTER_BATCH for all the devices on the
list and rely on the
needs_fib_flush flag to only call fib_flush once per namespace but it
seems like a waste to me.

Salam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html