From: Pablo Neira Ayuso <pablo@netfilter.org>
To: "Jose M. Guisado Gomez" <guigom@riseup.net>
Cc: netfilter-devel@vger.kernel.org, phil@nwl.cc
Subject: Re: [PATCH nft v2 1/1] src: enable output with "nft --echo --json" and nftables syntax
Date: Fri, 31 Jul 2020 11:22:12 +0200 [thread overview]
Message-ID: <20200731092212.GA1850@salvia> (raw)
In-Reply-To: <20200731000020.4230-2-guigom@riseup.net>
[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]
Hi,
Cc'ing Phil.
On Fri, Jul 31, 2020 at 02:00:22AM +0200, Jose M. Guisado Gomez wrote:
> diff --git a/src/parser_json.c b/src/parser_json.c
> index 59347168..237b6f3e 100644
> --- a/src/parser_json.c
> +++ b/src/parser_json.c
> @@ -3884,11 +3884,15 @@ 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)
> + if (!ctx->json_echo)
> return;
>
> - json_dumpf(ctx->json_root, ctx->output.output_fp, JSON_PRESERVE_ORDER);
> + ctx->json_echo = json_pack("{s:o}", "nftables", ctx->json_echo);
> + json_dumpf(ctx->json_echo, ctx->output.output_fp, JSON_PRESERVE_ORDER);
> + printf("\n");
> json_cmd_assoc_free();
> - json_decref(ctx->json_root);
> - ctx->json_root = NULL;
> + if (ctx->json_echo) {
> + json_decref(ctx->json_echo);
> + ctx->json_echo = NULL;
> + }
I think json_print_echo() should look like this - note I replaced the
printf("\n"); by fprintf. Also remove the if (ctx->json_echo) branch.
void json_print_echo(struct nft_ctx *ctx)
{
if (!ctx->json_echo)
return;
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);
}
Please, include this update. I'm also attaching a patch that you can
squash to your v3 patch.
@Phil, I think the entire assoc code can just go away? Maybe you can also
run firewalld tests to make sure v3 works fine? IIRC that is a heavy user
of --echo and --json.
Thanks.
[-- Attachment #2: remove-json-assoc-code.patch --]
[-- Type: text/x-diff, Size: 4444 bytes --]
diff --git a/src/parser_json.c b/src/parser_json.c
index 237b6f3e1732..66539ef5c13b 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3645,46 +3645,6 @@ static int json_verify_metainfo(struct json_ctx *ctx, json_t *root)
return 0;
}
-struct json_cmd_assoc {
- struct json_cmd_assoc *next;
- const struct cmd *cmd;
- json_t *json;
-};
-
-static struct json_cmd_assoc *json_cmd_list = NULL;
-
-static void json_cmd_assoc_free(void)
-{
- struct json_cmd_assoc *cur;
-
- while (json_cmd_list) {
- cur = json_cmd_list;
- json_cmd_list = cur->next;
- free(cur);
- }
-}
-
-static void json_cmd_assoc_add(json_t *json, const struct cmd *cmd)
-{
- struct json_cmd_assoc *new = xzalloc(sizeof *new);
-
- new->next = json_cmd_list;
- new->json = json;
- new->cmd = cmd;
- json_cmd_list = new;
-}
-
-static json_t *seqnum_to_json(const uint32_t seqnum)
-{
- const struct json_cmd_assoc *cur;
-
- for (cur = json_cmd_list; cur; cur = cur->next) {
- if (cur->cmd->seqnum == seqnum)
- return cur->json;
- }
- return NULL;
-}
-
static int __json_parse(struct json_ctx *ctx)
{
json_t *tmp, *value;
@@ -3729,9 +3689,6 @@ static int __json_parse(struct json_ctx *ctx)
list_add_tail(&cmd->list, &list);
list_splice_tail(&list, ctx->cmds);
-
- if (nft_output_echo(&ctx->nft->output))
- json_cmd_assoc_add(value, cmd);
}
return 0;
@@ -3757,10 +3714,9 @@ int nft_parse_json_buffer(struct nft_ctx *nft, const char *buf,
ret = __json_parse(&ctx);
- if (!nft_output_echo(&nft->output)) {
- json_decref(nft->json_root);
- nft->json_root = NULL;
- }
+ json_decref(nft->json_root);
+ nft->json_root = NULL;
+
return ret;
}
@@ -3792,96 +3748,6 @@ int nft_parse_json_filename(struct nft_ctx *nft, const char *filename,
return ret;
}
-static int json_echo_error(struct netlink_mon_handler *monh,
- const char *fmt, ...)
-{
- struct error_record *erec;
- va_list ap;
-
- va_start(ap, fmt);
- erec = erec_vcreate(EREC_ERROR, int_loc, fmt, ap);
- va_end(ap);
- erec_queue(erec, monh->ctx->msgs);
-
- return MNL_CB_ERROR;
-}
-
-static uint64_t handle_from_nlmsg(const struct nlmsghdr *nlh)
-{
- struct nftnl_table *nlt;
- struct nftnl_chain *nlc;
- struct nftnl_rule *nlr;
- struct nftnl_set *nls;
- struct nftnl_obj *nlo;
- uint64_t handle = 0;
- uint32_t flags;
-
- switch (NFNL_MSG_TYPE(nlh->nlmsg_type)) {
- case NFT_MSG_NEWTABLE:
- nlt = netlink_table_alloc(nlh);
- handle = nftnl_table_get_u64(nlt, NFTNL_TABLE_HANDLE);
- nftnl_table_free(nlt);
- break;
- case NFT_MSG_NEWCHAIN:
- nlc = netlink_chain_alloc(nlh);
- handle = nftnl_chain_get_u64(nlc, NFTNL_CHAIN_HANDLE);
- nftnl_chain_free(nlc);
- break;
- case NFT_MSG_NEWRULE:
- nlr = netlink_rule_alloc(nlh);
- handle = nftnl_rule_get_u64(nlr, NFTNL_RULE_HANDLE);
- nftnl_rule_free(nlr);
- break;
- case NFT_MSG_NEWSET:
- nls = netlink_set_alloc(nlh);
- flags = nftnl_set_get_u32(nls, NFTNL_SET_FLAGS);
- if (!set_is_anonymous(flags))
- handle = nftnl_set_get_u64(nls, NFTNL_SET_HANDLE);
- nftnl_set_free(nls);
- break;
- case NFT_MSG_NEWOBJ:
- nlo = netlink_obj_alloc(nlh);
- handle = nftnl_obj_get_u64(nlo, NFTNL_OBJ_HANDLE);
- nftnl_obj_free(nlo);
- break;
- }
- return handle;
-}
-int json_events_cb(const struct nlmsghdr *nlh, struct netlink_mon_handler *monh)
-{
- uint64_t handle = handle_from_nlmsg(nlh);
- json_t *tmp, *json;
- void *iter;
-
- if (!handle)
- return MNL_CB_OK;
-
- json = seqnum_to_json(nlh->nlmsg_seq);
- if (!json)
- return MNL_CB_OK;
-
- tmp = json_object_get(json, "add");
- if (!tmp)
- tmp = json_object_get(json, "insert");
- if (!tmp)
- /* assume loading JSON dump */
- tmp = json;
-
- iter = json_object_iter(tmp);
- if (!iter) {
- json_echo_error(monh, "Empty JSON object in cmd list\n");
- return MNL_CB_OK;
- }
- json = json_object_iter_value(iter);
- if (!json_is_object(json) || json_object_iter_next(tmp, iter)) {
- json_echo_error(monh, "Malformed JSON object in cmd list\n");
- return MNL_CB_OK;
- }
-
- json_object_set_new(json, "handle", json_integer(handle));
- return MNL_CB_OK;
-}
-
void json_print_echo(struct nft_ctx *ctx)
{
if (!ctx->json_echo)
@@ -3890,7 +3756,6 @@ void json_print_echo(struct nft_ctx *ctx)
ctx->json_echo = json_pack("{s:o}", "nftables", ctx->json_echo);
json_dumpf(ctx->json_echo, ctx->output.output_fp, JSON_PRESERVE_ORDER);
printf("\n");
- json_cmd_assoc_free();
if (ctx->json_echo) {
json_decref(ctx->json_echo);
ctx->json_echo = NULL;
next prev parent reply other threads:[~2020-07-31 9:22 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 [this message]
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
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=20200731092212.GA1850@salvia \
--to=pablo@netfilter.org \
--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).