Hi, On Tue, Nov 19, 2024 at 05:09:17PM +0100, Phil Sutter wrote: [...] > Checking callers of nft_unregister_flowtable_net_hooks(): > > nf_tables_commit() calls it for DELFLOWTABLE, code-paths differ for > flowtable updates or complete deletions: With the latter, > nft_commit_release() calls nf_tables_flowtable_destroy() which does the > UNBIND. So if deleting individual interfaces from an offloaded flowtable > is supported, we may miss the UNBIND there. > > __nf_tables_abort() calls it for NEWFLOWTABLE. The hooks should have > been bound by nf_tables_newflowtable() (or nft_flowtable_update(), > respectively) so this seems like missing UNBIND there. > > Now about __nft_release_hook, I see: > > nf_tables_pre_exit_net > -> __nft_release_hooks > -> __nft_release_hook > > Do we have to UNBIND at netns exit? > > There is also: > > nft_rcv_nl_event > -> __nft_release_hook > > I don't see where hooks of flowtables in owner flag tables are unbound. So I validated these findings by adding printks to BIND and UNBIND calls and performing these actions: - Delete an interface from a flowtable with multiple interfaces - Add a (device to a) flowtable with --check flag - Delete a netns containing a flowtable - In an interactive nft session, create a table with owner flag and flowtable inside, then quit All these cases cause imbalance between BIND and UNBIND calls. Looking at possible fixes, I wonder how things are supposed to be: When deleting a flowtable, nf_tables_commit will unregister hooks (via nf_unregister_net_hook), but not unlink/free them. Then, in nft_commit_release, the UNBIND happens along with unlink/free. Is this the correct process? Namely unregister and wait for RCU grace period before performing UNBIND? Or is this arbitrary and combining unregister with UBIND is OK in all cases? Cheers, Phil
