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]

Date:	Thu, 14 Oct 2010 08:09:07 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	ebiederm@...ssion.com
Cc:	hans.schillstrom@...csson.com, daniel.lezcano@...e.fr,
	netdev@...r.kernel.org
Subject: Re: BUG ? ipip unregister_netdevice_many()

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.

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