netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] add object userdata and comment support
@ 2020-09-02  9:12 Jose M. Guisado Gomez
  2020-09-02  9:12 ` [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object Jose M. Guisado Gomez
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-02  9:12 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

This patch series enables userdata for objects. Initially used to store
comments, can be extended for other use cases in the future.

There is a new API version for libnftnl as it did not export a necessary
function, nftnl_obj_get_data, to support getting object data.

nf-next:
  netfilter: nf_tables: add userdata support for nft_object

 include/net/netfilter/nf_tables.h        |  2 ++
 include/uapi/linux/netfilter/nf_tables.h |  2 ++
 net/netfilter/nf_tables_api.c            | 39 +++++++++++++++++++-----
 3 files changed, 35 insertions(+), 8 deletions(-)

libnftnl:
  object: add userdata and comment support

 include/libnftnl/object.h           |  1 +
 include/libnftnl/udata.h            |  6 ++++++
 include/linux/netfilter/nf_tables.h |  2 ++
 include/obj.h                       |  5 +++++
 src/libnftnl.map                    |  4 ++++
 src/object.c                        | 26 ++++++++++++++++++++++++++
 6 files changed, 44 insertions(+)

nftables:
  src: add comment support for objects

 include/rule.h                                |  1 +
 src/mnl.c                                     | 12 +++++
 src/netlink.c                                 | 31 +++++++++++++
 src/parser_bison.y                            | 40 +++++++++++++++++
 src/rule.c                                    | 20 +++++++++
 .../testcases/optionals/comments_objects_0    | 44 +++++++++++++++++++
 .../optionals/dumps/comments_objects_0.nft    | 37 ++++++++++++++++
 7 files changed, 185 insertions(+)
 create mode 100755 tests/shell/testcases/optionals/comments_objects_0
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_objects_0.nft

-- 
2.27.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object
  2020-09-02  9:12 [PATCH 0/3] add object userdata and comment support Jose M. Guisado Gomez
@ 2020-09-02  9:12 ` Jose M. Guisado Gomez
  2020-09-08 10:42   ` Pablo Neira Ayuso
  2020-09-02  9:12 ` [PATCH libnftnl 2/3] object: add userdata and comment support Jose M. Guisado Gomez
  2020-09-02  9:12 ` [PATCH nftables 3/3] src: add comment support for objects Jose M. Guisado Gomez
  2 siblings, 1 reply; 10+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-02  9:12 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

Enables storing userdata for nft_object. Initially this will store an
optional comment but can be extended in the future as needed.

Adds new attribute NFTA_OBJ_USERDATA to nft_object.

Renames error labels in nf_tables_newobj to make it more clear.

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
 include/net/netfilter/nf_tables.h        |  2 ++
 include/uapi/linux/netfilter/nf_tables.h |  2 ++
 net/netfilter/nf_tables_api.c            | 39 +++++++++++++++++++-----
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 97a7e147a59a..99c1b3188b1e 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1123,6 +1123,8 @@ struct nft_object {
 	u32				genmask:2,
 					use:30;
 	u64				handle;
+	u16				udlen;
+	u8				*udata;
 	/* runtime data below here */
 	const struct nft_object_ops	*ops ____cacheline_aligned;
 	unsigned char			data[]
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 543dc697b796..2a6e09dea1a0 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1559,6 +1559,7 @@ enum nft_ct_expectation_attributes {
  * @NFTA_OBJ_DATA: stateful object data (NLA_NESTED)
  * @NFTA_OBJ_USE: number of references to this expression (NLA_U32)
  * @NFTA_OBJ_HANDLE: object handle (NLA_U64)
+ * @NFTA_OBJ_USERDATA: user data (NLA_BINARY)
  */
 enum nft_object_attributes {
 	NFTA_OBJ_UNSPEC,
@@ -1569,6 +1570,7 @@ enum nft_object_attributes {
 	NFTA_OBJ_USE,
 	NFTA_OBJ_HANDLE,
 	NFTA_OBJ_PAD,
+	NFTA_OBJ_USERDATA,
 	__NFTA_OBJ_MAX
 };
 #define NFTA_OBJ_MAX		(__NFTA_OBJ_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 6ccce2a2e715..55111aefd3db 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5755,6 +5755,8 @@ static const struct nla_policy nft_obj_policy[NFTA_OBJ_MAX + 1] = {
 	[NFTA_OBJ_TYPE]		= { .type = NLA_U32 },
 	[NFTA_OBJ_DATA]		= { .type = NLA_NESTED },
 	[NFTA_OBJ_HANDLE]	= { .type = NLA_U64},
+	[NFTA_OBJ_USERDATA]	= { .type = NLA_BINARY,
+				    .len = NFT_USERDATA_MAXLEN },
 };
 
 static struct nft_object *nft_obj_init(const struct nft_ctx *ctx,
@@ -5901,6 +5903,7 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
 	struct nft_table *table;
 	struct nft_object *obj;
 	struct nft_ctx ctx;
+	u16 udlen = 0;
 	u32 objtype;
 	int err;
 
@@ -5946,7 +5949,7 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
 	obj = nft_obj_init(&ctx, type, nla[NFTA_OBJ_DATA]);
 	if (IS_ERR(obj)) {
 		err = PTR_ERR(obj);
-		goto err1;
+		goto err_init;
 	}
 	obj->key.table = table;
 	obj->handle = nf_tables_alloc_handle(table);
@@ -5954,32 +5957,48 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
 	obj->key.name = nla_strdup(nla[NFTA_OBJ_NAME], GFP_KERNEL);
 	if (!obj->key.name) {
 		err = -ENOMEM;
-		goto err2;
+		goto err_strdup;
+	}
+
+	if(nla[NFTA_OBJ_USERDATA]) {
+		udlen = nla_len(nla[NFTA_OBJ_USERDATA]);
+		obj->udata = kzalloc(udlen, GFP_KERNEL);
+		if (obj->udata == NULL)
+			goto err_userdata;
+	} else {
+		obj->udata = NULL;
+	}
+
+	if (udlen) {
+		nla_memcpy(obj->udata, nla[NFTA_OBJ_USERDATA], udlen);
+		obj->udlen = udlen;
 	}
 
 	err = nft_trans_obj_add(&ctx, NFT_MSG_NEWOBJ, obj);
 	if (err < 0)
-		goto err3;
+		goto err_trans;
 
 	err = rhltable_insert(&nft_objname_ht, &obj->rhlhead,
 			      nft_objname_ht_params);
 	if (err < 0)
-		goto err4;
+		goto err_obj_ht;
 
 	list_add_tail_rcu(&obj->list, &table->objects);
 	table->use++;
 	return 0;
-err4:
+err_obj_ht:
 	/* queued in transaction log */
 	INIT_LIST_HEAD(&obj->list);
 	return err;
-err3:
+err_trans:
 	kfree(obj->key.name);
-err2:
+err_userdata:
+	kfree(obj->udata);
+err_strdup:
 	if (obj->ops->destroy)
 		obj->ops->destroy(&ctx, obj);
 	kfree(obj);
-err1:
+err_init:
 	module_put(type->owner);
 	return err;
 }
@@ -6011,6 +6030,10 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
 			 NFTA_OBJ_PAD))
 		goto nla_put_failure;
 
+	if (obj->udata &&
+	    nla_put(skb, NFTA_OBJ_USERDATA, obj->udlen, obj->udata))
+		goto nla_put_failure;
+
 	nlmsg_end(skb, nlh);
 	return 0;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH libnftnl 2/3] object: add userdata and comment support
  2020-09-02  9:12 [PATCH 0/3] add object userdata and comment support Jose M. Guisado Gomez
  2020-09-02  9:12 ` [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object Jose M. Guisado Gomez
@ 2020-09-02  9:12 ` Jose M. Guisado Gomez
  2020-09-08 14:52   ` Pablo Neira Ayuso
  2020-09-02  9:12 ` [PATCH nftables 3/3] src: add comment support for objects Jose M. Guisado Gomez
  2 siblings, 1 reply; 10+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-02  9:12 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

This patch adds NFTNL_OBJ_USERDATA to support userdata for objects.

Also adds NFTNL_UDATA_OBJ_COMMENT to support comments for objects,
stored in userdata space.

Bumps libnftnl.map to 15 as nftnl_obj_get_data needs to be exported to
enable getting object attributes/data.

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
 include/libnftnl/object.h           |  1 +
 include/libnftnl/udata.h            |  6 ++++++
 include/linux/netfilter/nf_tables.h |  2 ++
 include/obj.h                       |  5 +++++
 src/libnftnl.map                    |  4 ++++
 src/object.c                        | 26 ++++++++++++++++++++++++++
 6 files changed, 44 insertions(+)

diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
index 4c23774..9bd83a5 100644
--- a/include/libnftnl/object.h
+++ b/include/libnftnl/object.h
@@ -19,6 +19,7 @@ enum {
 	NFTNL_OBJ_FAMILY,
 	NFTNL_OBJ_USE,
 	NFTNL_OBJ_HANDLE,
+	NFTNL_OBJ_USERDATA,
 	NFTNL_OBJ_BASE		= 16,
 	__NFTNL_OBJ_MAX
 };
diff --git a/include/libnftnl/udata.h b/include/libnftnl/udata.h
index ba6b3ab..2e38fcc 100644
--- a/include/libnftnl/udata.h
+++ b/include/libnftnl/udata.h
@@ -22,6 +22,12 @@ enum nftnl_udata_rule_types {
 };
 #define NFTNL_UDATA_RULE_MAX (__NFTNL_UDATA_RULE_MAX - 1)
 
+enum nftnl_udata_obj_types {
+	NFTNL_UDATA_OBJ_COMMENT,
+	__NFTNL_UDATA_OBJ_MAX
+};
+#define NFTNL_UDATA_OBJ_MAX (__NFTNL_UDATA_OBJ_MAX - 1)
+
 #define NFTNL_UDATA_COMMENT_MAXLEN	128
 
 enum nftnl_udata_set_types {
diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index d508154..1d65ff9 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -1542,6 +1542,7 @@ enum nft_ct_expectation_attributes {
  * @NFTA_OBJ_DATA: stateful object data (NLA_NESTED)
  * @NFTA_OBJ_USE: number of references to this expression (NLA_U32)
  * @NFTA_OBJ_HANDLE: object handle (NLA_U64)
+ * @NFTA_OBJ_HANDLE: user data (NLA_BINARY)
  */
 enum nft_object_attributes {
 	NFTA_OBJ_UNSPEC,
@@ -1552,6 +1553,7 @@ enum nft_object_attributes {
 	NFTA_OBJ_USE,
 	NFTA_OBJ_HANDLE,
 	NFTA_OBJ_PAD,
+	NFTA_OBJ_USERDATA,
 	__NFTA_OBJ_MAX
 };
 #define NFTA_OBJ_MAX		(__NFTA_OBJ_MAX - 1)
diff --git a/include/obj.h b/include/obj.h
index 10f806c..d9e856a 100644
--- a/include/obj.h
+++ b/include/obj.h
@@ -22,6 +22,11 @@ struct nftnl_obj {
 	uint32_t		flags;
 	uint64_t		handle;
 
+	struct {
+		void		*data;
+		uint32_t	len;
+	} user;
+
 	union {
 		struct nftnl_obj_counter {
 			uint64_t	pkts;
diff --git a/src/libnftnl.map b/src/libnftnl.map
index 6042479..ceafa3f 100644
--- a/src/libnftnl.map
+++ b/src/libnftnl.map
@@ -368,3 +368,7 @@ LIBNFTNL_14 {
   nftnl_flowtable_set_array;
   nftnl_flowtable_get_array;
 } LIBNFTNL_13;
+
+LIBNFTNL_15 {
+  nftnl_obj_get_data;
+} LIBNFTNL_14;
diff --git a/src/object.c b/src/object.c
index 4f58272..008bade 100644
--- a/src/object.c
+++ b/src/object.c
@@ -57,6 +57,8 @@ void nftnl_obj_free(const struct nftnl_obj *obj)
 		xfree(obj->table);
 	if (obj->flags & (1 << NFTNL_OBJ_NAME))
 		xfree(obj->name);
+	if (obj->flags & (1 << NFTNL_OBJ_USERDATA))
+		xfree(obj->user.data);
 
 	xfree(obj);
 }
@@ -103,6 +105,16 @@ void nftnl_obj_set_data(struct nftnl_obj *obj, uint16_t attr,
 	case NFTNL_OBJ_HANDLE:
 		memcpy(&obj->handle, data, sizeof(obj->handle));
 		break;
+	case NFTNL_OBJ_USERDATA:
+		if (obj->flags & (1 << NFTNL_OBJ_USERDATA))
+			xfree(obj->user.data);
+
+		obj->user.data = malloc(data_len);
+		if (!obj->user.data)
+			return;
+		memcpy(obj->user.data, data, data_len);
+		obj->user.len = data_len;
+		break;
 	default:
 		if (obj->ops)
 			obj->ops->set(obj, attr, data, data_len);
@@ -174,6 +186,9 @@ const void *nftnl_obj_get_data(struct nftnl_obj *obj, uint16_t attr,
 	case NFTNL_OBJ_HANDLE:
 		*data_len = sizeof(uint64_t);
 		return &obj->handle;
+	case NFTNL_OBJ_USERDATA:
+		*data_len = obj->user.len;
+		return obj->user.data;
 	default:
 		if (obj->ops)
 			return obj->ops->get(obj, attr, data_len);
@@ -235,6 +250,8 @@ void nftnl_obj_nlmsg_build_payload(struct nlmsghdr *nlh,
 		mnl_attr_put_u32(nlh, NFTA_OBJ_TYPE, htonl(obj->ops->type));
 	if (obj->flags & (1 << NFTNL_OBJ_HANDLE))
 		mnl_attr_put_u64(nlh, NFTA_OBJ_HANDLE, htobe64(obj->handle));
+	if (obj->flags & (1 << NFTNL_OBJ_USERDATA))
+		mnl_attr_put(nlh, NFTA_OBJ_USERDATA, obj->user.len, obj->user.data);
 	if (obj->ops) {
 		struct nlattr *nest = mnl_attr_nest_start(nlh, NFTA_OBJ_DATA);
 
@@ -269,6 +286,10 @@ static int nftnl_obj_parse_attr_cb(const struct nlattr *attr, void *data)
 		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
 			abi_breakage();
 		break;
+	case NFTA_OBJ_USERDATA:
+		if (mnl_attr_validate(attr, MNL_TYPE_BINARY) < 0)
+			abi_breakage();
+		break;
 	}
 
 	tb[type] = attr;
@@ -315,6 +336,11 @@ int nftnl_obj_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_obj *obj)
 		obj->handle = be64toh(mnl_attr_get_u64(tb[NFTA_OBJ_HANDLE]));
 		obj->flags |= (1 << NFTNL_OBJ_HANDLE);
 	}
+	if (tb[NFTA_OBJ_USERDATA]) {
+		nftnl_obj_set_data(obj, NFTNL_OBJ_USERDATA,
+				   mnl_attr_get_payload(tb[NFTA_OBJ_USERDATA]),
+				   mnl_attr_get_payload_len(tb[NFTA_OBJ_USERDATA]));
+	}
 
 	obj->family = nfg->nfgen_family;
 	obj->flags |= (1 << NFTNL_OBJ_FAMILY);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH nftables 3/3] src: add comment support for objects
  2020-09-02  9:12 [PATCH 0/3] add object userdata and comment support Jose M. Guisado Gomez
  2020-09-02  9:12 ` [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object Jose M. Guisado Gomez
  2020-09-02  9:12 ` [PATCH libnftnl 2/3] object: add userdata and comment support Jose M. Guisado Gomez
@ 2020-09-02  9:12 ` Jose M. Guisado Gomez
  2020-09-03  9:16   ` [PATCH nftables 3/3, v2] " Jose M. Guisado Gomez
  2020-09-08 14:53   ` [PATCH nftables 3/3] " Pablo Neira Ayuso
  2 siblings, 2 replies; 10+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-02  9:12 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

Enables specifying an optional comment when declaring named objects. The
comment is to be specified inside the object's block ({} block)

Relies on libnftnl exporting nftnl_obj_get_data and kernel space support
to store the comments.

For consistency, this patch makes the comment be printed first when
listing objects.

Adds a testcase importing all commented named objects except for secmark,
although it's supported.

Example: Adding a quota with a comment

> add table inet filter
> nft add quota inet filter q { over 1200 bytes \; comment "test_comment"\; }
> list ruleset

table inet filter {
	quota q {
		comment "test_comment"
		over 1200 bytes
	}
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
 include/rule.h                                |  1 +
 src/mnl.c                                     | 12 +++++
 src/netlink.c                                 | 31 +++++++++++++
 src/parser_bison.y                            | 40 +++++++++++++++++
 src/rule.c                                    | 20 +++++++++
 .../testcases/optionals/comments_objects_0    | 44 +++++++++++++++++++
 .../optionals/dumps/comments_objects_0.nft    | 37 ++++++++++++++++
 7 files changed, 185 insertions(+)
 create mode 100755 tests/shell/testcases/optionals/comments_objects_0
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_objects_0.nft

diff --git a/include/rule.h b/include/rule.h
index 56f1951f..837005b1 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -479,6 +479,7 @@ struct obj {
 	struct handle			handle;
 	uint32_t			type;
 	unsigned int			refcnt;
+	const char			*comment;
 	union {
 		struct counter		counter;
 		struct quota		quota;
diff --git a/src/mnl.c b/src/mnl.c
index cdcf9490..ca4f4b2a 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -1189,6 +1189,7 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
 		    unsigned int flags)
 {
 	struct obj *obj = cmd->object;
+	struct nftnl_udata_buf *udbuf;
 	struct nftnl_obj *nlo;
 	struct nlmsghdr *nlh;
 
@@ -1199,6 +1200,17 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
 	nftnl_obj_set_u32(nlo, NFTNL_OBJ_FAMILY, cmd->handle.family);
 	nftnl_obj_set_u32(nlo, NFTNL_OBJ_TYPE, obj->type);
 
+	if (obj->comment) {
+		udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
+		if (!udbuf)
+			memory_allocation_error();
+		if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_OBJ_COMMENT, obj->comment))
+			memory_allocation_error();
+		nftnl_obj_set_data(nlo, NFTNL_OBJ_USERDATA, nftnl_udata_buf_data(udbuf),
+				     nftnl_udata_buf_len(udbuf));
+		nftnl_udata_buf_free(udbuf);
+	}
+
 	switch (obj->type) {
 	case NFT_OBJECT_COUNTER:
 		nftnl_obj_set_u64(nlo, NFTNL_OBJ_CTR_PKTS,
diff --git a/src/netlink.c b/src/netlink.c
index a107f492..6912b018 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1314,11 +1314,33 @@ void netlink_dump_obj(struct nftnl_obj *nln, struct netlink_ctx *ctx)
 	fprintf(fp, "\n");
 }
 
+static int obj_parse_udata_cb(const struct nftnl_udata *attr, void *data)
+{
+	unsigned char *value = nftnl_udata_get(attr);
+	uint8_t type = nftnl_udata_type(attr);
+	const struct nftnl_udata **tb = data;
+	uint8_t len = nftnl_udata_len(attr);
+
+	switch (type) {
+		case NFTNL_UDATA_OBJ_COMMENT:
+			if (value[len - 1] != '\0')
+				return -1;
+			break;
+		default:
+			return 0;
+	}
+	tb[type] = attr;
+	return 0;
+}
+
 struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 				    struct nftnl_obj *nlo)
 {
+	const struct nftnl_udata *ud[NFTNL_UDATA_OBJ_MAX + 1] = {};
+	const char *udata;
 	struct obj *obj;
 	uint32_t type;
+	uint32_t ulen;
 
 	obj = obj_alloc(&netlink_location);
 	obj->handle.family = nftnl_obj_get_u32(nlo, NFTNL_OBJ_FAMILY);
@@ -1328,6 +1350,15 @@ struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 		xstrdup(nftnl_obj_get_str(nlo, NFTNL_OBJ_NAME));
 	obj->handle.handle.id =
 		nftnl_obj_get_u64(nlo, NFTNL_OBJ_HANDLE);
+	if (nftnl_obj_is_set(nlo, NFTNL_OBJ_USERDATA)) {
+		udata = nftnl_obj_get_data(nlo, NFTNL_OBJ_USERDATA, &ulen);
+		if (nftnl_udata_parse(udata, ulen, obj_parse_udata_cb, ud) < 0) {
+			netlink_io_error(ctx, NULL, "Cannot parse userdata");
+			return NULL;
+		}
+		if (ud[NFTNL_UDATA_OBJ_COMMENT])
+			obj->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_OBJ_COMMENT]));
+	}
 
 	type = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TYPE);
 	switch (type) {
diff --git a/src/parser_bison.y b/src/parser_bison.y
index d938f566..cf689426 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1006,10 +1006,18 @@ add_cmd			:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
 			}
+			|	COUNTER		obj_spec	counter_obj	'{' counter_block '}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
+			}
 			|	QUOTA		obj_spec	quota_obj	quota_config
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
+			|	QUOTA		obj_spec	quota_obj	'{' quota_block	'}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
+			}
 			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_helper_block '}'
 			{
 				$$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
@@ -2039,6 +2047,10 @@ counter_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|	counter_block	  comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 quota_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2048,6 +2060,10 @@ quota_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|	quota_block	comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2057,6 +2073,10 @@ ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       ct_helper_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 ct_timeout_block	:	/*empty */
@@ -2070,6 +2090,10 @@ ct_timeout_block	:	/*empty */
 			{
 				$$ = $1;
 			}
+			|       ct_timeout_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 ct_expect_block		:	/*empty */	{ $$ = $<obj>-1; }
@@ -2079,6 +2103,10 @@ ct_expect_block		:	/*empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       ct_expect_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 limit_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2088,6 +2116,10 @@ limit_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       limit_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 secmark_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2097,6 +2129,10 @@ secmark_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       secmark_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 synproxy_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2106,6 +2142,10 @@ synproxy_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       synproxy_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 type_identifier		:	STRING	{ $$ = $1; }
diff --git a/src/rule.c b/src/rule.c
index 2c4b5dbe..4ebc8d81 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1893,6 +1893,7 @@ void obj_free(struct obj *obj)
 {
 	if (--obj->refcnt > 0)
 		return;
+	xfree(obj->comment);
 	handle_free(&obj->handle);
 	xfree(obj);
 }
@@ -1986,6 +1987,16 @@ static const char *synproxy_timestamp_to_str(const uint32_t flags)
         return "";
 }
 
+static void obj_print_comment(const struct obj *obj,
+			      struct print_fmt_options *opts,
+			      struct output_ctx *octx)
+{
+	if (obj->comment)
+		nft_print(octx, "%s%s%scomment \"%s\"",
+			  opts->nl, opts->tab, opts->tab,
+			  obj->comment);
+}
+
 static void obj_print_data(const struct obj *obj,
 			   struct print_fmt_options *opts,
 			   struct output_ctx *octx)
@@ -1995,6 +2006,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		if (nft_output_stateless(octx)) {
 			nft_print(octx, "packets 0 bytes 0");
@@ -2010,6 +2022,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		data_unit = get_rate(obj->quota.bytes, &bytes);
 		nft_print(octx, "%s%" PRIu64 " %s",
@@ -2027,6 +2040,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		nft_print(octx, "\"%s\"%s", obj->secmark.ctx, opts->nl);
 		break;
@@ -2034,6 +2048,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s", opts->nl);
 		nft_print(octx, "%s%stype \"%s\" protocol ",
 			  opts->tab, opts->tab, obj->ct_helper.name);
@@ -2048,6 +2063,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s", opts->nl);
 		nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
 		print_proto_name_proto(obj->ct_timeout.l4proto, octx);
@@ -2063,6 +2079,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s", opts->nl);
 		nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
 		print_proto_name_proto(obj->ct_expect.l4proto, octx);
@@ -2091,6 +2108,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		switch (obj->limit.type) {
 		case NFT_LIMIT_PKTS:
@@ -2128,6 +2146,8 @@ static void obj_print_data(const struct obj *obj,
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
 
+		obj_print_comment(obj, opts, octx);
+
 		if (flags & NF_SYNPROXY_OPT_MSS) {
 			nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 			nft_print(octx, "mss %u", obj->synproxy.mss);
diff --git a/tests/shell/testcases/optionals/comments_objects_0 b/tests/shell/testcases/optionals/comments_objects_0
new file mode 100755
index 00000000..7437c77b
--- /dev/null
+++ b/tests/shell/testcases/optionals/comments_objects_0
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+EXPECTED='table ip filter {
+	quota q {
+		over 1200 bytes
+		comment "test1"
+	}
+
+	counter c {
+		packets 0 bytes 0
+		comment "test2"
+	}
+
+	ct helper h {
+		type "sip" protocol tcp
+		l3proto ip
+		comment "test3"
+	}
+
+	ct expectation e {
+		protocol tcp
+		dport 666
+		timeout 100ms
+		size 96
+		l3proto ip
+		comment "test4"
+	}
+
+	limit l {
+		rate 400/hour
+		comment "test5"
+	}
+
+	synproxy s {
+		mss 1460
+		wscale 2
+		comment "test6"
+	}
+}
+'
+
+set -e
+
+$NFT -f - <<< "$EXPECTED"
diff --git a/tests/shell/testcases/optionals/dumps/comments_objects_0.nft b/tests/shell/testcases/optionals/dumps/comments_objects_0.nft
new file mode 100644
index 00000000..b760ced6
--- /dev/null
+++ b/tests/shell/testcases/optionals/dumps/comments_objects_0.nft
@@ -0,0 +1,37 @@
+table ip filter {
+	quota q {
+		comment "test1"
+		over 1200 bytes
+	}
+
+	counter c {
+		comment "test2"
+		packets 0 bytes 0
+	}
+
+	ct helper h {
+		comment "test3"
+		type "sip" protocol tcp
+		l3proto ip
+	}
+
+	ct expectation e {
+		comment "test4"
+		protocol tcp
+		dport 666
+		timeout 100ms
+		size 96
+		l3proto ip
+	}
+
+	limit l {
+		comment "test5"
+		rate 400/hour
+	}
+
+	synproxy s {
+		comment "test6"
+		mss 1460
+		wscale 2
+	}
+}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH nftables 3/3, v2] src: add comment support for objects
  2020-09-02  9:12 ` [PATCH nftables 3/3] src: add comment support for objects Jose M. Guisado Gomez
@ 2020-09-03  9:16   ` Jose M. Guisado Gomez
  2020-09-08 14:53   ` [PATCH nftables 3/3] " Pablo Neira Ayuso
  1 sibling, 0 replies; 10+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-03  9:16 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

Enables specifying an optional comment when declaring named objects. The
comment is to be specified inside the object's block ({} block)

Relies on libnftnl exporting nftnl_obj_get_data and kernel space support
to store the comments.

For consistency, this patch makes the comment be printed first when
listing objects.

Adds a testcase importing all commented named objects except for secmark,
although it's supported.

Example: Adding a quota with a comment

> add table inet filter
> nft add quota inet filter q { over 1200 bytes \; comment "test_comment"\; }
> list ruleset

table inet filter {
	quota q {
		comment "test_comment"
		over 1200 bytes
	}
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
Adds missing parser { *_block } for some objects in v1

 include/rule.h                                |  1 +
 src/mnl.c                                     | 12 +++++
 src/netlink.c                                 | 31 +++++++++++
 src/parser_bison.y                            | 52 +++++++++++++++++++
 src/rule.c                                    | 20 +++++++
 .../testcases/optionals/comments_objects_0    | 44 ++++++++++++++++
 .../optionals/dumps/comments_objects_0.nft    | 37 +++++++++++++
 7 files changed, 197 insertions(+)
 create mode 100755 tests/shell/testcases/optionals/comments_objects_0
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_objects_0.nft

diff --git a/include/rule.h b/include/rule.h
index 56f1951f..837005b1 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -479,6 +479,7 @@ struct obj {
 	struct handle			handle;
 	uint32_t			type;
 	unsigned int			refcnt;
+	const char			*comment;
 	union {
 		struct counter		counter;
 		struct quota		quota;
diff --git a/src/mnl.c b/src/mnl.c
index cdcf9490..ca4f4b2a 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -1189,6 +1189,7 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
 		    unsigned int flags)
 {
 	struct obj *obj = cmd->object;
+	struct nftnl_udata_buf *udbuf;
 	struct nftnl_obj *nlo;
 	struct nlmsghdr *nlh;
 
@@ -1199,6 +1200,17 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
 	nftnl_obj_set_u32(nlo, NFTNL_OBJ_FAMILY, cmd->handle.family);
 	nftnl_obj_set_u32(nlo, NFTNL_OBJ_TYPE, obj->type);
 
+	if (obj->comment) {
+		udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
+		if (!udbuf)
+			memory_allocation_error();
+		if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_OBJ_COMMENT, obj->comment))
+			memory_allocation_error();
+		nftnl_obj_set_data(nlo, NFTNL_OBJ_USERDATA, nftnl_udata_buf_data(udbuf),
+				     nftnl_udata_buf_len(udbuf));
+		nftnl_udata_buf_free(udbuf);
+	}
+
 	switch (obj->type) {
 	case NFT_OBJECT_COUNTER:
 		nftnl_obj_set_u64(nlo, NFTNL_OBJ_CTR_PKTS,
diff --git a/src/netlink.c b/src/netlink.c
index a107f492..6912b018 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1314,11 +1314,33 @@ void netlink_dump_obj(struct nftnl_obj *nln, struct netlink_ctx *ctx)
 	fprintf(fp, "\n");
 }
 
+static int obj_parse_udata_cb(const struct nftnl_udata *attr, void *data)
+{
+	unsigned char *value = nftnl_udata_get(attr);
+	uint8_t type = nftnl_udata_type(attr);
+	const struct nftnl_udata **tb = data;
+	uint8_t len = nftnl_udata_len(attr);
+
+	switch (type) {
+		case NFTNL_UDATA_OBJ_COMMENT:
+			if (value[len - 1] != '\0')
+				return -1;
+			break;
+		default:
+			return 0;
+	}
+	tb[type] = attr;
+	return 0;
+}
+
 struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 				    struct nftnl_obj *nlo)
 {
+	const struct nftnl_udata *ud[NFTNL_UDATA_OBJ_MAX + 1] = {};
+	const char *udata;
 	struct obj *obj;
 	uint32_t type;
+	uint32_t ulen;
 
 	obj = obj_alloc(&netlink_location);
 	obj->handle.family = nftnl_obj_get_u32(nlo, NFTNL_OBJ_FAMILY);
@@ -1328,6 +1350,15 @@ struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 		xstrdup(nftnl_obj_get_str(nlo, NFTNL_OBJ_NAME));
 	obj->handle.handle.id =
 		nftnl_obj_get_u64(nlo, NFTNL_OBJ_HANDLE);
+	if (nftnl_obj_is_set(nlo, NFTNL_OBJ_USERDATA)) {
+		udata = nftnl_obj_get_data(nlo, NFTNL_OBJ_USERDATA, &ulen);
+		if (nftnl_udata_parse(udata, ulen, obj_parse_udata_cb, ud) < 0) {
+			netlink_io_error(ctx, NULL, "Cannot parse userdata");
+			return NULL;
+		}
+		if (ud[NFTNL_UDATA_OBJ_COMMENT])
+			obj->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_OBJ_COMMENT]));
+	}
 
 	type = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TYPE);
 	switch (type) {
diff --git a/src/parser_bison.y b/src/parser_bison.y
index d938f566..7242c4c3 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1006,10 +1006,18 @@ add_cmd			:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
 			}
+			|	COUNTER		obj_spec	counter_obj	'{' counter_block '}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
+			}
 			|	QUOTA		obj_spec	quota_obj	quota_config
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
 			}
+			|	QUOTA		obj_spec	quota_obj	'{' quota_block	'}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
+			}
 			|	CT	HELPER	obj_spec	ct_obj_alloc	'{' ct_helper_block '}'
 			{
 				$$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
@@ -1026,14 +1034,26 @@ add_cmd			:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_LIMIT, &$2, &@$, $3);
 			}
+			|	LIMIT		obj_spec	limit_obj	'{' limit_block '}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_LIMIT, &$2, &@$, $3);
+			}
 			|	SECMARK		obj_spec	secmark_obj	secmark_config
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_SECMARK, &$2, &@$, $3);
 			}
+			|	SECMARK		obj_spec	secmark_obj	'{' secmark_block '}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_SECMARK, &$2, &@$, $3);
+			}
 			|	SYNPROXY	obj_spec	synproxy_obj	synproxy_config
 			{
 				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_SYNPROXY, &$2, &@$, $3);
 			}
+			|	SYNPROXY	obj_spec	synproxy_obj	'{' synproxy_block '}'
+			{
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_SYNPROXY, &$2, &@$, $3);
+			}
 			;
 
 replace_cmd		:	RULE		ruleid_spec	rule
@@ -2039,6 +2059,10 @@ counter_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|	counter_block	  comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 quota_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2048,6 +2072,10 @@ quota_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|	quota_block	comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2057,6 +2085,10 @@ ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       ct_helper_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 ct_timeout_block	:	/*empty */
@@ -2070,6 +2102,10 @@ ct_timeout_block	:	/*empty */
 			{
 				$$ = $1;
 			}
+			|       ct_timeout_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 ct_expect_block		:	/*empty */	{ $$ = $<obj>-1; }
@@ -2079,6 +2115,10 @@ ct_expect_block		:	/*empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       ct_expect_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 limit_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2088,6 +2128,10 @@ limit_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       limit_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 secmark_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2097,6 +2141,10 @@ secmark_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       secmark_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 synproxy_block		:	/* empty */	{ $$ = $<obj>-1; }
@@ -2106,6 +2154,10 @@ synproxy_block		:	/* empty */	{ $$ = $<obj>-1; }
 			{
 				$$ = $1;
 			}
+			|       synproxy_block     comment_spec
+			{
+				$<obj>1->comment = $2;
+			}
 			;
 
 type_identifier		:	STRING	{ $$ = $1; }
diff --git a/src/rule.c b/src/rule.c
index 2c4b5dbe..4ebc8d81 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1893,6 +1893,7 @@ void obj_free(struct obj *obj)
 {
 	if (--obj->refcnt > 0)
 		return;
+	xfree(obj->comment);
 	handle_free(&obj->handle);
 	xfree(obj);
 }
@@ -1986,6 +1987,16 @@ static const char *synproxy_timestamp_to_str(const uint32_t flags)
         return "";
 }
 
+static void obj_print_comment(const struct obj *obj,
+			      struct print_fmt_options *opts,
+			      struct output_ctx *octx)
+{
+	if (obj->comment)
+		nft_print(octx, "%s%s%scomment \"%s\"",
+			  opts->nl, opts->tab, opts->tab,
+			  obj->comment);
+}
+
 static void obj_print_data(const struct obj *obj,
 			   struct print_fmt_options *opts,
 			   struct output_ctx *octx)
@@ -1995,6 +2006,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		if (nft_output_stateless(octx)) {
 			nft_print(octx, "packets 0 bytes 0");
@@ -2010,6 +2022,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		data_unit = get_rate(obj->quota.bytes, &bytes);
 		nft_print(octx, "%s%" PRIu64 " %s",
@@ -2027,6 +2040,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		nft_print(octx, "\"%s\"%s", obj->secmark.ctx, opts->nl);
 		break;
@@ -2034,6 +2048,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s", opts->nl);
 		nft_print(octx, "%s%stype \"%s\" protocol ",
 			  opts->tab, opts->tab, obj->ct_helper.name);
@@ -2048,6 +2063,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s", opts->nl);
 		nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
 		print_proto_name_proto(obj->ct_timeout.l4proto, octx);
@@ -2063,6 +2079,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s", opts->nl);
 		nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
 		print_proto_name_proto(obj->ct_expect.l4proto, octx);
@@ -2091,6 +2108,7 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, " %s {", obj->handle.obj.name);
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+		obj_print_comment(obj, opts, octx);
 		nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 		switch (obj->limit.type) {
 		case NFT_LIMIT_PKTS:
@@ -2128,6 +2146,8 @@ static void obj_print_data(const struct obj *obj,
 		if (nft_output_handle(octx))
 			nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
 
+		obj_print_comment(obj, opts, octx);
+
 		if (flags & NF_SYNPROXY_OPT_MSS) {
 			nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
 			nft_print(octx, "mss %u", obj->synproxy.mss);
diff --git a/tests/shell/testcases/optionals/comments_objects_0 b/tests/shell/testcases/optionals/comments_objects_0
new file mode 100755
index 00000000..7437c77b
--- /dev/null
+++ b/tests/shell/testcases/optionals/comments_objects_0
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+EXPECTED='table ip filter {
+	quota q {
+		over 1200 bytes
+		comment "test1"
+	}
+
+	counter c {
+		packets 0 bytes 0
+		comment "test2"
+	}
+
+	ct helper h {
+		type "sip" protocol tcp
+		l3proto ip
+		comment "test3"
+	}
+
+	ct expectation e {
+		protocol tcp
+		dport 666
+		timeout 100ms
+		size 96
+		l3proto ip
+		comment "test4"
+	}
+
+	limit l {
+		rate 400/hour
+		comment "test5"
+	}
+
+	synproxy s {
+		mss 1460
+		wscale 2
+		comment "test6"
+	}
+}
+'
+
+set -e
+
+$NFT -f - <<< "$EXPECTED"
diff --git a/tests/shell/testcases/optionals/dumps/comments_objects_0.nft b/tests/shell/testcases/optionals/dumps/comments_objects_0.nft
new file mode 100644
index 00000000..b760ced6
--- /dev/null
+++ b/tests/shell/testcases/optionals/dumps/comments_objects_0.nft
@@ -0,0 +1,37 @@
+table ip filter {
+	quota q {
+		comment "test1"
+		over 1200 bytes
+	}
+
+	counter c {
+		comment "test2"
+		packets 0 bytes 0
+	}
+
+	ct helper h {
+		comment "test3"
+		type "sip" protocol tcp
+		l3proto ip
+	}
+
+	ct expectation e {
+		comment "test4"
+		protocol tcp
+		dport 666
+		timeout 100ms
+		size 96
+		l3proto ip
+	}
+
+	limit l {
+		comment "test5"
+		rate 400/hour
+	}
+
+	synproxy s {
+		comment "test6"
+		mss 1460
+		wscale 2
+	}
+}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object
  2020-09-02  9:12 ` [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object Jose M. Guisado Gomez
@ 2020-09-08 10:42   ` Pablo Neira Ayuso
  2020-09-08 11:01     ` [PATCH nf-next 1/3, v2] " Jose M. Guisado Gomez
  0 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-08 10:42 UTC (permalink / raw)
  To: Jose M. Guisado Gomez; +Cc: netfilter-devel

On Wed, Sep 02, 2020 at 11:12:39AM +0200, Jose M. Guisado Gomez wrote:
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 6ccce2a2e715..55111aefd3db 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
[...]
> @@ -5954,32 +5957,48 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
>  	obj->key.name = nla_strdup(nla[NFTA_OBJ_NAME], GFP_KERNEL);
>  	if (!obj->key.name) {
>  		err = -ENOMEM;
> -		goto err2;
> +		goto err_strdup;
> +	}
> +
> +	if(nla[NFTA_OBJ_USERDATA]) {
> +		udlen = nla_len(nla[NFTA_OBJ_USERDATA]);
> +		obj->udata = kzalloc(udlen, GFP_KERNEL);
> +		if (obj->udata == NULL)
> +			goto err_userdata;
> +	} else {
> +		obj->udata = NULL;
> +	}
> +
> +	if (udlen) {
> +		nla_memcpy(obj->udata, nla[NFTA_OBJ_USERDATA], udlen);
> +		obj->udlen = udlen;
>  	}

Probably simplify this?

	if(nla[NFTA_OBJ_USERDATA]) {
		udlen = nla_len(nla[NFTA_OBJ_USERDATA]);
		obj->udata = kzalloc(udlen, GFP_KERNEL);
		if (obj->udata == NULL)
			goto err_userdata;

        	nla_memcpy(obj->udata, nla[NFTA_OBJ_USERDATA], udlen);
        	obj->udlen = udlen;
        }

obj is allocated via kzalloc(), so obj->udata is already guaranteed to
be initialized to NULL, no need for the 'else' side of the branch.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH nf-next 1/3, v2] netfilter: nf_tables: add userdata support for nft_object
  2020-09-08 10:42   ` Pablo Neira Ayuso
@ 2020-09-08 11:01     ` Jose M. Guisado Gomez
  2020-09-08 14:36       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 10+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-08 11:01 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel

Enables storing userdata for nft_object. Initially this will store an
optional comment but can be extended in the future as needed.

Adds new attribute NFTA_OBJ_USERDATA to nft_object.

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
Simplifications from v1, as proposed.

 include/net/netfilter/nf_tables.h        |  2 ++
 include/uapi/linux/netfilter/nf_tables.h |  2 ++
 net/netfilter/nf_tables_api.c            | 35 ++++++++++++++++++------
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 97a7e147a59a..99c1b3188b1e 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1123,6 +1123,8 @@ struct nft_object {
 	u32				genmask:2,
 					use:30;
 	u64				handle;
+	u16				udlen;
+	u8				*udata;
 	/* runtime data below here */
 	const struct nft_object_ops	*ops ____cacheline_aligned;
 	unsigned char			data[]
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 543dc697b796..2a6e09dea1a0 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1559,6 +1559,7 @@ enum nft_ct_expectation_attributes {
  * @NFTA_OBJ_DATA: stateful object data (NLA_NESTED)
  * @NFTA_OBJ_USE: number of references to this expression (NLA_U32)
  * @NFTA_OBJ_HANDLE: object handle (NLA_U64)
+ * @NFTA_OBJ_USERDATA: user data (NLA_BINARY)
  */
 enum nft_object_attributes {
 	NFTA_OBJ_UNSPEC,
@@ -1569,6 +1570,7 @@ enum nft_object_attributes {
 	NFTA_OBJ_USE,
 	NFTA_OBJ_HANDLE,
 	NFTA_OBJ_PAD,
+	NFTA_OBJ_USERDATA,
 	__NFTA_OBJ_MAX
 };
 #define NFTA_OBJ_MAX		(__NFTA_OBJ_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 6ccce2a2e715..7d6cd49c246e 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5755,6 +5755,8 @@ static const struct nla_policy nft_obj_policy[NFTA_OBJ_MAX + 1] = {
 	[NFTA_OBJ_TYPE]		= { .type = NLA_U32 },
 	[NFTA_OBJ_DATA]		= { .type = NLA_NESTED },
 	[NFTA_OBJ_HANDLE]	= { .type = NLA_U64},
+	[NFTA_OBJ_USERDATA]	= { .type = NLA_BINARY,
+				    .len = NFT_USERDATA_MAXLEN },
 };
 
 static struct nft_object *nft_obj_init(const struct nft_ctx *ctx,
@@ -5901,6 +5903,7 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
 	struct nft_table *table;
 	struct nft_object *obj;
 	struct nft_ctx ctx;
+	u16 udlen = 0;
 	u32 objtype;
 	int err;
 
@@ -5946,7 +5949,7 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
 	obj = nft_obj_init(&ctx, type, nla[NFTA_OBJ_DATA]);
 	if (IS_ERR(obj)) {
 		err = PTR_ERR(obj);
-		goto err1;
+		goto err_init;
 	}
 	obj->key.table = table;
 	obj->handle = nf_tables_alloc_handle(table);
@@ -5954,32 +5957,44 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
 	obj->key.name = nla_strdup(nla[NFTA_OBJ_NAME], GFP_KERNEL);
 	if (!obj->key.name) {
 		err = -ENOMEM;
-		goto err2;
+		goto err_strdup;
+	}
+
+	if(nla[NFTA_OBJ_USERDATA]) {
+		udlen = nla_len(nla[NFTA_OBJ_USERDATA]);
+		obj->udata = kzalloc(udlen, GFP_KERNEL);
+		if (obj->udata == NULL)
+			goto err_userdata;
+
+		nla_memcpy(obj->udata, nla[NFTA_OBJ_USERDATA], udlen);
+		obj->udlen = udlen;
 	}
 
 	err = nft_trans_obj_add(&ctx, NFT_MSG_NEWOBJ, obj);
 	if (err < 0)
-		goto err3;
+		goto err_trans;
 
 	err = rhltable_insert(&nft_objname_ht, &obj->rhlhead,
 			      nft_objname_ht_params);
 	if (err < 0)
-		goto err4;
+		goto err_obj_ht;
 
 	list_add_tail_rcu(&obj->list, &table->objects);
 	table->use++;
 	return 0;
-err4:
+err_obj_ht:
 	/* queued in transaction log */
 	INIT_LIST_HEAD(&obj->list);
 	return err;
-err3:
+err_trans:
 	kfree(obj->key.name);
-err2:
+err_userdata:
+	kfree(obj->udata);
+err_strdup:
 	if (obj->ops->destroy)
 		obj->ops->destroy(&ctx, obj);
 	kfree(obj);
-err1:
+err_init:
 	module_put(type->owner);
 	return err;
 }
@@ -6011,6 +6026,10 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
 			 NFTA_OBJ_PAD))
 		goto nla_put_failure;
 
+	if (obj->udata &&
+	    nla_put(skb, NFTA_OBJ_USERDATA, obj->udlen, obj->udata))
+		goto nla_put_failure;
+
 	nlmsg_end(skb, nlh);
 	return 0;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH nf-next 1/3, v2] netfilter: nf_tables: add userdata support for nft_object
  2020-09-08 11:01     ` [PATCH nf-next 1/3, v2] " Jose M. Guisado Gomez
@ 2020-09-08 14:36       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-08 14:36 UTC (permalink / raw)
  To: Jose M. Guisado Gomez; +Cc: netfilter-devel

On Tue, Sep 08, 2020 at 01:01:41PM +0200, Jose M. Guisado Gomez wrote:
> Enables storing userdata for nft_object. Initially this will store an
> optional comment but can be extended in the future as needed.
> 
> Adds new attribute NFTA_OBJ_USERDATA to nft_object.

Applied, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH libnftnl 2/3] object: add userdata and comment support
  2020-09-02  9:12 ` [PATCH libnftnl 2/3] object: add userdata and comment support Jose M. Guisado Gomez
@ 2020-09-08 14:52   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-08 14:52 UTC (permalink / raw)
  To: Jose M. Guisado Gomez; +Cc: netfilter-devel

On Wed, Sep 02, 2020 at 11:12:40AM +0200, Jose M. Guisado Gomez wrote:
> This patch adds NFTNL_OBJ_USERDATA to support userdata for objects.
> 
> Also adds NFTNL_UDATA_OBJ_COMMENT to support comments for objects,
> stored in userdata space.
> 
> Bumps libnftnl.map to 15 as nftnl_obj_get_data needs to be exported to
> enable getting object attributes/data.

Applied, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH nftables 3/3] src: add comment support for objects
  2020-09-02  9:12 ` [PATCH nftables 3/3] src: add comment support for objects Jose M. Guisado Gomez
  2020-09-03  9:16   ` [PATCH nftables 3/3, v2] " Jose M. Guisado Gomez
@ 2020-09-08 14:53   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-08 14:53 UTC (permalink / raw)
  To: Jose M. Guisado Gomez; +Cc: netfilter-devel

On Wed, Sep 02, 2020 at 11:12:41AM +0200, Jose M. Guisado Gomez wrote:
> Enables specifying an optional comment when declaring named objects. The
> comment is to be specified inside the object's block ({} block)
> 
> Relies on libnftnl exporting nftnl_obj_get_data and kernel space support
> to store the comments.
> 
> For consistency, this patch makes the comment be printed first when
> listing objects.
> 
> Adds a testcase importing all commented named objects except for secmark,
> although it's supported.
> 
> Example: Adding a quota with a comment
> 
> > add table inet filter
> > nft add quota inet filter q { over 1200 bytes \; comment "test_comment"\; }
> > list ruleset
> 
> table inet filter {
> 	quota q {
> 		comment "test_comment"
> 		over 1200 bytes
> 	}
> }

Also applied, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-09-08 20:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02  9:12 [PATCH 0/3] add object userdata and comment support Jose M. Guisado Gomez
2020-09-02  9:12 ` [PATCH nf-next 1/3] netfilter: nf_tables: add userdata support for nft_object Jose M. Guisado Gomez
2020-09-08 10:42   ` Pablo Neira Ayuso
2020-09-08 11:01     ` [PATCH nf-next 1/3, v2] " Jose M. Guisado Gomez
2020-09-08 14:36       ` Pablo Neira Ayuso
2020-09-02  9:12 ` [PATCH libnftnl 2/3] object: add userdata and comment support Jose M. Guisado Gomez
2020-09-08 14:52   ` Pablo Neira Ayuso
2020-09-02  9:12 ` [PATCH nftables 3/3] src: add comment support for objects Jose M. Guisado Gomez
2020-09-03  9:16   ` [PATCH nftables 3/3, v2] " Jose M. Guisado Gomez
2020-09-08 14:53   ` [PATCH nftables 3/3] " Pablo Neira Ayuso

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).