Re: [iptables PATCH v3 00/12] Implement among match support

[Phil Sutter <phil@nwl.cc>]

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