Skip to contentSkip to conversation

rtnetlink: fix dump+module unload races, take 2

PATCH In review Florian Westphal 3 participants 20 messages Nov 6, 2017 – Nov 13, 2017

Subsystems (6)
Networking Network drivers Ethernet bridge CAN network layer +2 more subsystems
lore
Share & tools
lore

Florian Westphal

author
#0 lore raw

Peter Zijlstra reported:
----------
I just ran across commit:
019a316992ee ("rtnetlink: add reference counting to prevent module unload while dump is in progress") And that commit is _completely_ broken.

1) it not in fact a refcount, so using refcount_t is silly
2) there is a distinct lack of memory barriers, so we can easily observe the decrement while the msg_handler is still in progress.
3) waiting with a schedule()/yield() loop is complete crap and subject life-locks, imagine doing that rtnl_unregister_all() from a RT task.
----------

Moreover, the commit doesn't work.

Problem is that netlink keeps the dump function address in the control block to resume dumps, i.e. when netlink_dump_start() returns then we may not be done with the dump, it can be restarted on next recv.

netlink already has a mechanism to prevent module unload while dumps are in progress: netlink_dump_control struct contains a pointer to struct module, and netlink will take care of module_get/put as needed if that is set.

To make use of this however we need to store pointer to module that registered the particular dump function. This series does that.

First, revert the broken commit to start from scratch. Second patch adds new rtnl_register_module() that takes pointer to the owning module.

Rest of patches convert affected modules.
IPv6 is not converted as ipv6 module cannot be unloaded.

One alternative to this series would be to decrement rtnl_msg_handlers_ref count via netlink_dump_control->done(), but that doesn't address any of Peters points.

I am submitting this for -next for 2 reasons:
1. This problem has existed for so long its evidently not a big deal
2. I think its a bad idea to cram this in at last second in current development cycle.

ps: I will be travelling, replies will be delayed.

author
#1 lore raw

PATCH 1/8 rtnetlink: Revert "rtnetlink: add reference counting to prevent module unload while dump is in progress"

This reverts commit 019a316992ee0d9832b1c480c899d6bdf2a0a77e.

Peter Zijlstra reported:
1) it not in fact a refcount, so using refcount_t is silly
2) there is a distinct lack of memory barriers, so we can easily observe the decrement while the msg_handler is still in progress.
3) waiting with a schedule()/yield() loop is complete crap and subject life-locks, imagine doing that rtnl_unregister_all() from a RT task.
----------

Moreover, the commit doesn't even work.
Problem is that netlink keeps the dump function address in the control block to resume dumps, i.e. when netlink_dump_start() returns then we may not be done with the dump, it can be restarted on next recv.

First, revert the broken commit to start from scratch.
Next patch will allow to store a pointer the owning module so we can let netlink infrastructure grab a refcount to module while dump function address is in use.

Signed-off-by Florian Westphal <[email protected]>
---
 net/core/rtnetlink.c | 13 -------------
 1 file changed, 13 deletions(-)
net/core/rtnetlink.c +0 -13
index dc5ad84ac096..c70f62137dd8 100644
@@ -128,7 +128,6 @@ EXPORT_SYMBOL(lockdep_rtnl_is_held);
 #endif /* #ifdef CONFIG_PROVE_LOCKING */
 
 static struct rtnl_link __rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
-static refcount_t rtnl_msg_handlers_ref[RTNL_FAMILY_MAX + 1];
 
 static inline int rtm_msgindex(int msgtype)
 {

@@ -262,8 +261,6 @@ void rtnl_unregister_all(int protocol)
 
 	synchronize_net();
 
-	while (refcount_read(&rtnl_msg_handlers_ref[protocol]) > 1)
-		schedule();
 	kfree(handlers);
 }
 EXPORT_SYMBOL_GPL(rtnl_unregister_all);

@@ -4365,8 +4362,6 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 				goto err_unlock;
 		}
 
-		refcount_inc(&rtnl_msg_handlers_ref[family]);
-
 		if (type == RTM_GETLINK - RTM_BASE)
 			min_dump_alloc = rtnl_calcit(skb, nlh);
 

@@ -4380,7 +4375,6 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 			};
 			err = netlink_dump_start(rtnl, skb, nlh, &c);
 		}
-		refcount_dec(&rtnl_msg_handlers_ref[family]);
 		return err;
 	}
 

@@ -4392,12 +4386,10 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	flags = READ_ONCE(handlers[type].flags);
 	if (flags & RTNL_FLAG_DOIT_UNLOCKED) {
-		refcount_inc(&rtnl_msg_handlers_ref[family]);
 		doit = READ_ONCE(handlers[type].doit);
 		rcu_read_unlock();
 		if (doit)
 			err = doit(skb, nlh, extack);
-		refcount_dec(&rtnl_msg_handlers_ref[family]);
 		return err;
 	}
 

@@ -4498,11 +4490,6 @@ static struct pernet_operations rtnetlink_net_ops = {
 
 void __init rtnetlink_init(void)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(rtnl_msg_handlers_ref); i++)
-		refcount_set(&rtnl_msg_handlers_ref[i], 1);
-
 	if (register_pernet_subsys(&rtnetlink_net_ops))
 		panic("rtnetlink_init: cannot initialize rtnetlink\n");
Signature · 1 line
2.13.6
author
#2 lore raw

PATCH 2/8 rtnetlink: add rtnl_register_module

In reply to Florian Westphal

Add yet another rtnl_register function. It will be used by modules that can be removed.

The passed module struct is used to take a reference while a netlink dump is in progress to prevent module unload while netlink core can invoke registered dumper function again.

Signed-off-by Florian Westphal <[email protected]>
---
 include/net/rtnetlink.h |   2 +
 net/core/rtnetlink.c    | 103 +++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 83 insertions(+), 22 deletions(-)
include/net/rtnetlink.h +2 -0
index ead018744ff5..e326b3f9eb5f 100644
@@ -17,6 +17,8 @@ int __rtnl_register(int protocol, int msgtype,
 		    rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
 void rtnl_register(int protocol, int msgtype,
 		   rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
+int rtnl_register_module(struct module *owner, int protocol, int msgtype,
+			 rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
 int rtnl_unregister(int protocol, int msgtype);
 void rtnl_unregister_all(int protocol);
net/core/rtnetlink.c +81 -22

158 lines not on this page · raw message

Signature · 1 line
2.13.6
#3 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 15 lines · Florian Westphal
net/core/rtnetlink.c +6 -0
@@ -180,6 +164,12 @@ int __rtnl_register(int protocol, int msgtype,
 		rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
 	}
 
+	WARN_ON(tab[msgindex].owner && tab[msgindex].owner != owner);
+
+	tab[msgindex].owner = owner;
+	/* make sure owner is always visible first */
+	smp_wmb();
+
 	if (doit)
 		tab[msgindex].doit = doit;
 	if (dumpit)

net/core/rtnetlink.c int rtnl_unregister(int protocol, int msgtype) · line 287

Quoted 9 lines
net/core/rtnetlink.c +3 -0
@@ -235,6 +279,9 @@ int rtnl_unregister(int protocol, int msgtype)
 	handlers[msgindex].doit = NULL;
 	handlers[msgindex].dumpit = NULL;
 	handlers[msgindex].flags = 0;
+	/* make sure we clear owner last */
+	smp_wmb();
+	handlers[msgindex].owner = NULL;
 	rtnl_unlock();
 
 	return 0;

These wmb()'s don't make sense; and the comments are incomplete. What do they pair with? Who cares about this ordering?

author
#4 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 24 lines · Peter Zijlstra
Quoted 23 lines · Florian Westphal
net/core/rtnetlink.c +9 -0
@@ -180,6 +164,12 @@ int __rtnl_register(int protocol, int msgtype,
 		rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
 	}
 
+	WARN_ON(tab[msgindex].owner && tab[msgindex].owner != owner);
+
+	tab[msgindex].owner = owner;
+	/* make sure owner is always visible first */
+	smp_wmb();
+
 	if (doit)
 		tab[msgindex].doit = doit;
 	if (dumpit)

@@ -235,6 +279,9 @@ int rtnl_unregister(int protocol, int msgtype)
 	handlers[msgindex].doit = NULL;
 	handlers[msgindex].dumpit = NULL;
 	handlers[msgindex].flags = 0;
+	/* make sure we clear owner last */
+	smp_wmb();
+	handlers[msgindex].owner = NULL;
 	rtnl_unlock();
Quoted 4 lines

Review int rtnl_unregister(int protocol, int msgtype) · line 287

Quoted 1 line
(quoted diff) +0 -0
 	return 0;

These wmb()'s don't make sense; and the comments are incomplete. What do they pair with? Who cares about this ordering?

rtnetlink_rcv_msg:

4406 dumpit = READ_ONCE(handlers[type].dumpit);
4407 if (!dumpit)
4408 goto err_unlock;
4409 owner = READ_ONCE(handlers[type].owner);
4410 }
..
4417 if (!try_module_get(owner))
4418 err = -EPROTONOSUPPORT;
4419

I don't want dumpit function address to be visible before owner. Does that make sense?

#5 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 30 lines · Florian Westphal
Quoted 24 lines · Peter Zijlstra
Quoted 23 lines · Florian Westphal
net/core/rtnetlink.c +9 -0
@@ -180,6 +164,12 @@ int __rtnl_register(int protocol, int msgtype,
 		rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
 	}
 
+	WARN_ON(tab[msgindex].owner && tab[msgindex].owner != owner);
+
+	tab[msgindex].owner = owner;
+	/* make sure owner is always visible first */
+	smp_wmb();
+
 	if (doit)
 		tab[msgindex].doit = doit;
 	if (dumpit)

@@ -235,6 +279,9 @@ int rtnl_unregister(int protocol, int msgtype)
 	handlers[msgindex].doit = NULL;
 	handlers[msgindex].dumpit = NULL;
 	handlers[msgindex].flags = 0;
+	/* make sure we clear owner last */
+	smp_wmb();
+	handlers[msgindex].owner = NULL;
 	rtnl_unlock();
Quoted 4 lines

Review int rtnl_unregister(int protocol, int msgtype) · line 287

Quoted 1 line
(quoted diff) +0 -0
 	return 0;

These wmb()'s don't make sense; and the comments are incomplete. What do they pair with? Who cares about this ordering?

Quoted 6 lines

rtnetlink_rcv_msg:

4406 dumpit = READ_ONCE(handlers[type].dumpit);
4407 if (!dumpit)
4408 goto err_unlock;
4409 owner = READ_ONCE(handlers[type].owner);

So what stops the CPU from hoisting this load before the dumpit load?

Quoted 8 lines

4410 }
..
4417 if (!try_module_get(owner))
4418 err = -EPROTONOSUPPORT;
4419

I don't want dumpit function address to be visible before owner. Does that make sense?

And no. That's insane, how can it ever observe an incomplete tab in the first place.

The problem is that __rtnl_register() and rtnl_unregister are broken.

__rtnl_register() publishes the tab before it initializes it; allowing people to observe the thing incomplete.

Also, are we required to hold rtnl_lock() across __rtnl_register()? I'd hope so, otherwise what stops concurrent allocations and leaking of tab?

Also, rtnl_register() doesn't seen to employ rtnl_lock() and panic() WTF?!

rtnl_unregister() should then RCU free the tab.

None of that is happening, so what is that RCU stuff supposed to do?

#6 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 60 lines · Peter Zijlstra
Quoted 30 lines · Florian Westphal
Quoted 24 lines · Peter Zijlstra
Quoted 23 lines · Florian Westphal
(quoted diff) +9 -0
@@ -180,6 +164,12 @@ int __rtnl_register(int protocol, int msgtype,
 		rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
 	}
 
+	WARN_ON(tab[msgindex].owner && tab[msgindex].owner != owner);
+
+	tab[msgindex].owner = owner;
+	/* make sure owner is always visible first */
+	smp_wmb();
+
 	if (doit)
 		tab[msgindex].doit = doit;
 	if (dumpit)

@@ -235,6 +279,9 @@ int rtnl_unregister(int protocol, int msgtype)
 	handlers[msgindex].doit = NULL;
 	handlers[msgindex].dumpit = NULL;
 	handlers[msgindex].flags = 0;
+	/* make sure we clear owner last */
+	smp_wmb();
+	handlers[msgindex].owner = NULL;
 	rtnl_unlock();
Quoted 4 lines

Review int rtnl_unregister(int protocol, int msgtype) · line 287

Quoted 1 line
(quoted diff) +0 -0
 	return 0;

These wmb()'s don't make sense; and the comments are incomplete. What do they pair with? Who cares about this ordering?

Quoted 6 lines

rtnetlink_rcv_msg:

4406 dumpit = READ_ONCE(handlers[type].dumpit);
4407 if (!dumpit)
4408 goto err_unlock;
4409 owner = READ_ONCE(handlers[type].owner);

So what stops the CPU from hoisting this load before the dumpit load?

Quoted 8 lines

4410 }
..
4417 if (!try_module_get(owner))
4418 err = -EPROTONOSUPPORT;
4419

I don't want dumpit function address to be visible before owner. Does that make sense?

And no. That's insane, how can it ever observe an incomplete tab in the first place.

The problem is that __rtnl_register() and rtnl_unregister are broken.

__rtnl_register() publishes the tab before it initializes it; allowing people to observe the thing incomplete.

Also, are we required to hold rtnl_lock() across __rtnl_register()? I'd hope so, otherwise what stops concurrent allocations and leaking of tab?

Quoted 6 lines

Also, rtnl_register() doesn't seen to employ rtnl_lock() and panic() WTF?!

rtnl_unregister() should then RCU free the tab.

None of that is happening, so what is that RCU stuff supposed to do?

Something like the below would go some way toward sanitizing this stuff; rcu_assign_pointer() is a store-release, meaning it happens after everything coming before.

Therefore, when you observe that tab (through rcu_dereference) you're guaranteed to see the thing initialized. The memory ordering on the consume side is through an address dependency; we need to have completed the load of the tab pointer before we can compute the address of its members and load from there, these are not things a CPU is allowed to reorder (lets forget about Alpha).

Quite possibly, if rtnl_unregister() is called from module unload, this is broken; in that case we'd need something like:

	rcu_assign_pointer(rtnl_msg_handler[protocol], NULL);
	/*
	 * Ensure nobody can still observe our old protocol handler
	 * before continuing to free the module that includes the
	 * functions called from it.
	 */
	synchronize_rcu();

---

net/core/rtnetlink.c +14 -12
index 5ace48926b19..25391c7b9c5d 100644
@@ -63,6 +63,7 @@ struct rtnl_link {
 	rtnl_doit_func		doit;
 	rtnl_dumpit_func	dumpit;
 	unsigned int		flags;
+	struct rcu_head		rcu;
 };
 
 static DEFINE_MUTEX(rtnl_mutex);

@@ -172,14 +173,15 @@ int __rtnl_register(int protocol, int msgtype,
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 	msgindex = rtm_msgindex(msgtype);
 
-	tab = rcu_dereference_raw(rtnl_msg_handlers[protocol]);
-	if (tab == NULL) {
-		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(*tab), GFP_KERNEL);
-		if (tab == NULL)
-			return -ENOBUFS;
+	if (WARN_ONCE(rtnl_msg_handler[protocol],
+		      "Double registration for protocol: %d\n", protcol))
+		return -EEXIST;
 
-		rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
-	}
+	lockdep_assert_held(&rtnl_mutex);
+
+	tab = kcalloc(RTM_NR_MSGTYPES, sizeof(*tab), GFP_KERNEL);
+	if (tab == NULL)
+		return -ENOBUFS;
 
 	if (doit)
 		tab[msgindex].doit = doit;

@@ -187,6 +189,8 @@ int __rtnl_register(int protocol, int msgtype,
 		tab[msgindex].dumpit = dumpit;
 	tab[msgindex].flags |= flags;
 
+	rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__rtnl_register);

@@ -227,15 +231,13 @@ int rtnl_unregister(int protocol, int msgtype)
 	msgindex = rtm_msgindex(msgtype);
 
 	rtnl_lock();
-	handlers = rtnl_dereference(rtnl_msg_handlers[protocol]);
+	handlers = rtnl_msg_handlers[protocol];
 	if (!handlers) {
 		rtnl_unlock();
 		return -ENOENT;
 	}
-
-	handlers[msgindex].doit = NULL;
-	handlers[msgindex].dumpit = NULL;
-	handlers[msgindex].flags = 0;
+	rcu_assign_pointer(rtnl_msg_handler[protocol], NULL);
+	kfree_rcu(handlers, rcu);
 	rtnl_unlock();
 
 	return 0;
author
#7 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 38 lines · Peter Zijlstra

Something like the below would go some way toward sanitizing this stuff; rcu_assign_pointer() is a store-release, meaning it happens after everything coming before.

Therefore, when you observe that tab (through rcu_dereference) you're guaranteed to see the thing initialized. The memory ordering on the consume side is through an address dependency; we need to have completed the load of the tab pointer before we can compute the address of its members and load from there, these are not things a CPU is allowed to reorder (lets forget about Alpha).

Quite possibly, if rtnl_unregister() is called from module unload, this is broken; in that case we'd need something like:

	rcu_assign_pointer(rtnl_msg_handler[protocol], NULL);
	/*
	 * Ensure nobody can still observe our old protocol handler
	 * before continuing to free the module that includes the
	 * functions called from it.
	 */
	synchronize_rcu();

---

net/core/rtnetlink.c +1 -0
index 5ace48926b19..25391c7b9c5d 100644
@@ -63,6 +63,7 @@ struct rtnl_link {
 	rtnl_doit_func		doit;
 	rtnl_dumpit_func	dumpit;
 	unsigned int		flags;
+	struct rcu_head		rcu;
 };
 
 static DEFINE_MUTEX(rtnl_mutex);

@@ -172,14 +173,15 @@ int __rtnl_register(int protocol, int msgtype,
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);

net/core/rtnetlink.c int __rtnl_register(int protocol, int msgtype, · line 178

Quoted 10 lines
net/core/rtnetlink.c +3 -5
 	msgindex = rtm_msgindex(msgtype);
 
-	tab = rcu_dereference_raw(rtnl_msg_handlers[protocol]);
-	if (tab == NULL) {
-		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(*tab), GFP_KERNEL);
-		if (tab == NULL)
-			return -ENOBUFS;
+	if (WARN_ONCE(rtnl_msg_handler[protocol],
+		      "Double registration for protocol: %d\n", protcol))
+		return -EEXIST;

I would expect this to trigger all the time, due to

rtnl_register(AF_INET, RTM_GETROUTE, ...
rtnl_register(AF_INET, RTM_GETADDR, ...

etc.

Quoted 6 lines
net/core/rtnetlink.c +1 -1
@@ -227,15 +231,13 @@ int rtnl_unregister(int protocol, int msgtype)
 	msgindex = rtm_msgindex(msgtype);
 
 	rtnl_lock();
-	handlers = rtnl_dereference(rtnl_msg_handlers[protocol]);
+	handlers = rtnl_msg_handlers[protocol];

net/core/rtnetlink.c int rtnl_unregister(int protocol, int msgtype) · line 240

Quoted 10 lines
net/core/rtnetlink.c +2 -4
 	if (!handlers) {
 		rtnl_unlock();
 		return -ENOENT;
 	}
-
-	handlers[msgindex].doit = NULL;
-	handlers[msgindex].dumpit = NULL;
-	handlers[msgindex].flags = 0;
+	rcu_assign_pointer(rtnl_msg_handler[protocol], NULL);
+	kfree_rcu(handlers, rcu);

This unregisters all handlers of "protocol" instead of "protocol:msgtype".

#8 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 4 lines · Florian Westphal

I would expect this to trigger all the time, due to

rtnl_register(AF_INET, RTM_GETROUTE, ...
rtnl_register(AF_INET, RTM_GETADDR, ...

Ah, sure, then something like so then...

There's bound to be bugs there too, as I pretty much typed this without thinking, but it should show the idea.

---

net/core/rtnetlink.c +83 -55
index 5ace48926b19..de1336775602 100644
@@ -63,6 +63,7 @@ struct rtnl_link {
 	rtnl_doit_func		doit;
 	rtnl_dumpit_func	dumpit;
 	unsigned int		flags;
+	struct rcu_head		rcu;
 };
 
 static DEFINE_MUTEX(rtnl_mutex);

@@ -127,7 +128,7 @@ bool lockdep_rtnl_is_held(void)
 EXPORT_SYMBOL(lockdep_rtnl_is_held);
 #endif /* #ifdef CONFIG_PROVE_LOCKING */
 
-static struct rtnl_link __rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
+static struct rtnl_link __rcu **rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
 static refcount_t rtnl_msg_handlers_ref[RTNL_FAMILY_MAX + 1];
 
 static inline int rtm_msgindex(int msgtype)

@@ -144,6 +145,23 @@ static inline int rtm_msgindex(int msgtype)
 	return msgindex;
 }
 
+static struct rtnl_link *rtnl_get_link(int protocol, int msgtype)
+{
+	struct rtnl_link **tab;
+
+	if (protocol >= ARRAY_SIZE(rtnl_msg_handlers))
+		protocol = PF_UNSPEC;
+
+	tab = rcu_dereference(rtnl_msg_handlers[protocol]);
+	if (!tab) {
+		tab = rcu_dereference(rtnl_msg_handlers[PF_UNSPECl]);
+		if (!tab)
+			return NULL;
+	}
+
+	return rcu_dereference(handlers[rtm_msgindex(msgtype)]);
+}
+
 /**
  * __rtnl_register - Register a rtnetlink message type
  * @protocol: Protocol family or PF_UNSPEC

@@ -166,28 +184,39 @@ int __rtnl_register(int protocol, int msgtype,
 		    rtnl_doit_func doit, rtnl_dumpit_func dumpit,
 		    unsigned int flags)
 {
-	struct rtnl_link *tab;
+	struct rtnl_link **tab, *link;
 	int msgindex;
+	int ret = -ENOBUFS;
 
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 	msgindex = rtm_msgindex(msgtype);
 
-	tab = rcu_dereference_raw(rtnl_msg_handlers[protocol]);
+	rtnl_lock();
+	tab = rtnl_msg_handlers[protocol];
 	if (tab == NULL) {
-		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(*tab), GFP_KERNEL);
-		if (tab == NULL)
-			return -ENOBUFS;
+		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
+		if (!tab)
+			goto unlock;
 
+		/* ensures we see the 0 stores */
 		rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
 	}
 
-	if (doit)
-		tab[msgindex].doit = doit;
-	if (dumpit)
-		tab[msgindex].dumpit = dumpit;
-	tab[msgindex].flags |= flags;
+	WARN_ONCE(tab[msgindex], "Double allocated rtnl(%d:%d)\n", protocol, msgtype);
+	link = kzalloc(sizeof(struct rtnl_link), GFP_KERNEL);
+	if (!link)
+		goto unlock;
 
-	return 0;
+	link->doit = doit;
+	link->dumpit = dumpit;
+	link->flags |= flags;
+	/* publish protocol:msgtype */
+	rcu_assign_pointer(tab[msgindex], link);
+	ret = 0;
+unlock:
+	rtnl_unlock();
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(__rtnl_register);
 

@@ -220,22 +249,22 @@ EXPORT_SYMBOL_GPL(rtnl_register);
  */
 int rtnl_unregister(int protocol, int msgtype)
 {
-	struct rtnl_link *handlers;
+	struct rtnl_link **tab, *link;
 	int msgindex;
 
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 	msgindex = rtm_msgindex(msgtype);
 
 	rtnl_lock();
-	handlers = rtnl_dereference(rtnl_msg_handlers[protocol]);
-	if (!handlers) {
+	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
+	if (!tab) {
 		rtnl_unlock();
 		return -ENOENT;
 	}
 
-	handlers[msgindex].doit = NULL;
-	handlers[msgindex].dumpit = NULL;
-	handlers[msgindex].flags = 0;
+	link = tab[msgindex];
+	rcu_assign_pointer(tab[msgindex], NULL);
+	kfree_rcu(link, rcu);
 	rtnl_unlock();
 
 	return 0;

@@ -251,13 +280,22 @@ EXPORT_SYMBOL_GPL(rtnl_unregister);
  */
 void rtnl_unregister_all(int protocol)
 {
-	struct rtnl_link *handlers;
+	struct rtnl_link **tab, *link;
+	int msgindex;
 
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 
 	rtnl_lock();
-	handlers = rtnl_dereference(rtnl_msg_handlers[protocol]);
+	tab = rtnl_msg_handlers[protocol];
 	RCU_INIT_POINTER(rtnl_msg_handlers[protocol], NULL);
+	for (msgindex = 0; msgindex < RTM_NR_MSGTYPES; msgindex++) {
+		link = tab[msgindex];
+		if (!link)
+			continue;
+
+		rcu_assign_pointer(tab[msgindex], NULL);
+		kfree_rcu(link, rcu);
+	}
 	rtnl_unlock();
 
 	synchronize_net();

@@ -2830,17 +2868,17 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 
 	for (idx = 1; idx <= RTNL_FAMILY_MAX; idx++) {
 		int type = cb->nlh->nlmsg_type-RTM_BASE;
-		struct rtnl_link *handlers;
+		struct rtnl_link *link;
 		rtnl_dumpit_func dumpit;
 
 		if (idx < s_idx || idx == PF_PACKET)
 			continue;
 
-		handlers = rtnl_dereference(rtnl_msg_handlers[idx]);
-		if (!handlers)
+		link = rtnl_get_link(idx, type);
+		if (!link)
 			continue;
 
-		dumpit = READ_ONCE(handlers[type].dumpit);
+		dumpit = link->dumpit;
 		if (!dumpit)
 			continue;
 

@@ -4155,7 +4193,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 			     struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(skb->sk);
-	struct rtnl_link *handlers;
+	struct rtnl_link *link;
 	int err = -EOPNOTSUPP;
 	rtnl_doit_func doit;
 	unsigned int flags;

@@ -4179,32 +4217,19 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (kind != 2 && !netlink_net_capable(skb, CAP_NET_ADMIN))
 		return -EPERM;
 
-	if (family >= ARRAY_SIZE(rtnl_msg_handlers))
-		family = PF_UNSPEC;
-
 	rcu_read_lock();
-	handlers = rcu_dereference(rtnl_msg_handlers[family]);
-	if (!handlers) {
-		family = PF_UNSPEC;
-		handlers = rcu_dereference(rtnl_msg_handlers[family]);
-	}
-
 	if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
 		struct sock *rtnl;
 		rtnl_dumpit_func dumpit;
 		u16 min_dump_alloc = 0;
 
-		dumpit = READ_ONCE(handlers[type].dumpit);
-		if (!dumpit) {
-			family = PF_UNSPEC;
-			handlers = rcu_dereference(rtnl_msg_handlers[PF_UNSPEC]);
-			if (!handlers)
-				goto err_unlock;
-
-			dumpit = READ_ONCE(handlers[type].dumpit);
-			if (!dumpit)
+		link = rtnl_get_link(family, type);
+		if (!link || !link->dumpit) {
+			link = rtnl_get_link(PF_UNSPEC, type);
+			if (!link || !link->dumpit)
 				goto err_unlock;
 		}
+		dumpit = link->dumpit;
 
 		refcount_inc(&rtnl_msg_handlers_ref[family]);
 

@@ -4225,33 +4250,36 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return err;
 	}
 
-	doit = READ_ONCE(handlers[type].doit);
-	if (!doit) {
+	link = rtnl_get_link(family, type);
+	if (!link || !link->doit) {
 		family = PF_UNSPEC;
-		handlers = rcu_dereference(rtnl_msg_handlers[family]);
+		link = rtnl_get_link(PF_UNSPEC, type);
+		if (!link || !link->doit)
+			goto out_unlock;
 	}
 
-	flags = READ_ONCE(handlers[type].flags);
+	flags = link->flags;
 	if (flags & RTNL_FLAG_DOIT_UNLOCKED) {
 		refcount_inc(&rtnl_msg_handlers_ref[family]);
-		doit = READ_ONCE(handlers[type].doit);
+		doit = link->oit;
 		rcu_read_unlock();
 		if (doit)
 			err = doit(skb, nlh, extack);
 		refcount_dec(&rtnl_msg_handlers_ref[family]);
 		return err;
 	}
-
 	rcu_read_unlock();
 
 	rtnl_lock();
-	handlers = rtnl_dereference(rtnl_msg_handlers[family]);
-	if (handlers) {
-		doit = READ_ONCE(handlers[type].doit);
-		if (doit)
-			err = doit(skb, nlh, extack);
-	}
+	link = rtnl_get_link(family, type);
+	if (link && link->doit)
+		err = link->doit(skb, nlh, extack);
 	rtnl_unlock();
+
+	return err;
+
+out_unlock:
+	rcu_read_unlock();
 	return err;
 
 err_unlock:
#9 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Review

Quoted 2 lines · On Tue, 7 Nov 2017 15:57:36 +0100 Peter Zijlstra <[email protected]> wrote:
(quoted diff) +2 -0
+	link = tab[msgindex];
+	rcu_assign_pointer(tab[msgindex], NULL);

It is not necessary (or useful) to use rcu_assign_pointer with NULL. The rcu_assign_pointer used to special case NULL but doesn't in recent years.

author
#10 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

In reply to Peter Zijlstra
Quoted 10 lines · Peter Zijlstra
Quoted 4 lines · Florian Westphal

I would expect this to trigger all the time, due to

rtnl_register(AF_INET, RTM_GETROUTE, ...
rtnl_register(AF_INET, RTM_GETADDR, ...

Ah, sure, then something like so then...

There's bound to be bugs there too, as I pretty much typed this without thinking, but it should show the idea.

Just o let you know, I am backlogged at the moment so I Will not have time to work on this for the time being.

net/core/rtnetlink.c struct rtnl_link { · line 67

Quoted 11 lines

---

net/core/rtnetlink.c +1 -0
index 5ace48926b19..de1336775602 100644
@@ -63,6 +63,7 @@ struct rtnl_link {
 	rtnl_doit_func		doit;
 	rtnl_dumpit_func	dumpit;
 	unsigned int		flags;
+	struct rcu_head		rcu;
 };

This will need to be split:

struct rtnl_link {
	rtnl_doit_func          doit;
	unsigned int		flags;
	struct rcu_head		rcu;
};
struct rtnl_link_dump {
  	rtnl_dumpit_func	dumpit;
	struct rcu_head		rcu;
};

net/core/rtnetlink.c bool lockdep_rtnl_is_held(void) · line 131

Quoted 2 lines
net/core/rtnetlink.c +1 -1
-static struct rtnl_link __rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
+static struct rtnl_link __rcu **rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];

So this will need to be two arrays.

Reason is that some places do this:

rtnl_register(pf, RTM_FOO, doit, NULL, 0);
rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

(from different call sites in the stack).

net/core/rtnetlink.c int __rtnl_register(int protocol, int msgtype, · line 187

Quoted 4 lines
net/core/rtnetlink.c +0 -4
-	if (doit)
-		tab[msgindex].doit = doit;
-	if (dumpit)
-		tab[msgindex].dumpit = dumpit;

Which is the reason for these if () tests.

#11 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 4 lines · Florian Westphal

Reason is that some places do this:

rtnl_register(pf, RTM_FOO, doit, NULL, 0);
rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

Sure, however,

Quoted 7 lines

(from different call sites in the stack).

net/core/rtnetlink.c int __rtnl_register(int protocol, int msgtype, · line 187

Quoted 4 lines
net/core/rtnetlink.c +0 -4
-	if (doit)
-		tab[msgindex].doit = doit;
-	if (dumpit)
-		tab[msgindex].dumpit = dumpit;

Which is the reason for these if () tests.

then we assign NULL, which is fine, no?

author
#12 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Quoted 9 lines · Peter Zijlstra
Quoted 4 lines · Florian Westphal

Reason is that some places do this:

rtnl_register(pf, RTM_FOO, doit, NULL, 0);
rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

Sure, however,

Quoted 1 line

(from different call sites in the stack).

Quoted 8 lines
Quoted 6 lines

net/core/rtnetlink.c int __rtnl_register(int protocol, int msgtype, · line 187

Quoted 4 lines
net/core/rtnetlink.c +0 -4
-	if (doit)
-		tab[msgindex].doit = doit;
-	if (dumpit)
-		tab[msgindex].dumpit = dumpit;

Which is the reason for these if () tests.

then we assign NULL, which is fine, no?

I meant that
1) rtnl_register(pf, RTM_FOO, doit, NULL, 0);
2) rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

2) overwrites doit() back to NULL.

(it doesn't at the moment due to if() guard quoted above).

We could not do this split, and keep the if () around.

But then we change a member of the link array after it has been published via rcu_assign_pointer.

AFAIU this is exactly what you want to avoid with this patch.

author
#13 lore raw

Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

In reply to Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines

rtnetlink_rcv_msg:

4406 dumpit = READ_ONCE(handlers[type].dumpit);
4407 if (!dumpit)
4408 goto err_unlock;
4409 owner = READ_ONCE(handlers[type].owner);

So what stops the CPU from hoisting this load before the dumpit load?

I was under impression READ_ONCE also includes rmb but I see i was wrong.

Quoted 6 lines
Quoted 2 lines

I don't want dumpit function address to be visible before owner. Does that make sense?

And no. That's insane, how can it ever observe an incomplete tab in the first place.

Quoted 7 lines

The problem is that __rtnl_register() and rtnl_unregister are broken.

__rtnl_register() publishes the tab before it initializes it; allowing people to observe the thing incomplete.

Also, are we required to hold rtnl_lock() across __rtnl_register()? I'd hope so, otherwise what stops concurrent allocations and leaking of tab?

I don't think these ever acquired rtnl mutex.
Hostorically the rtnl callbacks were statically allocated and only ran from initcalls.

Use of of kmalloc came later, and then use in modules.

Quoted 1 line

rtnl_unregister() should then RCU free the tab.

I do not think that will work since that will make it behave like rtnl_unregister_all(), i.e. removes all callbacks of the family.

Quoted 1 line

None of that is happening, so what is that RCU stuff supposed to do?

Its supposed to delay rmmod until all places that are still executing a registered callback are done.

author
#14 lore raw

PATCH 3/8 qtr: use rtnl_register_module

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 net/qrtr/qrtr.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
net/qrtr/qrtr.c +6 -2
index e458ece96d3d..5098625469e0 100644
@@ -1116,9 +1116,13 @@ static int __init qrtr_proto_init(void)
 		return rc;
 	}
 
-	rtnl_register(PF_QIPCRTR, RTM_NEWADDR, qrtr_addr_doit, NULL, 0);
+	rc = rtnl_register_module(THIS_MODULE, PF_QIPCRTR, RTM_NEWADDR, qrtr_addr_doit, NULL, 0);
+	if (rc) {
+		sock_unregister(qrtr_family.family);
+		proto_unregister(&qrtr_proto);
+	}
 
-	return 0;
+	return rc;
 }
 module_init(qrtr_proto_init);
Signature · 1 line
2.13.6
author
#15 lore raw

PATCH 4/8 bridge: use rtnl_register_module

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 net/bridge/br_mdb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/bridge/br_mdb.c +3 -3
index 31ddff22563e..f56eb480abb2 100644
@@ -714,9 +714,9 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 void br_mdb_init(void)
 {
-	rtnl_register(PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, 0);
-	rtnl_register(PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, 0);
-	rtnl_register(PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, 0);
+	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, 0);
 }
 
 void br_mdb_uninit(void)
Signature · 1 line
2.13.6
author
#16 lore raw

PATCH 5/8 can: use rtnl_register_module

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 net/can/gw.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
net/can/gw.c +10 -4
index 73a02af4b5d7..398dd0395ad9 100644
@@ -1014,6 +1014,8 @@ static struct pernet_operations cangw_pernet_ops = {
 
 static __init int cgw_module_init(void)
 {
+	int ret;
+
 	/* sanitize given module parameter */
 	max_hops = clamp_t(unsigned int, max_hops, CGW_MIN_HOPS, CGW_MAX_HOPS);
 

@@ -1031,15 +1033,19 @@ static __init int cgw_module_init(void)
 	notifier.notifier_call = cgw_notifier;
 	register_netdevice_notifier(&notifier);
 
-	if (__rtnl_register(PF_CAN, RTM_GETROUTE, NULL, cgw_dump_jobs, 0)) {
+	ret = rtnl_register_module(THIS_MODULE, PF_CAN, RTM_GETROUTE,
+				   NULL, cgw_dump_jobs, 0);
+	if (ret) {
 		unregister_netdevice_notifier(&notifier);
 		kmem_cache_destroy(cgw_cache);
 		return -ENOBUFS;
 	}
 
-	/* Only the first call to __rtnl_register can fail */
-	__rtnl_register(PF_CAN, RTM_NEWROUTE, cgw_create_job, NULL, 0);
-	__rtnl_register(PF_CAN, RTM_DELROUTE, cgw_remove_job, NULL, 0);
+	/* Only the first call to rtnl_register_module can fail */
+	rtnl_register_module(THIS_MODULE, PF_CAN, RTM_NEWROUTE,
+			     cgw_create_job, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_CAN, RTM_DELROUTE,
+			     cgw_remove_job, NULL, 0);
 
 	return 0;
 }
Signature · 1 line
2.13.6
author
#17 lore raw

PATCH 6/8 decnet: use rtnl_register_module

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 net/decnet/dn_dev.c   | 9 ++++++---
 net/decnet/dn_fib.c   | 6 ++++--
 net/decnet/dn_route.c | 8 ++++----
 3 files changed, 14 insertions(+), 9 deletions(-)
net/decnet/dn_dev.c +6 -3
index 9153247dad28..d1885cf59319 100644
@@ -1418,9 +1418,12 @@ void __init dn_dev_init(void)
 
 	dn_dev_devices_on();
 
-	rtnl_register(PF_DECnet, RTM_NEWADDR, dn_nl_newaddr, NULL, 0);
-	rtnl_register(PF_DECnet, RTM_DELADDR, dn_nl_deladdr, NULL, 0);
-	rtnl_register(PF_DECnet, RTM_GETADDR, NULL, dn_nl_dump_ifaddr, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_NEWADDR,
+			     dn_nl_newaddr, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_DELADDR,
+			     dn_nl_deladdr, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETADDR,
+			     NULL, dn_nl_dump_ifaddr, 0);
 
 	proc_create("decnet_dev", S_IRUGO, init_net.proc_net, &dn_dev_seq_fops);
net/decnet/dn_fib.c +4 -2
index b37a1b833c77..fce94cbd4378 100644
@@ -792,8 +792,10 @@ void __init dn_fib_init(void)
 
 	register_dnaddr_notifier(&dn_fib_dnaddr_notifier);
 
-	rtnl_register(PF_DECnet, RTM_NEWROUTE, dn_fib_rtm_newroute, NULL, 0);
-	rtnl_register(PF_DECnet, RTM_DELROUTE, dn_fib_rtm_delroute, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_NEWROUTE,
+			     dn_fib_rtm_newroute, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_DELROUTE,
+			     dn_fib_rtm_delroute, NULL, 0);
 }
net/decnet/dn_route.c +4 -4
index bff5ab88cdbb..27bf6e5dc8d9 100644
@@ -1921,11 +1921,11 @@ void __init dn_route_init(void)
 		    &dn_rt_cache_seq_fops);
 
 #ifdef CONFIG_DECNET_ROUTER
-	rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute,
-		      dn_fib_dump, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETROUTE,
+			     dn_cache_getroute, dn_fib_dump, 0);
 #else
-	rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute,
-		      dn_cache_dump, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETROUTE,
+			     dn_cache_getroute, dn_cache_dump, 0);
 #endif
 }
Signature · 1 line
2.13.6
author
#18 lore raw

PATCH 7/8 phonet: use rtnl_register_module

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 net/phonet/pn_netlink.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
net/phonet/pn_netlink.c +13 -8
index da754fc926e7..871eaf2cb85e 100644
@@ -299,16 +299,21 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 
 int __init phonet_netlink_register(void)
 {
-	int err = __rtnl_register(PF_PHONET, RTM_NEWADDR, addr_doit,
-				  NULL, 0);
+	int err = rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWADDR,
+				       addr_doit, NULL, 0);
 	if (err)
 		return err;
 
-	/* Further __rtnl_register() cannot fail */
-	__rtnl_register(PF_PHONET, RTM_DELADDR, addr_doit, NULL, 0);
-	__rtnl_register(PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, 0);
-	__rtnl_register(PF_PHONET, RTM_NEWROUTE, route_doit, NULL, 0);
-	__rtnl_register(PF_PHONET, RTM_DELROUTE, route_doit, NULL, 0);
-	__rtnl_register(PF_PHONET, RTM_GETROUTE, NULL, route_dumpit, 0);
+	/* Further rtnl_register_module() cannot fail */
+	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELADDR,
+			     addr_doit, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETADDR,
+			     NULL, getaddr_dumpit, 0);
+	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWROUTE,
+			     route_doit, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELROUTE,
+			     route_doit, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETROUTE,
+			     NULL, route_dumpit, 0);
 	return 0;
 }
Signature · 1 line
2.13.6
author
#19 lore raw

PATCH 8/8 mpls: use rtnl_register_module

In reply to Florian Westphal
Signed-off-by Florian Westphal <[email protected]>
---
 net/mpls/af_mpls.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
net/mpls/af_mpls.c +9 -6
index 8ca9915befc8..5dce8336d33f 100644
@@ -2510,12 +2510,15 @@ static int __init mpls_init(void)
 
 	rtnl_af_register(&mpls_af_ops);
 
-	rtnl_register(PF_MPLS, RTM_NEWROUTE, mpls_rtm_newroute, NULL, 0);
-	rtnl_register(PF_MPLS, RTM_DELROUTE, mpls_rtm_delroute, NULL, 0);
-	rtnl_register(PF_MPLS, RTM_GETROUTE, mpls_getroute, mpls_dump_routes,
-		      0);
-	rtnl_register(PF_MPLS, RTM_GETNETCONF, mpls_netconf_get_devconf,
-		      mpls_netconf_dump_devconf, 0);
+	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_NEWROUTE,
+			     mpls_rtm_newroute, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_DELROUTE,
+			     mpls_rtm_delroute, NULL, 0);
+	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_GETROUTE,
+			     mpls_getroute, mpls_dump_routes, 0);
+	rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_GETNETCONF,
+			     mpls_netconf_get_devconf,
+			     mpls_netconf_dump_devconf, 0);
 	err = ipgre_tunnel_encap_add_mpls_ops();
 	if (err)
 		pr_err("Can't add mpls over gre tunnel ops\n");
Signature · 1 line
2.13.6