On Tue, Mar 12, 2024 at 10:26:17PM +0800, Quan Tian wrote: > On Tue, Mar 12, 2024 at 02:01:34PM +0100, Florian Westphal wrote: > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > AFAICS this means that if the table as udata attached, and userspace > > > > makes an update request without a UDATA netlink attribute, we will > > > > delete the existing udata. > > > > > > > > Is that right? > > > > > > > > My question is, should we instead leave the existing udata as-is and not > > > > support removal, only replace? > > > > > > I would leave it in place too if no _USERDATA is specified. > > > > > Sure, I will change it in the proposed way. > > > > One more question is if the memcmp() with old and new udata makes > > > sense considering two consecutive requests for _USERDATA update in one > > > batch. > > > > Great point, any second udata change request in the same batch must fail. > > > > We learned this the hard way with flag updates :( > > Is it the same as two consectutive requests for chain name update and > chain stats update? > > In nf_tables_commit(): > The 1st trans swaps old udata with 1st new udata; > The 2nd trans swaps 1st new udata with 2nd new udata. > > In nft_commit_release(): > The 1st trans frees old udata; > The 2nd trans frees 1st new udata. And abort path simply releases the transaction objects, since udata was left intact in place. > So multiple udata requests in a batch could work? Yes, if rcu is used correctly, that should work fine. But memcmp() needs to be removed.
