Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Roberto Nibali]

> In 2.4.21, arp code uses gc_timer to check for stale arp cache
> entries. In 2.6, each entry has its own timer to check for stale arp
> cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.

Regarding NUD_REACHABLE <-> NUD_STALE transition it has a timer check.
Due to the fast path it's not enabled per default. Use neigh_sync() to
check, although I believe the installed tasklet does this for you already.

The knowledgeable netdev people will know better.

> This causes problems in environments where IPs or MACs are reassigned
> - saw this problem on load balancing router based networks that use
> VMACs. Tested this code on load balancing router based networks as
> well as peer-linux systems.

How do you use VMACs in 2.4.x?

> diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
> --- old/net/core/neighbour.c    Wed Nov 23 17:15:30 2005
> +++ new/net/core/neighbour.c    Wed Nov 23 17:26:01 2005
> @@ -14,6 +14,7 @@
> *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
> *     Harald Welte            Add neighbour cache statistics like rtstat
> *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
> + *      Pradeep Vincent         Move neighbour cache entry to stale state
> */
> 
> #include <linux/config.h>
> @@ -705,6 +706,14 @@
>                        neigh_release(n);
>                        continue;
>                }
> +
> +               /* Mark it stale - To be reconfirmed later when used */
> +               if (n->nud_state&NUD_REACHABLE &&
> +                   now - n->confirmed > n->parms->reachable_time) {
> +                       n->nud_state = NUD_STALE;
> +                       neigh_suspect(n);
> +               }
> +

If this is really a problem, why not simply call neigh_sync()? Your
patch also seems to be whitespace damaged.

>                write_unlock(&n->lock);
> 
> next_elt:

I've cc'd netdev since this is where such patches should go for
discussion; left Linus in the loop (netiquette) although he's nothing to
do with this ;).

Cheers,
Roberto Nibali, ratz
-- 
echo
'[q]sa[ln0=aln256%Pln256/snlbx]sb3135071790101768542287578439snlbxq' | dc

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Pradeep Vincent]

>From the source,
/*
   Transitions NUD_STALE <-> NUD_REACHABLE do not occur
   when fast path is built: we have no timers associated with
   these states, we do not have time to check state when sending.
   neigh_periodic_timer check periodically neigh->confirmed
   time and moves NUD_REACHABLE -> NUD_STALE.

   If a routine wants to know TRUE entry state, it calls
   neigh_sync before checking state.

   Called with write_locked neigh.
 */

I got the impression that neigh_periodic_timer is supposed to check
for stale entries although it doesn't. Which tasklet are you referring
to ? Or are you saying I should be calling neigh_sync instead of
checking for stale entries myself.

VMACS are used outside the linux system - for e.g. certain routers
present different VMACS for a default gateway ip for active-active
load-balancing and when one dies off, the failed VMAC is removed
eventually causing linux system using the failed VMAC to balk as it
doesn't stale out the arp cache entries even when traffic isn't able
to flow (confirmed doesn't move ahead).

netdev folks, can you CC me on your reply.

Thanks,

Pradeep Vincent


On 11/28/05, Roberto Nibali <ratz@drugphish.ch> wrote:
> > In 2.4.21, arp code uses gc_timer to check for stale arp cache
> > entries. In 2.6, each entry has its own timer to check for stale arp
> > cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.
>
> Regarding NUD_REACHABLE <-> NUD_STALE transition it has a timer check.
> Due to the fast path it's not enabled per default. Use neigh_sync() to
> check, although I believe the installed tasklet does this for you already.
>
> The knowledgeable netdev people will know better.
>
> > This causes problems in environments where IPs or MACs are reassigned
> > - saw this problem on load balancing router based networks that use
> > VMACs. Tested this code on load balancing router based networks as
> > well as peer-linux systems.
>
> How do you use VMACs in 2.4.x?
>
> > diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
> > --- old/net/core/neighbour.c    Wed Nov 23 17:15:30 2005
> > +++ new/net/core/neighbour.c    Wed Nov 23 17:26:01 2005
> > @@ -14,6 +14,7 @@
> > *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
> > *     Harald Welte            Add neighbour cache statistics like rtstat
> > *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
> > + *      Pradeep Vincent         Move neighbour cache entry to stale state
> > */
> >
> > #include <linux/config.h>
> > @@ -705,6 +706,14 @@
> >                        neigh_release(n);
> >                        continue;
> >                }
> > +
> > +               /* Mark it stale - To be reconfirmed later when used */
> > +               if (n->nud_state&NUD_REACHABLE &&
> > +                   now - n->confirmed > n->parms->reachable_time) {
> > +                       n->nud_state = NUD_STALE;
> > +                       neigh_suspect(n);
> > +               }
> > +
>
> If this is really a problem, why not simply call neigh_sync()? Your
> patch also seems to be whitespace damaged.
>
> >                write_unlock(&n->lock);
> >
> > next_elt:
>
> I've cc'd netdev since this is where such patches should go for
> discussion; left Linus in the loop (netiquette) although he's nothing to
> do with this ;).
>
> Cheers,
> Roberto Nibali, ratz
> --
> echo
> '[q]sa[ln0=aln256%Pln256/snlbx]sb3135071790101768542287578439snlbxq' | dc
>

Fwd: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Pradeep Vincent]

Resending..

---------- Forwarded message ----------
From: Pradeep Vincent <pradeep.vincent@gmail.com>
Date: Nov 28, 2005 12:57 PM
Subject: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed
To: linux-kernel@vger.kernel.org, torvalds@osdl.org
Cc: pradeep.vincent@gmail.com


In 2.4.21, arp code uses gc_timer to check for stale arp cache
entries. In 2.6, each entry has its own timer to check for stale arp
cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.
This causes problems in environments where IPs or MACs are reassigned
- saw this problem on load balancing router based networks that use
VMACs. Tested this code on load balancing router based networks as
well as peer-linux systems.

Let me know if I need to contact someone else about this,

Thanks,

Pradeep Vincent


diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
--- old/net/core/neighbour.c    Wed Nov 23 17:15:30 2005
+++ new/net/core/neighbour.c    Wed Nov 23 17:26:01 2005
@@ -14,6 +14,7 @@
*     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
*     Harald Welte            Add neighbour cache statistics like rtstat
*     Harald Welte            port neighbour cache rework from 2.6.9-rcX
+ *      Pradeep Vincent         Move neighbour cache entry to stale state
*/

#include <linux/config.h>
@@ -705,6 +706,14 @@
                      neigh_release(n);
                      continue;
              }
+
+               /* Mark it stale - To be reconfirmed later when used */
+               if (n->nud_state&NUD_REACHABLE &&
+                   now - n->confirmed > n->parms->reachable_time) {
+                       n->nud_state = NUD_STALE;
+                       neigh_suspect(n);
+               }
+
              write_unlock(&n->lock);

next_elt:

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[David S. Miller]

From: Pradeep Vincent <pradeep.vincent@gmail.com>
Date: Fri, 3 Feb 2006 18:06:53 -0800

> Resending..

Your email client has tab and newline mangled the patch so it
cannot be applied.  Please fix this up and also supply an
appropriate "Signed-off-by: " line.

Thanks.

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Pradeep Vincent]

In 2.4.21, arp code uses gc_timer to check for stale arp cache
entries. In 2.6, each entry has its own timer to check for stale arp
cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.
This causes problems in environments where IPs or MACs are reassigned
- saw this problem on load balancing router based networks that use
VMACs. Tested this code on load balancing router based networks as
well as peer-linux systems.


Thanks,


Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>

diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
--- old/net/core/neighbour.c    Wed Nov 23 17:15:30 2005
+++ new/net/core/neighbour.c    Wed Nov 23 17:26:01 2005
@@ -14,6 +14,7 @@
*     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
*     Harald Welte            Add neighbour cache statistics like rtstat
*     Harald Welte            port neighbour cache rework from 2.6.9-rcX
+ *      Pradeep Vincent         Move neighbour cache entry to stale state
*/

#include <linux/config.h>
@@ -705,6 +706,14 @@
                      neigh_release(n);
                      continue;
              }
+
+               /* Mark it stale - To be reconfirmed later when used */
+               if (n->nud_state&NUD_REACHABLE &&
+                   now - n->confirmed > n->parms->reachable_time) {
+                       n->nud_state = NUD_STALE;
+                       neigh_suspect(n);
+               }
+
              write_unlock(&n->lock);


On 2/3/06, David S. Miller <davem@davemloft.net> wrote:
> From: Pradeep Vincent <pradeep.vincent@gmail.com>
> Date: Fri, 3 Feb 2006 18:06:53 -0800
>
> > Resending..
>
> Your email client has tab and newline mangled the patch so it
> cannot be applied.  Please fix this up and also supply an
> appropriate "Signed-off-by: " line.
>
> Thanks.
>

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Willy Tarreau]

Hi,

On Tue, Feb 07, 2006 at 12:57:43AM -0700, Pradeep Vincent wrote:
> In 2.4.21, arp code uses gc_timer to check for stale arp cache
> entries. In 2.6, each entry has its own timer to check for stale arp
> cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.
> This causes problems in environments where IPs or MACs are reassigned
> - saw this problem on load balancing router based networks that use
> VMACs. Tested this code on load balancing router based networks as
> well as peer-linux systems.
> 
> 
> Thanks,
> 
> 
> Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
> 
> diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
> --- old/net/core/neighbour.c    Wed Nov 23 17:15:30 2005
> +++ new/net/core/neighbour.c    Wed Nov 23 17:26:01 2005
> @@ -14,6 +14,7 @@
> *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
> *     Harald Welte            Add neighbour cache statistics like rtstat
> *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
> + *      Pradeep Vincent         Move neighbour cache entry to stale state
> */

As you can see above, your mailer is still broken. Leading spaces get
removed and it seems like tabs are replaced with spaces. This makes it
really annoying to fix by hand because we all have to do your work again.
You should try to fix your mailer options, possibly by sending a few
mails to yourself or someone else (if you send *a few* mails to me, I
can confirm which one looks OK). If your mailer is definitely broken,
then you may send it as plain text first (for review), with a text
attachment for people to apply it without trouble.

Thanks,
Willy

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Pradeep Vincent]

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

One more attempt. Attaching the diff file as well.

Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>

--- old/net/core/neighbour.c    Wed Nov  9 16:48:10 2005
+++ new/net/core/neighbour.c    Tue Feb  7 17:38:26 2006
@@ -14,6 +14,7 @@
  *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
  *     Harald Welte            Add neighbour cache statistics like rtstat
  *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
+ *     Pradeep Vincent         fix neighbour cache state machine
  */

 #include <linux/config.h>
@@ -705,6 +706,13 @@
                        neigh_release(n);
                        continue;
                }
+               /* Move to NUD_STALE state */
+               if (n->nud_state&NUD_REACHABLE &&
+                   now - n->confirmed > n->parms->reachable_time) {
+                       n->nud_state = NUD_STALE;
+                       neigh_suspect(n);
+               }
+
                write_unlock(&n->lock);

 next_elt:

Thanks,

Pradeep
On 2/7/06, Willy Tarreau <willy@w.ods.org> wrote:
> Hi,
>
> On Tue, Feb 07, 2006 at 12:57:43AM -0700, Pradeep Vincent wrote:
> > In 2.4.21, arp code uses gc_timer to check for stale arp cache
> > entries. In 2.6, each entry has its own timer to check for stale arp
> > cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.
> > This causes problems in environments where IPs or MACs are reassigned
> > - saw this problem on load balancing router based networks that use
> > VMACs. Tested this code on load balancing router based networks as
> > well as peer-linux systems.
> >
> >
> > Thanks,
> >
> >
> > Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
> >
> > diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
> > --- old/net/core/neighbour.c    Wed Nov 23 17:15:30 2005
> > +++ new/net/core/neighbour.c    Wed Nov 23 17:26:01 2005
> > @@ -14,6 +14,7 @@
> > *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
> > *     Harald Welte            Add neighbour cache statistics like rtstat
> > *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
> > + *      Pradeep Vincent         Move neighbour cache entry to stale state
> > */
>
> As you can see above, your mailer is still broken. Leading spaces get
> removed and it seems like tabs are replaced with spaces. This makes it
> really annoying to fix by hand because we all have to do your work again.
> You should try to fix your mailer options, possibly by sending a few
> mails to yourself or someone else (if you send *a few* mails to me, I
> can confirm which one looks OK). If your mailer is definitely broken,
> then you may send it as plain text first (for review), with a text
> attachment for people to apply it without trouble.
>
> Thanks,
> Willy
>
>

[-- Attachment #2: linux-2.4.29-arp-fix.patch --]
[-- Type: application/octet-stream, Size: 691 bytes --]

--- old/net/core/neighbour.c	Wed Nov  9 16:48:10 2005
+++ new/net/core/neighbour.c	Tue Feb  7 17:38:26 2006
@@ -14,6 +14,7 @@
  *	Vitaly E. Lavrov	releasing NULL neighbor in neigh_add.
  *	Harald Welte		Add neighbour cache statistics like rtstat
  *	Harald Welte		port neighbour cache rework from 2.6.9-rcX
+ *	Pradeep Vincent         fix neighbour cache state machine 
  */
 
 #include <linux/config.h>
@@ -705,6 +706,13 @@
 			neigh_release(n);
 			continue;
 		}
+		/* Move to NUD_STALE state */
+		if (n->nud_state&NUD_REACHABLE &&
+		    now - n->confirmed > n->parms->reachable_time) {
+			n->nud_state = NUD_STALE;
+			neigh_suspect(n);
+		}
+
 		write_unlock(&n->lock);
 
 next_elt:

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Grant Coady]

On Tue, 7 Feb 2006 17:50:03 -0800, Pradeep Vincent <pradeep.vincent@gmail.com> wrote:

>One more attempt. Attaching the diff file as well.
>
>Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
>
>--- old/net/core/neighbour.c    Wed Nov  9 16:48:10 2005
>+++ new/net/core/neighbour.c    Tue Feb  7 17:38:26 2006
>@@ -14,6 +14,7 @@
>  *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
>  *     Harald Welte            Add neighbour cache statistics like rtstat
>  *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
>+ *     Pradeep Vincent         fix neighbour cache state machine
>  */
>
> #include <linux/config.h>
>@@ -705,6 +706,13 @@
>                        neigh_release(n);
>                        continue;
>                }
>+               /* Move to NUD_STALE state */
>+               if (n->nud_state&NUD_REACHABLE &&
>+                   now - n->confirmed > n->parms->reachable_time) {

Hmm, you're suffering tab -> space conversion syndrome :(

Grant.

Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed

[Bill Davidsen]

Grant Coady wrote:
> On Tue, 7 Feb 2006 17:50:03 -0800, Pradeep Vincent <pradeep.vincent@gmail.com> wrote:
> 
> 
>>One more attempt. Attaching the diff file as well.
>>
>>Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
>>
>>--- old/net/core/neighbour.c    Wed Nov  9 16:48:10 2005
>>+++ new/net/core/neighbour.c    Tue Feb  7 17:38:26 2006
>>@@ -14,6 +14,7 @@
>> *     Vitaly E. Lavrov        releasing NULL neighbor in neigh_add.
>> *     Harald Welte            Add neighbour cache statistics like rtstat
>> *     Harald Welte            port neighbour cache rework from 2.6.9-rcX
>>+ *     Pradeep Vincent         fix neighbour cache state machine
>> */
>>
>>#include <linux/config.h>
>>@@ -705,6 +706,13 @@
>>                       neigh_release(n);
>>                       continue;
>>               }
>>+               /* Move to NUD_STALE state */
>>+               if (n->nud_state&NUD_REACHABLE &&
>>+                   now - n->confirmed > n->parms->reachable_time) {
> 
> 
> Hmm, you're suffering tab -> space conversion syndrome :(
> 
> Grant.

The attachment has tabs here, don't know what you're seeing.

-- 
bill davidsen <davidsen@tmr.com>
   CTO TMR Associates, Inc
   Doing interesting things with small computers since 1979