On 8/21/19 12:09 PM, Florian Westphal wrote: > Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx> wrote: >> This patch adds the infrastructure needed for the stateful object update >> support. >> >> Signed-off-by: Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx> >> --- >> include/net/netfilter/nf_tables.h | 6 +++ >> net/netfilter/nf_tables_api.c | 71 ++++++++++++++++++++++++++++--- >> 2 files changed, 70 insertions(+), 7 deletions(-) >> >> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h >> index dc301e3d6739..dc4e32040ea9 100644 >> --- a/include/net/netfilter/nf_tables.h >> +++ b/include/net/netfilter/nf_tables.h >> @@ -1123,6 +1123,9 @@ struct nft_object_ops { >> int (*dump)(struct sk_buff *skb, >> struct nft_object *obj, >> bool reset); >> + int (*update)(const struct nft_ctx *ctx, >> + const struct nlattr *const tb[], >> + struct nft_object *obj); > > maybe adda 'bool commit' argument here. > How is that argument going to be used? If 'commit' is false we should just check that values are fine but not update them? >> + err = obj->ops->update(ctx, (const struct nlattr * const *)tb, obj); > > Then, set it to 'false' here. > You would have to keep 'tb' allocated and place it on the 'trans' > object. > Yes, I agree on updating the object in the commit phase. But I am not sure about how I should place it on 'trans'. Any hints? Thanks :-) I am also writing some userspace shell tests. >> + nft_trans_obj_update(trans) = true; > > nft_trans_obj_update_tb(trans) = tb; > >> - nft_clear(net, nft_trans_obj(trans)); >> - nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans), >> - NFT_MSG_NEWOBJ); >> - nft_trans_destroy(trans); >> + if (nft_trans_obj_update(trans)) { > > nft_trans_obj(trans)->ops->update(&trans->ctx, > nft_trans_obj_update_tb(trans), > nft_trans_obj(trans), > true); > > kfree(nft_trans_obj_update_tb(trans)); > > > Because otherwise we will update objects while we're not yet sure that > we can process/handle the entire batch. > > I think we should, if possible, only update once we've made it to > the commit phase. >