On Thu, Sep 12, 2024 at 06:06:39PM +0200, Florian Westphal wrote: > Phil Sutter <phil@xxxxxx> 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. This seems like a relief, but doesn't apply to NETDEV_CHANGENAME case. At least I can't handle it like UNREG && REG - or vice versa, have to consider the above for UNREG. > > > 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. I can't bind a device to multiple flowtables of the same family, so I can't bind first, then unbind. > Otherwise, we might unreg, then fail to re-register? Assuming I can't bind to the new flowtable while still bound to the old one, I can only try to roll back. Which means trying to bind to the old flowtable again in error case which might still fail. I could return NOTIFY_BAD then, but it won't help - the transaction is broken. Cheers, Phil
