From: Herbert Xu <[EMAIL PROTECTED]>
Date: Sat, 08 Apr 2006 13:46:25 +1000
> David S. Miller <[EMAIL PROTECTED]> wrote:
> > From: Zach Brown <[EMAIL PROTECTED]>
> > Date: Fri, 07 Apr 2006 16:59:01 -0700
> >
> >> b) Just calculate the hashes under the lock, we're already doing lots of
> >> work there anyway.
> >
> > I think this is the best way to go. Then we don't need to think
> > about it, and frankly I think the "recheck hash rnd after getting
> > lock" idea would turn out to be more expensive :)
>
> Agreed. We should also fix up ip_frag_create/ip_frag_intern by getting
> rid of the hash argument and recomputing it in ip_frag_intern.
I got tired of waiting for Zach to cook up a patch so I tossed
the following into my tree :-)
diff-tree 55c0022e53452360064ea264c41410c70565d9f8 (from
19910d1aec0b2b96ff4b4a93e2aed0d32643e850)
Author: David S. Miller <[EMAIL PROTECTED]>
Date: Sun Apr 9 22:43:55 2006 -0700
[IPV4] ip_fragment: Always compute hash with ipfrag_lock held.
Otherwise we could compute an inaccurate hash due to the
random seed changing.
Noticed by Zach Brown and patch is based upon some feedback
from Herbert Xu.
Signed-off-by: David S. Miller <[EMAIL PROTECTED]>
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 2a8adda..da734c4 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -304,13 +304,17 @@ out:
/* Creation primitives. */
-static struct ipq *ip_frag_intern(unsigned int hash, struct ipq *qp_in)
+static struct ipq *ip_frag_intern(struct ipq *qp_in)
{
struct ipq *qp;
#ifdef CONFIG_SMP
struct hlist_node *n;
#endif
+ unsigned int hash;
+
write_lock(&ipfrag_lock);
+ hash = ipqhashfn(qp_in->id, qp_in->saddr, qp_in->daddr,
+ qp_in->protocol);
#ifdef CONFIG_SMP
/* With SMP race we have to recheck hash table, because
* such entry could be created on other cpu, while we
@@ -345,7 +349,7 @@ #endif
}
/* Add an entry to the 'ipq' queue for a newly received IP datagram. */
-static struct ipq *ip_frag_create(unsigned hash, struct iphdr *iph, u32 user)
+static struct ipq *ip_frag_create(struct iphdr *iph, u32 user)
{
struct ipq *qp;
@@ -371,7 +375,7 @@ static struct ipq *ip_frag_create(unsign
spin_lock_init(&qp->lock);
atomic_set(&qp->refcnt, 1);
- return ip_frag_intern(hash, qp);
+ return ip_frag_intern(qp);
out_nomem:
LIMIT_NETDEBUG(KERN_ERR "ip_frag_create: no memory left !\n");
@@ -387,11 +391,12 @@ static inline struct ipq *ip_find(struct
__u32 saddr = iph->saddr;
__u32 daddr = iph->daddr;
__u8 protocol = iph->protocol;
- unsigned int hash = ipqhashfn(id, saddr, daddr, protocol);
+ unsigned int hash;
struct ipq *qp;
struct hlist_node *n;
read_lock(&ipfrag_lock);
+ hash = ipqhashfn(id, saddr, daddr, protocol);
hlist_for_each_entry(qp, n, &ipq_hash[hash], list) {
if(qp->id == id &&
qp->saddr == saddr &&
@@ -405,7 +410,7 @@ static inline struct ipq *ip_find(struct
}
read_unlock(&ipfrag_lock);
- return ip_frag_create(hash, iph, user);
+ return ip_frag_create(iph, user);
}
/* Is the fragment too far ahead to be part of ipq? */
-
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
