From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [libnftables PATCH 2/2] set: XML parse
Date: Thu, 25 Jul 2013 22:49:09 +0200 [thread overview]
Message-ID: <20130725204909.GB3407@localhost> (raw)
In-Reply-To: <20130725202038.21531.87739.stgit@nfdev.cica.es>
On Thu, Jul 25, 2013 at 10:20:39PM +0200, Arturo Borrero Gonzalez wrote:
> Sets are now parsed, following this previous snprintf pattern:
>
> <set name="xx" table="xx" version="xx">
> <set_flags>uint32_t</set_flags>
> <key_type>uint32_t</key_type>
> <key_len>size_t</key_len>
> <data_type>uint32_t</data_type>
> <data_len>size_t</data_len>
> <set_elem>
> <set_elem_flags>uint32_t</set_elem_flags>
> <set_elem_key>
> <data_reg type="value">
> <len></len>
> <dataN></dataN>
> </data_reg>
> </set_elem_key>
> <set_elem_data>
> <data_reg type="xx">
> [...]
> </data_reg>
> </set_elem_data>
> </set_elem>
> </set>
>
>
> Signed-off-by: Arturo Borrero González <arturo.borrero.glez@gmail.com>
> ---
> include/libnftables/set.h | 9 ++
> src/libnftables.map | 2 +
> src/mxml.c | 2 -
> src/set.c | 174 +++++++++++++++++++++++++++++++++++++++++++++
> src/set_elem.c | 85 +++++++++++++++++++++-
> tests/nft-parsing-test.c | 10 +++
> tests/xmlfiles/36-set.xml | 51 +++++++++++++
> 7 files changed, 328 insertions(+), 5 deletions(-)
> create mode 100644 tests/xmlfiles/36-set.xml
>
> diff --git a/include/libnftables/set.h b/include/libnftables/set.h
> index 6023d50..4fc3a8d 100644
> --- a/include/libnftables/set.h
> +++ b/include/libnftables/set.h
> @@ -52,6 +52,14 @@ struct nft_set *nft_set_list_iter_cur(struct nft_set_list_iter *iter);
> struct nft_set *nft_set_list_iter_next(struct nft_set_list_iter *iter);
> void nft_set_list_iter_destroy(struct nft_set_list_iter *iter);
>
> +enum nft_set_parse_type {
> + NFT_SET_PARSE_NONE = 0,
> + NFT_SET_PARSE_XML,
> + NFT_SET_PARSE_MAX,
> +};
> +
> +int nft_set_parse(struct nft_set *s, enum nft_set_parse_type type, char *data);
> +
> /*
> * Set elements
> */
> @@ -94,6 +102,7 @@ void nft_set_elem_nlmsg_build_payload(struct nlmsghdr *nlh, struct nft_set_elem
>
> int nft_set_elem_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set_elem *s);
>
> +int nft_set_elem_parse(struct nft_set_elem *e, enum nft_set_parse_type type, char *data);
> int nft_set_elem_snprintf(char *buf, size_t size, struct nft_set_elem *s, uint32_t type, uint32_t flags);
>
> int nft_set_elem_foreach(struct nft_set *s, int (*cb)(struct nft_set_elem *e, void *data), void *data);
> diff --git a/src/libnftables.map b/src/libnftables.map
> index f2084d9..614c705 100644
> --- a/src/libnftables.map
> +++ b/src/libnftables.map
> @@ -120,6 +120,7 @@ global:
> nft_set_nlmsg_build_hdr;
> nft_set_nlmsg_build_payload;
> nft_set_nlmsg_parse;
> + nft_set_parse;
> nft_set_snprintf;
>
> nft_set_list_alloc;
> @@ -149,6 +150,7 @@ global:
> nft_set_elem_nlmsg_build_hdr;
> nft_set_elem_nlmsg_build_payload;
> nft_set_elem_nlmsg_parse;
> + nft_set_elem_parse;
> nft_set_elem_snprintf;
>
> nft_set_elems_nlmsg_build_payload;
> diff --git a/src/mxml.c b/src/mxml.c
> index f812bf6..3d6ada4 100644
> --- a/src/mxml.c
> +++ b/src/mxml.c
> @@ -111,7 +111,7 @@ int nft_mxml_data_reg_parse(mxml_node_t *tree, const char *node_name,
> }
>
> node = mxmlFindElement(node, node, "data_reg", NULL, NULL,
> - MXML_DESCEND);
> + MXML_DESCEND_FIRST);
> if (node == NULL || node->child == NULL) {
> errno = EINVAL;
> goto err;
> diff --git a/src/set.c b/src/set.c
> index ef15527..5e1f8ed 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -16,6 +16,8 @@
> #include <stdlib.h>
> #include <string.h>
> #include <netinet/in.h>
> +#include <limits.h>
> +#include <errno.h>
>
> #include <libmnl/libmnl.h>
> #include <linux/netfilter/nfnetlink.h>
> @@ -301,6 +303,178 @@ int nft_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set *s)
> }
> EXPORT_SYMBOL(nft_set_nlmsg_parse);
>
> +static int nft_set_xml_parse(struct nft_set *s, char *xml)
> +{
> +#ifdef XML_PARSING
> + mxml_node_t *tree = NULL;
no need to init this variable.
> + mxml_node_t *node = NULL;
> + mxml_node_t *save = NULL;
> + char *set_elem_str = NULL;
> + struct nft_set_elem *elem;
> + int version;
> + int family;
> +
> + tree = mxmlLoadString(NULL, xml, MXML_OPAQUE_CALLBACK);
> + if (tree == NULL) {
> + errno = EINVAL;
> + return -1;
> + }
> +
> + if (strcmp(tree->value.opaque, "set") != 0)
> + goto einval;
> +
> + if (mxmlElementGetAttr(tree, "version") == NULL)
> + goto einval;
> +
> + if (nft_strtoi(mxmlElementGetAttr(tree, "version"), 10, &version,
> + NFT_TYPE_U64) != 0)
> + goto err;
> +
> + if (version != NFT_SET_XML_VERSION)
> + goto einval;
> +
> + if (mxmlElementGetAttr(tree, "name") == NULL)
> + goto einval;
> +
> + if (s->name)
> + free(s->name);
> +
> + s->name = strdup(mxmlElementGetAttr(tree, "name"));
> + s->flags |= (1 << NFT_SET_ATTR_NAME);
> +
> + if (mxmlElementGetAttr(tree, "table") == NULL)
> + goto einval;
> +
> + if (s->table)
> + free(s->table);
> +
> + s->table = strdup(mxmlElementGetAttr(tree, "table"));
> + s->flags |= (1 << NFT_SET_ATTR_TABLE);
> +
> + node = mxmlFindElement(tree, tree, "family", NULL, NULL,
> + MXML_DESCEND_FIRST);
> + if (node == NULL)
> + goto einval;
> +
> + if (node->child == NULL)
> + goto einval;
> +
> + family = nft_str2family(node->child->value.opaque);
> +
> + if (family < 0)
> + goto eafnosupport;
> +
> + s->family = family;
> +
> + s->flags |= (1 << NFT_SET_ATTR_FAMILY);
> +
> + if (nft_mxml_num_parse(tree, "set_flags", MXML_DESCEND_FIRST,
> + BASE_DEC, &s->set_flags, NFT_TYPE_U32) != 0)
> + goto einval;
nft_mxml_num_parse already sets errno.
> +
> + s->flags |= (1 << NFT_SET_ATTR_FLAGS);
> +
> +
> + if (nft_mxml_num_parse(tree, "key_type", MXML_DESCEND_FIRST,
> + BASE_DEC, &s->key_type, NFT_TYPE_U32) != 0)
> + goto einval;
> +
> + s->flags |= (1 << NFT_SET_ATTR_KEY_TYPE);
> +
> + if (nft_mxml_num_parse(tree, "key_len", MXML_DESCEND_FIRST,
> + BASE_DEC, &s->key_type, NFT_TYPE_U32) != 0)
> + goto einval;
> +
> + s->flags |= (1 << NFT_SET_ATTR_KEY_LEN);
> +
> + if (nft_mxml_num_parse(tree, "data_type", MXML_DESCEND_FIRST,
> + BASE_DEC, &s->data_type, NFT_TYPE_U32) != 0)
> + goto einval;
> +
> + s->flags |= (1 << NFT_SET_ATTR_DATA_TYPE);
> +
> + if (nft_mxml_num_parse(tree, "data_len", MXML_DESCEND_FIRST,
> + BASE_DEC, &s->data_len, NFT_TYPE_U32) != 0)
> + goto einval;
> +
> + s->flags |= (1 << NFT_SET_ATTR_DATA_LEN);
> +
> + /* Iterate over each <set_elem> */
> + for (node = mxmlFindElement(tree, tree, "set_elem", NULL,
> + NULL, MXML_DESCEND);
> + node != NULL;
> + node = mxmlFindElement(node, tree, "set_elem", NULL,
> + NULL, MXML_DESCEND)) {
> +
> + elem = nft_set_elem_alloc();
> + if (elem == NULL)
> + goto enomem;
already sets ENOMEM, no need for this.
> +
> + /* This is a hack for mxml to print just the current node */
> + save = node->next;
> + node->next = NULL;
> +
> + set_elem_str = mxmlSaveAllocString(node, MXML_NO_CALLBACK);
> + if (set_elem_str == NULL) {
> + free(elem);
> + goto enomem;
> + }
> +
> + if (nft_set_elem_parse(elem, NFT_SET_PARSE_XML,
> + set_elem_str) != 0) {
> + printf("nft_set_elem_parse err: %s\n", set_elem_str);
> + free(set_elem_str);
> + free(elem);
> + goto err;
> + }
> +
> + node->next = save;
> + free(set_elem_str);
> +
> + list_add_tail(&elem->head, &s->element_list);
> + }
Please, move element parsing to another function. Making functions
smaller help maintainability.
> +
> + mxmlDelete(tree);
> + return 0;
> +einval:
rename this to err:, now it's the only possible error in this
function.
> + errno = EINVAL;
> + mxmlDelete(tree);
> + return -1;
> +eafnosupport:
> + errno = EAFNOSUPPORT;
> + mxmlDelete(tree);
> + return -1;
no need for this, not nft_family2str sets it.
> +enomem:
> + errno = ENOMEM;
> + mxmlDelete(tree);
> + return -1;
remove this.
> +err:
> + mxmlDelete(tree);
> + return -1;
> +#else
> + errno = EOPNOTSUPP;
> + return -1;
> +#endif
> +}
> +
> +int nft_set_parse(struct nft_set *s, enum nft_set_parse_type type, char *data)
> +{
> + int ret;
> +
> + switch (type) {
> + case NFT_SET_PARSE_XML:
> + ret = nft_set_xml_parse(s, data);
> + break;
> + default:
> + ret = -1;
> + errno = EOPNOTSUPP;
> + break;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(nft_set_parse);
> +
> static int nft_set_snprintf_json(char *buf, size_t size, struct nft_set *s,
> uint32_t type, uint32_t flags)
> {
> diff --git a/src/set_elem.c b/src/set_elem.c
> index 4adba91..724c312 100644
> --- a/src/set_elem.c
> +++ b/src/set_elem.c
> @@ -16,6 +16,7 @@
> #include <stdlib.h>
> #include <string.h>
> #include <netinet/in.h>
> +#include <errno.h>
>
> #include <libmnl/libmnl.h>
> #include <linux/netfilter/nfnetlink.h>
> @@ -374,8 +375,83 @@ int nft_set_elems_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set *s)
> }
> EXPORT_SYMBOL(nft_set_elems_nlmsg_parse);
>
> +static int nft_set_elem_xml_parse(struct nft_set_elem *e, char *xml)
> +{
> +#ifdef XML_PARSING
> + mxml_node_t *tree = NULL;
> + int set_elem_data;
> +
> + tree = mxmlLoadString(NULL, xml, MXML_OPAQUE_CALLBACK);
> + if (tree == NULL) {
> + errno = EINVAL;
> + return -1;
> + }
> +
> + if (strcmp(tree->value.opaque, "set_elem") != 0)
> + goto einval;
> +
> + if (nft_mxml_num_parse(tree, "set_elem_flags", MXML_DESCEND_FIRST,
> + BASE_DEC, &e->set_elem_flags,
> + NFT_TYPE_U32) != 0)
> + goto einval;
no need to einval here, num_parse already sets.
> +
> + e->flags |= (1 << NFT_SET_ELEM_ATTR_FLAGS);
> +
> + if (nft_mxml_data_reg_parse(tree, "set_elem_key",
> + &e->key) != DATA_VALUE)
> + goto einval;
> +
> + e->flags |= (1 << NFT_SET_ELEM_ATTR_KEY);
> +
> + set_elem_data = nft_mxml_data_reg_parse(tree, "set_elem_data",
> + &e->data);
> + switch (set_elem_data) {
> + case DATA_VALUE:
> + e->flags |= (1 << NFT_SET_ELEM_ATTR_DATA);
> + break;
> + case DATA_VERDICT:
> + e->flags |= (1 << NFT_SET_ELEM_ATTR_VERDICT);
> + break;
> + case DATA_CHAIN:
> + e->flags |= (1 << NFT_SET_ELEM_ATTR_CHAIN);
> + break;
> + default:
> + goto einval;
> + }
> +
> + mxmlDelete(tree);
> + return 0;
> +
> +einval:
> + errno = EINVAL;
> + mxmlDelete(tree);
> + return -1;
> +#else
> + errno = EOPNOTSUPP;
> + return -1;
> +#endif
> +}
> +
> +int nft_set_elem_parse(struct nft_set_elem *e,
> + enum nft_set_parse_type type, char *data) {
> + int ret;
> +
> + switch (type) {
> + case NFT_SET_PARSE_XML:
> + ret = nft_set_elem_xml_parse(e, data);
> + break;
> + default:
> + errno = EOPNOTSUPP;
> + ret = -1;
> + break;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(nft_set_elem_parse);
> +
> static int nft_set_elem_snprintf_json(char *buf, size_t size,
> - struct nft_set_elem *e, uint32_t flags)
> + struct nft_set_elem *e, uint32_t flags)
> {
> int ret, len = size, offset = 0, type = -1;
>
> @@ -414,8 +490,9 @@ static int nft_set_elem_snprintf_json(char *buf, size_t size,
> return offset;
> }
>
> -static int nft_set_elem_snprintf_default(char *buf, size_t size,
> - struct nft_set_elem *e)
> +static
> +int nft_set_elem_snprintf_default(char *buf, size_t size,
> + struct nft_set_elem *e, uint32_t flags)
> {
> int ret, len = size, offset = 0, i;
>
> @@ -505,7 +582,7 @@ int nft_set_elem_snprintf(char *buf, size_t size, struct nft_set_elem *e,
> {
> switch(type) {
> case NFT_SET_O_DEFAULT:
> - return nft_set_elem_snprintf_default(buf, size, e);
> + return nft_set_elem_snprintf_default(buf, size, e, flags);
> case NFT_SET_O_XML:
> return nft_set_elem_snprintf_xml(buf, size, e, flags);
> case NFT_SET_O_JSON:
> diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
> index 4fe60c3..c75d97b 100644
> --- a/tests/nft-parsing-test.c
> +++ b/tests/nft-parsing-test.c
> @@ -9,6 +9,7 @@
> #include <libnftables/table.h>
> #include <libnftables/chain.h>
> #include <libnftables/rule.h>
> +#include <libnftables/set.h>
>
> #ifdef XML_PARSING
> #include <mxml.h>
> @@ -21,6 +22,7 @@ static int test_xml(const char *filename)
> struct nft_table *t = NULL;
> struct nft_chain *c = NULL;
> struct nft_rule *r = NULL;
> + struct nft_set *s = NULL;
> FILE *fp;
> mxml_node_t *tree = NULL;;
> char *xml = NULL;
> @@ -61,6 +63,14 @@ static int test_xml(const char *filename)
>
> nft_rule_free(r);
> }
> + } else if (strcmp(tree->value.opaque, "set") == 0) {
> + s = nft_set_alloc();
> + if (s != NULL) {
> + if (nft_set_parse(s, NFT_SET_PARSE_XML, xml) == 0)
> + ret = 0;
> +
> + nft_set_free(s);
> + }
> }
>
> return ret;
> diff --git a/tests/xmlfiles/36-set.xml b/tests/xmlfiles/36-set.xml
> new file mode 100644
> index 0000000..71ca189
> --- /dev/null
> +++ b/tests/xmlfiles/36-set.xml
> @@ -0,0 +1,51 @@
> +<set name="set1" table="filter" version="0">
> + <family>ip</family>
> + <set_flags>6</set_flags>
> + <key_type>0</key_type>
> + <key_len>12</key_len>
> + <data_type>0</data_type>
> + <data_len>12</data_len>
> + <set_elem>
> + <set_elem_flags>1</set_elem_flags>
> + <set_elem_key>
> + <data_reg type="value">
> + <len>4</len>
> + <data0>0xffaabbdd</data0>
> + </data_reg>
> + </set_elem_key>
> + <set_elem_data>
> + <data_reg type="verdict">
> + <verdict>accept</verdict>
> + </data_reg>
> + </set_elem_data>
> + </set_elem>
> + <set_elem>
> + <set_elem_flags>1</set_elem_flags>
> + <set_elem_key>
> + <data_reg type="value">
> + <len>4</len>
> + <data0>0xffaabb11</data0>
> + </data_reg>
> + </set_elem_key>
> + <set_elem_data>
> + <data_reg type="chain">
> + <chain>test</chain>
> + </data_reg>
> + </set_elem_data>
> + </set_elem>
> + <set_elem>
> + <set_elem_flags>1</set_elem_flags>
> + <set_elem_key>
> + <data_reg type="value">
> + <len>4</len>
> + <data0>0xffaabb11</data0>
> + </data_reg>
> + </set_elem_key>
> + <set_elem_data>
> + <data_reg type="value">
> + <len>4</len>
> + <data0>0xcafecafe</data0>
> + </data_reg>
> + </set_elem_data>
> + </set_elem>
> +</set>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-07-25 20:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-25 20:20 [libnftables PATCH 1/2] src: utils: add verdict2str Arturo Borrero Gonzalez
2013-07-25 20:20 ` [libnftables PATCH 2/2] set: XML parse Arturo Borrero Gonzalez
2013-07-25 20:49 ` Pablo Neira Ayuso [this message]
2013-07-25 20:33 ` [libnftables PATCH 1/2] src: utils: add verdict2str 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=20130725204909.GB3407@localhost \
--to=pablo@netfilter.org \
--cc=arturo.borrero.glez@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.