| Message ID | 20170529172557.7065-1-lrichard@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2017-05-29 at 13:25 -0400, Lance Richardson wrote: > After commit 0c1d70af924b ("net: use dst_cache for vxlan device"), > cached dst entries could be leaked when more than one remote was > present for a given vxlan_fdb entry, causing subsequent netns > operations to block indefinitely and "unregister_netdevice: waiting > for lo to become free." messages to appear in the kernel log. > > Fix by properly releasing cached dst and freeing resources in this > case. > > Fixes: commit 0c1d70af924b ("net: use dst_cache for vxlan device") > Signed-off-by: Lance Richardson <lrichard@redhat.com> > --- > drivers/net/vxlan.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index 328b471..5c1d69e 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -740,6 +740,22 @@ static void vxlan_fdb_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f) > call_rcu(&f->rcu, vxlan_fdb_free); > } > > +static void vxlan_dst_free(struct rcu_head *head) > +{ > + struct vxlan_rdst *rd = container_of(head, struct vxlan_rdst, rcu); > + > + dst_cache_destroy(&rd->dst_cache); > + kfree(rd); > +} > + > +static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f, > + struct vxlan_rdst *rd) > +{ > + list_del_rcu(&rd->list); > + vxlan_fdb_notify(vxlan, f, rd, RTM_DELNEIGH); > + call_rcu(&rd->rcu, vxlan_dst_free); > +} > + > static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan, > union vxlan_addr *ip, __be16 *port, __be32 *src_vni, > __be32 *vni, u32 *ifindex) > @@ -864,9 +880,7 @@ static int __vxlan_fdb_delete(struct vxlan_dev *vxlan, > * otherwise destroy the fdb entry > */ > if (rd && !list_is_singular(&f->remotes)) { > - list_del_rcu(&rd->list); > - vxlan_fdb_notify(vxlan, f, rd, RTM_DELNEIGH); > - kfree_rcu(rd, rcu); > + vxlan_fdb_dst_destroy(vxlan, f, rd); > goto out; > } > LGTM, thanks Lance! Acked-by: Paolo Abeni <pabeni@redhat.com>
> From: "Lance Richardson" <lrichard@redhat.com> > To: netdev@vger.kernel.org, pabeni@redhat.com > Sent: Monday, 29 May, 2017 1:25:57 PM > Subject: [PATCH net] vxlan: eliminate cached dst leak > > After commit 0c1d70af924b ("net: use dst_cache for vxlan device"), > cached dst entries could be leaked when more than one remote was > present for a given vxlan_fdb entry, causing subsequent netns > operations to block indefinitely and "unregister_netdevice: waiting > for lo to become free." messages to appear in the kernel log. > > Fix by properly releasing cached dst and freeing resources in this > case. > > Fixes: commit 0c1d70af924b ("net: use dst_cache for vxlan device") > Signed-off-by: Lance Richardson <lrichard@redhat.com> > --- This problem was originally debugged and the patch tested in an OpenStack (devstack) test environment. Here's a small(-ish) reproducer script that was cooked up after posting: ip netns add ns0 ip netns add ns1 ip netns add ns2 ip link add p0 type veth peer name p1 ip link add p2 type veth peer name p3 ip link add p4 type veth peer name p5 ip link add name br0 type bridge ip link set br0 up ip link set p0 master br0 up ip link set p1 netns ns0 ip link set p2 master br0 up ip link set p3 netns ns1 ip link set p4 master br0 up ip link set p5 netns ns2 ip netns exec ns0 ip addr add "1.1.1.1/24" dev p1 ip netns exec ns0 ip link set dev p1 up ip netns exec ns1 ip addr add "1.1.1.2/24" dev p3 ip netns exec ns1 ip link set dev p3 up ip netns exec ns2 ip addr add "1.1.1.3/24" dev p5 ip netns exec ns2 ip link set dev p5 up ip netns exec ns0 ip link add vxlan0 type vxlan dstport 4789 id 10 dev p1 ip netns exec ns0 ip addr add "4.1.1.1/24" dev vxlan0 ip netns exec ns0 ip link set dev vxlan0 up mtu 1450 ip netns exec ns1 ip link add vxlan1 type vxlan dstport 4789 id 10 dev p3 ip netns exec ns1 ip addr add "4.1.1.2/24" dev vxlan1 ip netns exec ns1 ip link set dev vxlan1 up mtu 1450 ip netns exec ns2 ip link add vxlan2 type vxlan dstport 4789 id 10 dev p5 ip netns exec ns2 ip addr add "4.1.1.3/24" dev vxlan2 ip netns exec ns2 ip link set dev vxlan2 up mtu 1450 # Create a vxlan default fdb entry with two remotes in the list ip netns exec ns0 bridge fdb append to 00:00:00:00:00:00 dst 1.1.1.2 dev vxlan0 ip netns exec ns0 bridge fdb append to 00:00:00:00:00:00 dst 1.1.1.3 dev vxlan0 # Forward some packets to populate dst cache for default fdb ip netns exec ns0 ping -c 2 4.1.1.2 ip netns exec ns0 ping -c 2 4.1.1.3 # delete one of the entries in the fdb remotes list to trigger the bug ip netns exec ns0 bridge fdb del to 00:00:00:00:00:00 dst 1.1.1.3 dev vxlan0 ip netns del ns2 ip netns del ns1 ip netns del ns0 # If bug is triggered, kernel messages similar to this should be logged: # # kernel:unregister_netdevice: waiting for lo to become free. Usage count = 1 # # Netns commands like "ip netns add ns3" will hang indefinitely.
From: Lance Richardson <lrichard@redhat.com> Date: Mon, 29 May 2017 13:25:57 -0400 > After commit 0c1d70af924b ("net: use dst_cache for vxlan device"), > cached dst entries could be leaked when more than one remote was > present for a given vxlan_fdb entry, causing subsequent netns > operations to block indefinitely and "unregister_netdevice: waiting > for lo to become free." messages to appear in the kernel log. > > Fix by properly releasing cached dst and freeing resources in this > case. > > Fixes: commit 0c1d70af924b ("net: use dst_cache for vxlan device") In the future please do not put that "commit " string there, it's not needed. > Signed-off-by: Lance Richardson <lrichard@redhat.com> Applied and queued up for -stable, thank you.
> From: "David Miller" <davem@davemloft.net> > To: lrichard@redhat.com > Cc: netdev@vger.kernel.org, pabeni@redhat.com > Sent: Thursday, 1 June, 2017 2:46:48 PM > Subject: Re: [PATCH net] vxlan: eliminate cached dst leak > > From: Lance Richardson <lrichard@redhat.com> > Date: Mon, 29 May 2017 13:25:57 -0400 > > > After commit 0c1d70af924b ("net: use dst_cache for vxlan device"), > > cached dst entries could be leaked when more than one remote was > > present for a given vxlan_fdb entry, causing subsequent netns > > operations to block indefinitely and "unregister_netdevice: waiting > > for lo to become free." messages to appear in the kernel log. > > > > Fix by properly releasing cached dst and freeing resources in this > > case. > > > > Fixes: commit 0c1d70af924b ("net: use dst_cache for vxlan device") > > In the future please do not put that "commit " string there, it's not > needed. Oops, sorry about that, I know I've made that mistake before (both times because a checkpatch.pl warning in the body made me think I needed to change the Fixes: tag as well). Now I'm tempted to roll a checkpatch.pl improvement... > > > Signed-off-by: Lance Richardson <lrichard@redhat.com> > > Applied and queued up for -stable, thank you. >
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 328b471..5c1d69e 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -740,6 +740,22 @@ static void vxlan_fdb_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f) call_rcu(&f->rcu, vxlan_fdb_free); } +static void vxlan_dst_free(struct rcu_head *head) +{ + struct vxlan_rdst *rd = container_of(head, struct vxlan_rdst, rcu); + + dst_cache_destroy(&rd->dst_cache); + kfree(rd); +} + +static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f, + struct vxlan_rdst *rd) +{ + list_del_rcu(&rd->list); + vxlan_fdb_notify(vxlan, f, rd, RTM_DELNEIGH); + call_rcu(&rd->rcu, vxlan_dst_free); +} + static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan, union vxlan_addr *ip, __be16 *port, __be32 *src_vni, __be32 *vni, u32 *ifindex) @@ -864,9 +880,7 @@ static int __vxlan_fdb_delete(struct vxlan_dev *vxlan, * otherwise destroy the fdb entry */ if (rd && !list_is_singular(&f->remotes)) { - list_del_rcu(&rd->list); - vxlan_fdb_notify(vxlan, f, rd, RTM_DELNEIGH); - kfree_rcu(rd, rcu); + vxlan_fdb_dst_destroy(vxlan, f, rd); goto out; }
After commit 0c1d70af924b ("net: use dst_cache for vxlan device"), cached dst entries could be leaked when more than one remote was present for a given vxlan_fdb entry, causing subsequent netns operations to block indefinitely and "unregister_netdevice: waiting for lo to become free." messages to appear in the kernel log. Fix by properly releasing cached dst and freeing resources in this case. Fixes: commit 0c1d70af924b ("net: use dst_cache for vxlan device") Signed-off-by: Lance Richardson <lrichard@redhat.com> --- drivers/net/vxlan.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)