[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130528214142.4e41db6f@nehalam.linuxnetplumber.net>
Date: Tue, 28 May 2013 21:41:42 -0700
From: Stephen Hemminger <stephen@...workplumber.org>
To: Cong Wang <amwang@...hat.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>
Subject: Re: [Patch net-next] vxlan: do real refcnt for vn_sock
Why not just fix the requirement to drop rtnl when calling igmp.
The code comes out cleaner and safer as well.
The follow COMPILE TESTED ONLY is a starting point...
--- a/drivers/net/vxlan.c 2013-05-20 10:24:19.624427346 -0700
+++ b/drivers/net/vxlan.c 2013-05-28 21:39:57.502351889 -0700
@@ -657,20 +657,12 @@ static int vxlan_join_group(struct net_d
.imr_multiaddr.s_addr = vxlan->default_dst.remote_ip,
.imr_ifindex = vxlan->default_dst.remote_ifindex,
};
- int err;
/* Already a member of group */
if (vxlan_group_used(vn, vxlan))
return 0;
- /* Need to drop RTNL to call multicast join */
- rtnl_unlock();
- lock_sock(sk);
- err = ip_mc_join_group(sk, &mreq);
- release_sock(sk);
- rtnl_lock();
-
- return err;
+ return _ip_mc_join_group(sk, &mreq);
}
@@ -679,7 +671,6 @@ static int vxlan_leave_group(struct net_
{
struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id);
- int err = 0;
struct sock *sk = vxlan->vn_sock->sock->sk;
struct ip_mreqn mreq = {
.imr_multiaddr.s_addr = vxlan->default_dst.remote_ip,
@@ -690,14 +681,7 @@ static int vxlan_leave_group(struct net_
if (vxlan_group_used(vn, vxlan))
return 0;
- /* Need to drop RTNL to call multicast leave */
- rtnl_unlock();
- lock_sock(sk);
- err = ip_mc_leave_group(sk, &mreq);
- release_sock(sk);
- rtnl_lock();
-
- return err;
+ return _ip_mc_leave_group(sk, &mreq);
}
/* Callback from net/ipv4/udp.c to receive packets */
@@ -1244,6 +1228,7 @@ static int vxlan_open(struct net_device
struct vxlan_dev *vxlan = netdev_priv(dev);
int err;
+ dev_hold(dev);
if (IN_MULTICAST(ntohl(vxlan->default_dst.remote_ip))) {
err = vxlan_join_group(dev);
if (err)
@@ -1252,6 +1237,7 @@ static int vxlan_open(struct net_device
if (vxlan->age_interval)
mod_timer(&vxlan->age_timer, jiffies + FDB_AGE_INTERVAL);
+ dev_put(dev);
return 0;
}
--- a/include/linux/igmp.h 2013-05-11 15:58:53.692325370 -0700
+++ b/include/linux/igmp.h 2013-05-28 21:37:40.560328674 -0700
@@ -109,7 +109,9 @@ struct ip_mc_list {
extern int ip_check_mc_rcu(struct in_device *dev, __be32 mc_addr, __be32 src_addr, u16 proto);
extern int igmp_rcv(struct sk_buff *);
+extern int _ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr);
extern int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr);
+extern int _ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr);
extern int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr);
extern void ip_mc_drop_socket(struct sock *sk);
extern int ip_mc_source(int add, int omode, struct sock *sk,
--- a/net/ipv4/igmp.c 2013-05-11 15:58:53.992318551 -0700
+++ b/net/ipv4/igmp.c 2013-05-28 21:39:43.302554701 -0700
@@ -1781,48 +1781,36 @@ static void ip_mc_clear_src(struct ip_mc
pmc->sfcount[MCAST_EXCLUDE] = 1;
}
-
-/*
- * Join a multicast group
- */
-int ip_mc_join_group(struct sock *sk , struct ip_mreqn *imr)
+int _ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
{
- int err;
__be32 addr = imr->imr_multiaddr.s_addr;
- struct ip_mc_socklist *iml = NULL, *i;
+ struct ip_mc_socklist *iml, *i;
struct in_device *in_dev;
struct inet_sock *inet = inet_sk(sk);
struct net *net = sock_net(sk);
int ifindex;
int count = 0;
- if (!ipv4_is_multicast(addr))
- return -EINVAL;
-
- rtnl_lock();
+ ASSERT_RTNL();
in_dev = ip_mc_find_dev(net, imr);
+ if (!in_dev)
+ return -ENODEV;
- if (!in_dev) {
- iml = NULL;
- err = -ENODEV;
- goto done;
- }
-
- err = -EADDRINUSE;
ifindex = imr->imr_ifindex;
for_each_pmc_rtnl(inet, i) {
if (i->multi.imr_multiaddr.s_addr == addr &&
i->multi.imr_ifindex == ifindex)
- goto done;
+ return -EADDRINUSE;
count++;
}
- err = -ENOBUFS;
+
if (count >= sysctl_igmp_max_memberships)
- goto done;
+ return -ENOBUFS;
+
iml = sock_kmalloc(sk, sizeof(*iml), GFP_KERNEL);
if (iml == NULL)
- goto done;
+ return -ENOMEM;
memcpy(&iml->multi, imr, sizeof(*imr));
iml->next_rcu = inet->mc_list;
@@ -1830,8 +1818,24 @@ int ip_mc_join_group(struct sock *sk , s
iml->sfmode = MCAST_EXCLUDE;
rcu_assign_pointer(inet->mc_list, iml);
ip_mc_inc_group(in_dev, addr);
- err = 0;
-done:
+
+ return 0;
+}
+EXPORT_SYMBOL(_ip_mc_join_group);
+
+/*
+ * Join a multicast group
+ */
+int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
+{
+ int err;
+ __be32 addr = imr->imr_multiaddr.s_addr;
+
+ if (!ipv4_is_multicast(addr))
+ return -EINVAL;
+
+ rtnl_lock();
+ err = _ip_mc_join_group(sk, imr);
rtnl_unlock();
return err;
}
@@ -1857,11 +1861,7 @@ static int ip_mc_leave_src(struct sock *
return err;
}
-/*
- * Ask a socket to leave a group.
- */
-
-int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
+int _ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
{
struct inet_sock *inet = inet_sk(sk);
struct ip_mc_socklist *iml;
@@ -1870,9 +1870,9 @@ int ip_mc_leave_group(struct sock *sk, s
struct net *net = sock_net(sk);
__be32 group = imr->imr_multiaddr.s_addr;
u32 ifindex;
- int ret = -EADDRNOTAVAIL;
- rtnl_lock();
+ ASSERT_RTNL();
+
in_dev = ip_mc_find_dev(net, imr);
ifindex = imr->imr_ifindex;
for (imlp = &inet->mc_list;
@@ -1880,6 +1880,7 @@ int ip_mc_leave_group(struct sock *sk, s
imlp = &iml->next_rcu) {
if (iml->multi.imr_multiaddr.s_addr != group)
continue;
+
if (ifindex) {
if (iml->multi.imr_ifindex != ifindex)
continue;
@@ -1887,20 +1888,32 @@ int ip_mc_leave_group(struct sock *sk, s
iml->multi.imr_address.s_addr)
continue;
- (void) ip_mc_leave_src(sk, iml, in_dev);
+ ip_mc_leave_src(sk, iml, in_dev);
*imlp = iml->next_rcu;
if (in_dev)
ip_mc_dec_group(in_dev, group);
- rtnl_unlock();
+
/* decrease mem now to avoid the memleak warning */
atomic_sub(sizeof(*iml), &sk->sk_omem_alloc);
kfree_rcu(iml, rcu);
return 0;
}
- if (!in_dev)
- ret = -ENODEV;
+
+ return !in_dev ? -ENODEV : -EADDRNOTAVAIL;
+}
+EXPORT_SYMBOL(_ip_mc_leave_group);
+
+/*
+ * Ask a socket to leave a group.
+ */
+int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
+{
+ int ret;
+
+ rtnl_lock();
+ ret = _ip_mc_leave_group(sk, imr);
rtnl_unlock();
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists