Ben Greear wrote:
I added a separate flag to keep track of whether a neighbour struct is on the timer list or not. I also added some logic to dump the stack if the add-timer method was called while we are already on the timer list.
A cleaned up patch against 2.6.13 is attached. This (also) seems to fix the problem with removing net-devices. Hopefully with fewer side affects than the last kludge. In case it passes review: Signed-off-by Ben Greear <[EMAIL PROTECTED]> -- Ben Greear <[EMAIL PROTECTED]> Candela Technologies Inc http://www.candelatech.com
diff --git a/include/net/neighbour.h b/include/net/neighbour.h --- a/include/net/neighbour.h +++ b/include/net/neighbour.h @@ -1,3 +1,4 @@ +/* -*- linux-c -*- */ #ifndef _NET_NEIGHBOUR_H #define _NET_NEIGHBOUR_H @@ -138,6 +139,7 @@ struct neighbour int (*output)(struct sk_buff *skb); struct sk_buff_head arp_queue; struct timer_list timer; + u32 in_timer; /* boolean, space for more flags as needed. */ struct neigh_ops *ops; u8 primary_key[0]; }; diff --git a/net/core/neighbour.c b/net/core/neighbour.c --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -1,4 +1,4 @@ -/* +/* -*- linux-c -*- * Generic address resolution entity * * Authors: @@ -153,11 +153,34 @@ static int neigh_forced_gc(struct neigh_ return shrunk; } -static int neigh_del_timer(struct neighbour *n) -{ - if ((n->nud_state & NUD_IN_TIMER) && - del_timer(&n->timer)) { - neigh_release(n); +static int neigh_del_timer(struct neighbour *n) { + if (n->in_timer) { + if (!n->nud_state & NUD_IN_TIMER) { + printk("ERROR: in_timer true, but NUD_IN_TIMER false, neigh: %p\n", n); + /* dump_stack(); */ + } + if (del_timer(&n->timer)) { + n->in_timer = 0; + neigh_release(n); + return 1; + } + } + return 0; +} + +static int neigh_add_timer(struct neighbour *n, u32 expires) { + if (n->in_timer) { + /* This hits fairly often, not too sure why/how. */ + /* TODO: Maybe should remove existing timer if it expires + * later than this one and then insert the new timer? --Ben*/ + NEIGH_PRINTK2("ERROR: trying to add timer, but already there, neigh: %p\n", n); + /* dump_stack(); */ + } + else { + n->timer.expires = expires; + neigh_hold(n); + add_timer(&n->timer); + n->in_timer = 1; return 1; } return 0; @@ -737,6 +760,9 @@ static void neigh_timer_handler(unsigned write_lock(&neigh->lock); + /* We are out of the timer list at this point. */ + neigh->in_timer = 0; + state = neigh->nud_state; now = jiffies; next = now + HZ; @@ -806,11 +832,9 @@ static void neigh_timer_handler(unsigned } if (neigh->nud_state & NUD_IN_TIMER) { - neigh_hold(neigh); if (time_before(next, jiffies + HZ/2)) next = jiffies + HZ/2; - neigh->timer.expires = next; - add_timer(&neigh->timer); + neigh_add_timer(neigh, next); } if (neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) { struct sk_buff *skb = skb_peek(&neigh->arp_queue); @@ -851,9 +875,7 @@ int __neigh_event_send(struct neighbour if (neigh->parms->mcast_probes + neigh->parms->app_probes) { atomic_set(&neigh->probes, neigh->parms->ucast_probes); neigh->nud_state = NUD_INCOMPLETE; - neigh_hold(neigh); - neigh->timer.expires = now + 1; - add_timer(&neigh->timer); + neigh_add_timer(neigh, now + 1); } else { neigh->nud_state = NUD_FAILED; write_unlock_bh(&neigh->lock); @@ -864,10 +886,8 @@ int __neigh_event_send(struct neighbour } } else if (neigh->nud_state & NUD_STALE) { NEIGH_PRINTK2("neigh %p is delayed.\n", neigh); - neigh_hold(neigh); neigh->nud_state = NUD_DELAY; - neigh->timer.expires = jiffies + neigh->parms->delay_probe_time; - add_timer(&neigh->timer); + neigh_add_timer(neigh, jiffies + neigh->parms->delay_probe_time); } if (neigh->nud_state == NUD_INCOMPLETE) { @@ -1012,11 +1032,9 @@ int neigh_update(struct neighbour *neigh if (new != old) { neigh_del_timer(neigh); if (new & NUD_IN_TIMER) { - neigh_hold(neigh); - neigh->timer.expires = jiffies + + neigh_add_timer(neigh, jiffies + ((new & NUD_REACHABLE) ? - neigh->parms->reachable_time : 0); - add_timer(&neigh->timer); + neigh->parms->reachable_time : 0)); } neigh->nud_state = new; }