Skip to content

Commit 7e4d784

Browse files
Stanislav Fomichevkuba-moo
authored andcommitted
net: hold netdev instance lock during rtnetlink operations
To preserve the atomicity, hold the lock while applying multiple attributes. The major issue with a full conversion to the instance lock are software nesting devices (bonding/team/vrf/etc). Those devices call into the core stack for their lower (potentially real hw) devices. To avoid explicitly wrapping all those places into instance lock/unlock, introduce new API boundaries: - (some) existing dev_xxx calls are now considered "external" (to drivers) APIs and they transparently grab the instance lock if needed (dev_api.c) - new netif_xxx calls are internal core stack API (naming is sketchy, I've tried netdev_xxx_locked per Jakub's suggestion, but it feels a bit verbose; but happy to get back to this naming scheme if this is the preference) This avoids touching most of the existing ioctl/sysfs/drivers paths. Note the special handling of ndo_xxx_slave operations: I exploit the fact that none of the drivers that call these functions need/use instance lock. At the same time, they use dev_xxx APIs, so the lower device has to be unlocked. Changes in unregister_netdevice_many_notify (to protect dev->state with instance lock) trigger lockdep - the loop over close_list (mostly from cleanup_net) introduces spurious ordering issues. netdev_lock_cmp_fn has a justification on why it's ok to suppress for now. Cc: Saeed Mahameed <[email protected]> Signed-off-by: Stanislav Fomichev <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent cae03e5 commit 7e4d784

File tree

6 files changed

+329
-150
lines changed

6 files changed

+329
-150
lines changed

include/linux/netdevice.h

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2620,16 +2620,35 @@ static inline void netdev_for_each_tx_queue(struct net_device *dev,
26202620
f(dev, &dev->_tx[i], arg);
26212621
}
26222622

2623+
static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,
2624+
const struct lockdep_map *b)
2625+
{
2626+
/* Only lower devices currently grab the instance lock, so no
2627+
* real ordering issues can occur. In the near future, only
2628+
* hardware devices will grab instance lock which also does not
2629+
* involve any ordering. Suppress lockdep ordering warnings
2630+
* until (if) we start grabbing instance lock on pure SW
2631+
* devices (bond/team/veth/etc).
2632+
*/
2633+
if (a == b)
2634+
return 0;
2635+
return -1;
2636+
}
2637+
26232638
#define netdev_lockdep_set_classes(dev) \
26242639
{ \
26252640
static struct lock_class_key qdisc_tx_busylock_key; \
26262641
static struct lock_class_key qdisc_xmit_lock_key; \
26272642
static struct lock_class_key dev_addr_list_lock_key; \
2643+
static struct lock_class_key dev_instance_lock_key; \
26282644
unsigned int i; \
26292645
\
26302646
(dev)->qdisc_tx_busylock = &qdisc_tx_busylock_key; \
26312647
lockdep_set_class(&(dev)->addr_list_lock, \
26322648
&dev_addr_list_lock_key); \
2649+
lockdep_set_class(&(dev)->lock, \
2650+
&dev_instance_lock_key); \
2651+
lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL); \
26332652
for (i = 0; i < (dev)->num_tx_queues; i++) \
26342653
lockdep_set_class(&(dev)->_tx[i]._xmit_lock, \
26352654
&qdisc_xmit_lock_key); \
@@ -2776,6 +2795,12 @@ static inline void netdev_unlock_ops(struct net_device *dev)
27762795
netdev_unlock(dev);
27772796
}
27782797

2798+
static inline void netdev_ops_assert_locked(struct net_device *dev)
2799+
{
2800+
if (netdev_need_ops_lock(dev))
2801+
lockdep_assert_held(&dev->lock);
2802+
}
2803+
27792804
void netif_napi_set_irq_locked(struct napi_struct *napi, int irq);
27802805

27812806
static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
@@ -3350,7 +3375,9 @@ struct net_device *dev_get_by_name_rcu(struct net *net, const char *name);
33503375
struct net_device *__dev_get_by_name(struct net *net, const char *name);
33513376
bool netdev_name_in_use(struct net *net, const char *name);
33523377
int dev_alloc_name(struct net_device *dev, const char *name);
3378+
int netif_open(struct net_device *dev, struct netlink_ext_ack *extack);
33533379
int dev_open(struct net_device *dev, struct netlink_ext_ack *extack);
3380+
void netif_close(struct net_device *dev);
33543381
void dev_close(struct net_device *dev);
33553382
void dev_close_many(struct list_head *head, bool unlink);
33563383
int dev_setup_tc(struct net_device *dev, enum tc_setup_type type,
@@ -4211,25 +4238,26 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *userdata);
42114238
unsigned int dev_get_flags(const struct net_device *);
42124239
int __dev_change_flags(struct net_device *dev, unsigned int flags,
42134240
struct netlink_ext_ack *extack);
4241+
int netif_change_flags(struct net_device *dev, unsigned int flags,
4242+
struct netlink_ext_ack *extack);
42144243
int dev_change_flags(struct net_device *dev, unsigned int flags,
42154244
struct netlink_ext_ack *extack);
4245+
int netif_set_alias(struct net_device *dev, const char *alias, size_t len);
42164246
int dev_set_alias(struct net_device *, const char *, size_t);
42174247
int dev_get_alias(const struct net_device *, char *, size_t);
4218-
int __dev_change_net_namespace(struct net_device *dev, struct net *net,
4248+
int netif_change_net_namespace(struct net_device *dev, struct net *net,
42194249
const char *pat, int new_ifindex,
42204250
struct netlink_ext_ack *extack);
4221-
static inline
42224251
int dev_change_net_namespace(struct net_device *dev, struct net *net,
4223-
const char *pat)
4224-
{
4225-
return __dev_change_net_namespace(dev, net, pat, 0, NULL);
4226-
}
4252+
const char *pat);
42274253
int __dev_set_mtu(struct net_device *, int);
42284254
int dev_set_mtu(struct net_device *, int);
42294255
int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
42304256
struct netlink_ext_ack *extack);
42314257
int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
42324258
struct netlink_ext_ack *extack);
4259+
int netif_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
4260+
struct netlink_ext_ack *extack);
42334261
int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
42344262
struct netlink_ext_ack *extack);
42354263
int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name);

net/core/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ obj-y := sock.o request_sock.o skbuff.o datagram.o stream.o scm.o \
99

1010
obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
1111

12-
obj-y += dev.o dev_addr_lists.o dst.o netevent.o \
12+
obj-y += dev.o dev_api.o dev_addr_lists.o dst.o netevent.o \
1313
neighbour.o rtnetlink.o utils.o link_watch.o filter.o \
1414
sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \
1515
fib_notifier.o xdp.o flow_offload.o gro.o \

0 commit comments

Comments
 (0)