David Miller schrieb:
From: Blaschka [EMAIL PROTECTED]
Date: Mon, 4 Feb 2008 15:27:17 +0100
I'm running a SMP maschine (2 CPUs) configured as a router. During heavy
traffic kernel dies with following message:
2kernel BUG at
/home/autobuild/BUILD/linux-2.6.23-20080125/net/core/skbuff.c:648!
...
Following patch fixes the problem but I do not know if it is a good
sollution.
From: Frank Blaschka [EMAIL PROTECTED]
neigh_update sends skb from neigh-arp_queue while
neigh_timer_handler has increased skbs refcount and calls
solicit with the skb. Do not send neighbour skbs
marked for solicit (skb_shared).
Signed-off-by: Frank Blaschka [EMAIL PROTECTED]
Thanks for finding this bug.
I'm fine with your approach as a temporary fix, but there is a slight
problem with your patch. If the skb is shared we have to free it if
we don't pass it on to -output(), otherwise this creates a leak.
In the longer term, this is an unfortunate limitation. The
-solicit() code just wants to look at a few header fields to
determine how to construct the solicitation request.
What's funny is that we added these skb_get() calls for
the solications exactly to deal with this race condition.
I considered various ways to fix this. The simplest is probably just
to skb_copy() in the -solicit() case. Solicitation is a rare event
so it's not big deal to copy the packet until the neighbour is
resolved.
The other option is holding the write lock on neigh-lock during the
-solicit() call. I looked at all of the ndisc_ops implementations
and this seems workable. The only case that needs special care is the
IPV4 ARP implementation of arp_solicit(). It wants to take
neigh-lock as a reader to protect the header entry in neigh-ha
during the emission of the soliciation. We can simply remove the read
lock calls to take care of that since holding the lock as a writer at
the caller providers a superset of the protection afforded by the
existing read locking.
The rest of the -solicit() implementations don't care whether
the neigh is locked or not.
Can you see if this version of the patch fixes your problem?
Thanks!
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index a16cf1e..7bb6a9a 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -834,18 +834,12 @@ static void neigh_timer_handler(unsigned long arg)
}
if (neigh-nud_state (NUD_INCOMPLETE | NUD_PROBE)) {
struct sk_buff *skb = skb_peek(neigh-arp_queue);
- /* keep skb alive even if arp_queue overflows */
- if (skb)
- skb_get(skb);
- write_unlock(neigh-lock);
+
neigh-ops-solicit(neigh, skb);
atomic_inc(neigh-probes);
- if (skb)
- kfree_skb(skb);
- } else {
-out:
- write_unlock(neigh-lock);
}
+out:
+ write_unlock(neigh-lock);
if (notify)
neigh_update_notify(neigh);
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 8e17f65..c663fa5 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -368,7 +368,6 @@ static void arp_solicit(struct neighbour *neigh, struct
sk_buff *skb)
if (!(neigh-nud_stateNUD_VALID))
printk(KERN_DEBUG trying to ucast probe in
NUD_INVALID\n);
dst_ha = neigh-ha;
- read_lock_bh(neigh-lock);
} else if ((probes -= neigh-parms-app_probes) 0) {
#ifdef CONFIG_ARPD
neigh_app_ns(neigh);
@@ -378,8 +377,6 @@ static void arp_solicit(struct neighbour *neigh, struct
sk_buff *skb)
arp_send(ARPOP_REQUEST, ETH_P_ARP, target, dev, saddr,
dst_ha, dev-dev_addr, NULL);
- if (dst_ha)
- read_unlock_bh(neigh-lock);
}
static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dave,
we run your patch during the weekend on single CPU and SMP machines. We do not
see any problems. Thanks for providing the fix.
Best regards,
Frank
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
