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

[Florian Westphal <fw@strlen.de>]

Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Nov 07, 2017 at 10:47:51AM +0100, Florian Westphal wrote:
> > 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.

> ---
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 5ace48926b19..de1336775602 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -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;
};

> -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).
> -	if (doit)
> -		tab[msgindex].doit = doit;
> -	if (dumpit)
> -		tab[msgindex].dumpit = dumpit;

Which is the reason for these if () tests.