Hi, On Thu, Oct 31, 2019 at 03:14:52PM +0100, Pablo Neira Ayuso wrote: > On Thu, Oct 31, 2019 at 03:13:14PM +0100, Pablo Neira Ayuso wrote: > > On Wed, Oct 30, 2019 at 06:26:49PM +0100, Phil Sutter wrote: > > [...] > > > Patches 1 to 5 implement required changes and are rather boring by > > > themselves: When converting an nftnl rule to iptables command state, > > > cache access is required (to lookup set references). > > > > nft_handle is passed now all over the place, this allows anyone to > > access all of its content. This layering design was done on purpose, > > to avoid giving access to all information to the callers, instead > > force the developer to give a reason to show why it needs something > > else from wherever he is. I'm not entirely convinced exposing the > > handle everywhere just because you need to access the set cache is the > > way to go. > > In other words: You only need the cache, right? Why don't you just > expose cache to these functions which what you need? When creating a new rule with among match, code needs to call batch_add() to add the NFT_COMPAT_SET_ADD job. So in that direction I don't see an alternative to passing nft_handle around. When parsing a lookup expression, we may get by without having to call __nft_build_cache() as cache might be present already (not sure if I miss something). If not, nft_handle is mandatory - cache update functions access many fields in nft_handle. So when passing cache and builtin_table pointers to rule_to_cs, pure set lookups should be possible without nft_handle access. We need builtin_table pointer to identify the right table array item in cache. With only table name, we need to call nft_table_builtin_find() and that takes nft_handle as well. I could give it a try if you still think it's feasible to keep nft_handle away from nft_xt_ctx. Thanks, Phil
