* [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test
2017-09-22 6:10 [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal Florian Westphal
@ 2017-09-22 6:10 ` Florian Westphal
2017-09-23 16:59 ` David Ahern
2017-09-23 17:07 ` David Ahern
2017-09-22 6:10 ` [PATCH net-next v2 2/6] rtnetlink: add helper to put master ifindex Florian Westphal
` (4 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, David Ahern
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test
2017-09-22 6:10 ` [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test Florian Westphal
@ 2017-09-23 16:59 ` David Ahern
2017-09-23 17:07 ` David Ahern
1 sibling, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 16:59 UTC (permalink / raw)
To: Florian Westphal, netdev; +Cc: David Ahern
On 9/22/17 12:10 AM, Florian Westphal wrote:
> 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(+)
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test
2017-09-22 6:10 ` [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test Florian Westphal
2017-09-23 16:59 ` David Ahern
@ 2017-09-23 17:07 ` David Ahern
1 sibling, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 17:07 UTC (permalink / raw)
To: Florian Westphal, netdev
On 9/22/17 12:10 AM, Florian Westphal wrote:
> +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
BTW, if there is a v3 of this set, that ip command is shifted - uses
spaces instead of tab.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 2/6] rtnetlink: add helper to put master ifindex
2017-09-22 6:10 [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal Florian Westphal
2017-09-22 6:10 ` [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test Florian Westphal
@ 2017-09-22 6:10 ` Florian Westphal
2017-09-23 17:01 ` David Ahern
2017-09-22 6:10 ` [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name Florian Westphal
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 2/6] rtnetlink: add helper to put master ifindex
2017-09-22 6:10 ` [PATCH net-next v2 2/6] rtnetlink: add helper to put master ifindex Florian Westphal
@ 2017-09-23 17:01 ` David Ahern
0 siblings, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 17:01 UTC (permalink / raw)
To: Florian Westphal, netdev
On 9/22/17 12:10 AM, Florian Westphal wrote:
> 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(-)
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
2017-09-22 6:10 [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal Florian Westphal
2017-09-22 6:10 ` [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test Florian Westphal
2017-09-22 6:10 ` [PATCH net-next v2 2/6] rtnetlink: add helper to put master ifindex Florian Westphal
@ 2017-09-22 6:10 ` Florian Westphal
2017-09-23 17:03 ` David Ahern
2017-09-23 17:31 ` Eric Dumazet
2017-09-22 6:10 ` [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias Florian Westphal
` (2 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
We can use rcu here to make this safe even if we would not hold rtnl:
qdisc_destroy uses call_rcu to free the Qdisc struct.
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
2017-09-22 6:10 ` [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name Florian Westphal
@ 2017-09-23 17:03 ` David Ahern
2017-09-23 17:31 ` Eric Dumazet
1 sibling, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 17:03 UTC (permalink / raw)
To: Florian Westphal, netdev
On 9/22/17 12:10 AM, Florian Westphal wrote:
> We can use rcu here to make this safe even if we would not hold rtnl:
> qdisc_destroy uses call_rcu to free the Qdisc struct.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/core/rtnetlink.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
2017-09-22 6:10 ` [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name Florian Westphal
2017-09-23 17:03 ` David Ahern
@ 2017-09-23 17:31 ` Eric Dumazet
2017-09-23 18:38 ` Florian Westphal
1 sibling, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2017-09-23 17:31 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
On Fri, 2017-09-22 at 08:10 +0200, Florian Westphal wrote:
> We can use rcu here to make this safe even if we would not hold rtnl:
> qdisc_destroy uses call_rcu to free the Qdisc struct.
Where do you see call_rcu() called from qdisc_destroy() ?
You missed this commit I guess
752fbcc33405d6f8249465e4b2c4e420091bb825
("net_sched: no need to free qdisc in RCU callback")
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
2017-09-23 17:31 ` Eric Dumazet
@ 2017-09-23 18:38 ` Florian Westphal
0 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2017-09-23 18:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-09-22 at 08:10 +0200, Florian Westphal wrote:
> > We can use rcu here to make this safe even if we would not hold rtnl:
> > qdisc_destroy uses call_rcu to free the Qdisc struct.
>
>
> Where do you see call_rcu() called from qdisc_destroy() ?
>
> You missed this commit I guess
>
> 752fbcc33405d6f8249465e4b2c4e420091bb825
> ("net_sched: no need to free qdisc in RCU callback")
Indeed, I did, patch dropped, thanks for the heads-up.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias
2017-09-22 6:10 [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal Florian Westphal
` (2 preceding siblings ...)
2017-09-22 6:10 ` [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name Florian Westphal
@ 2017-09-22 6:10 ` Florian Westphal
2017-09-23 17:14 ` David Ahern
2017-09-22 6:10 ` [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information Florian Westphal
2017-09-22 6:10 ` [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl Florian Westphal
5 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: 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..42ff582a010e 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 noinline int 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias
2017-09-22 6:10 ` [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias Florian Westphal
@ 2017-09-23 17:14 ` David Ahern
0 siblings, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 17:14 UTC (permalink / raw)
To: Florian Westphal, netdev
On 9/22/17 12:10 AM, Florian Westphal wrote:
> 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..42ff582a010e 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 noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
> +{
> + ASSERT_RTNL();
The assert is not needed given the code path.
> +
> + 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))
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information
2017-09-22 6:10 [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal Florian Westphal
` (3 preceding siblings ...)
2017-09-22 6:10 ` [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias Florian Westphal
@ 2017-09-22 6:10 ` Florian Westphal
2017-09-23 17:12 ` David Ahern
2017-09-22 6:10 ` [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl Florian Westphal
5 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: 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 42ff582a010e..590823f70cc3 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 noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
return 0;
}
+static noinline int 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information
2017-09-22 6:10 ` [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information Florian Westphal
@ 2017-09-23 17:12 ` David Ahern
0 siblings, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 17:12 UTC (permalink / raw)
To: Florian Westphal, netdev
On 9/22/17 12:10 AM, Florian Westphal wrote:
> +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 noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
> return 0;
> }
>
> +static noinline int 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;
> +}
> +
No reason to combine vf and netnsid into 1 patch; completely separate topics
> 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;
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl
2017-09-22 6:10 [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal Florian Westphal
` (4 preceding siblings ...)
2017-09-22 6:10 ` [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information Florian Westphal
@ 2017-09-22 6:10 ` Florian Westphal
2017-09-23 17:17 ` David Ahern
5 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
Switch it to rcu.
rtnl_link_slave_info_fill on to other hand does need rtnl mutex for now so
add an annotation to its caller as a reminder.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Change since v1:
- don't add ASSERT_RTNL to rtnl_link_slave_info_fill and
rtnl_link_info_fill they are only called via rtnl_link_fill.
net/core/rtnetlink.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 590823f70cc3..e6f9e36d9d5a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -522,11 +522,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,
@@ -598,6 +602,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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl
2017-09-22 6:10 ` [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl Florian Westphal
@ 2017-09-23 17:17 ` David Ahern
0 siblings, 0 replies; 16+ messages in thread
From: David Ahern @ 2017-09-23 17:17 UTC (permalink / raw)
To: Florian Westphal, netdev
On 9/22/17 12:10 AM, Florian Westphal wrote:
> Switch it to rcu.
>
> rtnl_link_slave_info_fill on to other hand does need rtnl mutex for now so
> add an annotation to its caller as a reminder.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Change since v1:
> - don't add ASSERT_RTNL to rtnl_link_slave_info_fill and
> rtnl_link_info_fill they are only called via rtnl_link_fill.
>
> net/core/rtnetlink.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 590823f70cc3..e6f9e36d9d5a 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -522,11 +522,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,
> @@ -598,6 +602,8 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
> struct nlattr *linkinfo;
> int err = -EMSGSIZE;
>
> + ASSERT_RTNL();
Rather than sprinkling the ASSERT_RTNL why not just add a comment above
the function which is done in many places. Since this is really meant as
your notes as you remove rtnl usage a comment serves the same purpose.
> +
> linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
> if (linkinfo == NULL)
> goto out;
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread