On Mon, Mar 07, 2016 at 06:10:41PM +0100, Carlos Falgueras García wrote:
> Now it is possible to store multiple variable length user data into rule.
> Modify the parser in order to fill the nftnl_udata with the comment, and the
> print function for extract these commentary and print it to user.
>
> Signed-off-by: Carlos Falgueras García <carlosfg@xxxxxxxxxx>
> ---
> include/rule.h | 11 +++++++++--
> src/netlink_delinearize.c | 7 +++++--
> src/netlink_linearize.c | 6 ++++--
> src/parser_bison.y | 15 ++++++++++++++-
> src/rule.c | 41 ++++++++++++++++++++++++++++++++++++++---
> 5 files changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/include/rule.h b/include/rule.h
> index c848f0f..c500e88 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -4,6 +4,7 @@
> #include <stdint.h>
> #include <nftables.h>
> #include <list.h>
> +#include <libnftnl/udata.h>
>
> /**
> * struct handle - handle for tables, chains, rules and sets
> @@ -155,7 +156,7 @@ extern void chain_print_plain(const struct chain *chain);
> * @location: location the rule was defined at
> * @stmt: list of statements
> * @num_stmts: number of statements in stmts list
> - * @comment: comment
> + * @udata: user data
> */
> struct rule {
> struct list_head list;
> @@ -163,7 +164,7 @@ struct rule {
> struct location location;
> struct list_head stmts;
> unsigned int num_stmts;
> - const char *comment;
> + struct nftnl_udata_buf *udata;
> };
>
> extern struct rule *rule_alloc(const struct location *loc,
> @@ -396,4 +397,10 @@ extern int do_command(struct netlink_ctx *ctx, struct cmd *cmd);
> extern int cache_update(enum cmd_ops cmd, struct list_head *msgs);
> extern void cache_release(void);
>
> +enum udata_type {
> + UDATA_TYPE_COMMENT,
> + __UDATA_TYPE_MAX,
> +};
> +#define UDATA_TYPE_MAX (__UDATA_TYPE_MAX - 1)
> +
> #endif /* NFTABLES_RULE_H */
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index ae6abb0..2f8c512 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -25,6 +25,7 @@
> #include <utils.h>
> #include <erec.h>
> #include <sys/socket.h>
> +#include <libnftnl/udata.h>
>
> struct netlink_parse_ctx {
> struct list_head *msgs;
> @@ -1738,8 +1739,10 @@ struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
> uint32_t len;
>
> data = nftnl_rule_get_data(nlr, NFTNL_RULE_USERDATA, &len);
> - pctx->rule->comment = xmalloc(len);
> - memcpy((char *)pctx->rule->comment, data, len);
> + pctx->rule->udata = nftnl_udata_alloc(len);
> + if (!pctx->rule->udata)
> + memory_allocation_error();
> + nftnl_udata_copy_data(pctx->rule->udata, data, len);
> }
>
> nftnl_expr_foreach((struct nftnl_rule *)nlr, netlink_parse_expr, pctx);
> diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
> index 86b49c6..00f81ea 100644
> --- a/src/netlink_linearize.c
> +++ b/src/netlink_linearize.c
> @@ -21,6 +21,7 @@
> #include <netinet/in.h>
>
> #include <linux/netfilter.h>
> +#include <libnftnl/udata.h>
>
>
> struct netlink_linearize_ctx {
> @@ -1108,9 +1109,10 @@ void netlink_linearize_rule(struct netlink_ctx *ctx, struct nftnl_rule *nlr,
> list_for_each_entry(stmt, &rule->stmts, list)
> netlink_gen_stmt(&lctx, stmt);
>
> - if (rule->comment)
> + if (rule->udata)
> nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA,
> - rule->comment, strlen(rule->comment) + 1);
> + nftnl_udata_data(rule->udata),
> + nftnl_udata_len(rule->udata));
>
> netlink_dump_rule(nlr);
> }
> diff --git a/src/parser_bison.y b/src/parser_bison.y
> index 05ade0f..ed1b63a 100644
> --- a/src/parser_bison.y
> +++ b/src/parser_bison.y
> @@ -24,6 +24,7 @@
> #include <netinet/icmp6.h>
> #include <libnftnl/common.h>
> #include <libnftnl/set.h>
> +#include <libnftnl/attr.h>
>
> #include <rule.h>
> #include <statement.h>
> @@ -1304,7 +1305,19 @@ rule : stmt_list comment_spec
> struct stmt *i;
>
> $$ = rule_alloc(&@$, NULL);
> - $$->comment = $2;
> +
> + if ($2) {
> + if (!($$->udata = nftnl_udata_alloc(NFT_USERDATA_MAXLEN)))
> + memory_allocation_error();
> +
> + if (!nftnl_udata_put_strz($$->udata,
> + UDATA_TYPE_COMMENT, $2)) {
Mind coding style:
if (!nftnl_udata_put_strz($$->udata,
DATA_TYPE_COMMENT, $2)) {
You have to align the parameters to the parens.
> + erec_queue(error(&@2, "Comment too long: \"%s\"", $2),
> + state->msgs);
> + YYERROR;
> + }
> + }
> +
> list_for_each_entry(i, $1, list)
> $$->num_stmts++;
> list_splice_tail($1, &$$->stmts);
> diff --git a/src/rule.c b/src/rule.c
> index 18ff592..60d9b38 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -23,6 +23,7 @@
>
> #include <libnftnl/common.h>
> #include <libnftnl/ruleset.h>
> +#include <libnftnl/udata.h>
> #include <netinet/ip.h>
> #include <linux/netfilter.h>
> #include <linux/netfilter_arp.h>
> @@ -366,6 +367,7 @@ struct rule *rule_alloc(const struct location *loc, const struct handle *h)
> rule->location = *loc;
> init_list_head(&rule->list);
> init_list_head(&rule->stmts);
> + rule->udata = NULL;
No need to set this to null. IIRC the rule object is already allocated
in an already initialized memory area, right?
> if (h != NULL)
> rule->handle = *h;
> return rule;
> @@ -375,21 +377,54 @@ void rule_free(struct rule *rule)
> {
> stmt_list_free(&rule->stmts);
> handle_free(&rule->handle);
> - xfree(rule->comment);
> + nftnl_udata_free(rule->udata);
> xfree(rule);
> }
>
> +static int rule_parse_userdata_cb(const struct nftnl_udata *attr,
> + void *data)
> +{
> + const struct nftnl_udata **tb = data;
> + uint8_t type = nftnl_udata_attr_type(attr);
> + uint8_t len = nftnl_udata_attr_len(attr);
> + unsigned char *value = nftnl_udata_attr_value(attr);
It is prefered to define variables in this way:
unsigned char *value = nftnl_udata_attr_value(attr);
uint8_t type = nftnl_udata_attr_type(attr);
uint8_t len = nftnl_udata_attr_len(attr);
const struct nftnl_udata **tb = data;
ie. longest line first. It's easier to read the code in this way.
> +
> + /* Validation */
Remove this comment, it's obvious what this is doing ;-)
> + switch (type) {
> + case UDATA_TYPE_COMMENT:
> + if (value[len-1] != '\0')
> + return NFTNL_CB_ERROR;
You can remove the NFTNL_CB_* definitions.
I know you're based on libmnl, but in this case I think it's fine to
return the values.
> + break;
> + default:
> + break;
> + };
> +
> + tb[type] = attr;
> + return NFTNL_CB_OK;
> +}
> +
> +
> void rule_print(const struct rule *rule)
> {
> const struct stmt *stmt;
> + const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] = {};
> + const struct nftnl_udata *attr;
>
> list_for_each_entry(stmt, &rule->stmts, list) {
> stmt->ops->print(stmt);
> printf(" ");
> }
>
> - if (rule->comment)
> - printf("comment \"%s\" ", rule->comment);
> + if (rule->udata) {
> + if (nftnl_udata_parse(rule->udata, rule_parse_userdata_cb, tb)
> + != NFTNL_CB_ERROR
> + ) {
^^^
This coding style is not correct.
> + attr = tb[UDATA_TYPE_COMMENT];
> + if (attr)
You can do this instead:
if (tb[UDATA_TYPE_COMMENT]) {
printf("comment \"%s\" ",
(char *)nftnl_udata_attr_value(tb[UDATA_TYPE_COMMENT]));
> + printf("comment \"%s\" ",
> + (char *)nftnl_udata_attr_value(attr));
> + }
> + }
>
> if (handle_output > 0)
> printf("# handle %" PRIu64, rule->handle.handle);
> --
> 2.7.2
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
