Julian Anastasov wrote:
Hello,
On Wed, 7 Sep 2005, Ben Greear wrote:
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]>
This code is wrong: if (!n->nud_state & NUD_IN_TIMER)
Must be if (!(n->nud_state & NUD_IN_TIMER))
Thanks for catching that. A new patch is attached. I didn't see any
printouts for this message even with the fix, so this case must not be
hit (at least not often).
Please let me know if you see any more areas for improvement.
Thanks,
Ben
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;
}
