[PATCH net-next] rtnetlink: prepare for further rtnl lock pushdown/removal

[PATCH net-next] rtnetlink: prepare for further rtnl lock pushdown/removal

[Florian Westphal]

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(-)

[PATCH net-next 1/7] selftests: rtnetlink.sh: add rudimentary vrf test

[Florian Westphal]

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/testing/selftests/net/rtnetlink.sh | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 4b48de565cae..6ee2bbaebcd4 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -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
 }
-- 
2.13.5

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

[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 <fw@strlen.de>
---
 net/core/rtnetlink.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a78fd61da0ec..c801212ee40e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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)) ||
-- 
2.13.5

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

[David Ahern]

On 9/21/17 10:58 AM, Florian Westphal wrote:

> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index a78fd61da0ec..c801212ee40e 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -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,

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

[PATCH net-next 3/7] rtnetlink: add helper to dump qdisc name

[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 <fw@strlen.de>
---
 net/core/rtnetlink.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c801212ee40e..ad3f27da37a8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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,
-- 
2.13.5

[PATCH net-next 4/7] rtnetlink: add helper to dump ifalias

[Florian Westphal]

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

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/core/rtnetlink.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ad3f27da37a8..1af3ef7f329d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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))
-- 
2.13.5

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

[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 <fw@strlen.de>
---
 net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 30 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1af3ef7f329d..7503021fe308 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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;
-- 
2.13.5

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

[David Ahern]

On 9/21/17 10:59 AM, Florian Westphal wrote:
> 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 <fw@strlen.de>
> ---
>  net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 50 insertions(+), 30 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 1af3ef7f329d..7503021fe308 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -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,

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

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

[Florian Westphal]

David Ahern <dsahern@gmail.com> wrote:
> On 9/21/17 10:59 AM, Florian Westphal wrote:
> > similar to earlier patches, split out more parts of this function to
> > better see what is happening and where we assume rtnl is locked.

[..]

> > +static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
> > +					   struct net_device *dev,
> > +					   u32 ext_filter_mask)
[..]
> > +static int noinline rtnl_fill_link_netnsid(struct sk_buff *skb,
> > +				  const struct net_device *dev)
[..]
> >  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).

[PATCH net-next 6/7] rtnetlink: link ops lookup must occur with rtnl lock held

[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 <fw@strlen.de>
---
 net/core/rtnetlink.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7503021fe308..7af9774aec40 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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;
-- 
2.13.5

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

[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 <fw@strlen.de>
---
 net/core/rtnetlink.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7af9774aec40..9fd48b437d64 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -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;
-- 
2.13.5

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

[David Ahern]

On 9/21/17 10:59 AM, Florian Westphal wrote:
> @@ -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;
> 


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?

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

[Florian Westphal]

David Ahern <dsahern@gmail.com> wrote:
> On 9/21/17 10:59 AM, Florian Westphal wrote:
> > @@ -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);

> 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.