Skip to content

Commit

Permalink
macvtap: Convert to using rtnl lock
Browse files Browse the repository at this point in the history
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: Vlad Yasevich <vyasevic@redhat.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Vlad Yasevich authored and davem330 committed Jun 25, 2013
1 parent 2d48d67 commit 441ac0f
Showing 1 changed file with 25 additions and 37 deletions.
62 changes: 25 additions & 37 deletions drivers/net/macvtap.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static const struct proto_ops macvtap_socket_ops;
* RCU usage:
* 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
* or macvtap_lock is held.
* or rtnl is held.
*
* 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,
Expand All @@ -81,15 +81,14 @@ static const struct proto_ops macvtap_socket_ops;
* file or the dev. The data structure is freed through __sk_free
* 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,
struct macvtap_queue *q)
{
struct macvlan_dev *vlan = netdev_priv(dev);
int err = -EINVAL;

spin_lock(&macvtap_lock);
ASSERT_RTNL();

if (q->enabled)
goto out;
Expand All @@ -101,7 +100,6 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file,

vlan->numvtaps++;
out:
spin_unlock(&macvtap_lock);
return err;
}

Expand All @@ -111,7 +109,7 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
struct macvlan_dev *vlan = netdev_priv(dev);
int err = -EBUSY;

spin_lock(&macvtap_lock);
rtnl_lock();
if (vlan->numqueues == MAX_MACVTAP_QUEUES)
goto out;

Expand All @@ -130,26 +128,25 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
vlan->numqueues++;

out:
spin_unlock(&macvtap_lock);
rtnl_unlock();
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 macvtap_queue *nq;

vlan = rcu_dereference_protected(q->vlan,
lockdep_is_held(&macvtap_lock));

ASSERT_RTNL();
if (!q->enabled)
return -EINVAL;

vlan = rtnl_dereference(q->vlan);

if (vlan) {
int index = q->queue_index;
BUG_ON(index >= vlan->numvtaps);
nq = rcu_dereference_protected(vlan->taps[vlan->numvtaps - 1],
lockdep_is_held(&macvtap_lock));
nq = rtnl_dereference(vlan->taps[vlan->numvtaps - 1]);
nq->queue_index = index;

rcu_assign_pointer(vlan->taps[index], nq);
Expand All @@ -162,17 +159,6 @@ static int __macvtap_disable_queue(struct macvtap_queue *q)
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 reference that the files holds as well as the
Expand All @@ -185,20 +171,20 @@ static void macvtap_put_queue(struct macvtap_queue *q)
{
struct macvlan_dev *vlan;

spin_lock(&macvtap_lock);
vlan = rcu_dereference_protected(q->vlan,
lockdep_is_held(&macvtap_lock));
rtnl_lock();
vlan = rtnl_dereference(q->vlan);

if (vlan) {
if (q->enabled)
BUG_ON(__macvtap_disable_queue(q));
BUG_ON(macvtap_disable_queue(q));

vlan->numqueues--;
RCU_INIT_POINTER(q->vlan, NULL);
sock_put(&q->sk);
list_del_init(&q->next);
}

spin_unlock(&macvtap_lock);
rtnl_unlock();

synchronize_rcu();
sock_put(&q->sk);
Expand Down Expand Up @@ -260,7 +246,7 @@ static void macvtap_del_queues(struct net_device *dev)
struct macvtap_queue *q, *tmp, *qlist[MAX_MACVTAP_QUEUES];
int i, j = 0;

spin_lock(&macvtap_lock);
ASSERT_RTNL();
list_for_each_entry_safe(q, tmp, &vlan->queue_list, next) {
list_del_init(&q->next);
qlist[j++] = q;
Expand All @@ -275,9 +261,6 @@ static void macvtap_del_queues(struct net_device *dev)
BUG_ON(vlan->numqueues);
/* guarantee that any future macvtap_set_queue will fail */
vlan->numvtaps = MAX_MACVTAP_QUEUES;
spin_unlock(&macvtap_lock);

synchronize_rcu();

for (--j; j >= 0; j--)
sock_put(&qlist[j]->sk);
Expand Down Expand Up @@ -941,11 +924,10 @@ static struct macvlan_dev *macvtap_get_vlan(struct macvtap_queue *q)
{
struct macvlan_dev *vlan;

rcu_read_lock_bh();
vlan = rcu_dereference_bh(q->vlan);
ASSERT_RTNL();
vlan = rtnl_dereference(q->vlan);
if (vlan)
dev_hold(vlan->dev);
rcu_read_unlock_bh();

return vlan;
}
Expand Down Expand Up @@ -1008,21 +990,27 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
return ret;

case TUNGETIFF:
rtnl_lock();
vlan = macvtap_get_vlan(q);
if (!vlan)
if (!vlan) {
rtnl_unlock();
return -ENOLINK;
}

ret = 0;
if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) ||
put_user(q->flags, &ifr->ifr_flags))
ret = -EFAULT;
macvtap_put_vlan(vlan);
rtnl_unlock();
return ret;

case TUNSETQUEUE:
if (get_user(u, &ifr->ifr_flags))
return -EFAULT;
return macvtap_ioctl_set_queue(file, u);
rtnl_lock();
ret = macvtap_ioctl_set_queue(file, u);
rtnl_unlock();

case TUNGETFEATURES:
if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR |
Expand Down

0 comments on commit 441ac0f

Please sign in to comment.