* [PATCH] ip_gre: fix percpu stats accounting
@ 2010-10-28 16:07 Pavel Emelyanov
2010-10-28 16:33 ` Eric Dumazet
0 siblings, 1 reply; 11+ messages in thread
From: Pavel Emelyanov @ 2010-10-28 16:07 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, Linux Netdev List
commit e985aad7 (ip_gre: percpu stats accounting) forgot the fallback
tunnel case (gre0).
This is the 4th part of the "foo: fix percpu stats accounting" series ;)
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 01087e0..be05d5b 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1321,7 +1321,7 @@ static int ipgre_tunnel_init(struct net_device *dev)
return 0;
}
-static void ipgre_fb_tunnel_init(struct net_device *dev)
+static int ipgre_fb_tunnel_init(struct net_device *dev)
{
struct ip_tunnel *tunnel = netdev_priv(dev);
struct iphdr *iph = &tunnel->parms.iph;
@@ -1335,8 +1335,13 @@ static void ipgre_fb_tunnel_init(struct net_device *dev)
iph->ihl = 5;
tunnel->hlen = sizeof(struct iphdr) + 4;
+ dev->tstats = alloc_percpu(struct pcpu_tstats);
+ if (!dev->tstats)
+ return -ENOMEM;
+
dev_hold(dev);
rcu_assign_pointer(ign->tunnels_wc[0], tunnel);
+ return 0;
}
@@ -1377,7 +1382,10 @@ static int __net_init ipgre_init_net(struct net *net)
}
dev_net_set(ign->fb_tunnel_dev, net);
- ipgre_fb_tunnel_init(ign->fb_tunnel_dev);
+ err = ipgre_fb_tunnel_init(ign->fb_tunnel_dev);
+ if (err)
+ goto err_reg_dev;
+
ign->fb_tunnel_dev->rtnl_link_ops = &ipgre_link_ops;
if ((err = register_netdev(ign->fb_tunnel_dev)))
@@ -1386,7 +1394,7 @@ static int __net_init ipgre_init_net(struct net *net)
return 0;
err_reg_dev:
- free_netdev(ign->fb_tunnel_dev);
+ ipgre_dev_free(ign->fb_tunnel_dev);
err_alloc_dev:
return err;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix percpu stats accounting
2010-10-28 16:07 [PATCH] ip_gre: fix percpu stats accounting Pavel Emelyanov
@ 2010-10-28 16:33 ` Eric Dumazet
2010-10-28 17:29 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Eric Dumazet @ 2010-10-28 16:33 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List
Le jeudi 28 octobre 2010 à 20:07 +0400, Pavel Emelyanov a écrit :
> commit e985aad7 (ip_gre: percpu stats accounting) forgot the fallback
> tunnel case (gre0).
>
> This is the 4th part of the "foo: fix percpu stats accounting" series ;)
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>
Indeed, right you are ;)
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Anyway, I suspect more work is needed...
(check patch against ixgbe : http://patchwork.ozlabs.org/patch/69458/ )
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix percpu stats accounting
2010-10-28 16:33 ` Eric Dumazet
@ 2010-10-28 17:29 ` David Miller
2010-10-28 18:38 ` Eric Dumazet
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2010-10-28 17:29 UTC (permalink / raw)
To: eric.dumazet; +Cc: xemul, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 28 Oct 2010 18:33:54 +0200
> Le jeudi 28 octobre 2010 à 20:07 +0400, Pavel Emelyanov a écrit :
>> commit e985aad7 (ip_gre: percpu stats accounting) forgot the fallback
>> tunnel case (gre0).
>>
>> This is the 4th part of the "foo: fix percpu stats accounting" series ;)
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>>
>
> Indeed, right you are ;)
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix percpu stats accounting
2010-10-28 17:29 ` David Miller
@ 2010-10-28 18:38 ` Eric Dumazet
2010-10-28 18:41 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Eric Dumazet @ 2010-10-28 18:38 UTC (permalink / raw)
To: David Miller; +Cc: xemul, netdev
Le jeudi 28 octobre 2010 à 10:29 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 28 Oct 2010 18:33:54 +0200
>
> > Le jeudi 28 octobre 2010 à 20:07 +0400, Pavel Emelyanov a écrit :
> >> commit e985aad7 (ip_gre: percpu stats accounting) forgot the fallback
> >> tunnel case (gre0).
> >>
> >> This is the 4th part of the "foo: fix percpu stats accounting" series ;)
> >>
> >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> >>
> >
> > Indeed, right you are ;)
> >
> > Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Applied.
Hmm, actually patch is buggy, sorry...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix percpu stats accounting
2010-10-28 18:38 ` Eric Dumazet
@ 2010-10-28 18:41 ` David Miller
2010-10-28 18:47 ` Eric Dumazet
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2010-10-28 18:41 UTC (permalink / raw)
To: eric.dumazet; +Cc: xemul, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 28 Oct 2010 20:38:58 +0200
> Le jeudi 28 octobre 2010 à 10:29 -0700, David Miller a écrit :
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu, 28 Oct 2010 18:33:54 +0200
>>
>> > Le jeudi 28 octobre 2010 à 20:07 +0400, Pavel Emelyanov a écrit :
>> >> commit e985aad7 (ip_gre: percpu stats accounting) forgot the fallback
>> >> tunnel case (gre0).
>> >>
>> >> This is the 4th part of the "foo: fix percpu stats accounting" series ;)
>> >>
>> >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>> >>
>> >
>> > Indeed, right you are ;)
>> >
>> > Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
>>
>> Applied.
>
>
> Hmm, actually patch is buggy, sorry...
I am still able to revert this I think without screwing up
publicly visible history, so I will double check and do the
revert if I can.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix percpu stats accounting
2010-10-28 18:41 ` David Miller
@ 2010-10-28 18:47 ` Eric Dumazet
2010-10-28 18:49 ` David Miller
2010-10-28 19:11 ` [PATCH] ip_gre: fix fallback tunnel setup Eric Dumazet
0 siblings, 2 replies; 11+ messages in thread
From: Eric Dumazet @ 2010-10-28 18:47 UTC (permalink / raw)
To: David Miller; +Cc: xemul, netdev
Le jeudi 28 octobre 2010 à 11:41 -0700, David Miller a écrit :
> I am still able to revert this I think without screwing up
> publicly visible history, so I will double check and do the
> revert if I can.
Cool, I'll provide a patch in a couple of minutes, when tested.
Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix percpu stats accounting
2010-10-28 18:47 ` Eric Dumazet
@ 2010-10-28 18:49 ` David Miller
2010-10-28 19:11 ` [PATCH] ip_gre: fix fallback tunnel setup Eric Dumazet
1 sibling, 0 replies; 11+ messages in thread
From: David Miller @ 2010-10-28 18:49 UTC (permalink / raw)
To: eric.dumazet; +Cc: xemul, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 28 Oct 2010 20:47:59 +0200
> Le jeudi 28 octobre 2010 à 11:41 -0700, David Miller a écrit :
>> I am still able to revert this I think without screwing up
>> publicly visible history, so I will double check and do the
>> revert if I can.
>
> Cool, I'll provide a patch in a couple of minutes, when tested.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ip_gre: fix fallback tunnel setup
2010-10-28 18:47 ` Eric Dumazet
2010-10-28 18:49 ` David Miller
@ 2010-10-28 19:11 ` Eric Dumazet
2010-10-28 19:29 ` Pavel Emelyanov
1 sibling, 1 reply; 11+ messages in thread
From: Eric Dumazet @ 2010-10-28 19:11 UTC (permalink / raw)
To: David Miller; +Cc: xemul, netdev
Le jeudi 28 octobre 2010 à 20:47 +0200, Eric Dumazet a écrit :
> Le jeudi 28 octobre 2010 à 11:41 -0700, David Miller a écrit :
> > I am still able to revert this I think without screwing up
> > publicly visible history, so I will double check and do the
> > revert if I can.
>
> Cool, I'll provide a patch in a couple of minutes, when tested.
>
I believe the right fix is this one, Pavel what do you think ?
With your patch, we allocate the per_cpu data twice for the fallback
tunnel, thus leaking memory.
Thanks
Note: free_percpu(NULL) is legal
[PATCH] ip_gre: fix fallback tunnel setup
Before making the fallback tunnel visible to lookups, we should make
sure it is completely setup, once ipgre_tunnel_init() had been called
and tstats per_cpu pointer allocated.
move rcu_assign_pointer(ign->tunnels_wc[0], tunnel); from
ipgre_fb_tunnel_init() to ipgre_init_net()
Based on a patch from Pavel Emelyanov
Reported-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/ip_gre.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 01087e0..70ff77f 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1325,7 +1325,6 @@ static void ipgre_fb_tunnel_init(struct net_device *dev)
{
struct ip_tunnel *tunnel = netdev_priv(dev);
struct iphdr *iph = &tunnel->parms.iph;
- struct ipgre_net *ign = net_generic(dev_net(dev), ipgre_net_id);
tunnel->dev = dev;
strcpy(tunnel->parms.name, dev->name);
@@ -1336,7 +1335,6 @@ static void ipgre_fb_tunnel_init(struct net_device *dev)
tunnel->hlen = sizeof(struct iphdr) + 4;
dev_hold(dev);
- rcu_assign_pointer(ign->tunnels_wc[0], tunnel);
}
@@ -1383,10 +1381,12 @@ static int __net_init ipgre_init_net(struct net *net)
if ((err = register_netdev(ign->fb_tunnel_dev)))
goto err_reg_dev;
+ rcu_assign_pointer(ign->tunnels_wc[0],
+ netdev_priv(ign->fb_tunnel_dev));
return 0;
err_reg_dev:
- free_netdev(ign->fb_tunnel_dev);
+ ipgre_dev_free(ign->fb_tunnel_dev);
err_alloc_dev:
return err;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix fallback tunnel setup
2010-10-28 19:11 ` [PATCH] ip_gre: fix fallback tunnel setup Eric Dumazet
@ 2010-10-28 19:29 ` Pavel Emelyanov
2010-10-28 19:34 ` Eric Dumazet
2010-10-30 23:21 ` David Miller
0 siblings, 2 replies; 11+ messages in thread
From: Pavel Emelyanov @ 2010-10-28 19:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
On 10/28/2010 11:11 PM, Eric Dumazet wrote:
> Le jeudi 28 octobre 2010 à 20:47 +0200, Eric Dumazet a écrit :
>> Le jeudi 28 octobre 2010 à 11:41 -0700, David Miller a écrit :
>>> I am still able to revert this I think without screwing up
>>> publicly visible history, so I will double check and do the
>>> revert if I can.
>>
>> Cool, I'll provide a patch in a couple of minutes, when tested.
>>
>
> I believe the right fix is this one, Pavel what do you think ?
Indeed. I missed the fact, that the gre driver uses ndo_init for
all devices including the fb one :(
Acked-by: Pavel Emelyanov <xemul@openvz.org>
> With your patch, we allocate the per_cpu data twice for the fallback
> tunnel, thus leaking memory.
Yup...
> Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix fallback tunnel setup
2010-10-28 19:29 ` Pavel Emelyanov
@ 2010-10-28 19:34 ` Eric Dumazet
2010-10-30 23:21 ` David Miller
1 sibling, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2010-10-28 19:34 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, netdev
Le jeudi 28 octobre 2010 à 23:29 +0400, Pavel Emelyanov a écrit :
> Indeed. I missed the fact, that the gre driver uses ndo_init for
> all devices including the fb one :(
>
> Acked-by: Pavel Emelyanov <xemul@openvz.org>
Well, I discovered this right now, it was not that obvious, maybe we
should use similar setup for all fb tunnels as well...
Thanks !
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ip_gre: fix fallback tunnel setup
2010-10-28 19:29 ` Pavel Emelyanov
2010-10-28 19:34 ` Eric Dumazet
@ 2010-10-30 23:21 ` David Miller
1 sibling, 0 replies; 11+ messages in thread
From: David Miller @ 2010-10-30 23:21 UTC (permalink / raw)
To: xemul; +Cc: eric.dumazet, netdev
From: Pavel Emelyanov <xemul@parallels.com>
Date: Thu, 28 Oct 2010 23:29:10 +0400
> On 10/28/2010 11:11 PM, Eric Dumazet wrote:
>> Le jeudi 28 octobre 2010 à 20:47 +0200, Eric Dumazet a écrit :
>>> Le jeudi 28 octobre 2010 à 11:41 -0700, David Miller a écrit :
>>>> I am still able to revert this I think without screwing up
>>>> publicly visible history, so I will double check and do the
>>>> revert if I can.
>>>
>>> Cool, I'll provide a patch in a couple of minutes, when tested.
>>>
>>
>> I believe the right fix is this one, Pavel what do you think ?
>
> Indeed. I missed the fact, that the gre driver uses ndo_init for
> all devices including the fb one :(
>
> Acked-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-10-30 23:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-28 16:07 [PATCH] ip_gre: fix percpu stats accounting Pavel Emelyanov
2010-10-28 16:33 ` Eric Dumazet
2010-10-28 17:29 ` David Miller
2010-10-28 18:38 ` Eric Dumazet
2010-10-28 18:41 ` David Miller
2010-10-28 18:47 ` Eric Dumazet
2010-10-28 18:49 ` David Miller
2010-10-28 19:11 ` [PATCH] ip_gre: fix fallback tunnel setup Eric Dumazet
2010-10-28 19:29 ` Pavel Emelyanov
2010-10-28 19:34 ` Eric Dumazet
2010-10-30 23:21 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.