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

From: Florian Westphal <[email-masked]>
Date: Mon, 13 Nov 2017 08:59:49 +0100

Peter Zijlstra <[email-masked]> wrote:
> On Mon, Nov 13, 2017 at 08:21:59AM +0100, Florian Westphal wrote:
> > 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,
> 
> > (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.
> 
> 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.