Re: [PATCH nft 2/2] json: drop handling missing json() hook for "struct expr_ops"

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Thomas,

On Fri, Nov 03, 2023 at 09:45:38AM +0100, Thomas Haller wrote:
> On Thu, 2023-11-02 at 21:51 +0100, Pablo Neira Ayuso wrote:
> > On Thu, Nov 02, 2023 at 05:17:56PM +0100, Thomas Haller wrote:
> > 
> > 
> > Yes, chain statement is lacking a json output, that is correct, that
> > needs to be done.
> 
> What is the correct JSON syntax for printing a chain?

There is currently no syntax, so this needs to be defined.

> For example, for test "tests/shell/testcases/nft-f/sample-ruleset" I
> get the following from `nft -j list ruleset`:
> 
>     [...]
>     {
>       "rule": {
>         "family": "inet",
>         "table": "filter",
>         "chain": "home_input",
>         "handle": 91,
>         "expr": [
>           {
>             "match": {
>               "op": "==",
>               "left": {
>                 "meta": {
>                   "key": "l4proto"
>                 }
>               },
>               "right": {
>                 "set": [
>                   "tcp",
>                   "udp"
>                 ]
>               }
>             }
>           },
>           {
>             "match": {
>               "op": "==",
>               "left": {
>                 "payload": {
>                   "protocol": "th",
>                   "field": "dport"
>                 }
>               },
>               "right": 53
>             }
>           },
>           "jump {\n\t\t\tip6 saddr != { fd00::/8, fe80::/64 } counter packets 0 bytes 0 reject with icmpv6 port-unreachable\n\t\t\taccept\n\t\t}"
>         ]
>       }
>     },
>     [...]
> 
> 
> In `man libnftables-json`, searching for "jump" only gives:
> 
>     { "jump": { "target": * STRING *}}
> 
> 
> Is there an example how this JSON output should look like?
> 
> (or a test, after all, I want to feed this output back into `nft -j --check -f -`).

Maybe something like:

     { "jump": { "chain" : [ rules here ] }

but I would need to sketch some code to explore how complicate this is
to reuse existing JSON code.

> > But, as for variable and symbol expressions, I do not see how those
> > can be found in the 'list ruleset' path. Note that symbol expressions
> > represent a preliminary state of the expression, these type of
> > expressions go away after evaluation. Same thing applies to variable
> > expression. They have no use for listing path.
> 
> ACK about symbol_expr_ops + variable_expr_ops. I will send a minor
> patch about that (essentially with code comments and remove the
> elaborate fallback code).

OK, so it is chain statement that is missing the json callback.

> > Do you have tests that explicitly refer to the lack of json callback
> > for variable and symbol expressions just like in the warning above?
> > 
> > > /tmp/nft-test.latest.thom/test-tests-shell-testcases-chains-
> > > 0041chain_binding_0.4/rc-failed-chkdump:<<<<
> > > 
> > > There are also other failures. e.g.
> > > tests/shell/testcases/parsing/large_rule_pipe does not give stable
> > > output. I need to drop that .json-nft file in v2.
> > 
> > What does 'unstable' mean in this case?
> 
> It seems, that the order of the elements of the list is unstable.

Ah, I see, so it is not easy to compare. Thanks for explaining.

> I didn't investigate. At this point, I only want to add the
> .json-nft files for tests that pass, and worry about the remaining
> issues after the basic test infrastructure about .json-nft tests is
> up.