Commit 441ac0fc authored by Vlad Yasevich's avatar Vlad Yasevich Committed by David S. Miller

macvtap: Convert to using rtnl lock

Macvtap uses a private lock to protect the relationship between
macvtap_queue and macvlan_dev.  The private lock is not needed
since the relationship is managed by user via open(), release(),
and dellink() calls.  dellink() already happens under rtnl, so
we can safely convert open() and release(), and use it in ioctl()
as well.

Suggested by Eric Dumazet.
Signed-off-by: default avatarVlad Yasevich <vyasevic@redhat.com>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2d48d67f
...@@ -69,7 +69,7 @@ static const struct proto_ops macvtap_socket_ops; ...@@ -69,7 +69,7 @@ static const struct proto_ops macvtap_socket_ops;
* RCU usage: * RCU usage:
* The macvtap_queue and the macvlan_dev are loosely coupled, the * The macvtap_queue and the macvlan_dev are loosely coupled, the
* pointers from one to the other can only be read while rcu_read_lock * pointers from one to the other can only be read while rcu_read_lock
* or macvtap_lock is held. * or rtnl is held.
* *
* Both the file and the macvlan_dev hold a reference on the macvtap_queue * Both the file and the macvlan_dev hold a reference on the macvtap_queue
* through sock_hold(&q->sk). When the macvlan_dev goes away first, * through sock_hold(&q->sk). When the macvlan_dev goes away first,
...@@ -81,7 +81,6 @@ static const struct proto_ops macvtap_socket_ops; ...@@ -81,7 +81,6 @@ static const struct proto_ops macvtap_socket_ops;
* file or the dev. The data structure is freed through __sk_free * file or the dev. The data structure is freed through __sk_free
* when both our references and any pending SKBs are gone. * when both our references and any pending SKBs are gone.
*/ */
static DEFINE_SPINLOCK(macvtap_lock);
static int macvtap_enable_queue(struct net_device *dev, struct file *file, static int macvtap_enable_queue(struct net_device *dev, struct file *file,
struct macvtap_queue *q) struct macvtap_queue *q)
...@@ -89,7 +88,7 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file, ...@@ -89,7 +88,7 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file,
struct macvlan_dev *vlan = netdev_priv(dev); struct macvlan_dev *vlan = netdev_priv(dev);
int err = -EINVAL; int err = -EINVAL;
spin_lock(&macvtap_lock); ASSERT_RTNL();
if (q->enabled) if (q->enabled)
goto out; goto out;
...@@ -101,7 +100,6 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file, ...@@ -101,7 +100,6 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file,
vlan->numvtaps++; vlan->numvtaps++;
out: out:
spin_unlock(&macvtap_lock);
return err; return err;
} }
...@@ -111,7 +109,7 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file, ...@@ -111,7 +109,7 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
struct macvlan_dev *vlan = netdev_priv(dev); struct macvlan_dev *vlan = netdev_priv(dev);
int err = -EBUSY; int err = -EBUSY;
spin_lock(&macvtap_lock); rtnl_lock();
if (vlan->numqueues == MAX_MACVTAP_QUEUES) if (vlan->numqueues == MAX_MACVTAP_QUEUES)
goto out; goto out;
...@@ -130,26 +128,25 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file, ...@@ -130,26 +128,25 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
vlan->numqueues++; vlan->numqueues++;
out: out:
spin_unlock(&macvtap_lock); rtnl_unlock();
return err; return err;
} }
static int __macvtap_disable_queue(struct macvtap_queue *q) static int macvtap_disable_queue(struct macvtap_queue *q)
{ {
struct macvlan_dev *vlan; struct macvlan_dev *vlan;
struct macvtap_queue *nq; struct macvtap_queue *nq;
vlan = rcu_dereference_protected(q->vlan, ASSERT_RTNL();
lockdep_is_held(&macvtap_lock));
if (!q->enabled) if (!q->enabled)
return -EINVAL; return -EINVAL;
vlan = rtnl_dereference(q->vlan);
if (vlan) { if (vlan) {
int index = q->queue_index; int index = q->queue_index;
BUG_ON(index >= vlan->numvtaps); BUG_ON(index >= vlan->numvtaps);
nq = rcu_dereference_protected(vlan->taps[vlan->numvtaps - 1], nq = rtnl_dereference(vlan->taps[vlan->numvtaps - 1]);
lockdep_is_held(&macvtap_lock));
nq->queue_index = index; nq->queue_index = index;
rcu_assign_pointer(vlan->taps[index], nq); rcu_assign_pointer(vlan->taps[index], nq);
...@@ -162,17 +159,6 @@ static int __macvtap_disable_queue(struct macvtap_queue *q) ...@@ -162,17 +159,6 @@ static int __macvtap_disable_queue(struct macvtap_queue *q)
return 0; return 0;
} }
static int macvtap_disable_queue(struct macvtap_queue *q)
{
int err;
spin_lock(&macvtap_lock);
err = __macvtap_disable_queue(q);
spin_unlock(&macvtap_lock);
return err;
}
/* /*
* The file owning the queue got closed, give up both * The file owning the queue got closed, give up both
* the reference that the files holds as well as the * the reference that the files holds as well as the
...@@ -185,12 +171,12 @@ static void macvtap_put_queue(struct macvtap_queue *q) ...@@ -185,12 +171,12 @@ static void macvtap_put_queue(struct macvtap_queue *q)
{ {
struct macvlan_dev *vlan; struct macvlan_dev *vlan;
spin_lock(&macvtap_lock); rtnl_lock();
vlan = rcu_dereference_protected(q->vlan, vlan = rtnl_dereference(q->vlan);
lockdep_is_held(&macvtap_lock));
if (vlan) { if (vlan) {
if (q->enabled) if (q->enabled)
BUG_ON(__macvtap_disable_queue(q)); BUG_ON(macvtap_disable_queue(q));
vlan->numqueues--; vlan->numqueues--;
RCU_INIT_POINTER(q->vlan, NULL); RCU_INIT_POINTER(q->vlan, NULL);
...@@ -198,7 +184,7 @@ static void macvtap_put_queue(struct macvtap_queue *q) ...@@ -198,7 +184,7 @@ static void macvtap_put_queue(struct macvtap_queue *q)
list_del_init(&q->next); list_del_init(&q->next);
} }
spin_unlock(&macvtap_lock); rtnl_unlock();
synchronize_rcu(); synchronize_rcu();
sock_put(&q->sk); sock_put(&q->sk);
...@@ -260,7 +246,7 @@ static void macvtap_del_queues(struct net_device *dev) ...@@ -260,7 +246,7 @@ static void macvtap_del_queues(struct net_device *dev)
struct macvtap_queue *q, *tmp, *qlist[MAX_MACVTAP_QUEUES]; struct macvtap_queue *q, *tmp, *qlist[MAX_MACVTAP_QUEUES];
int i, j = 0; int i, j = 0;
spin_lock(&macvtap_lock); ASSERT_RTNL();
list_for_each_entry_safe(q, tmp, &vlan->queue_list, next) { list_for_each_entry_safe(q, tmp, &vlan->queue_list, next) {
list_del_init(&q->next); list_del_init(&q->next);
qlist[j++] = q; qlist[j++] = q;
...@@ -275,9 +261,6 @@ static void macvtap_del_queues(struct net_device *dev) ...@@ -275,9 +261,6 @@ static void macvtap_del_queues(struct net_device *dev)
BUG_ON(vlan->numqueues); BUG_ON(vlan->numqueues);
/* guarantee that any future macvtap_set_queue will fail */ /* guarantee that any future macvtap_set_queue will fail */
vlan->numvtaps = MAX_MACVTAP_QUEUES; vlan->numvtaps = MAX_MACVTAP_QUEUES;
spin_unlock(&macvtap_lock);
synchronize_rcu();
for (--j; j >= 0; j--) for (--j; j >= 0; j--)
sock_put(&qlist[j]->sk); sock_put(&qlist[j]->sk);
...@@ -941,11 +924,10 @@ static struct macvlan_dev *macvtap_get_vlan(struct macvtap_queue *q) ...@@ -941,11 +924,10 @@ static struct macvlan_dev *macvtap_get_vlan(struct macvtap_queue *q)
{ {
struct macvlan_dev *vlan; struct macvlan_dev *vlan;
rcu_read_lock_bh(); ASSERT_RTNL();
vlan = rcu_dereference_bh(q->vlan); vlan = rtnl_dereference(q->vlan);
if (vlan) if (vlan)
dev_hold(vlan->dev); dev_hold(vlan->dev);
rcu_read_unlock_bh();
return vlan; return vlan;
} }
...@@ -1008,21 +990,27 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, ...@@ -1008,21 +990,27 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
return ret; return ret;
case TUNGETIFF: case TUNGETIFF:
rtnl_lock();
vlan = macvtap_get_vlan(q); vlan = macvtap_get_vlan(q);
if (!vlan) if (!vlan) {
rtnl_unlock();
return -ENOLINK; return -ENOLINK;
}
ret = 0; ret = 0;
if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) || if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) ||
put_user(q->flags, &ifr->ifr_flags)) put_user(q->flags, &ifr->ifr_flags))
ret = -EFAULT; ret = -EFAULT;
macvtap_put_vlan(vlan); macvtap_put_vlan(vlan);
rtnl_unlock();
return ret; return ret;
case TUNSETQUEUE: case TUNSETQUEUE:
if (get_user(u, &ifr->ifr_flags)) if (get_user(u, &ifr->ifr_flags))
return -EFAULT; return -EFAULT;
return macvtap_ioctl_set_queue(file, u); rtnl_lock();
ret = macvtap_ioctl_set_queue(file, u);
rtnl_unlock();
case TUNGETFEATURES: case TUNGETFEATURES:
if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR |
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment