On Tue, Aug 04, 2020 at 04:20:27PM +0200, Phil Sutter wrote:
> Hi,
>
> On Tue, Aug 04, 2020 at 04:04:54PM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Aug 04, 2020 at 03:44:25PM +0200, Jose M. Guisado wrote:
> > > Hi Phil.
> > >
> > > On 4/8/20 15:14, Phil Sutter wrote:
> > > > Hi,
> > > >
> > > > On Tue, Aug 04, 2020 at 03:05:25PM +0200, Jose M. Guisado wrote:
> > > > > On 4/8/20 14:37, Phil Sutter wrote:
> > > > > > Why not just:
> > > > > >
> > > > > > --- a/src/monitor.c
> > > > > > +++ b/src/monitor.c
> > > > > > @@ -922,8 +922,11 @@ int netlink_echo_callback(const struct nlmsghdr *nlh, void *data)
> > > > > > if (!nft_output_echo(&echo_monh.ctx->nft->output))
> > > > > > return MNL_CB_OK;
> > > > > > - if (nft_output_json(&ctx->nft->output))
> > > > > > - return json_events_cb(nlh, &echo_monh);
> > > > > > + if (nft_output_json(&ctx->nft->output)) {
> > > > > > + if (ctx->nft->json_root)
> > > > > > + return json_events_cb(nlh, &echo_monh);
> > > > > > + echo_monh.format = NFTNL_OUTPUT_JSON;
> > > > > > + }
> > > > > > return netlink_events_cb(nlh, &echo_monh);
> > > > > > }
> > > > > >
> > > > > > At a first glance, this seems to work just fine.
> > > > > >
> > > > > > Cheers, Phil
> > > > >
> > > > > This does not output anything on my machine. This is because json_echo
> > > > > is not initialized before netlink_echo_callback.
> > > >
> > > > Please try my diff above on upstream's master without your changes. In
> > > > the tree I did above changes, no symbol named 'json_echo' exists.
> > > >
> > > > Cheers, Phil
> > >
> > > Just tested it, it works great on my machine. As it outputs the same that
> > > would a running nft monitor.
>
> Thanks for validating.
>
> > > I'm imagining this is preferred if there's no need having the json commands
> > > in the output be wrapped inside list of a single json object with its
> > > metainfo. That's the main difference with your patch.
>
> Yes, 'nft -j monitor' output has always been like this. Given that
> monitor potentially runs for a while and picks up multiple distinct
> ruleset changes, I wonder how it *should* behave.
>
> > If it's not wrapped by the top-level nftables root then this is
> > unparseable.
>
> We could change monitor code to add the wrapping "nftables" object to
> every line printed:
>
> --- a/src/json.c
> +++ b/src/json.c
> @@ -1857,7 +1857,8 @@ int do_command_list_json(struct netlink_ctx *ctx, struct cmd *cmd)
> static void monitor_print_json(struct netlink_mon_handler *monh,
> const char *cmd, json_t *obj)
> {
> - obj = json_pack("{s:o}", cmd, obj);
> + obj = json_pack("{s:[o, {s:o}]}", "nftables",
> + generate_json_metainfo(), cmd, obj);
> json_dumpf(obj, monh->ctx->nft->output.output_fp, 0);
> json_decref(obj);
> }
This is probably fine for the monitor + json.
However, nft --echo --json should provide a consistent output whether
the input comes from a json file or not.
