Re: [PATCH 4/6] net neighbour: convert to RCU

[Stephen Hemminger <shemminger@osdl.org>]

On Wed, 30 Aug 2006 03:21:26 +0400
Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> wrote:

> Hello!
> 
> > This should not be any more racy than the existing code.
> 
> Existing code is not racy.

Race 1: w/o RCU
					Cpu 0: is in neigh_lookup
						gets read_lock()
						finds entry
						++refcount to 2
						updates it
	Cpu 1: is in forced_gc()
		waits at write_lock()
						releases read_lock()
						drops ref count to 1.
		sees ref count is 1
		deletes it

Race 1: w RCU
					Cpu 0: is in __neigh_lookup
						updates it
	Cpu 1: is in forced_gc()
						leaves refcount=1
		sees ref count is 1
		deletes it


> 
> Critical place is interpretation of refcnt==1. Current code assumes,
> that when refcnt=1 and entry is in hash table, nobody can take this
> entry (table is locked). So, it can be unlinked from the table.
> 
> See? __neigh_lookup()/__neigh_lookup_errno() _MUST_ return alive
> and hashed entry. And will stay hashed until the reference is held.
> Or until neigh entry is forced for destruction by device going down,
> in this case referring dst entries will die as well.

Why must it be hashed, it could always get zapped just after the update.

> If dst cache grabs an entry, which is purged from table because for some
> time it had refcnt==1, you got a valid dst entry referring to dead
> neighbour entry.

Hmm.. Since this is a slow path, grabbing the write_lock on the neighbour
entry would block the gc from zapping it.

in __neigh_lookup()
		neigh_hold();
		write_lock(&n->lock);
		if (n->dead) {
			write_unlock()
			neigh_release()
			goto rescan;
		}

		write_unlock()





-- 
Stephen Hemminger <shemminger@osdl.org>