Re: BUG ? ipip unregister_netdevice_many()

[David Miller <davem@davemloft.net>]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 13 Oct 2010 22:20:28 -0700

> With the network namespace support we limit the scope of the test of
> the invalidate to just a single network namespace, and as such
> rt_is_expired stops being true for every cache entry.  So we cannot
> unconditionally throw away entire chains.
> 
> All of which can be either done by network namespace equality or by
> rt_is_expired().  Although Denis picked rt_is_expired() when he made
> his change.

Right, and I choose to use namespace equality which will completely
compile into no code at all when namespace support is not in the
kernel.

Therefore, making the non-namespace case equivalent and as efficient
as it always was.

> The only place it makes a noticable difference in practice is what
> happens when we do batched deleletes of lots of network devices in
> different network namespaces.
> 
> During batched network device deletes in fib_netdev_event we do
> rt_cache_flush(dev_net(dev), -1) for each network device.  and then a
> final rt_cache_flush_batch() to remove the invalidated entries.  These
> devices can be from multiple network namespaces, so I suspect that is
> a savings worth having.

How can it make a real difference even in this case?  We'll obliterate
all the entries, and then on subsequent passes we'll find nothing
matching that namespace any more.

Show me performance tests that show it makes any difference, please.

> So if we are going to change the tests we need to do something with
> rt_cache_flush_batch().  Further I do not see what is confusing about
> a test that asks if the routing cache entry is unusable.  Is
> rt_cache_expired() a bad name?

It's not a bad name, it's just an unnecessary test that we don't need
to even make in this specific place.

Redundancy tends to accumulate.