Skip to contentSkip to conversation

rtnetlink: prepare for further rtnl lock pushdown/removal

PATCH New Florian Westphal 2 participants 13 messages Sep 21, 2017

Subsystems (3)
Networking Network drivers Kselftest
lore
Share & tools
lore

Florian Westphal

author
#0 lore raw

PATCH 1/1 rtnetlink: prepare for further rtnl lock pushdown/removal

First patch adds a rudimentary vrf test case.

Remaining patches split large rtnl_fill_ifinfo into smaller chunks to better see which parts

1. require rtnl
2. do not require it at all
3. rely on rtnl locking now but could be converted

add ASSERT_RTNL annotations/checks in a few spots as reminder for future rtnl removal/pushdown patches.

 net/core/rtnetlink.c                     |  161 ++++++++++++++++++++++---------
 tools/testing/selftests/net/rtnetlink.sh |   42 ++++++++
 2 files changed, 161 insertions(+), 42 deletions(-)
author
#1 lore raw

PATCH 1/7 selftests: rtnetlink.sh: add rudimentary vrf test

Cc David Ahern <[email protected]>
Signed-off-by Florian Westphal <[email protected]>
---
 tools/testing/selftests/net/rtnetlink.sh | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
tools/testing/selftests/net/rtnetlink.sh +42 -0
index 4b48de565cae..6ee2bbaebcd4 100755
@@ -291,6 +291,47 @@ kci_test_ifalias()
 	echo "PASS: set ifalias $namewant for $devdummy"
 }
 
+kci_test_vrf()
+{
+	vrfname="test-vrf"
+	ret=0
+
+	ip link show type vrf 2>/dev/null
+	if [ $? -ne 0 ]; then
+		echo "SKIP: vrf: iproute2 too old"
+		return 0
+	fi
+
+	ip link add "$vrfname" type vrf table 10
+	check_err $?
+	if [ $ret -ne 0 ];then
+		echo "FAIL: can't add vrf interface, skipping test"
+		return 0
+	fi
+
+	ip -br link show type vrf | grep -q "$vrfname"
+	check_err $?
+	if [ $ret -ne 0 ];then
+		echo "FAIL: created vrf device not found"
+		return 1
+	fi
+
+        ip link set dev "$vrfname" up
+	check_err $?
+
+	ip link set dev "$devdummy" master "$vrfname"
+	check_err $?
+	ip link del dev "$vrfname"
+	check_err $?
+
+	if [ $ret -ne 0 ];then
+		echo "FAIL: vrf"
+		return 1
+	fi
+
+	echo "PASS: vrf"
+}
+
 kci_test_rtnl()
 {
 	kci_add_dummy

@@ -306,6 +347,7 @@ kci_test_rtnl()
 	kci_test_bridge
 	kci_test_addrlabel
 	kci_test_ifalias
+	kci_test_vrf
 
 	kci_del_dummy
 }
Signature · 1 line
2.13.5
author
#2 lore raw

PATCH 2/7 rtnetlink: add helper to put master ifindex

In reply to Florian Westphal

rtnl_fill_ifinfo currently requires caller to hold the rtnl mutex. Unfortunately the function is quite large which makes it harder to see which spots need the lock, which spots assume it and which ones could do without.

Add helpers to factor out the ifindex dumping.

One helper can use rcu to remove rtnl dependency.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)
net/core/rtnetlink.c +27 -5
index a78fd61da0ec..c801212ee40e 100644
@@ -1307,6 +1307,31 @@ static u32 rtnl_get_event(unsigned long event)
 	return rtnl_event_type;
 }
 
+static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev)
+{
+	const struct net_device *upper_dev;
+	int ret = 0;
+
+	rcu_read_lock();
+
+	upper_dev = netdev_master_upper_dev_get_rcu(dev);
+	if (upper_dev)
+		ret = nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex);
+
+	rcu_read_unlock();
+	return ret;
+}
+
+static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
+{
+	int ifindex = dev_get_iflink(dev);
+
+	if (dev->ifindex == ifindex)
+		return 0;
+
+	return nla_put_u32(skb, IFLA_LINK, ifindex);
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags, u32 ext_filter_mask,

@@ -1316,7 +1341,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	struct nlmsghdr *nlh;
 	struct nlattr *af_spec;
 	struct rtnl_af_ops *af_ops;
-	struct net_device *upper_dev = netdev_master_upper_dev_get(dev);
 
 	ASSERT_RTNL();
 	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);

@@ -1345,10 +1369,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 #ifdef CONFIG_RPS
 	    nla_put_u32(skb, IFLA_NUM_RX_QUEUES, dev->num_rx_queues) ||
 #endif
-	    (dev->ifindex != dev_get_iflink(dev) &&
-	     nla_put_u32(skb, IFLA_LINK, dev_get_iflink(dev))) ||
-	    (upper_dev &&
-	     nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex)) ||
+	    nla_put_iflink(skb, dev) ||
+	    put_master_ifindex(skb, dev) ||
 	    nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
 	    (dev->qdisc &&
 	     nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
Signature · 1 line
2.13.5
#3 lore raw

Re: [PATCH net-next 2/7] rtnetlink: add helper to put master ifindex

Quoted 26 lines · Florian Westphal
net/core/rtnetlink.c +18 -0
index a78fd61da0ec..c801212ee40e 100644
@@ -1307,6 +1307,31 @@ static u32 rtnl_get_event(unsigned long event)
 	return rtnl_event_type;
 }
 
+static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev)
+{
+	const struct net_device *upper_dev;
+	int ret = 0;
+
+	rcu_read_lock();
+
+	upper_dev = netdev_master_upper_dev_get_rcu(dev);
+	if (upper_dev)
+		ret = nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex);
+
+	rcu_read_unlock();
+	return ret;
+}
+
+static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
+{
+	int ifindex = dev_get_iflink(dev);

net/core/rtnetlink.c static u32 rtnl_get_event(unsigned long event) · line 1337

Quoted 10 lines
net/core/rtnetlink.c +7 -0
+
+	if (dev->ifindex == ifindex)
+		return 0;
+
+	return nla_put_u32(skb, IFLA_LINK, ifindex);
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags, u32 ext_filter_mask,

Subject references only change for master index, put the patch is converting 2 things to helpers.

author
#4 lore raw

PATCH 3/7 rtnetlink: add helper to dump qdisc name

In reply to Florian Westphal

We can use rcu here to make this safe even if we would not hold rtnl, qdisc_destroy uses call_rcu to delay free of the qdisc for one grace period.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
net/core/rtnetlink.c +14 -2
index c801212ee40e..ad3f27da37a8 100644
@@ -1332,6 +1332,19 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
 	return nla_put_u32(skb, IFLA_LINK, ifindex);
 }
 
+static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
+{
+	struct Qdisc *q;
+	int ret = 0;
+
+	rcu_read_lock();
+	q = READ_ONCE(dev->qdisc);
+	if (q)
+		ret = nla_put_string(skb, IFLA_QDISC, q->ops->id);
+	rcu_read_unlock();
+	return ret;
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags, u32 ext_filter_mask,

@@ -1372,8 +1385,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	    nla_put_iflink(skb, dev) ||
 	    put_master_ifindex(skb, dev) ||
 	    nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
-	    (dev->qdisc &&
-	     nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
+	    nla_put_qdisc(skb, dev) ||
 	    (dev->ifalias &&
 	     nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
 	    nla_put_u32(skb, IFLA_CARRIER_CHANGES,
Signature · 1 line
2.13.5
author
#5 lore raw

PATCH 4/7 rtnetlink: add helper to dump ifalias

In reply to Florian Westphal

ifalias is currently protected by rtnl mutex, add assertion as a reminder.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
net/core/rtnetlink.c +11 -2
index ad3f27da37a8..1af3ef7f329d 100644
@@ -1345,6 +1345,16 @@ static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
 	return ret;
 }
 
+static int noinline nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
+{
+	ASSERT_RTNL();
+
+	if (dev->ifalias)
+		return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
+
+	return 0;
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags, u32 ext_filter_mask,

@@ -1386,8 +1396,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	    put_master_ifindex(skb, dev) ||
 	    nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
 	    nla_put_qdisc(skb, dev) ||
-	    (dev->ifalias &&
-	     nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
+	    nla_put_ifalias(skb, dev) ||
 	    nla_put_u32(skb, IFLA_CARRIER_CHANGES,
 			atomic_read(&dev->carrier_changes)) ||
 	    nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
Signature · 1 line
2.13.5
author
#6 lore raw

PATCH 5/7 rtnetlink: add helper to dump vf information

In reply to Florian Westphal

similar to earlier patches, split out more parts of this function to better see what is happening and where we assume rtnl is locked.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 30 deletions(-)
net/core/rtnetlink.c +50 -30
index 1af3ef7f329d..7503021fe308 100644
@@ -1211,6 +1211,36 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
+static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
+					   struct net_device *dev,
+					   u32 ext_filter_mask)
+{
+	struct nlattr *vfinfo;
+	int i, num_vfs;
+
+	if (!dev->dev.parent || ((ext_filter_mask & RTEXT_FILTER_VF) == 0))
+		return 0;
+
+	num_vfs = dev_num_vf(dev->dev.parent);
+	if (nla_put_u32(skb, IFLA_NUM_VF, num_vfs))
+		return -EMSGSIZE;
+
+	if (!dev->netdev_ops->ndo_get_vf_config)
+		return 0;
+
+	vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
+	if (!vfinfo)
+		return -EMSGSIZE;
+
+	for (i = 0; i < num_vfs; i++) {
+		if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
+			return -EMSGSIZE;
+	}
+
+	nla_nest_end(skb, vfinfo);
+	return 0;
+}
+
 static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
 {
 	struct rtnl_link_ifmap map;

@@ -1355,6 +1385,23 @@ static int noinline nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static int noinline rtnl_fill_link_netnsid(struct sk_buff *skb,
+				  const struct net_device *dev)
+{
+	if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
+		struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+		if (!net_eq(dev_net(dev), link_net)) {
+			int id = peernet2id_alloc(dev_net(dev), link_net);
+
+			if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
+				return -EMSGSIZE;
+		}
+	}
+
+	return 0;
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags, u32 ext_filter_mask,

@@ -1428,27 +1475,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (rtnl_fill_stats(skb, dev))
 		goto nla_put_failure;
 
-	if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF) &&
-	    nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)))
+	if (rtnl_fill_vf(skb, dev, ext_filter_mask))
 		goto nla_put_failure;
 
-	if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent &&
-	    ext_filter_mask & RTEXT_FILTER_VF) {
-		int i;
-		struct nlattr *vfinfo;
-		int num_vfs = dev_num_vf(dev->dev.parent);
-
-		vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
-		if (!vfinfo)
-			goto nla_put_failure;
-		for (i = 0; i < num_vfs; i++) {
-			if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
-				goto nla_put_failure;
-		}
-
-		nla_nest_end(skb, vfinfo);
-	}
-
 	if (rtnl_port_fill(skb, dev, ext_filter_mask))
 		goto nla_put_failure;
 

@@ -1460,17 +1489,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			goto nla_put_failure;
 	}
 
-	if (dev->rtnl_link_ops &&
-	    dev->rtnl_link_ops->get_link_net) {
-		struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
-
-		if (!net_eq(dev_net(dev), link_net)) {
-			int id = peernet2id_alloc(dev_net(dev), link_net);
-
-			if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
-				goto nla_put_failure;
-		}
-	}
+	if (rtnl_fill_link_netnsid(skb, dev))
+		goto nla_put_failure;
 
 	if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
 		goto nla_put_failure;
Signature · 1 line
2.13.5
#7 lore raw

Re: [PATCH net-next 5/7] rtnetlink: add helper to dump vf information

Quoted 61 lines · Florian Westphal

similar to earlier patches, split out more parts of this function to better see what is happening and where we assume rtnl is locked.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 30 deletions(-)
net/core/rtnetlink.c +38 -0
index 1af3ef7f329d..7503021fe308 100644
@@ -1211,6 +1211,36 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
+static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
+					   struct net_device *dev,
+					   u32 ext_filter_mask)
+{
+	struct nlattr *vfinfo;
+	int i, num_vfs;
+
+	if (!dev->dev.parent || ((ext_filter_mask & RTEXT_FILTER_VF) == 0))
+		return 0;
+
+	num_vfs = dev_num_vf(dev->dev.parent);
+	if (nla_put_u32(skb, IFLA_NUM_VF, num_vfs))
+		return -EMSGSIZE;
+
+	if (!dev->netdev_ops->ndo_get_vf_config)
+		return 0;
+
+	vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
+	if (!vfinfo)
+		return -EMSGSIZE;
+
+	for (i = 0; i < num_vfs; i++) {
+		if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
+			return -EMSGSIZE;
+	}
+
+	nla_nest_end(skb, vfinfo);
+	return 0;
+}
+
 static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
 {
 	struct rtnl_link_ifmap map;

@@ -1355,6 +1385,23 @@ static int noinline nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static int noinline rtnl_fill_link_netnsid(struct sk_buff *skb,
+				  const struct net_device *dev)
+{
+	if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
+		struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+		if (!net_eq(dev_net(dev), link_net)) {
+			int id = peernet2id_alloc(dev_net(dev), link_net);

net/core/rtnetlink.c static int noinline nla_put_ifalias(struct sk_buff *skb, struct net_device *dev) · line 1405

Quoted 10 lines
net/core/rtnetlink.c +9 -0
+
+			if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
+				return -EMSGSIZE;
+		}
+	}
+
+	return 0;
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,

Similarly for this patch -- subject references vf helper yet the code changes flips netnsid as well.

author
#8 lore raw

Re: [PATCH net-next 5/7] rtnetlink: add helper to dump vf information

Quoted 3 lines · David Ahern
Quoted 2 lines · Florian Westphal

similar to earlier patches, split out more parts of this function to better see what is happening and where we assume rtnl is locked.

[..]

Quoted 3 lines
Quoted 3 lines
net/core/rtnetlink.c +3 -0
+static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
+					   struct net_device *dev,
+					   u32 ext_filter_mask)

[..]

Quoted 2 lines
Quoted 2 lines
net/core/rtnetlink.c +2 -0
+static int noinline rtnl_fill_link_netnsid(struct sk_buff *skb,
+				  const struct net_device *dev)

[..]

Quoted 4 lines
Quoted 1 line

static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,

Similarly for this patch -- subject references vf helper yet the code changes flips netnsid as well.

Right, anomly due to improper squash-merge.
I'll send a v2 tomorrow (giving more time for others to comment).

author
#9 lore raw

PATCH 6/7 rtnetlink: link ops lookup must occur with rtnl lock held

In reply to Florian Westphal

all callers meet this requirement, this serves as reminder to not forget about this in future rtnl pushdown work.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 4 ++++
 1 file changed, 4 insertions(+)
net/core/rtnetlink.c +4 -0
index 7503021fe308..7af9774aec40 100644
@@ -274,6 +274,8 @@ static const struct rtnl_link_ops *rtnl_link_ops_get(const char *kind)
 {
 	const struct rtnl_link_ops *ops;
 
+	ASSERT_RTNL();
+
 	list_for_each_entry(ops, &link_ops, list) {
 		if (!strcmp(ops->kind, kind))
 			return ops;

@@ -1618,6 +1620,8 @@ static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla
 	const struct rtnl_link_ops *ops = NULL;
 	struct nlattr *linfo[IFLA_INFO_MAX + 1];
 
+	ASSERT_RTNL();
+
 	if (nla_parse_nested(linfo, IFLA_INFO_MAX, nla,
 			     ifla_info_policy, NULL) < 0)
 		return NULL;
Signature · 1 line
2.13.5
author
#10 lore raw

PATCH 7/7 rtnetlink: rtnl_have_link_slave_info doesn't need rtnl

In reply to Florian Westphal

it can be switched to rcu. rtnl_link_slave_info_fill on the other hand does need it, at least for now. Add ASSERT_RTNL annotation as a reminder.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
net/core/rtnetlink.c +13 -3
index 7af9774aec40..9fd48b437d64 100644
@@ -524,11 +524,15 @@ static size_t rtnl_link_get_af_size(const struct net_device *dev,
 static bool rtnl_have_link_slave_info(const struct net_device *dev)
 {
 	struct net_device *master_dev;
+	bool ret = false;
 
-	master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+	rcu_read_lock();
+
+	master_dev = netdev_master_upper_dev_get_rcu((struct net_device *) dev);
 	if (master_dev && master_dev->rtnl_link_ops)
-		return true;
-	return false;
+		ret = true;
+	rcu_read_unlock();
+	return ret;
 }
 
 static int rtnl_link_slave_info_fill(struct sk_buff *skb,

@@ -539,6 +543,8 @@ static int rtnl_link_slave_info_fill(struct sk_buff *skb,
 	struct nlattr *slave_data;
 	int err;
 
+	ASSERT_RTNL();
+
 	master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
 	if (!master_dev)
 		return 0;

@@ -570,6 +576,8 @@ static int rtnl_link_info_fill(struct sk_buff *skb,
 	struct nlattr *data;
 	int err;
 
+	ASSERT_RTNL();
+
 	if (!ops)
 		return 0;
 	if (nla_put_string(skb, IFLA_INFO_KIND, ops->kind) < 0)

@@ -600,6 +608,8 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
 	struct nlattr *linkinfo;
 	int err = -EMSGSIZE;
 
+	ASSERT_RTNL();
+
 	linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
 	if (linkinfo == NULL)
 		goto out;
Signature · 1 line
2.13.5
#11 lore raw

Re: [PATCH net-next 7/7] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl

Quoted 20 lines · Florian Westphal
net/core/rtnetlink.c +4 -0
@@ -539,6 +543,8 @@ static int rtnl_link_slave_info_fill(struct sk_buff *skb,
 	struct nlattr *slave_data;
 	int err;
 
+	ASSERT_RTNL();
+
 	master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
 	if (!master_dev)
 		return 0;

@@ -570,6 +576,8 @@ static int rtnl_link_info_fill(struct sk_buff *skb,
 	struct nlattr *data;
 	int err;
 
+	ASSERT_RTNL();
+
 	if (!ops)
 		return 0;
 	if (nla_put_string(skb, IFLA_INFO_KIND, ops->kind) < 0)

net/core/rtnetlink.c static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev) · line 615

Quoted 8 lines
net/core/rtnetlink.c +2 -0
@@ -600,6 +608,8 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
 	struct nlattr *linkinfo;
 	int err = -EMSGSIZE;
 
+	ASSERT_RTNL();
+
 	linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
 	if (linkinfo == NULL)
 		goto out;

Since rtnl_link_slave_info_fill and rtnl_link_info_fill are only called by rtnl_link_fill and rtnl_link_fill is only called rtnl_fill_ifinfo which as the ASSERT_RTNL why add to these lower functions as well?

author
#12 lore raw

Re: [PATCH net-next 7/7] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl

Quoted 7 lines · David Ahern

net/core/rtnetlink.c static int rtnl_link_slave_info_fill(struct sk_buff *skb, · line 547

Quoted 6 lines · Florian Westphal
net/core/rtnetlink.c +2 -0
@@ -539,6 +543,8 @@ static int rtnl_link_slave_info_fill(struct sk_buff *skb,
 	struct nlattr *slave_data;
 	int err;
 
+	ASSERT_RTNL();
+

[..]
master_dev = netdev_master_upper_dev_get((struct net_device *) dev);

Quoted 3 lines

Since rtnl_link_slave_info_fill and rtnl_link_info_fill are only called by rtnl_link_fill and rtnl_link_fill is only called rtnl_fill_ifinfo which as the ASSERT_RTNL why add to these lower functions as well?

I'll remove this patch in v2 and will hold it back in my private queue; these serve more as a reminder/TODO for myself rather than anything else.