From: Stephen Hemminger <stephen@networkplumber.org>
To: Cong Wang <amwang@redhat.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>
Subject: Re: [Patch net-next] vxlan: do real refcnt for vn_sock
Date: Tue, 28 May 2013 21:41:42 -0700 [thread overview]
Message-ID: <20130528214142.4e41db6f@nehalam.linuxnetplumber.net> (raw)
In-Reply-To: <1369739242-5944-1-git-send-email-amwang@redhat.com>
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;
}
next prev parent reply other threads:[~2013-05-29 4:41 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-28 11:07 [Patch net-next] vxlan: do real refcnt for vn_sock Cong Wang
2013-05-28 15:22 ` Stephen Hemminger
2013-05-29 2:08 ` Cong Wang
2013-05-29 4:22 ` Stephen Hemminger
2013-05-29 4:34 ` Cong Wang
2013-05-29 4:01 ` Cong Wang
2013-05-29 4:41 ` Stephen Hemminger [this message]
2013-05-29 5:14 ` Cong Wang
2013-05-29 8:39 ` Cong Wang
2013-05-31 2:55 ` Cong Wang
2013-05-31 3:56 ` Stephen Hemminger
2013-05-31 4:12 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130528214142.4e41db6f@nehalam.linuxnetplumber.net \
--to=stephen@networkplumber.org \
--cc=amwang@redhat.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.