On Tue, Nov 21, 2023 at 01:27:44PM +0100, Florian Westphal wrote: > struct nf_flowtable is currently wholly embedded in either nft_flowtable > or tcf_ct_flow_table. > > In order to allow flowtable acceleration via XDP, the XDP program will > need to map struct net_device to struct nf_flowtable. > > To make this work reliably, make a clear separation of the frontend > (nft, tc) and backend (nf_flowtable) representation. > > In this first patch, amke it so nft_flowtable and tcf_ct_flow_table > only store pointers to an nf_flowtable structure. > > The main goal is to have follow patches that allow us to keep the > nf_flowtable structure around for a bit longer (e.g. until after > an rcu grace period has elapesed) when the frontend(s) are tearing the > structures down. > > At this time, things are fine, but when xdp programs might be using > the nf_flowtable structure as well we will need a way to ensure that > no such users exist anymore. > > Right now there is inufficient guarantee: nftables only ensures > that the netfilter hooks are unregistered, and tc only ensures the > tc actions have been removed. > > Any future kfunc might still be called in parallel from an XDP > program. The easies way to resolve this is to let the nf_flowtable > core handle release and module reference counting itself. > > Signed-off-by: Florian Westphal <fw@xxxxxxxxx> ... > @@ -312,24 +313,29 @@ static int tcf_ct_flow_table_get(struct net *net, struct tcf_ct_params *params) > if (err) > goto err_insert; > > - ct_ft->nf_ft.type = &flowtable_ct; > - ct_ft->nf_ft.flags |= NF_FLOWTABLE_HW_OFFLOAD | > - NF_FLOWTABLE_COUNTER; > - err = nf_flow_table_init(&ct_ft->nf_ft); > + ct_ft->nf_ft = kzalloc(sizeof(*ct_ft->nf_ft), GFP_KERNEL); > + if (!ct_ft->nf_ft) > + goto err_alloc; Hi Florian, This branch will cause the function to return err, but err is 0 here. Perhaps it should be set to a negative error value instead? Flagged by Smatch. > + > + ct_ft->nf_ft->type = &flowtable_ct; > + ct_ft->nf_ft->flags |= NF_FLOWTABLE_HW_OFFLOAD | > + NF_FLOWTABLE_COUNTER; > + err = nf_flow_table_init(ct_ft->nf_ft); > if (err) > goto err_init; > - write_pnet(&ct_ft->nf_ft.net, net); > + write_pnet(&ct_ft->nf_ft->net, net); > > __module_get(THIS_MODULE); > out_unlock: > params->ct_ft = ct_ft; > - params->nf_ft = &ct_ft->nf_ft; > + params->nf_ft = ct_ft->nf_ft; > mutex_unlock(&zones_mutex); > > return 0; > > err_init: > rhashtable_remove_fast(&zones_ht, &ct_ft->node, zones_params); > + kfree(ct_ft->nf_ft); > err_insert: > kfree(ct_ft); > err_alloc: ...
