On Fri, Mar 07, 2025 at 08:47:26PM -0800, Stanislav Fomichev wrote: > There is a couple of places from which we can arrive to ndo_setup_tc > with TC_SETUP_BLOCK/TC_SETUP_FT: > - netlink > - netlink notifier > - netdev notifier > > Locking netdev too deep in this call chain seems to be problematic > (especially assuming some/all of the call_netdevice_notifiers > NETDEV_UNREGISTER) might soon be running with the instance lock). > Revert to lockless ndo_setup_tc for TC_SETUP_BLOCK/TC_SETUP_FT. NFT > framework already takes care of most of the locking. Document > the assumptions. > > ndo_setup_tc TC_SETUP_BLOCK > nft_block_offload_cmd > nft_chain_offload_cmd > nft_flow_block_chain > nft_flow_offload_chain > nft_flow_rule_offload_abort > nft_flow_rule_offload_commit > nft_flow_rule_offload_commit > nf_tables_commit > nfnetlink_rcv_batch > nfnetlink_rcv_skb_batch > nfnetlink_rcv > nft_offload_netdev_event > NETDEV_UNREGISTER notifier > > ndo_setup_tc TC_SETUP_FT > nf_flow_table_offload_cmd > nf_flow_table_offload_setup > nft_unregister_flowtable_hook > nft_register_flowtable_net_hooks > nft_flowtable_update > nf_tables_newflowtable > nfnetlink_rcv_batch (.call NFNL_CB_BATCH) > nft_flowtable_update > nf_tables_newflowtable > nft_flowtable_event > nf_tables_flowtable_event > NETDEV_UNREGISTER notifier > __nft_unregister_flowtable_net_hooks > nft_unregister_flowtable_net_hooks > nf_tables_commit > nfnetlink_rcv_batch (.call NFNL_CB_BATCH) > __nf_tables_abort > nf_tables_abort > nfnetlink_rcv_batch > __nft_release_hook > __nft_release_hooks > nf_tables_pre_exit_net -> module unload > nft_rcv_nl_event > netlink_register_notifier (oh boy) > nft_register_flowtable_net_hooks > nft_flowtable_update > nf_tables_newflowtable > nf_tables_newflowtable > > Fixes: c4f0f30b424e ("net: hold netdev instance lock during nft ndo_setup_tc") > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxxx> Thanks Stan, Thinking aloud: the dev_setup_tc() helper checked if ndo_setup_tc is non-NULL, but that is not the case here. But that seems ok because it was also the case prior to the cited commit. Reviewed-by: Simon Horman <horms@xxxxxxxxxx> ...