Skip to content

Commit 441ac0f

Browse files
Vlad Yasevichdavem330
Vlad Yasevich
authored andcommitted
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: Vlad Yasevich <vyasevic@redhat.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 2d48d67 commit 441ac0f

File tree

1 file changed

+25
-37
lines changed

1 file changed

+25
-37
lines changed

drivers/net/macvtap.c

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ static const struct proto_ops macvtap_socket_ops;
6969
* RCU usage:
7070
* The macvtap_queue and the macvlan_dev are loosely coupled, the
7171
* pointers from one to the other can only be read while rcu_read_lock
72-
* or macvtap_lock is held.
72+
* or rtnl is held.
7373
*
7474
* Both the file and the macvlan_dev hold a reference on the macvtap_queue
7575
* through sock_hold(&q->sk). When the macvlan_dev goes away first,
@@ -81,15 +81,14 @@ static const struct proto_ops macvtap_socket_ops;
8181
* file or the dev. The data structure is freed through __sk_free
8282
* when both our references and any pending SKBs are gone.
8383
*/
84-
static DEFINE_SPINLOCK(macvtap_lock);
8584

8685
static int macvtap_enable_queue(struct net_device *dev, struct file *file,
8786
struct macvtap_queue *q)
8887
{
8988
struct macvlan_dev *vlan = netdev_priv(dev);
9089
int err = -EINVAL;
9190

92-
spin_lock(&macvtap_lock);
91+
ASSERT_RTNL();
9392

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

102101
vlan->numvtaps++;
103102
out:
104-
spin_unlock(&macvtap_lock);
105103
return err;
106104
}
107105

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

114-
spin_lock(&macvtap_lock);
112+
rtnl_lock();
115113
if (vlan->numqueues == MAX_MACVTAP_QUEUES)
116114
goto out;
117115

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

132130
out:
133-
spin_unlock(&macvtap_lock);
131+
rtnl_unlock();
134132
return err;
135133
}
136134

137-
static int __macvtap_disable_queue(struct macvtap_queue *q)
135+
static int macvtap_disable_queue(struct macvtap_queue *q)
138136
{
139137
struct macvlan_dev *vlan;
140138
struct macvtap_queue *nq;
141139

142-
vlan = rcu_dereference_protected(q->vlan,
143-
lockdep_is_held(&macvtap_lock));
144-
140+
ASSERT_RTNL();
145141
if (!q->enabled)
146142
return -EINVAL;
147143

144+
vlan = rtnl_dereference(q->vlan);
145+
148146
if (vlan) {
149147
int index = q->queue_index;
150148
BUG_ON(index >= vlan->numvtaps);
151-
nq = rcu_dereference_protected(vlan->taps[vlan->numvtaps - 1],
152-
lockdep_is_held(&macvtap_lock));
149+
nq = rtnl_dereference(vlan->taps[vlan->numvtaps - 1]);
153150
nq->queue_index = index;
154151

155152
rcu_assign_pointer(vlan->taps[index], nq);
@@ -162,17 +159,6 @@ static int __macvtap_disable_queue(struct macvtap_queue *q)
162159
return 0;
163160
}
164161

165-
static int macvtap_disable_queue(struct macvtap_queue *q)
166-
{
167-
int err;
168-
169-
spin_lock(&macvtap_lock);
170-
err = __macvtap_disable_queue(q);
171-
spin_unlock(&macvtap_lock);
172-
173-
return err;
174-
}
175-
176162
/*
177163
* The file owning the queue got closed, give up both
178164
* the reference that the files holds as well as the
@@ -185,20 +171,20 @@ static void macvtap_put_queue(struct macvtap_queue *q)
185171
{
186172
struct macvlan_dev *vlan;
187173

188-
spin_lock(&macvtap_lock);
189-
vlan = rcu_dereference_protected(q->vlan,
190-
lockdep_is_held(&macvtap_lock));
174+
rtnl_lock();
175+
vlan = rtnl_dereference(q->vlan);
176+
191177
if (vlan) {
192178
if (q->enabled)
193-
BUG_ON(__macvtap_disable_queue(q));
179+
BUG_ON(macvtap_disable_queue(q));
194180

195181
vlan->numqueues--;
196182
RCU_INIT_POINTER(q->vlan, NULL);
197183
sock_put(&q->sk);
198184
list_del_init(&q->next);
199185
}
200186

201-
spin_unlock(&macvtap_lock);
187+
rtnl_unlock();
202188

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

263-
spin_lock(&macvtap_lock);
249+
ASSERT_RTNL();
264250
list_for_each_entry_safe(q, tmp, &vlan->queue_list, next) {
265251
list_del_init(&q->next);
266252
qlist[j++] = q;
@@ -275,9 +261,6 @@ static void macvtap_del_queues(struct net_device *dev)
275261
BUG_ON(vlan->numqueues);
276262
/* guarantee that any future macvtap_set_queue will fail */
277263
vlan->numvtaps = MAX_MACVTAP_QUEUES;
278-
spin_unlock(&macvtap_lock);
279-
280-
synchronize_rcu();
281264

282265
for (--j; j >= 0; j--)
283266
sock_put(&qlist[j]->sk);
@@ -941,11 +924,10 @@ static struct macvlan_dev *macvtap_get_vlan(struct macvtap_queue *q)
941924
{
942925
struct macvlan_dev *vlan;
943926

944-
rcu_read_lock_bh();
945-
vlan = rcu_dereference_bh(q->vlan);
927+
ASSERT_RTNL();
928+
vlan = rtnl_dereference(q->vlan);
946929
if (vlan)
947930
dev_hold(vlan->dev);
948-
rcu_read_unlock_bh();
949931

950932
return vlan;
951933
}
@@ -1008,21 +990,27 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
1008990
return ret;
1009991

1010992
case TUNGETIFF:
993+
rtnl_lock();
1011994
vlan = macvtap_get_vlan(q);
1012-
if (!vlan)
995+
if (!vlan) {
996+
rtnl_unlock();
1013997
return -ENOLINK;
998+
}
1014999

10151000
ret = 0;
10161001
if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) ||
10171002
put_user(q->flags, &ifr->ifr_flags))
10181003
ret = -EFAULT;
10191004
macvtap_put_vlan(vlan);
1005+
rtnl_unlock();
10201006
return ret;
10211007

10221008
case TUNSETQUEUE:
10231009
if (get_user(u, &ifr->ifr_flags))
10241010
return -EFAULT;
1025-
return macvtap_ioctl_set_queue(file, u);
1011+
rtnl_lock();
1012+
ret = macvtap_ioctl_set_queue(file, u);
1013+
rtnl_unlock();
10261014

10271015
case TUNGETFEATURES:
10281016
if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR |

0 commit comments

Comments
 (0)