netdev - Re: BUG ? ipip unregister_netdevice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m1lj60oc5o.fsf@fess.ebiederm.org>
Date:	Thu, 14 Oct 2010 11:35:31 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	David Miller <davem@...emloft.net>
Cc:	hans.schillstrom@...csson.com, daniel.lezcano@...e.fr,
	netdev@...r.kernel.org, Octavian Purdila <opurdila@...acom.com>
Subject: Re: BUG ? ipip unregister_netdevice_many()

David Miller <davem@...emloft.net> writes:

> From: ebiederm@...ssion.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.

Almost you still have the hash list inversion, which means you have
to at look at the rtable entry even on a one list long hash chain.
Perhaps I am looking at it wrong but once you look at the entries
I don't see the difference in the number of cache line faults
between one variant of the code and the other.

>> 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.

Octavian did you happen to measure the performance difference when you
added batching of routing table flushes?

>> 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.

As long as we do something that is correct in the batched flush case
I am happy either way.

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