netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: "Jose M. Guisado Gomez" <guigom@riseup.net>
Cc: netfilter-devel@vger.kernel.org, erig@erig.me, phil@nwl.cc
Subject: Re: [PATCH nft v4] src: enable json echo output when reading native syntax
Date: Tue, 4 Aug 2020 13:05:52 +0200	[thread overview]
Message-ID: <20200804110552.GA18345@salvia> (raw)
In-Reply-To: <20200804103846.58872-1-guigom@riseup.net>

On Tue, Aug 04, 2020 at 12:38:46PM +0200, Jose M. Guisado Gomez wrote:
> This patch fixes a bug in which nft did not print any output when
> specifying --echo and --json and reading nft native syntax.
> 
> This patch respects behavior when input is json, in which the output
> would be the identical input plus the handles.
> 
> Adds a json_echo member inside struct nft_ctx to build and store the json object
> containing the json command objects, the object is built using a mock
> monitor to reuse monitor json code. This json object is only used when
> we are sure we have not read json from input.
> 
> Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=1446
> 
> Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
[...]
> diff --git a/src/monitor.c b/src/monitor.c
> index 3872ebcf..868e31b5 100644
> --- a/src/monitor.c
> +++ b/src/monitor.c
> @@ -221,12 +221,14 @@ static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
>  		if (nft_output_handle(&monh->ctx->nft->output))
>  			nft_mon_print(monh, " # handle %" PRIu64 "",
>  				      t->handle.handle.id);
> +		nft_mon_print(monh, "\n");
>  		break;
>  	case NFTNL_OUTPUT_JSON:
>  		monitor_print_table_json(monh, cmd, t);
> +		if(!nft_output_echo(&monh->ctx->nft->output))
> +			nft_mon_print(monh, "\n");
>  		break;
>  	}
> -	nft_mon_print(monh, "\n");
>  	table_free(t);
>  	nftnl_table_free(nlt);
>  	return MNL_CB_OK;
> @@ -258,12 +260,14 @@ static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type,
>  				      c->handle.chain.name);
>  			break;
>  		}
> +		nft_mon_print(monh, "\n");
>  		break;
>  	case NFTNL_OUTPUT_JSON:
>  		monitor_print_chain_json(monh, cmd, c);
> +		if(!nft_output_echo(&monh->ctx->nft->output))
> +			nft_mon_print(monh, "\n");
>  		break;
>  	}
> -	nft_mon_print(monh, "\n");
>  	chain_free(c);
>  	nftnl_chain_free(nlc);
>  	return MNL_CB_OK;
> @@ -304,12 +308,14 @@ static int netlink_events_set_cb(const struct nlmsghdr *nlh, int type,
>  				      set->handle.set.name);
>  			break;
>  		}
> +		nft_mon_print(monh, "\n");
>  		break;
>  	case NFTNL_OUTPUT_JSON:
>  		monitor_print_set_json(monh, cmd, set);
> +		if(!nft_output_echo(&monh->ctx->nft->output))
> +			nft_mon_print(monh, "\n");
>  		break;
>  	}
> -	nft_mon_print(monh, "\n");
>  	set_free(set);
>  out:
>  	nftnl_set_free(nls);
> @@ -441,6 +447,7 @@ static int netlink_events_setelem_cb(const struct nlmsghdr *nlh, int type,
>  		nft_mon_print(monh, "%s element %s %s %s ",
>  			      cmd, family2str(family), table, setname);
>  		expr_print(dummyset->init, &monh->ctx->nft->output);
> +		nft_mon_print(monh, "\n");
>  		break;
>  	case NFTNL_OUTPUT_JSON:
>  		dummyset->handle.family = family;
> @@ -450,9 +457,10 @@ static int netlink_events_setelem_cb(const struct nlmsghdr *nlh, int type,
>  		/* prevent set_free() from trying to free those */
>  		dummyset->handle.set.name = NULL;
>  		dummyset->handle.table.name = NULL;
> +		if(!nft_output_echo(&monh->ctx->nft->output))
                  ^
nitpick: 'if' is not a function, add space between if and parens.

> +			nft_mon_print(monh, "\n");
>  		break;
>  	}
> -	nft_mon_print(monh, "\n");
>  	set_free(dummyset);
>  out:
>  	nftnl_set_free(nls);
> @@ -492,12 +500,14 @@ static int netlink_events_obj_cb(const struct nlmsghdr *nlh, int type,
>  			       obj->handle.obj.name);
>  			break;
>  		}
> +		nft_mon_print(monh, "\n");
>  		break;
>  	case NFTNL_OUTPUT_JSON:
>  		monitor_print_obj_json(monh, cmd, obj);
> +		if(!nft_output_echo(&monh->ctx->nft->output))
                  ^
same here and everywhere else.

> +			nft_mon_print(monh, "\n");
>  		break;
>  	}
> -	nft_mon_print(monh, "\n");
>  	obj_free(obj);
>  	nftnl_obj_free(nlo);
>  	return MNL_CB_OK;
> @@ -542,12 +552,14 @@ static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
>  				      r->handle.handle.id);
>  			break;
>  		}
> +		nft_mon_print(monh, "\n");
>  		break;
>  	case NFTNL_OUTPUT_JSON:
>  		monitor_print_rule_json(monh, cmd, r);
> +		if(!nft_output_echo(&monh->ctx->nft->output))
> +			nft_mon_print(monh, "\n");
>  		break;
>  	}
> -	nft_mon_print(monh, "\n");
>  	rule_free(r);
>  	nftnl_rule_free(nlr);
>  	return MNL_CB_OK;
> @@ -912,6 +924,8 @@ int netlink_echo_callback(const struct nlmsghdr *nlh, void *data)
>  {
>  	struct netlink_cb_data *nl_cb_data = data;
>  	struct netlink_ctx *ctx = nl_cb_data->nl_ctx;
> +	struct nft_ctx *nft = ctx->nft;
> +
>  	struct netlink_mon_handler echo_monh = {
>  		.format = NFTNL_OUTPUT_DEFAULT,
>  		.ctx = ctx,
> @@ -922,8 +936,15 @@ 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(&nft->output)) {
> +		if (!nft->json_root) {
> +			nft->json_echo = json_array();
> +			if (!nft->json_echo)
> +				memory_allocation_error();
> +			echo_monh.format = NFTNL_OUTPUT_JSON;
> +		} else

Nitpick: Use curly brace '{' here in the else side of the branch for
consistency (even if it's only on single line).

> +			return json_events_cb(nlh, &echo_monh);
> +	}
>  
>  	return netlink_events_cb(nlh, &echo_monh);
>  }
> diff --git a/src/parser_json.c b/src/parser_json.c
> index 59347168..ef33063d 100644
> --- a/src/parser_json.c
> +++ b/src/parser_json.c
> @@ -3884,11 +3884,21 @@ int json_events_cb(const struct nlmsghdr *nlh, struct netlink_mon_handler *monh)
>  
>  void json_print_echo(struct nft_ctx *ctx)
>  {
> -	if (!ctx->json_root)
> -		return;
> -
> -	json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER);
> -	json_cmd_assoc_free();
> -	json_decref(ctx->json_root);
> -	ctx->json_root = NULL;
> +	if (!ctx->json_root) {
> +		if (!ctx->json_echo)
> +			return;
> +		else {
> +			ctx->json_echo = json_pack("{s:o}", "nftables", ctx->json_echo);
> +			json_dumpf(ctx->json_echo, ctx->output.output_fp, JSON_PRESERVE_ORDER);
> +			json_decref(ctx->json_echo);
> +			ctx->json_echo = NULL;
> +			fprintf(ctx->output.output_fp, "\n");
> +			fflush(ctx->output.output_fp);
> +		}
> +	} else {
> +		json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER);
> +		json_cmd_assoc_free();
> +		json_decref(ctx->json_root);
> +		ctx->json_root = NULL;
> +	}

I'd suggest:

void json_print_echo(struct nft_ctx *ctx)
{
	if (!ctx->json_root)
                return;

        if (ctx->json_echo) {
		ctx->json_echo = json_pack("{s:o}", "nftables", ctx->json_echo);
		json_dumpf(ctx->json_echo, ctx->output.output_fp, JSON_PRESERVE_ORDER);
		json_decref(ctx->json_echo);
		ctx->json_echo = NULL;
		fprintf(ctx->output.output_fp, "\n");
		fflush(ctx->output.output_fp);
	} else {
		json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER);
		json_cmd_assoc_free();
		json_decref(ctx->json_root);
		ctx->json_root = NULL;
	}
}

Thanks.

  reply	other threads:[~2020-08-04 11:17 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-30 19:53 [PATCH nft] src: enable output with "nft --echo --json" and nftables syntax Jose M. Guisado Gomez
2020-07-31  0:00 ` [PATCH nft v2 0/1] " Jose M. Guisado Gomez
2020-07-31  0:00 ` [PATCH nft v2 1/1] " Jose M. Guisado Gomez
2020-07-31  9:22   ` Pablo Neira Ayuso
2020-07-31 10:49     ` [PATCH nft v3] " Jose M. Guisado Gomez
2020-08-04 10:38       ` [PATCH nft v4] src: enable json echo output when reading native syntax Jose M. Guisado Gomez
2020-08-04 11:05         ` Pablo Neira Ayuso [this message]
2020-08-04 12:13           ` Jose M. Guisado
2020-08-04 12:15             ` Pablo Neira Ayuso
2020-08-04 12:37         ` Phil Sutter
2020-08-04 13:05           ` Jose M. Guisado
2020-08-04 13:14             ` Phil Sutter
2020-08-04 13:44               ` Jose M. Guisado
2020-08-04 14:04                 ` Pablo Neira Ayuso
2020-08-04 14:17                   ` Pablo Neira Ayuso
2020-08-04 14:20                   ` Phil Sutter
2020-08-04 15:47                     ` Jose M. Guisado
2020-08-04 19:10                     ` Pablo Neira Ayuso
2020-08-05  9:31                       ` Phil Sutter
2020-08-05  9:45                         ` Pablo Neira Ayuso
2020-08-06  7:28                           ` Phil Sutter
2020-08-04 12:57         ` Eric Garver
2020-07-31 12:33     ` [PATCH nft v2 1/1] src: enable output with "nft --echo --json" and nftables syntax Phil Sutter
2020-07-31 12:58       ` Pablo Neira Ayuso
2020-07-31 13:48         ` Phil Sutter
2020-07-31 14:17           ` Eric Garver
2020-07-31 17:19             ` Pablo Neira Ayuso
2020-07-31 18:36               ` Eric Garver
2020-07-31 20:14                 ` Eric Garver
2020-07-31 17:30           ` Pablo Neira Ayuso
2020-08-01  0:02             ` Phil Sutter
2020-08-01 19:27               ` Pablo Neira Ayuso
2020-08-03 12:52                 ` Phil Sutter
2020-08-04 10:20                   ` Jose M. Guisado
2020-08-04 10:32                     ` Phil Sutter

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=20200804110552.GA18345@salvia \
    --to=pablo@netfilter.org \
    --cc=erig@erig.me \
    --cc=guigom@riseup.net \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    /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).