On Thu, Mar 10, 2016 at 10:37:16PM +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 | 14 +++++++++++++- > src/rule.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- > 5 files changed, 73 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; I think we should keep the: const char *comment; The nftnl_udata_buf should only be used when building the TLV area from the netlink_linearize step, and when parsing the TLV and covert it to attribute from netlink_delinearize. Otherwise, we have to deal with this internal layout from other parts of nft that are not netlink specific. > }; > > 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 d431588..e655e00 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; > @@ -1773,8 +1774,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); Therefore, here you should extract the data from the udata and set pctx->rule->comment. > } > > nftnl_expr_foreach((struct nftnl_rule *)nlr, netlink_parse_expr, pctx); > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c > index bb51de7..5ac438e 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 { > @@ -1171,9 +1172,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)); When linearizing, you should do something like: if (rule->comment) { udata = nftnl_udata_alloc(NFT_USERDATA_MAXLEN)); if (!udata) memory_allocation_error(); if (!nftnl_udata_put_strz(udata, UDATA_TYPE_COMMENT, rule->comment)) memory_allocation_error(); nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA, nftnl_udata_data(udata), nftnl_udata_len(udata)); } > > netlink_dump_rule(nlr); > } > diff --git a/src/parser_bison.y b/src/parser_bison.y > index 90978ab..9f8c005 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/udata.h> > > #include <rule.h> > #include <statement.h> > @@ -1303,7 +1304,18 @@ 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)) { > + erec_queue(error(&@2, "Comment too long: \"%s\"", $2), > + state->msgs); > + YYERROR; > + } > + } With the change I propose above, you don't need to update the parser anymore. > + > 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 0b78549..112ae85 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> > @@ -375,21 +376,59 @@ void rule_free(struct rule *rule) > { > stmt_list_free(&rule->stmts); > handle_free(&rule->handle); > - xfree(rule->comment); > + nftnl_udata_free(rule->udata); This change, you will not need it with the change I'm proposing. > xfree(rule); > } > > +static int rule_parse_udata_cb(const struct nftnl_udata *attr, void *data) > +{ > + 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; > + > + switch (type) { > + case UDATA_TYPE_COMMENT: > + if (value[len - 1] != '\0') > + return -1; > + break; > + default: > + break; > + }; > + > + tb[type] = attr; > + return 1; > +} > + > +static void rule_print_udata(const struct nftnl_udata *tb[], uint8_t type) > +{ > + if (!tb[type]) > + return; > + > + switch (type) { > + case UDATA_TYPE_COMMENT: > + printf("comment \"%s\" ", > + (char *)nftnl_udata_attr_value(tb[type])); > + break; > + default: > + break; > + } > +} ... And these functions, you will place them in netlink_delinearize.c, which is where they belong since they are dealing with a low-level netlink aspect. > void rule_print(const struct rule *rule) > { > const struct stmt *stmt; > + const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] = {}; > > 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_udata_cb, tb) > 0) > + rule_print_udata(tb, UDATA_TYPE_COMMENT); > + } With the change I'm proposing above, you will not need this either. > 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 -- 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