[Patch net-next 0/2] netlink: improve netlink tap code

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [Patch net-next 0/2] netlink: improve netlink tap code
@ 2017-12-06 23:03 Cong Wang
  2017-12-06 23:03 ` [Patch net-next 1/2] netlink: make netlink tap per netns Cong Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Cong Wang @ 2017-12-06 23:03 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang

Cong Wang (2):
  netlink: make netlink tap per netns
  netlink: convert netlink tap spinlock to mutex

 net/netlink/af_netlink.c | 66 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 17 deletions(-)

-- 
2.13.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Patch net-next 1/2] netlink: make netlink tap per netns
  2017-12-06 23:03 [Patch net-next 0/2] netlink: improve netlink tap code Cong Wang
@ 2017-12-06 23:03 ` Cong Wang
  2017-12-07 14:59   ` Daniel Borkmann
  2017-12-06 23:03 ` [Patch net-next 2/2] netlink: convert netlink tap spinlock to mutex Cong Wang
  2017-12-11 15:57 ` [Patch net-next 0/2] netlink: improve netlink tap code David Miller
  2 siblings, 1 reply; 5+ messages in thread
From: Cong Wang @ 2017-12-06 23:03 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Daniel Borkmann, Kevin Cernekee

nlmon device is not supposed to capture netlink events from
other netns, so instead of filtering events, we can simply
make netlink tap itself per netns.

Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kevin Cernekee <cernekee@chromium.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/netlink/af_netlink.c | 66 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 17 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index b9e0ee4e22f5..de5324cc98d4 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -65,6 +65,7 @@
 #include <linux/net_namespace.h>
 
 #include <net/net_namespace.h>
+#include <net/netns/generic.h>
 #include <net/sock.h>
 #include <net/scm.h>
 #include <net/netlink.h>
@@ -145,8 +146,6 @@ static atomic_t nl_table_users = ATOMIC_INIT(0);
 
 static BLOCKING_NOTIFIER_HEAD(netlink_chain);
 
-static DEFINE_SPINLOCK(netlink_tap_lock);
-static struct list_head netlink_tap_all __read_mostly;
 
 static const struct rhashtable_params netlink_rhashtable_params;
 
@@ -173,14 +172,24 @@ static struct sk_buff *netlink_to_full_skb(const struct sk_buff *skb,
 	return new;
 }
 
+static unsigned int netlink_tap_net_id;
+
+struct netlink_tap_net {
+	struct list_head netlink_tap_all;
+	spinlock_t netlink_tap_lock;
+};
+
 int netlink_add_tap(struct netlink_tap *nt)
 {
+	struct net *net = dev_net(nt->dev);
+	struct netlink_tap_net *nn = net_generic(net, netlink_tap_net_id);
+
 	if (unlikely(nt->dev->type != ARPHRD_NETLINK))
 		return -EINVAL;
 
-	spin_lock(&netlink_tap_lock);
-	list_add_rcu(&nt->list, &netlink_tap_all);
-	spin_unlock(&netlink_tap_lock);
+	spin_lock(&nn->netlink_tap_lock);
+	list_add_rcu(&nt->list, &nn->netlink_tap_all);
+	spin_unlock(&nn->netlink_tap_lock);
 
 	__module_get(nt->module);
 
@@ -190,12 +199,14 @@ EXPORT_SYMBOL_GPL(netlink_add_tap);
 
 static int __netlink_remove_tap(struct netlink_tap *nt)
 {
+	struct net *net = dev_net(nt->dev);
+	struct netlink_tap_net *nn = net_generic(net, netlink_tap_net_id);
 	bool found = false;
 	struct netlink_tap *tmp;
 
-	spin_lock(&netlink_tap_lock);
+	spin_lock(&nn->netlink_tap_lock);
 
-	list_for_each_entry(tmp, &netlink_tap_all, list) {
+	list_for_each_entry(tmp, &nn->netlink_tap_all, list) {
 		if (nt == tmp) {
 			list_del_rcu(&nt->list);
 			found = true;
@@ -205,7 +216,7 @@ static int __netlink_remove_tap(struct netlink_tap *nt)
 
 	pr_warn("__netlink_remove_tap: %p not found\n", nt);
 out:
-	spin_unlock(&netlink_tap_lock);
+	spin_unlock(&nn->netlink_tap_lock);
 
 	if (found)
 		module_put(nt->module);
@@ -224,6 +235,26 @@ int netlink_remove_tap(struct netlink_tap *nt)
 }
 EXPORT_SYMBOL_GPL(netlink_remove_tap);
 
+static __net_init int netlink_tap_init_net(struct net *net)
+{
+	struct netlink_tap_net *nn = net_generic(net, netlink_tap_net_id);
+
+	INIT_LIST_HEAD(&nn->netlink_tap_all);
+	spin_lock_init(&nn->netlink_tap_lock);
+	return 0;
+}
+
+static void __net_exit netlink_tap_exit_net(struct net *net)
+{
+}
+
+static struct pernet_operations netlink_tap_net_ops = {
+	.init = netlink_tap_init_net,
+	.exit = netlink_tap_exit_net,
+	.id   = &netlink_tap_net_id,
+	.size = sizeof(struct netlink_tap_net),
+};
+
 static bool netlink_filter_tap(const struct sk_buff *skb)
 {
 	struct sock *sk = skb->sk;
@@ -274,7 +305,7 @@ static int __netlink_deliver_tap_skb(struct sk_buff *skb,
 	return ret;
 }
 
-static void __netlink_deliver_tap(struct sk_buff *skb)
+static void __netlink_deliver_tap(struct sk_buff *skb, struct netlink_tap_net *nn)
 {
 	int ret;
 	struct netlink_tap *tmp;
@@ -282,19 +313,21 @@ static void __netlink_deliver_tap(struct sk_buff *skb)
 	if (!netlink_filter_tap(skb))
 		return;
 
-	list_for_each_entry_rcu(tmp, &netlink_tap_all, list) {
+	list_for_each_entry_rcu(tmp, &nn->netlink_tap_all, list) {
 		ret = __netlink_deliver_tap_skb(skb, tmp->dev);
 		if (unlikely(ret))
 			break;
 	}
 }
 
-static void netlink_deliver_tap(struct sk_buff *skb)
+static void netlink_deliver_tap(struct net *net, struct sk_buff *skb)
 {
+	struct netlink_tap_net *nn = net_generic(net, netlink_tap_net_id);
+
 	rcu_read_lock();
 
-	if (unlikely(!list_empty(&netlink_tap_all)))
-		__netlink_deliver_tap(skb);
+	if (unlikely(!list_empty(&nn->netlink_tap_all)))
+		__netlink_deliver_tap(skb, nn);
 
 	rcu_read_unlock();
 }
@@ -303,7 +336,7 @@ static void netlink_deliver_tap_kernel(struct sock *dst, struct sock *src,
 				       struct sk_buff *skb)
 {
 	if (!(netlink_is_kernel(dst) && netlink_is_kernel(src)))
-		netlink_deliver_tap(skb);
+		netlink_deliver_tap(sock_net(dst), skb);
 }
 
 static void netlink_overrun(struct sock *sk)
@@ -1213,7 +1246,7 @@ static int __netlink_sendskb(struct sock *sk, struct sk_buff *skb)
 {
 	int len = skb->len;
 
-	netlink_deliver_tap(skb);
+	netlink_deliver_tap(sock_net(sk), skb);
 
 	skb_queue_tail(&sk->sk_receive_queue, skb);
 	sk->sk_data_ready(sk);
@@ -2730,12 +2763,11 @@ static int __init netlink_proto_init(void)
 		}
 	}
 
-	INIT_LIST_HEAD(&netlink_tap_all);
-
 	netlink_add_usersock_entry();
 
 	sock_register(&netlink_family_ops);
 	register_pernet_subsys(&netlink_net_ops);
+	register_pernet_subsys(&netlink_tap_net_ops);
 	/* The netlink device handler may be needed early. */
 	rtnetlink_init();
 out:
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch net-next 1/2] netlink: make netlink tap per netns
  2017-12-06 23:03 ` [Patch net-next 1/2] netlink: make netlink tap per netns Cong Wang
@ 2017-12-07 14:59   ` Daniel Borkmann
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2017-12-07 14:59 UTC (permalink / raw)
  To: Cong Wang, netdev; +Cc: Kevin Cernekee

On 12/07/2017 12:03 AM, Cong Wang wrote:
> nlmon device is not supposed to capture netlink events from
> other netns, so instead of filtering events, we can simply
> make netlink tap itself per netns.
> 
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Kevin Cernekee <cernekee@chromium.org>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Yeah, that should optimize it a bit further, looks fine to me.

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Patch net-next 2/2] netlink: convert netlink tap spinlock to mutex
  2017-12-06 23:03 [Patch net-next 0/2] netlink: improve netlink tap code Cong Wang
  2017-12-06 23:03 ` [Patch net-next 1/2] netlink: make netlink tap per netns Cong Wang
@ 2017-12-06 23:03 ` Cong Wang
  2017-12-11 15:57 ` [Patch net-next 0/2] netlink: improve netlink tap code David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2017-12-06 23:03 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Daniel Borkmann

Both netlink_add_tap() and netlink_remove_tap() are
called in process context, no need to bother spinlock.

Note, in fact, currently we always hold RTNL when calling
these two functions, so we don't need any other lock at
all, but keeping this lock doesn't harm anything.

Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/netlink/af_netlink.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index de5324cc98d4..f6e6d2278fa9 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -176,7 +176,7 @@ static unsigned int netlink_tap_net_id;
 
 struct netlink_tap_net {
 	struct list_head netlink_tap_all;
-	spinlock_t netlink_tap_lock;
+	struct mutex netlink_tap_lock;
 };
 
 int netlink_add_tap(struct netlink_tap *nt)
@@ -187,9 +187,9 @@ int netlink_add_tap(struct netlink_tap *nt)
 	if (unlikely(nt->dev->type != ARPHRD_NETLINK))
 		return -EINVAL;
 
-	spin_lock(&nn->netlink_tap_lock);
+	mutex_lock(&nn->netlink_tap_lock);
 	list_add_rcu(&nt->list, &nn->netlink_tap_all);
-	spin_unlock(&nn->netlink_tap_lock);
+	mutex_unlock(&nn->netlink_tap_lock);
 
 	__module_get(nt->module);
 
@@ -204,7 +204,7 @@ static int __netlink_remove_tap(struct netlink_tap *nt)
 	bool found = false;
 	struct netlink_tap *tmp;
 
-	spin_lock(&nn->netlink_tap_lock);
+	mutex_lock(&nn->netlink_tap_lock);
 
 	list_for_each_entry(tmp, &nn->netlink_tap_all, list) {
 		if (nt == tmp) {
@@ -216,7 +216,7 @@ static int __netlink_remove_tap(struct netlink_tap *nt)
 
 	pr_warn("__netlink_remove_tap: %p not found\n", nt);
 out:
-	spin_unlock(&nn->netlink_tap_lock);
+	mutex_unlock(&nn->netlink_tap_lock);
 
 	if (found)
 		module_put(nt->module);
@@ -240,7 +240,7 @@ static __net_init int netlink_tap_init_net(struct net *net)
 	struct netlink_tap_net *nn = net_generic(net, netlink_tap_net_id);
 
 	INIT_LIST_HEAD(&nn->netlink_tap_all);
-	spin_lock_init(&nn->netlink_tap_lock);
+	mutex_init(&nn->netlink_tap_lock);
 	return 0;
 }
 
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch net-next 0/2] netlink: improve netlink tap code
  2017-12-06 23:03 [Patch net-next 0/2] netlink: improve netlink tap code Cong Wang
  2017-12-06 23:03 ` [Patch net-next 1/2] netlink: make netlink tap per netns Cong Wang
  2017-12-06 23:03 ` [Patch net-next 2/2] netlink: convert netlink tap spinlock to mutex Cong Wang
@ 2017-12-11 15:57 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-12-11 15:57 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed,  6 Dec 2017 15:03:18 -0800

> Cong Wang (2):
>   netlink: make netlink tap per netns
>   netlink: convert netlink tap spinlock to mutex

Both applied, thanks Cong.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-12-11 15:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-06 23:03 [Patch net-next 0/2] netlink: improve netlink tap code Cong Wang
2017-12-06 23:03 ` [Patch net-next 1/2] netlink: make netlink tap per netns Cong Wang
2017-12-07 14:59   ` Daniel Borkmann
2017-12-06 23:03 ` [Patch net-next 2/2] netlink: convert netlink tap spinlock to mutex Cong Wang
2017-12-11 15:57 ` [Patch net-next 0/2] netlink: improve netlink tap code David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).