From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Alvaro Neira Ayuso <alvaroneay@gmail.com>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [libnftnl PATCH 2/4 v7] src: add support to import json/xml with the new command syntax
Date: Sun, 8 Feb 2015 21:40:46 +0100 [thread overview]
Message-ID: <20150208204046.GC4711@salvia> (raw)
In-Reply-To: <1423229069-652-2-git-send-email-alvaroneay@gmail.com>
On Fri, Feb 06, 2015 at 02:24:27PM +0100, Alvaro Neira Ayuso wrote:
> This patch adds support to parse the new syntax in xml/json. This patch adds two
> new different functions nft_ruleset_parse_*_cb. The ruleset contais differents
> objects (tables, chains, set and rules) of nftables. With these functions,
> we can pass a callback functions as parameter and we can call for each objects.
>
> These functions use a new structure as parameter called nft_parse_ctx. This
> structure contains the command associated, the type of the object (table, chain,
> rule, set or set element) and the object.
>
> Also, with this changes, we support to parse another new ruleset syntax:
>
> {"nftables":[{"add":[{"element":{"name":"blackhole","table":"filter"
> "family":"ip","key_type":7,"key_len":4,"set_elem":[{"key":{
> "reg":{"type":"value","len":4,"data0":"0x0403a8c0"}}}]}}]}]}
>
> With this new "element", we can make incremental changes to set elements.
>
> In short, these functions offer us the possibility to make incremental changes
> to the ruleset.
>
> Moreover, this patch consolidate the code in the ruleset xml/json import
> support.
>
> Another interesting goal is extend (in the future) the ruleset
> structure with information like the command associated to each object.
>
> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
> ---
> [changes in v7]
> * Add the atribute CTX_DATA to set it. Also, to get it in userspace.
> * Fix the conflict with the previously patch changes in libnftnl.map
>
> include/libnftnl/ruleset.h | 32 ++
> src/internal.h | 3 +
> src/libnftnl.map | 8 +
> src/ruleset.c | 824 +++++++++++++++++++++++++++-----------------
> src/set.c | 12 +
> src/utils.c | 17 +
> 6 files changed, 589 insertions(+), 307 deletions(-)
>
> diff --git a/include/libnftnl/ruleset.h b/include/libnftnl/ruleset.h
> index 1a3e22f..aa1d92d 100644
> --- a/include/libnftnl/ruleset.h
> +++ b/include/libnftnl/ruleset.h
> @@ -25,11 +25,43 @@ enum {
> NFT_RULESET_ATTR_RULELIST,
> };
>
> +enum nft_ruleset_type {
> + NFT_RULESET_UNSPEC = 0,
> + NFT_RULESET_RULESET,
> + NFT_RULESET_TABLE,
> + NFT_RULESET_CHAIN,
> + NFT_RULESET_RULE,
> + NFT_RULESET_SET,
> + NFT_RULESET_SET_ELEMS,
> +};
> +
> bool nft_ruleset_attr_is_set(const struct nft_ruleset *r, uint16_t attr);
> void nft_ruleset_attr_unset(struct nft_ruleset *r, uint16_t attr);
> void nft_ruleset_attr_set(struct nft_ruleset *r, uint16_t attr, void *data);
> void *nft_ruleset_attr_get(const struct nft_ruleset *r, uint16_t attr);
>
> +enum {
> + NFT_RULESET_CTX_CMD = 0,
> + NFT_RULESET_CTX_TYPE,
> + NFT_RULESET_CTX_TABLE,
> + NFT_RULESET_CTX_CHAIN,
> + NFT_RULESET_CTX_RULE,
> + NFT_RULESET_CTX_SET,
> + NFT_RULESET_CTX_DATA,
> +};
> +
> +struct nft_parse_ctx;
> +bool nft_ruleset_ctx_is_set(const struct nft_parse_ctx *ctx, uint16_t attr);
> +void *nft_ruleset_ctx_get(const struct nft_parse_ctx *ctx, uint16_t attr);
> +uint32_t nft_ruleset_ctx_get_u32(const struct nft_parse_ctx *ctx,
> + uint16_t attr);
> +
> +int nft_ruleset_parse_file_cb(enum nft_parse_type type, FILE *fp,
> + struct nft_parse_err *err, void *data,
> + int (*cb)(const struct nft_parse_ctx *ctx));
> +int nft_ruleset_parse_buffer_cb(enum nft_parse_type type, const char *buffer,
> + struct nft_parse_err *err, void *data,
> + int (*cb)(const struct nft_parse_ctx *ctx));
> int nft_ruleset_parse(struct nft_ruleset *rs, enum nft_parse_type type,
> const char *data, struct nft_parse_err *err);
> int nft_ruleset_parse_file(struct nft_ruleset *rs, enum nft_parse_type type,
> diff --git a/src/internal.h b/src/internal.h
> index a846d1e..f273140 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -139,6 +139,8 @@ int nft_jansson_parse_rule(struct nft_rule *r, json_t *tree,
> struct nft_set;
> int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
> struct nft_parse_err *err);
> +int nft_jansson_parse_elem(struct nft_set *s, json_t *tree,
> + struct nft_parse_err *err);
> #endif
>
> const char *nft_family2str(uint32_t family);
> @@ -148,6 +150,7 @@ const char *nft_verdict2str(uint32_t verdict);
> int nft_str2verdict(const char *verdict, int *verdict_num);
> int nft_flag2cmd(uint32_t flags, uint32_t *cmd);
> int nft_get_value(enum nft_type type, void *val, void *out);
> +uint32_t nft_str2cmd(const char *cmd);
>
> #include <stdio.h>
> int nft_fprintf(FILE *fp, void *obj, uint32_t cmd, uint32_t type,
> diff --git a/src/libnftnl.map b/src/libnftnl.map
> index be7b998..7c74fbc 100644
> --- a/src/libnftnl.map
> +++ b/src/libnftnl.map
> @@ -227,3 +227,11 @@ LIBNFTNL_1.2 {
> nft_gen_snprintf;
> nft_gen_fprintf;
> } LIBNFTNL_1.1;
> +
> +LIBNFTNL_1.2.0 {
> + nft_ruleset_ctx_is_set;
> + nft_ruleset_ctx_get;
> + nft_ruleset_ctx_get_u32;
> + nft_ruleset_parse_file_cb;
> + nft_ruleset_parse_buffer_cb;
> +} LIBNFTNL_1.2;
> diff --git a/src/ruleset.c b/src/ruleset.c
> index e9c22a1..1c58bba 100644
> --- a/src/ruleset.c
> +++ b/src/ruleset.c
> @@ -32,6 +32,32 @@ struct nft_ruleset {
> uint16_t flags;
> };
>
> +struct nft_parse_ctx {
> + enum nft_cmd_type cmd;
> + enum nft_ruleset_type type;
> + union {
> + struct nft_table *table;
> + struct nft_chain *chain;
> + struct nft_rule *rule;
> + struct nft_set *set;
> + struct nft_set_elem *set_elem;
> + };
> + void *data;
> +
> + /* These fields below are not exposed to the user */
> + union {
> + json_t *json;
> + mxml_node_t *xml;
> + };
> +
> + uint32_t format;
> + uint32_t set_id;
> + struct nft_set_list *set_list;
> +
> + int (*cb)(const struct nft_parse_ctx *ctx);
> + uint16_t flags;
> +};
> +
> struct nft_ruleset *nft_ruleset_alloc(void)
> {
> return calloc(1, sizeof(struct nft_ruleset));
> @@ -131,230 +157,419 @@ void *nft_ruleset_attr_get(const struct nft_ruleset *r, uint16_t attr)
> }
> EXPORT_SYMBOL(nft_ruleset_attr_get);
>
> -#ifdef JSON_PARSING
> -static int nft_ruleset_json_parse_tables(struct nft_ruleset *rs, json_t *array,
> - struct nft_parse_err *err)
> +bool nft_ruleset_ctx_is_set(const struct nft_parse_ctx *ctx, uint16_t attr)
> +{
> + return ctx->flags & (1 << attr);
> +}
> +EXPORT_SYMBOL(nft_ruleset_ctx_is_set);
> +
> +void *nft_ruleset_ctx_get(const struct nft_parse_ctx *ctx, uint16_t attr)
> +{
> + if (!(ctx->flags & (1 << attr)))
> + return NULL;
> +
> + switch (attr) {
> + case NFT_RULESET_CTX_CMD:
> + return (void *)&ctx->cmd;
> + case NFT_RULESET_CTX_TYPE:
> + return (void *)&ctx->type;
> + case NFT_RULESET_CTX_TABLE:
> + return ctx->table;
> + case NFT_RULESET_CTX_CHAIN:
> + return ctx->chain;
> + case NFT_RULESET_CTX_RULE:
> + return ctx->rule;
> + case NFT_RULESET_CTX_SET:
> + return ctx->set;
> + case NFT_RULESET_CTX_DATA:
> + return ctx->data;
> + default:
> + return NULL;
> + }
> +}
> +EXPORT_SYMBOL(nft_ruleset_ctx_get);
> +
> +uint32_t nft_ruleset_ctx_get_u32(const struct nft_parse_ctx *ctx, uint16_t attr)
> +{
> + const void *ret = nft_ruleset_ctx_get(ctx, attr);
> + return ret == NULL ? 0 : *((uint32_t *)ret);
> +}
> +EXPORT_SYMBOL(nft_ruleset_ctx_get_u32);
> +
> +#if defined(JSON_PARSING) || defined(XML_PARSING)
> +static void nft_ruleset_ctx_unset(struct nft_parse_ctx *ctx, uint16_t attr)
> +{
> + if (!(ctx->flags & (1 << attr)))
> + return;
> +
> + switch (attr) {
> + case NFT_RULESET_CTX_CMD:
> + case NFT_RULESET_CTX_TYPE:
> + break;
> + case NFT_RULESET_CTX_TABLE:
> + ctx->table = NULL;
> + break;
> + case NFT_RULESET_CTX_CHAIN:
> + ctx->chain = NULL;
> + break;
> + case NFT_RULESET_CTX_RULE:
> + ctx->rule = NULL;
> + break;
> + case NFT_RULESET_CTX_SET:
> + ctx->set = NULL;
> + break;
> + case NFT_RULESET_CTX_DATA:
> + ctx->data = NULL;
> + break;
> + }
I think you don't need to set all these pointers to NULL.
> + ctx->flags &= ~(1 << attr);
Unsetting the flag should be sufficient.
> +}
> +
> +static void nft_ruleset_ctx_set(struct nft_parse_ctx *ctx, uint16_t attr,
> + void *data)
> +{
> + switch (attr) {
> + case NFT_RULESET_CTX_CMD:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_CMD);
You don't need to call unset first.
> + ctx->cmd = *((uint32_t *)data);
> + break;
> + case NFT_RULESET_CTX_TYPE:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_TYPE);
> + ctx->type = *((uint32_t *)data);
> + break;
> + case NFT_RULESET_CTX_TABLE:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_TABLE);
> + ctx->table = data;
> + break;
> + case NFT_RULESET_CTX_CHAIN:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_CHAIN);
> + ctx->chain = data;
> + break;
> + case NFT_RULESET_CTX_RULE:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_RULE);
> + ctx->rule = data;
> + break;
> + case NFT_RULESET_CTX_SET:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_SET);
> + ctx->set = data;
> + break;
> + case NFT_RULESET_CTX_DATA:
> + nft_ruleset_ctx_unset(ctx, NFT_RULESET_CTX_DATA);
> + ctx->data = data;
> + break;
> + }
> + ctx->flags |= (1 << attr);
> +}
> +
> +static void nft_ruleset_ctx_set_u32(struct nft_parse_ctx *ctx, uint16_t attr,
> + uint32_t val)
> +{
> + nft_ruleset_ctx_set(ctx, attr, &val);
> +}
> +
> +static int nft_ruleset_parse_tables(struct nft_parse_ctx *ctx,
> + struct nft_parse_err *err)
> {
> - int i, len;
> - json_t *node;
> struct nft_table *table;
> - struct nft_table_list *list = nft_table_list_alloc();
>
> - if (list == NULL) {
> + table = nft_table_alloc();
> + if (table == NULL) {
> errno = ENOMEM;
> return -1;
> }
>
> - len = json_array_size(array);
> - for (i = 0; i < len; i++) {
> - node = json_array_get(array, i);
> - if (node == NULL) {
> - errno = EINVAL;
> - goto err;
> - }
> -
> - if (!(nft_jansson_node_exist(node, "table")))
> - continue;
> -
> - table = nft_table_alloc();
> - if (table == NULL) {
> - errno = ENOMEM;
> + switch (ctx->format) {
> + case NFT_OUTPUT_JSON:
> +#ifdef JSON_PARSING
Can you move this ifdef inside nft_jansson_parse_table().
I mean:
int nft_jansson_parse_table(...)
{
#ifdef JSON_PARSING
...
#else
errno = EOPNOTSUPP;
return -1;
#endif
}
> + if (nft_jansson_parse_table(table, ctx->json, err) < 0)
> goto err;
> - }
> -
> - if (nft_jansson_parse_table(table, node, err) < 0) {
> - nft_table_free(table);
> +#endif
> + break;
> + case NFT_OUTPUT_XML:
> +#ifdef XML_PARSING
> + if (nft_mxml_table_parse(ctx->xml, table, err) < 0)
> goto err;
> - }
> -
> - nft_table_list_add_tail(table, list);
> +#endif
> + break;
> + default:
> + return -1;
> }
>
> - if (!nft_table_list_is_empty(list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_TABLELIST, list);
> - else
> - nft_table_list_free(list);
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE, NFT_RULESET_TABLE);
> + nft_ruleset_ctx_set(ctx, NFT_RULESET_CTX_TABLE, table);
> + if (ctx->cb(ctx) < 0)
> + goto err;
>
> return 0;
> err:
> - nft_table_list_free(list);
> + nft_table_free(table);
> return -1;
> }
>
> -static int nft_ruleset_json_parse_chains(struct nft_ruleset *rs, json_t *array,
> - struct nft_parse_err *err)
> +static int nft_ruleset_parse_chains(struct nft_parse_ctx *ctx,
> + struct nft_parse_err *err)
> {
> - int i, len;
> - json_t *node;
> struct nft_chain *chain;
> - struct nft_chain_list *list = nft_chain_list_alloc();
>
> - if (list == NULL) {
> + chain = nft_chain_alloc();
> + if (chain == NULL) {
> errno = ENOMEM;
> return -1;
> }
>
> - len = json_array_size(array);
> - for (i = 0; i < len; i++) {
> - node = json_array_get(array, i);
> - if (node == NULL) {
> - errno = EINVAL;
> + switch (ctx->format) {
> + case NFT_OUTPUT_JSON:
> +#ifdef JSON_PARSING
> + if (nft_jansson_parse_chain(chain, ctx->json, err) < 0)
> goto err;
> - }
> -
> - if (!(nft_jansson_node_exist(node, "chain")))
> - continue;
> -
> - chain = nft_chain_alloc();
> - if (chain == NULL) {
> - errno = ENOMEM;
> +#endif
> + break;
> + case NFT_OUTPUT_XML:
> +#ifdef XML_PARSING
> + if (nft_mxml_chain_parse(ctx->xml, chain, err) < 0)
> goto err;
> - }
> +#endif
> + break;
> + default:
> + return -1;
> + }
>
> - if (nft_jansson_parse_chain(chain, node, err) < 0) {
> - nft_chain_free(chain);
> - goto err;
> - }
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE, NFT_RULESET_CHAIN);
> + nft_ruleset_ctx_set(ctx, NFT_RULESET_CTX_CHAIN, chain);
> + if (ctx->cb(ctx) < 0)
> + goto err;
>
> - nft_chain_list_add_tail(chain, list);
> - }
> + return 0;
> +err:
> + nft_chain_free(chain);
> + return -1;
> +}
> +
> +static int nft_ruleset_parse_set(struct nft_parse_ctx *ctx,
> + struct nft_set *set, uint32_t type,
> + struct nft_parse_err *err)
> +{
> + nft_set_attr_set_u32(set, NFT_SET_ATTR_ID, ctx->set_id++);
> + nft_set_list_add_tail(set, ctx->set_list);
>
> - if (!nft_chain_list_is_empty(list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_CHAINLIST, list);
> - else
> - nft_chain_list_free(list);
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE, type);
> + nft_ruleset_ctx_set(ctx, NFT_RULESET_CTX_SET, set);
> + if (ctx->cb(ctx) < 0)
> + goto err;
>
> return 0;
> err:
> - nft_chain_list_free(list);
> + nft_set_free(set);
Do you really have to release this set here? This object is not
allocated by us, this is received as a parameter. The caller should be
resposible for releasing this IMO.
> return -1;
> }
>
> -static int nft_ruleset_json_parse_sets(struct nft_ruleset *rs, json_t *array,
> +static int nft_ruleset_parse_set_elems(struct nft_parse_ctx *ctx,
> struct nft_parse_err *err)
> {
> - int i, len;
> - uint32_t set_id = 0;
> - json_t *node;
> struct nft_set *set;
> - struct nft_set_list *list = nft_set_list_alloc();
>
> - if (list == NULL) {
> + set = nft_set_alloc();
> + if (set == NULL) {
> errno = ENOMEM;
> return -1;
> }
>
> - len = json_array_size(array);
> - for (i = 0; i < len; i++) {
> - node = json_array_get(array, i);
> - if (node == NULL) {
> - errno = EINVAL;
> + switch (ctx->format) {
> + case NFT_OUTPUT_JSON:
> +#ifdef JSON_PARSING
> + if (nft_jansson_parse_elem(set, ctx->json, err) < 0)
> goto err;
> - }
> +#endif
> + break;
> + case NFT_OUTPUT_XML:
> +#ifdef XML_PARSING
> + if (nft_mxml_set_parse(ctx->xml, set, err) < 0)
> + goto err;
> +#endif
> + break;
> + default:
> + return -1;
> + }
>
> - if (!(nft_jansson_node_exist(node, "set")))
> - continue;
> + return nft_ruleset_parse_set(ctx, set, NFT_RULESET_SET_ELEMS, err);
> +err:
> + nft_set_free(set);
> + return -1;
> +}
>
> - set = nft_set_alloc();
> - if (set == NULL) {
> - errno = ENOMEM;
> - goto err;
> - }
> +static int nft_ruleset_parse_sets(struct nft_parse_ctx *ctx,
> + struct nft_parse_err *err)
> +{
> + struct nft_set *set;
> +
> + set = nft_set_alloc();
> + if (set == NULL) {
> + errno = ENOMEM;
No need to set errno: malloc() / calloc() already sets this for you
when it fails.
> + return -1;
> + }
>
> - if (nft_jansson_parse_set(set, node, err) < 0) {
> - nft_set_free(set);
> + switch (ctx->format) {
> + case NFT_OUTPUT_JSON:
> +#ifdef JSON_PARSING
> + if (nft_jansson_parse_set(set, ctx->json, err) < 0)
> goto err;
> - }
> +#endif
> + break;
> + case NFT_OUTPUT_XML:
> +#ifdef XML_PARSING
> + if (nft_mxml_set_parse(ctx->xml, set, err) < 0)
> + goto err;
> +#endif
> + break;
> + default:
> + return -1;
> + }
>
> - nft_set_attr_set_u32(set, NFT_SET_ATTR_ID, set_id++);
> - nft_set_list_add_tail(set, list);
> + return nft_ruleset_parse_set(ctx, set, NFT_RULESET_SET, err);
> +err:
> + nft_set_free(set);
> + return -1;
> +}
> +
> +static int nft_ruleset_parse_rules(struct nft_parse_ctx *ctx,
> + struct nft_parse_err *err)
> +{
> + struct nft_rule *rule;
> +
> + rule = nft_rule_alloc();
> + if (rule == NULL) {
> + errno = ENOMEM;
> + return -1;
> }
>
> - if (!nft_set_list_is_empty(list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_SETLIST, list);
> - else
> - nft_set_list_free(list);
> + switch (ctx->format) {
> + case NFT_OUTPUT_JSON:
> +#ifdef JSON_PARSING
> + if (nft_jansson_parse_rule(rule, ctx->json, err,
> + ctx->set_list) < 0)
> + goto err;
> +#endif
> + break;
> + case NFT_OUTPUT_XML:
> +#ifdef XML_PARSING
> + if (nft_mxml_rule_parse(ctx->xml, rule, err, ctx->set_list) < 0)
> + goto err;
> +#endif
> + break;
> + default:
> + return -1;
> + }
> +
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE, NFT_RULESET_RULE);
> + nft_ruleset_ctx_set(ctx, NFT_RULESET_CTX_RULE, rule);
> + if (ctx->cb(ctx) < 0)
> + goto err;
>
> return 0;
> err:
> - nft_set_list_free(list);
> + nft_rule_free(rule);
> return -1;
> }
> +#endif
>
> -static int nft_ruleset_json_parse_rules(struct nft_ruleset *rs, json_t *array,
> - struct nft_parse_err *err)
> +#ifdef JSON_PARSING
> +static int nft_ruleset_json_parse_ruleset(struct nft_parse_ctx *ctx,
> + struct nft_parse_err *err)
> {
> - int i, len;
> - json_t *node;
> - struct nft_rule *rule = NULL;
> - struct nft_rule_list *list = nft_rule_list_alloc();
> + json_t *node, *array = ctx->json;
> + int len, i;
>
> - if (list == NULL) {
> - errno = ENOMEM;
> + ctx->set_list = nft_set_list_alloc();
> + if (ctx->set_list == NULL)
> return -1;
> - }
>
> len = json_array_size(array);
> for (i = 0; i < len; i++) {
> node = json_array_get(array, i);
> if (node == NULL) {
> errno = EINVAL;
> - goto err;
> - }
> -
> - if (!(nft_jansson_node_exist(node, "rule")))
> - continue;
> -
> - rule = nft_rule_alloc();
> - if (rule == NULL) {
> - errno = ENOMEM;
> - goto err;
> + return -1;
> }
>
> - if (nft_jansson_parse_rule(rule, node, err, rs->set_list) < 0) {
> - nft_rule_free(rule);
> - goto err;
> + ctx->json = node;
> + if (nft_jansson_node_exist(node, "table")) {
> + if (nft_ruleset_parse_tables(ctx, err) < 0)
> + return -1;
You can get better code if you use this pattern instead:
err = nft_ruleset_parse_tables(...);
and see below...
> + } else if (nft_jansson_node_exist(node, "chain")) {
> + if (nft_ruleset_parse_chains(ctx, err) < 0)
> + return -1;
> + } else if (nft_jansson_node_exist(node, "set")) {
> + if (nft_ruleset_parse_sets(ctx, err) < 0)
> + return -1;
> + } else if (nft_jansson_node_exist(node, "rule")) {
> + if (nft_ruleset_parse_rules(ctx, err) < 0)
> + return -1;
> + } else if (nft_jansson_node_exist(node, "element")) {
> + if (nft_ruleset_parse_set_elems(ctx, err) < 0)
> + return -1;
> + } else {
> + return -1;
> }
Then, here you add:
if (err < 0)
return err;
> -
> - nft_rule_list_add_tail(rule, list);
> }
>
> - if (!nft_rule_list_is_empty(list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_RULELIST, list);
> - else
> - nft_rule_list_free(list);
> + if (len == 0 && ctx->cmd == NFT_CMD_FLUSH) {
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE,
> + NFT_RULESET_RULESET);
> + if (ctx->cb(ctx) < 0)
> + return -1;
> + }
>
> return 0;
> -err:
> - nft_rule_list_free(list);
> - return -1;
> }
>
> -static int nft_ruleset_json_parse_ruleset(struct nft_ruleset *rs, json_t *array,
> - struct nft_parse_err *err)
> +static int nft_ruleset_json_parse_cmd(const char *cmd,
> + struct nft_parse_err *err,
> + struct nft_parse_ctx *ctx)
> {
> - if (nft_ruleset_json_parse_tables(rs, array, err) != 0)
> - return -1;
> + uint32_t cmdnum;
> + json_t *nodecmd;
>
> - if (nft_ruleset_json_parse_chains(rs, array, err) != 0)
> + cmdnum = nft_str2cmd(cmd);
> + if (cmdnum == NFT_CMD_UNSPEC) {
> + err->error = NFT_PARSE_EMISSINGNODE;
> + err->node_name = strdup(cmd);
> return -1;
> + }
>
> - if (nft_ruleset_json_parse_sets(rs, array, err) != 0)
> - return -1;
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_CMD, cmdnum);
>
> - if (nft_ruleset_json_parse_rules(rs, array, err) != 0)
> - return -1;
> + nodecmd = json_object_get(ctx->json, cmd);
> + if (nodecmd == NULL)
> + return 0;
> +
> + ctx->json = nodecmd;
> + if (nft_ruleset_json_parse_ruleset(ctx, err) != 0)
> + goto err;
>
> return 0;
> +err:
> + return -1;
> }
> #endif
>
> -static int nft_ruleset_json_parse(struct nft_ruleset *rs, const void *json,
> - struct nft_parse_err *err, enum nft_parse_input input)
> +static int nft_ruleset_json_parse(const void *json,
> + struct nft_parse_err *err,
> + enum nft_parse_input input,
> + enum nft_parse_type type, void *arg,
> + int (*cb)(const struct nft_parse_ctx *ctx))
> {
> #ifdef JSON_PARSING
> - json_t *root, *array;
> + json_t *root, *array, *node;
> json_error_t error;
> + int i, len;
> + const char *key;
> + struct nft_parse_ctx ctx;
> +
> + ctx.cb = cb;
> + ctx.format = type;
> +
> + if (arg != NULL)
> + nft_ruleset_ctx_set(&ctx, NFT_RULESET_CTX_DATA, arg);
>
> root = nft_jansson_create_root(json, &error, err, input);
> if (root == NULL)
> @@ -366,8 +581,21 @@ static int nft_ruleset_json_parse(struct nft_ruleset *rs, const void *json,
> goto err;
> }
>
> - if (nft_ruleset_json_parse_ruleset(rs, array, err) != 0)
> - goto err;
> + len = json_array_size(array);
> + for (i = 0; i < len; i++) {
> + node = json_array_get(array, i);
> + if (node == NULL) {
> + errno = EINVAL;
> + goto err;
> + }
> + ctx.json = node;
> + key = json_object_iter_key(json_object_iter(node));
> + if (key == NULL)
> + goto err;
> +
> + if (nft_ruleset_json_parse_cmd(key, err, &ctx) < 0)
> + goto err;
> + }
>
> nft_jansson_free_root(root);
> return 0;
> @@ -381,203 +609,113 @@ err:
> }
>
> #ifdef XML_PARSING
> -static int
> -nft_ruleset_xml_parse_tables(struct nft_ruleset *rs, mxml_node_t *tree,
> - struct nft_parse_err *err)
> +static int nft_ruleset_xml_parse_ruleset(struct nft_parse_ctx *ctx,
> + struct nft_parse_err *err)
> {
> - mxml_node_t *node;
> - struct nft_table *table;
> - struct nft_table_list *table_list = nft_table_list_alloc();
> - if (table_list == NULL) {
> - errno = ENOMEM;
> - return -1;
> - }
> -
> - for (node = mxmlFindElement(tree, tree, "table", NULL, NULL,
> - MXML_DESCEND_FIRST);
> - node != NULL;
> - node = mxmlFindElement(node, tree, "table", NULL, NULL,
> - MXML_NO_DESCEND)) {
> - table = nft_table_alloc();
> - if (table == NULL)
> - goto err_free;
> -
> - if (nft_mxml_table_parse(node, table, err) != 0) {
> - nft_table_free(table);
> - goto err_free;
> - }
> -
> - nft_table_list_add_tail(table, table_list);
> - }
> -
> - if (!nft_table_list_is_empty(table_list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_TABLELIST,
> - table_list);
> - else
> - nft_table_list_free(table_list);
> + const char *node_type;
> + mxml_node_t *node, *array = ctx->xml;
> + int len = 0;
>
> - return 0;
> -err_free:
> - nft_table_list_free(table_list);
> - return -1;
> -}
> -
> -static int
> -nft_ruleset_xml_parse_chains(struct nft_ruleset *rs, mxml_node_t *tree,
> - struct nft_parse_err *err)
> -{
> - mxml_node_t *node;
> - struct nft_chain *chain;
> - struct nft_chain_list *chain_list = nft_chain_list_alloc();
> - if (chain_list == NULL) {
> + ctx->set_list = nft_set_list_alloc();
> + if (ctx->set_list == NULL) {
> errno = ENOMEM;
> return -1;
> }
>
> - for (node = mxmlFindElement(tree, tree, "chain", NULL, NULL,
> + for (node = mxmlFindElement(array, array, NULL, NULL, NULL,
> MXML_DESCEND_FIRST);
> node != NULL;
> - node = mxmlFindElement(node, tree, "chain", NULL, NULL,
> + node = mxmlFindElement(node, array, NULL, NULL, NULL,
> MXML_NO_DESCEND)) {
> - chain = nft_chain_alloc();
> - if (chain == NULL)
> - goto err_free;
> -
> - if (nft_mxml_chain_parse(node, chain, err) != 0) {
> - nft_chain_free(chain);
> - goto err_free;
> - }
> -
> - nft_chain_list_add_tail(chain, chain_list);
> - }
> -
> - if (!nft_chain_list_is_empty(chain_list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_CHAINLIST,
> - chain_list);
> - else
> - nft_chain_list_free(chain_list);
> -
> - return 0;
> -err_free:
> - nft_chain_list_free(chain_list);
> - return -1;
> -}
> -
> -static int
> -nft_ruleset_xml_parse_sets(struct nft_ruleset *rs, mxml_node_t *tree,
> - struct nft_parse_err *err)
> -{
> - uint32_t set_id = 0;
> - mxml_node_t *node;
> - struct nft_set *set;
> - struct nft_set_list *set_list = nft_set_list_alloc();
> - if (set_list == NULL) {
> - errno = ENOMEM;
> - return -1;
> + len++;
> + node_type = node->value.opaque;
> + ctx->xml = node;
> + if (strcmp(node_type, "table") == 0) {
> + if (nft_ruleset_parse_tables(ctx, err) != 0)
You can perform the same consolidation as above.
> + return -1;
> + } else if (strcmp(node_type, "chain") == 0) {
> + if (nft_ruleset_parse_chains(ctx, err) != 0)
> + return -1;
> + } else if (strcmp(node_type, "set") == 0) {
> + if (nft_ruleset_parse_sets(ctx, err) != 0)
> + return -1;
> + } else if (strcmp(node_type, "rule") == 0) {
> + if (nft_ruleset_parse_rules(ctx, err) != 0)
> + return -1;
> + } else if (strcmp(node_type, "element") == 0) {
> + if (nft_ruleset_parse_set_elems(ctx, err) != 0)
> + return -1;
> + } else
> + return -1;
> }
>
> - for (node = mxmlFindElement(tree, tree, "set", NULL, NULL,
> - MXML_DESCEND_FIRST);
> - node != NULL;
> - node = mxmlFindElement(node, tree, "set", NULL, NULL,
> - MXML_NO_DESCEND)) {
> - set = nft_set_alloc();
> - if (set == NULL)
> - goto err_free;
> -
> - if (nft_mxml_set_parse(node, set, err) != 0) {
> - nft_set_free(set);
> - goto err_free;
> - }
> -
> - nft_set_attr_set_u32(set, NFT_SET_ATTR_ID, set_id++);
> - nft_set_list_add_tail(set, set_list);
> + if (len == 0 && ctx->cmd == NFT_CMD_FLUSH) {
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE,
> + NFT_RULESET_RULESET);
> + if (ctx->cb(ctx) < 0)
> + return -1;
> }
>
> - if (!nft_set_list_is_empty(set_list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_SETLIST, set_list);
> - else
> - nft_set_list_free(set_list);
> -
> return 0;
> -err_free:
> - nft_set_list_free(set_list);
> - return -1;
> }
>
> -static int
> -nft_ruleset_xml_parse_rules(struct nft_ruleset *rs, mxml_node_t *tree,
> - struct nft_parse_err *err,
> - struct nft_set_list *set_list)
> +static int nft_ruleset_xml_parse_cmd(const char *cmd, struct nft_parse_err *err,
> + struct nft_parse_ctx *ctx)
> {
> - mxml_node_t *node;
> - struct nft_rule *rule;
> - struct nft_rule_list *rule_list = nft_rule_list_alloc();
> - if (rule_list == NULL) {
> - errno = ENOMEM;
> + uint32_t cmdnum;
> + mxml_node_t *nodecmd;
> +
> + cmdnum = nft_str2cmd(cmd);
> + if (cmdnum == NFT_CMD_UNSPEC) {
> + err->error = NFT_PARSE_EMISSINGNODE;
> + err->node_name = strdup(cmd);
> return -1;
> }
>
> - for (node = mxmlFindElement(tree, tree, "rule", NULL, NULL,
> - MXML_DESCEND_FIRST);
> - node != NULL;
> - node = mxmlFindElement(node, tree, "rule", NULL, NULL,
> - MXML_NO_DESCEND)) {
> - rule = nft_rule_alloc();
> - if (rule == NULL)
> - goto err_free;
> + nodecmd = mxmlFindElement(ctx->xml, ctx->xml, cmd, NULL, NULL,
> + MXML_DESCEND_FIRST);
>
> - if (nft_mxml_rule_parse(node, rule, err, set_list) != 0) {
> - nft_rule_free(rule);
> - goto err_free;
> - }
> + ctx->xml = nodecmd;
> + nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_CMD, cmdnum);
>
> - nft_rule_list_add_tail(rule, rule_list);
> - }
> -
> - if (!nft_rule_list_is_empty(rule_list))
> - nft_ruleset_attr_set(rs, NFT_RULESET_ATTR_RULELIST, rule_list);
> - else
> - nft_rule_list_free(rule_list);
> + if (nft_ruleset_xml_parse_ruleset(ctx, err) != 0)
> + goto err;
>
> return 0;
> -err_free:
> - nft_rule_list_free(rule_list);
> +err:
> return -1;
> }
> -
> -static int nft_ruleset_xml_parse_ruleset(struct nft_ruleset *rs,
> - mxml_node_t *tree,
> - struct nft_parse_err *err)
> -{
> - if (nft_ruleset_xml_parse_tables(rs, tree, err) != 0)
> - return -1;
> -
> - if (nft_ruleset_xml_parse_chains(rs, tree, err) != 0)
> - return -1;
> -
> - if (nft_ruleset_xml_parse_sets(rs, tree, err) != 0)
> - return -1;
> -
> - if (nft_ruleset_xml_parse_rules(rs, tree, err, rs->set_list) != 0)
> - return -1;
> -
> - return 0;
> -}
> #endif
>
> -static int nft_ruleset_xml_parse(struct nft_ruleset *rs, const void *xml,
> - struct nft_parse_err *err, enum nft_parse_input input)
> +static int nft_ruleset_xml_parse(const void *xml, struct nft_parse_err *err,
> + enum nft_parse_input input,
> + enum nft_parse_type type, void *arg,
> + int (*cb)(const struct nft_parse_ctx *ctx))
> {
> #ifdef XML_PARSING
> - mxml_node_t *tree;
> + mxml_node_t *tree, *nodecmd = NULL;
> + char *cmd;
> + struct nft_parse_ctx ctx;
> +
> + ctx.cb = cb;
> + ctx.format = type;
> +
> + if (arg != NULL)
> + nft_ruleset_ctx_set(&ctx, NFT_RULESET_CTX_DATA, arg);
>
> tree = nft_mxml_build_tree(xml, "nftables", err, input);
> if (tree == NULL)
> return -1;
>
> - if (nft_ruleset_xml_parse_ruleset(rs, tree, err) != 0)
> - goto err;
> + ctx.xml = tree;
> +
> + nodecmd = mxmlWalkNext(tree, tree, MXML_DESCEND_FIRST);
> + while (nodecmd != NULL) {
> + cmd = nodecmd->value.opaque;
> + if (nft_ruleset_xml_parse_cmd(cmd, err, &ctx) < 0)
> + goto err;
> + nodecmd = mxmlWalkNext(tree, tree, MXML_NO_DESCEND);
> + }
>
> mxmlDelete(tree);
> return 0;
> @@ -591,18 +729,18 @@ err:
> }
>
> static int
> -nft_ruleset_do_parse(struct nft_ruleset *r, enum nft_parse_type type,
> - const void *data, struct nft_parse_err *err,
> - enum nft_parse_input input)
> +nft_ruleset_do_parse(enum nft_parse_type type, const void *data,
> + struct nft_parse_err *err, enum nft_parse_input input,
> + void *arg, int (*cb)(const struct nft_parse_ctx *ctx))
> {
> int ret;
>
> switch (type) {
> case NFT_PARSE_XML:
> - ret = nft_ruleset_xml_parse(r, data, err, input);
> + ret = nft_ruleset_xml_parse(data, err, input, type, arg, cb);
> break;
> case NFT_PARSE_JSON:
> - ret = nft_ruleset_json_parse(r, data, err, input);
> + ret = nft_ruleset_json_parse(data, err, input, type, arg, cb);
> break;
> default:
> ret = -1;
> @@ -613,17 +751,89 @@ nft_ruleset_do_parse(struct nft_ruleset *r, enum nft_parse_type type,
> return ret;
> }
>
> +int nft_ruleset_parse_file_cb(enum nft_parse_type type, FILE *fp,
> + struct nft_parse_err *err, void *data,
> + int (*cb)(const struct nft_parse_ctx *ctx))
> +{
> + return nft_ruleset_do_parse(type, fp, err, NFT_PARSE_FILE, data, cb);
> +}
> +EXPORT_SYMBOL(nft_ruleset_parse_file_cb);
> +
> +int nft_ruleset_parse_buffer_cb(enum nft_parse_type type, const char *buffer,
> + struct nft_parse_err *err, void *data,
> + int (*cb)(const struct nft_parse_ctx *ctx))
> +{
> + return nft_ruleset_do_parse(type, buffer, err, NFT_PARSE_BUFFER, data,
> + cb);
> +}
> +EXPORT_SYMBOL(nft_ruleset_parse_buffer_cb);
> +
> +static int nft_ruleset_init_ruleset(const struct nft_parse_ctx *ctx)
^^^^^^^ ^^^^^^^
I'm pretty sure you can get a better name here. I'd suggest:
static int nft_ruleset_cb(...)
> +{
> + struct nft_ruleset *r = ctx->data;
> + struct nft_table_list *table_list = r->table_list;
> + struct nft_chain_list *chain_list = r->chain_list;
> + struct nft_rule_list *rule_list = r->rule_list;
> + struct nft_set_list *set_list = r->set_list;
Why don't you use r->object_list instead all alround this function?
> +
> + if (ctx->cmd != NFT_CMD_ADD)
> + return -1;
> +
> + switch (ctx->type) {
> + case NFT_RULESET_TABLE:
> + if (table_list == NULL) {
> + table_list = nft_table_list_alloc();
You have to check if list_alloc() returns NULL, in that case goto err
and release what you had.
> + nft_ruleset_attr_set(r, NFT_RULESET_ATTR_TABLELIST,
> + table_list);
> + }
> + nft_table_list_add_tail(ctx->table, table_list);
> + break;
next prev parent reply other threads:[~2015-02-08 20:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-06 13:24 [libnftnl PATCH 1/4 v7] src: add command tag in json/xml export support Alvaro Neira Ayuso
2015-02-06 13:24 ` [libnftnl PATCH 2/4 v7] src: add support to import json/xml with the new command syntax Alvaro Neira Ayuso
2015-02-08 20:40 ` Pablo Neira Ayuso [this message]
2015-02-06 13:24 ` [libnftnl PATCH 3/4 v7] example: Parse and create netlink message using the new parsing functions Alvaro Neira Ayuso
2015-02-08 21:10 ` Pablo Neira Ayuso
2015-02-06 13:24 ` [libnftnl PATCH 4/4 v7] test: update json/xml tests to the new syntax Alvaro Neira Ayuso
2015-02-08 20:18 ` [libnftnl PATCH 1/4 v7] src: add command tag in json/xml export support Pablo Neira Ayuso
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=20150208204046.GC4711@salvia \
--to=pablo@netfilter.org \
--cc=alvaroneay@gmail.com \
--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 NNTP newsgroup(s).