From: Phil Sutter <phil@nwl.cc>
To: Florian Westphal <fw@strlen.de>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nft 2/5] parser_json: move list_add into json_parse_cmd
Date: Thu, 7 Mar 2024 18:58:50 +0100 [thread overview]
Message-ID: <ZeoAWivnRYDjrpfo@orbyte.nwl.cc> (raw)
In-Reply-To: <20240307164422.GN4420@breakpoint.cc>
On Thu, Mar 07, 2024 at 05:44:22PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > So IIUC, JSON parser will now collapse all new ruleset items into a tree
> > and use the existing nft_cmd_expand() to split things up again. This may
> > impose significant overhead depending on input data (bogus/OpenShift use
> > cases involving many chains maybe) on one hand, on the other might allow
> > for overhead elimination in other cases (e.g. long lists of 'add
> > element' commands for different sets in alternating fashion).
> >
> > We may want to do this for standard syntax as well if the benefits
> > outweigh the downsides. Thus generalize the JSON-specific helpers you
> > wrote for use within bison parser, too?
>
> It tries to do same as bison parser when using nft -f with a standard
> 'list ruleset' input.
>
> A 'batch file' with sequential 'add table x', 'add chain x c' etc.
> does separate 'add' requests. The json parser is supposed to follow
> this, i.e. 'ctx->in_ruleset' is only supoosed to be set when this
> is a json listing, not when some input daemon is feeding independent
> add requests.
>
> > An alternative might be to reorder code in table_print_json_full(),
> > copying what nft_cmd_expand() does for CMD_OBJ_TABLE. AIUI, it should
> > solve the current issue of failing 'nft -j list ruleset | nft -j -f -'
> > for special cases.
>
> Its indeed possible to reorder things but I was not sure if there is
> a simple way to do this.
It seems there is! Taking nft_cmd_expand() as an example, all that's
missing for table_print_json_full() is to move (bare) chain listing
first and later list rules only instead of chain + rules. I have a patch
at hand and am currently tickling the testsuite to get things tested. It
should work though, because what nft_cmd_expand() does is proven to
work.
[...]
> If you prefer to resolve it by sorting the output (input) as needed
> please let me know.
I'm more confident with the reordering as it must work. Your approach is
interesting, but it may fail if e.g. input does not contain the table
(user knows it exists already). Though it may still be of value for
other purposes. Also my "reorder output" approach does not cover for
user-compiled input (although one may call PEBKAC there).
Thanks, Phil
next prev parent reply other threads:[~2024-03-07 17:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-07 12:26 [PATCH nft 0/5] parser_json: fix up transaction ordering Florian Westphal
2024-03-07 12:26 ` [PATCH nft 1/5] parser_json: move some code around Florian Westphal
2024-03-07 12:26 ` [PATCH nft 2/5] parser_json: move list_add into json_parse_cmd Florian Westphal
2024-03-07 14:31 ` Phil Sutter
2024-03-07 15:10 ` Florian Westphal
2024-03-07 15:52 ` Phil Sutter
2024-03-07 16:44 ` Florian Westphal
2024-03-07 17:58 ` Phil Sutter [this message]
2024-03-07 12:26 ` [PATCH nft 3/5] parser_json: add and use CMD_ERR helpers Florian Westphal
2024-03-07 12:26 ` [PATCH nft 4/5] parser_json: defer command allocation to nft_cmd_expand Florian Westphal
2024-03-07 12:26 ` [PATCH nft 5/5] tests: shell: add more json-nft dumps Florian Westphal
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=ZeoAWivnRYDjrpfo@orbyte.nwl.cc \
--to=phil@nwl.cc \
--cc=fw@strlen.de \
--cc=netfilter-devel@vger.kernel.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 read-only IMAP folder(s) and NNTP newsgroup(s).