Re: [nf-next PATCH v3 11/16] netfilter: nf_tables: chain: Respect NETDEV_REGISTER events

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> So it is possible that ops are still used somewhere after
> nf_unregister_net_hook() returns? I don't quite get that code, with all
> the RCU-bells and ONCE-whistles.

Not ops, but other core could still run the hookfn that
is being unregistered, i.e.:

cpu 0
 unreg hookfn                      nf_hook_slow
                                      runs hookfn
if nf_hook() fetched the hook blob (struct nf_hook_entries *)
before unreg path called its rcu_assign_pointer with the updated
incarnation.

That said, this might be a special case as entire nf_hook_entries blob
is tied to device that is going down, so packet flow might have stopped
already.

From a brief glance the device is already disabled at NETDEV_UNREGISTER
time, no packets are flowing anymore.

> > Device is the same, so maybe its enough to update the name
> > in nft_hooks structure?
> 
> You're putting the cart before the horse here: The user sets
> hook->ifname and we bind to whatever device matches that.
> 
> Now with a device being renamed, there are two options:
> 
> A) Unbind if the name doesn't match hook->ifname anymore and search for
>    another, matching hook. This is what I had (tried to) implement.
> 
> B) Just leave the interface in place as long as it exists. This is how
>    the old code behaves.
> 
> For users, I find (A) more intuitive.

Yes, that makes sense to me.  But can't you defer the unbind until after
you've figured out if there is a matching hook or not?

I.e., if no matching new hook, just unreg, else register new/unregister
old.

Otherwise, we might unreg, then fail to re-register?