[NET] Fix neighbour tbl->entries race

[NET] Fix neighbour tbl->entries race

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

Hi Dave:

This patch turns neigh_table's entries counter into an atomic_t.  This
is needed since tbl->entries is updated with no locks held.  This is
no big deal in practice since it'll be off by a few entries at most
which is way less than any of the neighbour thresholds.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2576 bytes --]

===== include/net/neighbour.h 1.24 vs edited =====
--- 1.24/include/net/neighbour.h	2004-10-22 14:51:12 +10:00
+++ edited/include/net/neighbour.h	2004-11-02 22:21:41 +11:00
@@ -189,7 +189,7 @@
 	struct timer_list 	gc_timer;
 	struct timer_list 	proxy_timer;
 	struct sk_buff_head	proxy_queue;
-	int			entries;
+	atomic_t		entries;
 	rwlock_t		lock;
 	unsigned long		last_rand;
 	struct neigh_parms	*parms_list;
===== net/core/neighbour.c 1.54 vs edited =====
--- 1.54/net/core/neighbour.c	2004-10-06 04:37:37 +10:00
+++ edited/net/core/neighbour.c	2004-11-02 22:23:18 +11:00
@@ -254,18 +254,20 @@
 {
 	struct neighbour *n = NULL;
 	unsigned long now = jiffies;
+	int entries;
 
-	if (tbl->entries > tbl->gc_thresh3 ||
-	    (tbl->entries > tbl->gc_thresh2 &&
+	entries = atomic_inc_return(&tbl->entries) - 1;
+	if (entries >= tbl->gc_thresh3 ||
+	    (entries >= tbl->gc_thresh2 &&
 	     time_after(now, tbl->last_flush + 5 * HZ))) {
-		if (!neigh_forced_gc(tbl) &&
-		    tbl->entries > tbl->gc_thresh3)
-			goto out;
+		neigh_forced_gc(tbl);
+		if (atomic_read(&tbl->entries) > tbl->gc_thresh3)
+			goto out_entries;
 	}
 
 	n = kmem_cache_alloc(tbl->kmem_cachep, SLAB_ATOMIC);
 	if (!n)
-		goto out;
+		goto out_entries;
 
 	memset(n, 0, tbl->entry_size);
 
@@ -281,12 +283,15 @@
 
 	NEIGH_CACHE_STAT_INC(tbl, allocs);
 	neigh_glbl_allocs++;
-	tbl->entries++;
 	n->tbl		  = tbl;
 	atomic_set(&n->refcnt, 1);
 	n->dead		  = 1;
 out:
 	return n;
+
+out_entries:
+	atomic_dec(&tbl->entries);
+	goto out;
 }
 
 static struct neighbour **neigh_hash_alloc(unsigned int entries)
@@ -427,7 +432,7 @@
 
 	write_lock_bh(&tbl->lock);
 
-	if (tbl->entries > (tbl->hash_mask + 1))
+	if (atomic_read(&tbl->entries) > (tbl->hash_mask + 1))
 		neigh_hash_grow(tbl, (tbl->hash_mask + 1) << 1);
 
 	hash_val = tbl->hash(pkey, dev) & tbl->hash_mask;
@@ -608,7 +613,7 @@
 	NEIGH_PRINTK2("neigh %p is destroyed.\n", neigh);
 
 	neigh_glbl_allocs--;
-	neigh->tbl->entries--;
+	atomic_dec(&neigh->tbl->entries);
 	kmem_cache_free(neigh->tbl->kmem_cachep, neigh);
 }
 
@@ -1394,7 +1399,7 @@
 	del_timer_sync(&tbl->proxy_timer);
 	pneigh_queue_purge(&tbl->proxy_queue);
 	neigh_ifdown(tbl, NULL);
-	if (tbl->entries)
+	if (atomic_read(&tbl->entries))
 		printk(KERN_CRIT "neighbour leakage\n");
 	write_lock(&neigh_tbl_lock);
 	for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
@@ -1951,7 +1956,7 @@
 
 	seq_printf(seq, "%08x  %08lx %08lx %08lx  %08lx %08lx  %08lx  "
 			"%08lx %08lx  %08lx %08lx\n",
-		   tbl->entries,
+		   atomic_read(&tbl->entries),
 
 		   st->allocs,
 		   st->destroys,

Re: [NET] Fix neighbour tbl->entries race

[jamal]

On Tue, 2004-11-02 at 06:26, Herbert Xu wrote:
> Hi Dave:
> 
> This patch turns neigh_table's entries counter into an atomic_t.  This
> is needed since tbl->entries is updated with no locks held.  This is
> no big deal in practice since it'll be off by a few entries at most
> which is way less than any of the neighbour thresholds.
> 

Is this still the same logic:

-------------
-               if (!neigh_forced_gc(tbl) &&
-                   tbl->entries > tbl->gc_thresh3)
-                       goto out;
+               neigh_forced_gc(tbl);
+               if (atomic_read(&tbl->entries) > tbl->gc_thresh3)
+                       goto out_entries;

-------

In previous code it seems you should short-circuit if
neigh_forced_gc(tbl) returns non-zero. why do you have to break that
if statement?

cheers,
jamal

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

On Tue, Nov 02, 2004 at 09:13:43AM -0500, jamal wrote:
> 
> Is this still the same logic:
> 
> -------------
> -               if (!neigh_forced_gc(tbl) &&
> -                   tbl->entries > tbl->gc_thresh3)
> -                       goto out;
> +               neigh_forced_gc(tbl);
> +               if (atomic_read(&tbl->entries) > tbl->gc_thresh3)
> +                       goto out_entries;
> 
> -------
> 
> In previous code it seems you should short-circuit if
> neigh_forced_gc(tbl) returns non-zero. why do you have to break that
> if statement?

The previous logic is slightly incorrect in that even if neigh_forced_gc
succeeded in removing some entries, the total number of entries may still
be above the threshold 3.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Wed, Nov 03, 2004 at 07:37:20AM +1100, herbert wrote:
> On Tue, Nov 02, 2004 at 09:13:43AM -0500, jamal wrote:
>
> > In previous code it seems you should short-circuit if
> > neigh_forced_gc(tbl) returns non-zero. why do you have to break that
> > if statement?
> 
> The previous logic is slightly incorrect in that even if neigh_forced_gc
> succeeded in removing some entries, the total number of entries may still
> be above the threshold 3.

Actually, after thinking about this a bit more, the short-circuit
is indeed correct.  The logic is that if we freed up at least one
spot in the hash table then we ought to be able to take it.

In fact, we can make it a bit more effective/fair by checking the
previous entries count as well.

Here is an updated patch.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2588 bytes --]

===== include/net/neighbour.h 1.24 vs edited =====
--- 1.24/include/net/neighbour.h	2004-10-22 14:51:12 +10:00
+++ edited/include/net/neighbour.h	2004-11-03 07:56:07 +11:00
@@ -189,7 +189,7 @@
 	struct timer_list 	gc_timer;
 	struct timer_list 	proxy_timer;
 	struct sk_buff_head	proxy_queue;
-	int			entries;
+	atomic_t		entries;
 	rwlock_t		lock;
 	unsigned long		last_rand;
 	struct neigh_parms	*parms_list;
===== net/core/neighbour.c 1.54 vs edited =====
--- 1.54/net/core/neighbour.c	2004-10-06 04:37:37 +10:00
+++ edited/net/core/neighbour.c	2004-11-03 07:57:32 +11:00
@@ -254,18 +254,21 @@
 {
 	struct neighbour *n = NULL;
 	unsigned long now = jiffies;
+	int entries;
 
-	if (tbl->entries > tbl->gc_thresh3 ||
-	    (tbl->entries > tbl->gc_thresh2 &&
+	entries = atomic_inc_return(&tbl->entries) - 1;
+	if (entries >= tbl->gc_thresh3 ||
+	    (entries >= tbl->gc_thresh2 &&
 	     time_after(now, tbl->last_flush + 5 * HZ))) {
 		if (!neigh_forced_gc(tbl) &&
-		    tbl->entries > tbl->gc_thresh3)
-			goto out;
+		    entries >= tbl->gc_thresh3 &&
+		    atomic_read(&tbl->entries) > tbl->gc_thresh3)
+			goto out_entries;
 	}
 
 	n = kmem_cache_alloc(tbl->kmem_cachep, SLAB_ATOMIC);
 	if (!n)
-		goto out;
+		goto out_entries;
 
 	memset(n, 0, tbl->entry_size);
 
@@ -281,12 +284,15 @@
 
 	NEIGH_CACHE_STAT_INC(tbl, allocs);
 	neigh_glbl_allocs++;
-	tbl->entries++;
 	n->tbl		  = tbl;
 	atomic_set(&n->refcnt, 1);
 	n->dead		  = 1;
 out:
 	return n;
+
+out_entries:
+	atomic_dec(&tbl->entries);
+	goto out;
 }
 
 static struct neighbour **neigh_hash_alloc(unsigned int entries)
@@ -427,7 +433,7 @@
 
 	write_lock_bh(&tbl->lock);
 
-	if (tbl->entries > (tbl->hash_mask + 1))
+	if (atomic_read(&tbl->entries) > (tbl->hash_mask + 1))
 		neigh_hash_grow(tbl, (tbl->hash_mask + 1) << 1);
 
 	hash_val = tbl->hash(pkey, dev) & tbl->hash_mask;
@@ -608,7 +614,7 @@
 	NEIGH_PRINTK2("neigh %p is destroyed.\n", neigh);
 
 	neigh_glbl_allocs--;
-	neigh->tbl->entries--;
+	atomic_dec(&neigh->tbl->entries);
 	kmem_cache_free(neigh->tbl->kmem_cachep, neigh);
 }
 
@@ -1394,7 +1400,7 @@
 	del_timer_sync(&tbl->proxy_timer);
 	pneigh_queue_purge(&tbl->proxy_queue);
 	neigh_ifdown(tbl, NULL);
-	if (tbl->entries)
+	if (atomic_read(&tbl->entries))
 		printk(KERN_CRIT "neighbour leakage\n");
 	write_lock(&neigh_tbl_lock);
 	for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
@@ -1951,7 +1957,7 @@
 
 	seq_printf(seq, "%08x  %08lx %08lx %08lx  %08lx %08lx  %08lx  "
 			"%08lx %08lx  %08lx %08lx\n",
-		   tbl->entries,
+		   atomic_read(&tbl->entries),
 
 		   st->allocs,
 		   st->destroys,

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

On Wed, Nov 03, 2004 at 08:02:59AM +1100, herbert wrote:
> 
> In fact, we can make it a bit more effective/fair by checking the
> previous entries count as well.

Call me a flip-flop :) This extra check is silly because in the
cases where it is done it is very unlikely to succeed.  So here
is yet another update without that check.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2533 bytes --]

===== include/net/neighbour.h 1.24 vs edited =====
--- 1.24/include/net/neighbour.h	2004-10-22 14:51:12 +10:00
+++ edited/include/net/neighbour.h	2004-11-03 07:56:07 +11:00
@@ -189,7 +189,7 @@
 	struct timer_list 	gc_timer;
 	struct timer_list 	proxy_timer;
 	struct sk_buff_head	proxy_queue;
-	int			entries;
+	atomic_t		entries;
 	rwlock_t		lock;
 	unsigned long		last_rand;
 	struct neigh_parms	*parms_list;
===== net/core/neighbour.c 1.54 vs edited =====
--- 1.54/net/core/neighbour.c	2004-10-06 04:37:37 +10:00
+++ edited/net/core/neighbour.c	2004-11-03 09:02:19 +11:00
@@ -254,18 +254,20 @@
 {
 	struct neighbour *n = NULL;
 	unsigned long now = jiffies;
+	int entries;
 
-	if (tbl->entries > tbl->gc_thresh3 ||
-	    (tbl->entries > tbl->gc_thresh2 &&
+	entries = atomic_inc_return(&tbl->entries) - 1;
+	if (entries >= tbl->gc_thresh3 ||
+	    (entries >= tbl->gc_thresh2 &&
 	     time_after(now, tbl->last_flush + 5 * HZ))) {
 		if (!neigh_forced_gc(tbl) &&
-		    tbl->entries > tbl->gc_thresh3)
-			goto out;
+		    entries >= tbl->gc_thresh3)
+			goto out_entries;
 	}
 
 	n = kmem_cache_alloc(tbl->kmem_cachep, SLAB_ATOMIC);
 	if (!n)
-		goto out;
+		goto out_entries;
 
 	memset(n, 0, tbl->entry_size);
 
@@ -281,12 +283,15 @@
 
 	NEIGH_CACHE_STAT_INC(tbl, allocs);
 	neigh_glbl_allocs++;
-	tbl->entries++;
 	n->tbl		  = tbl;
 	atomic_set(&n->refcnt, 1);
 	n->dead		  = 1;
 out:
 	return n;
+
+out_entries:
+	atomic_dec(&tbl->entries);
+	goto out;
 }
 
 static struct neighbour **neigh_hash_alloc(unsigned int entries)
@@ -427,7 +432,7 @@
 
 	write_lock_bh(&tbl->lock);
 
-	if (tbl->entries > (tbl->hash_mask + 1))
+	if (atomic_read(&tbl->entries) > (tbl->hash_mask + 1))
 		neigh_hash_grow(tbl, (tbl->hash_mask + 1) << 1);
 
 	hash_val = tbl->hash(pkey, dev) & tbl->hash_mask;
@@ -608,7 +613,7 @@
 	NEIGH_PRINTK2("neigh %p is destroyed.\n", neigh);
 
 	neigh_glbl_allocs--;
-	neigh->tbl->entries--;
+	atomic_dec(&neigh->tbl->entries);
 	kmem_cache_free(neigh->tbl->kmem_cachep, neigh);
 }
 
@@ -1394,7 +1399,7 @@
 	del_timer_sync(&tbl->proxy_timer);
 	pneigh_queue_purge(&tbl->proxy_queue);
 	neigh_ifdown(tbl, NULL);
-	if (tbl->entries)
+	if (atomic_read(&tbl->entries))
 		printk(KERN_CRIT "neighbour leakage\n");
 	write_lock(&neigh_tbl_lock);
 	for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
@@ -1951,7 +1956,7 @@
 
 	seq_printf(seq, "%08x  %08lx %08lx %08lx  %08lx %08lx  %08lx  "
 			"%08lx %08lx  %08lx %08lx\n",
-		   tbl->entries,
+		   atomic_read(&tbl->entries),
 
 		   st->allocs,
 		   st->destroys,

Re: [NET] Fix neighbour tbl->entries race

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20041102210259.GA12642@gondor.apana.org.au> (at Wed, 3 Nov 2004 08:02:59 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:

> -	if (tbl->entries > tbl->gc_thresh3 ||
> -	    (tbl->entries > tbl->gc_thresh2 &&
> +	entries = atomic_inc_return(&tbl->entries) - 1;
> +	if (entries >= tbl->gc_thresh3 ||
> +	    (entries >= tbl->gc_thresh2 &&

Why don't you do something like this?

        entries = atomic_inc_return(&tbl->entries);
        if (entries > tbl->gc_thresh3 ||
            (entries > tbl->gc_thresh2 &&

--yoshfuji

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

On Wed, Nov 03, 2004 at 10:14:03AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> In article <20041102210259.GA12642@gondor.apana.org.au> (at Wed, 3 Nov 2004 08:02:59 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:
> 
> > -	if (tbl->entries > tbl->gc_thresh3 ||
> > -	    (tbl->entries > tbl->gc_thresh2 &&
> > +	entries = atomic_inc_return(&tbl->entries) - 1;
> > +	if (entries >= tbl->gc_thresh3 ||
> > +	    (entries >= tbl->gc_thresh2 &&
> 
> Why don't you do something like this?
> 
>         entries = atomic_inc_return(&tbl->entries);
>         if (entries > tbl->gc_thresh3 ||
>             (entries > tbl->gc_thresh2 &&

We could do that.  The first form generates slightly better code on
i386.  However, the situation is probably reversed for other
architectures.

A more serious issue is that some architectures arm/arm26/um/x86_64
don't have atomic_inc_return.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

On Wed, Nov 03, 2004 at 12:25:20PM +1100, herbert wrote:
>
> A more serious issue is that some architectures arm/arm26/um/x86_64
> don't have atomic_inc_return.

Actually arm/arm26/x86_64 have atomic_inc_return now so it's just um.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

On Wed, Nov 03, 2004 at 12:48:14PM +1100, herbert wrote:
> On Wed, Nov 03, 2004 at 12:25:20PM +1100, herbert wrote:
> >
> > A more serious issue is that some architectures arm/arm26/um/x86_64
> > don't have atomic_inc_return.
> 
> Actually arm/arm26/x86_64 have atomic_inc_return now so it's just um.

And of course um doesn't need it since it just includes
asm/arch/atomic.h :)
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [NET] Fix neighbour tbl->entries race

[David S. Miller]

On Wed, 3 Nov 2004 09:06:03 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, Nov 03, 2004 at 08:02:59AM +1100, herbert wrote:
> > 
> > In fact, we can make it a bit more effective/fair by checking the
> > previous entries count as well.
> 
> Call me a flip-flop :) This extra check is silly because in the
> cases where it is done it is very unlikely to succeed.  So here
> is yet another update without that check.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied.  We can make the proposed atomic_inc_return() enhancement
as a followon patch.

Can you cook up a 2.4.x version of this Herbert?  The new neighbour
code is now there too.

Re: [NET] Fix neighbour tbl->entries race

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Wed, Nov 03, 2004 at 03:54:38PM -0800, David S. Miller wrote:
> 
> Can you cook up a 2.4.x version of this Herbert?  The new neighbour
> code is now there too.

Sure.  BTW this bug is a lot older than the new neighbours code.
It's in the very first revision of neighbour.c :)

Here is a patch for 2.4.  I simply used atomic_read instead of
doing atomic_inc_return as in 2.6 since I thought that the
downside of going a bit over gc_thresh3 wasn't worth the
trouble of back-porting all the atomic_*_return work.

Of course if that ever gets backported then we can do what 2.6
does here too.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2338 bytes --]

===== include/net/neighbour.h 1.4 vs edited =====
--- 1.4/include/net/neighbour.h	2004-10-06 04:22:37 +10:00
+++ edited/include/net/neighbour.h	2004-11-07 19:03:05 +11:00
@@ -174,7 +174,7 @@
 	struct timer_list 	gc_timer;
 	struct timer_list 	proxy_timer;
 	struct sk_buff_head	proxy_queue;
-	int			entries;
+	atomic_t		entries;
 	rwlock_t		lock;
 	unsigned long		last_rand;
 	struct neigh_parms	*parms_list;
===== net/core/neighbour.c 1.14 vs edited =====
--- 1.14/net/core/neighbour.c	2004-10-06 04:40:25 +10:00
+++ edited/net/core/neighbour.c	2004-11-07 19:04:24 +11:00
@@ -257,11 +257,11 @@
 	struct neighbour *n;
 	unsigned long now = jiffies;
 
-	if (tbl->entries > tbl->gc_thresh3 ||
-	    (tbl->entries > tbl->gc_thresh2 &&
+	if (atomic_read(&tbl->entries) > tbl->gc_thresh3 ||
+	    (atomic_read(&tbl->entries) > tbl->gc_thresh2 &&
 	     now - tbl->last_flush > 5*HZ)) {
 		if (neigh_forced_gc(tbl) == 0 &&
-		    tbl->entries > tbl->gc_thresh3)
+		    atomic_read(&tbl->entries) > tbl->gc_thresh3)
 			return NULL;
 	}
 
@@ -282,7 +282,7 @@
 	n->timer.data = (unsigned long)n;
 	NEIGH_CACHE_STAT_INC(tbl, allocs);
 	neigh_glbl_allocs++;
-	tbl->entries++;
+	atomic_inc(&tbl->entries);
 	n->tbl = tbl;
 	atomic_set(&n->refcnt, 1);
 	n->dead = 1;
@@ -428,7 +428,7 @@
 	hash_val = tbl->hash(pkey, dev) & tbl->hash_mask;
 
 	write_lock_bh(&tbl->lock);
-	if (tbl->entries > (tbl->hash_mask + 1))
+	if (atomic_read(&tbl->entries) > (tbl->hash_mask + 1))
 		neigh_hash_grow(tbl, (tbl->hash_mask + 1) << 1);
 	for (n1 = tbl->hash_buckets[hash_val]; n1; n1 = n1->next) {
 		if (dev == n1->dev &&
@@ -583,7 +583,7 @@
 	NEIGH_PRINTK2("neigh %p is destroyed.\n", neigh);
 
 	neigh_glbl_allocs--;
-	neigh->tbl->entries--;
+	atomic_dec(&neigh->tbl->entries);
 	kmem_cache_free(neigh->tbl->kmem_cachep, neigh);
 }
 
@@ -1306,7 +1306,7 @@
 	del_timer_sync(&tbl->proxy_timer);
 	pneigh_queue_purge(&tbl->proxy_queue);
 	neigh_ifdown(tbl, NULL);
-	if (tbl->entries)
+	if (atomic_read(&tbl->entries))
 		printk(KERN_CRIT "neighbour leakage\n");
 	write_lock(&neigh_tbl_lock);
 	for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
@@ -1858,7 +1858,7 @@
 
 	seq_printf(seq, "%08x  %08lx %08lx %08lx  %08lx %08lx  %08lx  "
 			"%08lx %08lx  %08lx %08lx\n",
-		   tbl->entries,
+		   atomic_read(&tbl->entries),
 
 		   st->allocs,
 		   st->destroys,

Re: [NET] Fix neighbour tbl->entries race

[David S. Miller]

On Sun, 7 Nov 2004 19:08:39 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Sure.  BTW this bug is a lot older than the new neighbours code.
> It's in the very first revision of neighbour.c :)

Indeed. :-)

Patch applied, thanks a lot Herbert.