* [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild
@ 2010-03-24 17:43 Pavel Emelyanov
2010-03-24 20:21 ` Neil Horman
2010-03-25 7:51 ` [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2) Pavel Emelyanov
0 siblings, 2 replies; 6+ messages in thread
From: Pavel Emelyanov @ 2010-03-24 17:43 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, Neil Horman, Linux Netdev List
The the rebuild changes the genid which in turn is used at the
hash calculation. Thus if we don't restart and go on with the
inserting the rt will happen in wrong chain.
This doesn't seem critical to me, since the proper rt entry will
be hashed at next skb.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
net/ipv4/route.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 28c1c2f..bbce53e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1097,7 +1097,7 @@ static int slow_chain_length(const struct rtable *head)
}
static int rt_intern_hash(unsigned hash, struct rtable *rt,
- struct rtable **rp, struct sk_buff *skb)
+ struct rtable **rp, struct sk_buff *skb, int in)
{
struct rtable *rth, **rthp;
unsigned long now;
@@ -1217,6 +1217,12 @@ restart:
rt->u.dst.dev->name, num);
}
rt_emergency_hash_rebuild(net);
+ spin_unlock_bh(rt_hash_lock_addr(hash));
+
+ hash = rt_hash(rt->fl.fl4_dst, rt->fl.fl4_src,
+ in ? rt->fl.iif : rt->fl.oif,
+ rt_genid(net));
+ goto restart;
}
}
@@ -1477,7 +1483,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
&netevent);
rt_del(hash, rth);
- if (!rt_intern_hash(hash, rt, &rt, NULL))
+ if (!rt_intern_hash(hash, rt, &rt, NULL, 0))
ip_rt_put(rt);
goto do_next;
}
@@ -1930,7 +1936,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
in_dev_put(in_dev);
hash = rt_hash(daddr, saddr, dev->ifindex, rt_genid(dev_net(dev)));
- return rt_intern_hash(hash, rth, NULL, skb);
+ return rt_intern_hash(hash, rth, NULL, skb, 1);
e_nobufs:
in_dev_put(in_dev);
@@ -2097,7 +2103,7 @@ static int ip_mkroute_input(struct sk_buff *skb,
/* put it into the cache */
hash = rt_hash(daddr, saddr, fl->iif,
rt_genid(dev_net(rth->u.dst.dev)));
- return rt_intern_hash(hash, rth, NULL, skb);
+ return rt_intern_hash(hash, rth, NULL, skb, 1);
}
/*
@@ -2254,7 +2260,7 @@ local_input:
}
rth->rt_type = res.type;
hash = rt_hash(daddr, saddr, fl.iif, rt_genid(net));
- err = rt_intern_hash(hash, rth, NULL, skb);
+ err = rt_intern_hash(hash, rth, NULL, skb, 1);
goto done;
no_route:
@@ -2501,7 +2507,7 @@ static int ip_mkroute_output(struct rtable **rp,
if (err == 0) {
hash = rt_hash(oldflp->fl4_dst, oldflp->fl4_src, oldflp->oif,
rt_genid(dev_net(dev_out)));
- err = rt_intern_hash(hash, rth, rp, NULL);
+ err = rt_intern_hash(hash, rth, rp, NULL, 0);
}
return err;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild
2010-03-24 17:43 [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild Pavel Emelyanov
@ 2010-03-24 20:21 ` Neil Horman
2010-03-25 7:44 ` Pavel Emelyanov
2010-03-25 7:51 ` [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2) Pavel Emelyanov
1 sibling, 1 reply; 6+ messages in thread
From: Neil Horman @ 2010-03-24 20:21 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Eric Dumazet, Linux Netdev List
On Wed, Mar 24, 2010 at 08:43:27PM +0300, Pavel Emelyanov wrote:
> The the rebuild changes the genid which in turn is used at the
> hash calculation. Thus if we don't restart and go on with the
> inserting the rt will happen in wrong chain.
>
> This doesn't seem critical to me, since the proper rt entry will
> be hashed at next skb.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> ---
> net/ipv4/route.c | 18 ++++++++++++------
> 1 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 28c1c2f..bbce53e 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1097,7 +1097,7 @@ static int slow_chain_length(const struct rtable *head)
> }
>
> static int rt_intern_hash(unsigned hash, struct rtable *rt,
> - struct rtable **rp, struct sk_buff *skb)
> + struct rtable **rp, struct sk_buff *skb, int in)
> {
> struct rtable *rth, **rthp;
> unsigned long now;
> @@ -1217,6 +1217,12 @@ restart:
> rt->u.dst.dev->name, num);
> }
> rt_emergency_hash_rebuild(net);
> + spin_unlock_bh(rt_hash_lock_addr(hash));
> +
> + hash = rt_hash(rt->fl.fl4_dst, rt->fl.fl4_src,
> + in ? rt->fl.iif : rt->fl.oif,
> + rt_genid(net));
> + goto restart;
> }
> }
Why pass in a flag here? Perhaps it would be easier just to pass in the ifindex
that was origionally used by the caller of rt_intern_hash so that the hash could
be redone without needing to toggle on the value of the flag.
Regards
Neil
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild
2010-03-24 20:21 ` Neil Horman
@ 2010-03-25 7:44 ` Pavel Emelyanov
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Emelyanov @ 2010-03-25 7:44 UTC (permalink / raw)
To: Neil Horman; +Cc: David Miller, Eric Dumazet, Linux Netdev List
>> @@ -1217,6 +1217,12 @@ restart:
>> rt->u.dst.dev->name, num);
>> }
>> rt_emergency_hash_rebuild(net);
>> + spin_unlock_bh(rt_hash_lock_addr(hash));
>> +
>> + hash = rt_hash(rt->fl.fl4_dst, rt->fl.fl4_src,
>> + in ? rt->fl.iif : rt->fl.oif,
>> + rt_genid(net));
>> + goto restart;
>> }
>> }
> Why pass in a flag here? Perhaps it would be easier just to pass in the ifindex
> that was origionally used by the caller of rt_intern_hash so that the hash could
> be redone without needing to toggle on the value of the flag.
That's perfectly fine with me. I'll resend the patch shortly.
> Regards
> Neil
>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2)
2010-03-24 17:43 [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild Pavel Emelyanov
2010-03-24 20:21 ` Neil Horman
@ 2010-03-25 7:51 ` Pavel Emelyanov
2010-03-25 13:17 ` Neil Horman
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Emelyanov @ 2010-03-25 7:51 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, Neil Horman, Linux Netdev List
The the rebuild changes the genid which in turn is used at
the hash calculation. Thus if we don't restart and go on with
inserting the rt will happen in wrong chain.
(Fixed Neil's comment about the index passed into the rt_intern_hash)
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
net/ipv4/route.c | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 28c1c2f..54ddc84 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1097,7 +1097,7 @@ static int slow_chain_length(const struct rtable *head)
}
static int rt_intern_hash(unsigned hash, struct rtable *rt,
- struct rtable **rp, struct sk_buff *skb)
+ struct rtable **rp, struct sk_buff *skb, int ifindex)
{
struct rtable *rth, **rthp;
unsigned long now;
@@ -1217,6 +1217,11 @@ restart:
rt->u.dst.dev->name, num);
}
rt_emergency_hash_rebuild(net);
+ spin_unlock_bh(rt_hash_lock_addr(hash));
+
+ hash = rt_hash(rt->fl.fl4_dst, rt->fl.fl4_src,
+ ifindex, rt_genid(net));
+ goto restart;
}
}
@@ -1477,7 +1482,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
&netevent);
rt_del(hash, rth);
- if (!rt_intern_hash(hash, rt, &rt, NULL))
+ if (!rt_intern_hash(hash, rt, &rt, NULL, rt->fl.oif))
ip_rt_put(rt);
goto do_next;
}
@@ -1930,7 +1935,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
in_dev_put(in_dev);
hash = rt_hash(daddr, saddr, dev->ifindex, rt_genid(dev_net(dev)));
- return rt_intern_hash(hash, rth, NULL, skb);
+ return rt_intern_hash(hash, rth, NULL, skb, dev->ifindex);
e_nobufs:
in_dev_put(in_dev);
@@ -2097,7 +2102,7 @@ static int ip_mkroute_input(struct sk_buff *skb,
/* put it into the cache */
hash = rt_hash(daddr, saddr, fl->iif,
rt_genid(dev_net(rth->u.dst.dev)));
- return rt_intern_hash(hash, rth, NULL, skb);
+ return rt_intern_hash(hash, rth, NULL, skb, fl->iif);
}
/*
@@ -2254,7 +2259,7 @@ local_input:
}
rth->rt_type = res.type;
hash = rt_hash(daddr, saddr, fl.iif, rt_genid(net));
- err = rt_intern_hash(hash, rth, NULL, skb);
+ err = rt_intern_hash(hash, rth, NULL, skb, fl.iif);
goto done;
no_route:
@@ -2501,7 +2506,7 @@ static int ip_mkroute_output(struct rtable **rp,
if (err == 0) {
hash = rt_hash(oldflp->fl4_dst, oldflp->fl4_src, oldflp->oif,
rt_genid(dev_net(dev_out)));
- err = rt_intern_hash(hash, rth, rp, NULL);
+ err = rt_intern_hash(hash, rth, rp, NULL, oldflp->oif);
}
return err;
--
1.5.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2)
2010-03-25 7:51 ` [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2) Pavel Emelyanov
@ 2010-03-25 13:17 ` Neil Horman
2010-03-27 3:58 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Neil Horman @ 2010-03-25 13:17 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Eric Dumazet, Linux Netdev List
On Thu, Mar 25, 2010 at 10:51:22AM +0300, Pavel Emelyanov wrote:
> The the rebuild changes the genid which in turn is used at
> the hash calculation. Thus if we don't restart and go on with
> inserting the rt will happen in wrong chain.
>
> (Fixed Neil's comment about the index passed into the rt_intern_hash)
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Thanks! I think this is pretty reasonable.
Reviewed-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2)
2010-03-25 13:17 ` Neil Horman
@ 2010-03-27 3:58 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-03-27 3:58 UTC (permalink / raw)
To: nhorman; +Cc: xemul, dada1, netdev
From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu, 25 Mar 2010 09:17:21 -0400
> On Thu, Mar 25, 2010 at 10:51:22AM +0300, Pavel Emelyanov wrote:
>> The the rebuild changes the genid which in turn is used at
>> the hash calculation. Thus if we don't restart and go on with
>> inserting the rt will happen in wrong chain.
>>
>> (Fixed Neil's comment about the index passed into the rt_intern_hash)
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> Thanks! I think this is pretty reasonable.
> Reviewed-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-27 3:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-24 17:43 [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild Pavel Emelyanov
2010-03-24 20:21 ` Neil Horman
2010-03-25 7:44 ` Pavel Emelyanov
2010-03-25 7:51 ` [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2) Pavel Emelyanov
2010-03-25 13:17 ` Neil Horman
2010-03-27 3:58 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).