From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [iptables PATCH v3 00/12] Implement among match support
Date: Thu, 31 Oct 2019 16:01:53 +0100 [thread overview]
Message-ID: <20191031150153.GE8531@orbyte.nwl.cc> (raw)
In-Reply-To: <20191031141452.h3hknkc3qze3xm3r@salvia>
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
prev parent reply other threads:[~2019-10-31 15:01 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-30 17:26 [iptables PATCH v3 00/12] Implement among match support Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 01/12] nft: family_ops: Pass nft_handle to 'add' callback Phil Sutter
2019-10-31 14:05 ` Pablo Neira Ayuso
2019-10-30 17:26 ` [iptables PATCH v3 02/12] nft: family_ops: Pass nft_handle to 'rule_find' callback Phil Sutter
2019-10-31 14:05 ` Pablo Neira Ayuso
2019-10-30 17:26 ` [iptables PATCH v3 03/12] nft: family_ops: Pass nft_handle to 'print_rule' callback Phil Sutter
2019-10-31 14:05 ` Pablo Neira Ayuso
2019-10-30 17:26 ` [iptables PATCH v3 04/12] nft: family_ops: Pass nft_handle to 'rule_to_cs' callback Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 05/12] nft: Keep nft_handle pointer in nft_xt_ctx Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 06/12] nft: Eliminate pointless calls to nft_family_ops_lookup() Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 07/12] nft: Introduce NFT_CL_SETS cache level Phil Sutter
2019-10-31 14:04 ` Pablo Neira Ayuso
2019-10-31 14:08 ` Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 08/12] nft: Support NFT_COMPAT_SET_ADD Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 09/12] nft: Bore up nft_parse_payload() Phil Sutter
2019-10-30 17:26 ` [iptables PATCH v3 10/12] nft: Embed rule's table name in nft_xt_ctx Phil Sutter
2019-10-30 17:27 ` [iptables PATCH v3 11/12] nft: Support parsing lookup expression Phil Sutter
2019-10-30 17:27 ` [iptables PATCH v3 12/12] nft: bridge: Rudimental among extension support Phil Sutter
2019-10-31 14:13 ` [iptables PATCH v3 00/12] Implement among match support Pablo Neira Ayuso
2019-10-31 14:14 ` Pablo Neira Ayuso
2019-10-31 15:01 ` Phil Sutter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191031150153.GE8531@orbyte.nwl.cc \
--to=phil@nwl.cc \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).