All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects
@ 2016-03-15 20:28 Carlos Falgueras García
  2016-03-15 20:28 ` [PATCH 2/4 v5] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer Carlos Falgueras García
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-03-15 20:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, kaber

These functions allow to create a buffer (nftnl_udata_buf) of TLV objects
(nftnl_udata). It is inspired by libmnl/src/attr.c. It can be used to store
several variable length user data into an object.

Example usage:
	```
	struct nftnl_udata_buf *buf;
	struct nftnl_udata *attr;
	const char str[] = "Hello World!";

	buf = nftnl_udata_alloc(UDATA_SIZE);
	if (!buf) {
		perror("OOM");
		exit(EXIT_FAILURE);
	}

	if (!nftnl_udata_put_strz(buf, MY_TYPE, str)) {
		perror("Can't put attribute \"%s\"", str);
		exit(EXIT_FAILURE);
	}

	nftnl_udata_for_each(buf, attr) {
		printf("%s\n", (char *)nftnl_udata_attr_value(attr));
	}

	nftnl_udata_free(buf);
	```

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 include/Makefile.am          |   1 +
 include/libnftnl/Makefile.am |   1 +
 include/libnftnl/udata.h     |  49 ++++++++++++++++
 include/udata.h              |  40 +++++++++++++
 src/Makefile.am              |   1 +
 src/libnftnl.map             |  32 +++++++++++
 src/udata.c                  | 133 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 257 insertions(+)
 create mode 100644 include/libnftnl/udata.h
 create mode 100644 include/udata.h
 create mode 100644 src/udata.c

diff --git a/include/Makefile.am b/include/Makefile.am
index be9eb9b..9f55737 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -12,4 +12,5 @@ noinst_HEADERS = internal.h	\
 		 expr.h		\
 		 json.h		\
 		 set_elem.h	\
+		 udata.h	\
 		 utils.h
diff --git a/include/libnftnl/Makefile.am b/include/libnftnl/Makefile.am
index 84f01b6..457ec95 100644
--- a/include/libnftnl/Makefile.am
+++ b/include/libnftnl/Makefile.am
@@ -7,4 +7,5 @@ pkginclude_HEADERS = batch.h		\
 		     set.h		\
 		     ruleset.h		\
 		     common.h		\
+		     udata.h		\
 		     gen.h
diff --git a/include/libnftnl/udata.h b/include/libnftnl/udata.h
new file mode 100644
index 0000000..f65a1dc
--- /dev/null
+++ b/include/libnftnl/udata.h
@@ -0,0 +1,49 @@
+#ifndef _LIBNFTNL_UDATA_H_
+#define _LIBNFTNL_UDATA_H_
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+/*
+ * nftnl user data attributes API
+ */
+struct nftnl_udata;
+struct nftnl_udata_buf;
+
+/* nftnl_udata_buf */
+struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size);
+void nftnl_udata_free(struct nftnl_udata_buf *buf);
+uint32_t nftnl_udata_len(const struct nftnl_udata_buf *buf);
+uint32_t nftnl_udata_size(const struct nftnl_udata_buf *buf);
+void *nftnl_udata_data(const struct nftnl_udata_buf *buf);
+void nftnl_udata_copy_data(struct nftnl_udata_buf *buf, const void *data,
+			   uint32_t len);
+struct nftnl_udata *nftnl_udata_start(const struct nftnl_udata_buf *buf);
+struct nftnl_udata *nftnl_udata_end(const struct nftnl_udata_buf *buf);
+
+/* putters */
+bool nftnl_udata_put(struct nftnl_udata_buf *buf, uint8_t type, uint32_t len,
+		     const void *value);
+bool nftnl_udata_put_strz(struct nftnl_udata_buf *buf, uint8_t type,
+			  const char *strz);
+
+/* nftnl_udata_attr */
+uint8_t nftnl_udata_attr_type(const struct nftnl_udata *attr);
+uint8_t nftnl_udata_attr_len(const struct nftnl_udata *attr);
+void *nftnl_udata_attr_value(const struct nftnl_udata *attr);
+
+/* iterator */
+struct nftnl_udata *nftnl_udata_attr_next(const struct nftnl_udata *attr);
+
+#define nftnl_udata_for_each(buf, attr)                       \
+	for ((attr) = nftnl_udata_start(buf);                 \
+	     (char *)(nftnl_udata_end(buf)) > (char *)(attr); \
+	     (attr) = nftnl_udata_attr_next(attr))
+
+typedef int (*nftnl_udata_cb_t)(const struct nftnl_udata *attr,
+				void *data);
+int nftnl_udata_parse(const struct nftnl_udata_buf *buf, nftnl_udata_cb_t cb,
+		      void *data);
+
+#endif /* _LIBNFTNL_UDATA_H_ */
diff --git a/include/udata.h b/include/udata.h
new file mode 100644
index 0000000..407a3b9
--- /dev/null
+++ b/include/udata.h
@@ -0,0 +1,40 @@
+#ifndef _LIBNFTNL_UDATA_INTERNAL_H_
+#define _LIBNFTNL_UDATA_INTERNAL_H_
+
+#include <stdint.h>
+#include <stddef.h>
+
+/*
+ * TLV structures:
+ * nftnl_udata
+ *  <-------- HEADER --------> <------ PAYLOAD ------>
+ * +------------+-------------+- - - - - - - - - - - -+
+ * |    type    |     len     |         value         |
+ * |  (1 byte)  |   (1 byte)  |                       |
+ * +--------------------------+- - - - - - - - - - - -+
+ *  <-- sizeof(nftnl_udata) -> <-- nftnl_udata->len -->
+ */
+struct nftnl_udata {
+	uint8_t		type;
+	uint8_t		len;
+	unsigned char	value[];
+} __attribute__((__packed__));
+
+/*
+ *              +---------------------------------++
+ *              | data[]                          ||
+ *              |   ||                            ||
+ *              |   \/                            \/
+ *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
+ *  | size  |  end  |  TLV  |  TLV  |     |  TLV  |    Empty    |
+ *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
+ *                  |<---- nftnl_udata_len() ---->|
+ *                  |<----------- nftnl_udata_size() ---------->|
+ */
+struct nftnl_udata_buf {
+	uint32_t	size;
+	char		*end;
+	char		data[];
+};
+
+#endif /* _LIBNFTNL_UDATA_INTERNAL_H_ */
diff --git a/src/Makefile.am b/src/Makefile.am
index a27e292..7e580e4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -19,6 +19,7 @@ libnftnl_la_SOURCES = utils.c		\
 		      ruleset.c		\
 		      mxml.c		\
 		      jansson.c		\
+		      udata.c		\
 		      expr.c		\
 		      expr_ops.c	\
 		      expr/bitwise.c	\
diff --git a/src/libnftnl.map b/src/libnftnl.map
index 2e193b7..329caca 100644
--- a/src/libnftnl.map
+++ b/src/libnftnl.map
@@ -336,6 +336,22 @@ global:
   nftnl_set_snprintf;
   nftnl_set_fprintf;
 
+  nftnl_udata_alloc;
+  nftnl_udata_free;
+  nftnl_udata_len;
+  nftnl_udata_size;
+  nftnl_udata_data;
+  nftnl_udata_copy_data;
+  nftnl_udata_start;
+  nftnl_udata_end;
+  nftnl_udata_put;
+  nftnl_udata_put_strz;
+  nftnl_udata_attr_type;
+  nftnl_udata_attr_len;
+  nftnl_udata_attr_value;
+  nftnl_udata_attr_next;
+  nftnl_udata_parse;
+
   nftnl_set_list_alloc;
   nftnl_set_list_free;
   nftnl_set_list_add;
@@ -512,4 +528,20 @@ LIBNFTNL_4.1 {
 	nftnl_trace_get_data;
 
 	nftnl_trace_nlmsg_parse;
+
+	nftnl_udata_alloc;
+	nftnl_udata_free;
+	nftnl_udata_len;
+	nftnl_udata_size;
+	nftnl_udata_data;
+	nftnl_udata_copy_data;
+	nftnl_udata_start;
+	nftnl_udata_end;
+	nftnl_udata_put;
+	nftnl_udata_put_strz;
+	nftnl_udata_attr_type;
+	nftnl_udata_attr_len;
+	nftnl_udata_attr_value;
+	nftnl_udata_attr_next;
+	nftnl_udata_parse;
 } LIBNFTNL_4;
diff --git a/src/udata.c b/src/udata.c
new file mode 100644
index 0000000..61ee6c9
--- /dev/null
+++ b/src/udata.c
@@ -0,0 +1,133 @@
+#include <libnftnl/udata.h>
+#include <udata.h>
+#include <utils.h>
+
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+
+struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size)
+{
+	struct nftnl_udata_buf *buf;
+
+	buf = malloc(sizeof(struct nftnl_udata_buf) + data_size);
+	if (!buf)
+		return NULL;
+	buf->size = data_size;
+	buf->end = buf->data;
+
+	return buf;
+}
+EXPORT_SYMBOL(nftnl_udata_alloc);
+
+void nftnl_udata_free(struct nftnl_udata_buf *buf)
+{
+	buf->size = 0;
+	buf->end = NULL;
+	free(buf);
+}
+EXPORT_SYMBOL(nftnl_udata_free);
+
+uint32_t nftnl_udata_len(const struct nftnl_udata_buf *buf)
+{
+	return (uint32_t)(buf->end - buf->data);
+}
+EXPORT_SYMBOL(nftnl_udata_len);
+
+uint32_t nftnl_udata_size(const struct nftnl_udata_buf *buf)
+{
+	return buf->size;
+}
+EXPORT_SYMBOL(nftnl_udata_size);
+
+void *nftnl_udata_data(const struct nftnl_udata_buf *buf)
+{
+	return (void *)buf->data;
+}
+EXPORT_SYMBOL(nftnl_udata_data);
+
+void nftnl_udata_copy_data(struct nftnl_udata_buf *buf, const void *data,
+			   uint32_t len)
+{
+	memcpy(buf->data, data, len <= buf->size ? len : buf->size);
+	buf->end = buf->data + len;
+}
+EXPORT_SYMBOL(nftnl_udata_copy_data);
+
+struct nftnl_udata *nftnl_udata_start(const struct nftnl_udata_buf *buf)
+{
+	return (struct nftnl_udata *)buf->data;
+}
+EXPORT_SYMBOL(nftnl_udata_start);
+
+struct nftnl_udata *nftnl_udata_end(const struct nftnl_udata_buf *buf)
+{
+	return (struct nftnl_udata *)buf->end;
+}
+EXPORT_SYMBOL(nftnl_udata_end);
+
+bool nftnl_udata_put(struct nftnl_udata_buf *buf, uint8_t type, uint32_t len,
+		     const void *value)
+{
+	struct nftnl_udata *attr;
+
+	if (buf->size < len + sizeof(struct nftnl_udata))
+		return false;
+
+	attr = (struct nftnl_udata *)buf->end;
+	attr->len  = len;
+	attr->type = type;
+	memcpy(attr->value, value, len);
+
+	buf->end = (char *)nftnl_udata_attr_next(attr);
+
+	return true;
+}
+EXPORT_SYMBOL(nftnl_udata_put);
+
+bool nftnl_udata_put_strz(struct nftnl_udata_buf *buf, uint8_t type,
+			  const char *strz)
+{
+	return nftnl_udata_put(buf, type, strlen(strz) + 1, strz);
+}
+EXPORT_SYMBOL(nftnl_udata_put_strz);
+
+uint8_t nftnl_udata_attr_type(const struct nftnl_udata *attr)
+{
+	return attr->type;
+}
+EXPORT_SYMBOL(nftnl_udata_attr_type);
+
+uint8_t nftnl_udata_attr_len(const struct nftnl_udata *attr)
+{
+	return attr->len;
+}
+EXPORT_SYMBOL(nftnl_udata_attr_len);
+
+void *nftnl_udata_attr_value(const struct nftnl_udata *attr)
+{
+	return (void *)attr->value;
+}
+EXPORT_SYMBOL(nftnl_udata_attr_value);
+
+struct nftnl_udata *nftnl_udata_attr_next(const struct nftnl_udata *attr)
+{
+	return (struct nftnl_udata *)&attr->value[attr->len];
+}
+EXPORT_SYMBOL(nftnl_udata_attr_next);
+
+int nftnl_udata_parse(const struct nftnl_udata_buf *buf, nftnl_udata_cb_t cb,
+		      void *data)
+{
+	int ret = 0;
+	const struct nftnl_udata *attr;
+
+	nftnl_udata_for_each(buf, attr) {
+		ret = cb(attr, data);
+		if (ret <= 0)
+			return ret;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(nftnl_udata_parse);
-- 
2.7.2

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

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

* [PATCH 2/4 v5] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer
  2016-03-15 20:28 [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Carlos Falgueras García
@ 2016-03-15 20:28 ` Carlos Falgueras García
  2016-03-15 20:28 ` [PATCH 3/4 v5] libnftnl: test: Update test to check new nftnl_udata features of nftnl_rule Carlos Falgueras García
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-03-15 20:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, kaber

Now is it possible to store multiple variable length user data into a rule.
Modify XML and JSON parsers to support this new feature.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 include/json.h  |   7 ++
 include/utils.h |   2 +
 include/xml.h   |   6 ++
 src/jansson.c   |  66 +++++++++++++++
 src/mxml.c      |  72 ++++++++++++++++
 src/rule.c      | 249 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 src/utils.c     |  44 ++++++++++
 7 files changed, 420 insertions(+), 26 deletions(-)

diff --git a/include/json.h b/include/json.h
index bd70cec..2526a5b 100644
--- a/include/json.h
+++ b/include/json.h
@@ -3,6 +3,7 @@
 
 #ifdef JSON_PARSING
 #include <jansson.h>
+#include <libnftnl/udata.h>
 #include <stdbool.h>
 #include "common.h"
 
@@ -51,6 +52,12 @@ int nftnl_jansson_parse_elem(struct nftnl_set *s, json_t *tree,
 
 int nftnl_data_reg_json_parse(union nftnl_data_reg *reg, json_t *data,
 			    struct nftnl_parse_err *err);
+
+struct nftnl_udata_buf *nftnl_jansson_udata_parse(json_t *attr_array,
+						  json_t *root,
+						  struct nftnl_parse_err *err,
+						  struct nftnl_set_list *set_list);
+
 #else
 #define json_t void
 #endif
diff --git a/include/utils.h b/include/utils.h
index 0087dbb..1bbabff 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -69,6 +69,8 @@ enum nftnl_type {
 int nftnl_strtoi(const char *string, int base, void *number, enum nftnl_type type);
 int nftnl_get_value(enum nftnl_type type, void *val, void *out);
 
+char *str2value(const char *str, size_t strlen);
+
 const char *nftnl_verdict2str(uint32_t verdict);
 int nftnl_str2verdict(const char *verdict, int *verdict_num);
 
diff --git a/include/xml.h b/include/xml.h
index 7b33a83..b23b318 100644
--- a/include/xml.h
+++ b/include/xml.h
@@ -4,6 +4,7 @@
 #ifdef XML_PARSING
 #include <mxml.h>
 #include "common.h"
+#include <libnftnl/udata.h>
 
 #define NFTNL_XML_MAND 0
 #define NFTNL_XML_OPT (1 << 0)
@@ -51,6 +52,11 @@ int nftnl_mxml_set_parse(mxml_node_t *tree, struct nftnl_set *s,
 
 int nftnl_data_reg_xml_parse(union nftnl_data_reg *reg, mxml_node_t *tree,
 			   struct nftnl_parse_err *err);
+
+struct nftnl_udata_buf *nftnl_mxml_udata_parse(mxml_node_t *rootn,
+					       uint32_t mxml_flags,
+					       uint16_t flags,
+					       struct nftnl_parse_err *err);
 #else
 #define mxml_node_t void
 #endif
diff --git a/src/jansson.c b/src/jansson.c
index 3476ed2..8cc8f9a 100644
--- a/src/jansson.c
+++ b/src/jansson.c
@@ -19,6 +19,7 @@
 #include <libnftnl/set.h>
 
 #include <libnftnl/expr.h>
+#include <libnftnl/udata.h>
 #include <linux/netfilter/nf_tables.h>
 
 #ifdef JSON_PARSING
@@ -276,4 +277,69 @@ int nftnl_jansson_set_elem_parse(struct nftnl_set_elem *e, json_t *root,
 
 	return 0;
 }
+
+static int nftnl_jansson_udata_attr_parse(struct nftnl_udata_buf *buf,
+					  json_t *root,
+					  struct nftnl_parse_err *err,
+					  struct nftnl_set_list *set_list)
+{
+	const char *value_str;
+	char *value = NULL;
+	uint8_t type;
+	uint8_t len;
+	int ret;
+
+	ret = nftnl_jansson_parse_val(root, "type", NFTNL_TYPE_U8, &type, err);
+	if (ret != 0)
+		return 0;
+
+	ret = nftnl_jansson_parse_val(root, "length", NFTNL_TYPE_U8, &len, err);
+	if (ret != 0)
+		return 0;
+
+	value_str = nftnl_jansson_parse_str(root, "value", err);
+	if (ret != 0)
+		return 0;
+
+	if (strlen(value_str) != 2 * len)
+		return 0;
+
+	value = str2value(value_str, 2 * len);
+	if (!value) {
+		err->error = NFTNL_PARSE_EBADTYPE;
+		err->node_name = "value";
+		errno = ERANGE;
+		return 0;
+	}
+
+	if (!nftnl_udata_put(buf, type, len, (void *)value))
+		ret = 0;
+
+	free(value);
+	return 1;
+}
+
+struct nftnl_udata_buf *nftnl_jansson_udata_parse(json_t *attr_array,
+						  json_t *root,
+						  struct nftnl_parse_err *err,
+						  struct nftnl_set_list *set_list)
+{
+	struct nftnl_udata_buf *buf = NULL;
+	json_t *jattr;
+	int i;
+
+	buf = nftnl_udata_alloc(NFT_USERDATA_MAXLEN);
+	if (!buf) {
+		free(buf);
+		return NULL;
+	}
+
+	for (i = 0; (jattr = json_array_get(attr_array, i)); ++i) {
+		if (!nftnl_jansson_udata_attr_parse(buf, jattr, err, set_list))
+			return NULL;
+	}
+
+	return buf;
+}
+
 #endif
diff --git a/src/mxml.c b/src/mxml.c
index 51dbf1b..ae04eb1 100644
--- a/src/mxml.c
+++ b/src/mxml.c
@@ -20,6 +20,7 @@
 #include <libnftnl/rule.h>
 #include <libnftnl/expr.h>
 #include <libnftnl/set.h>
+#include <libnftnl/udata.h>
 
 #ifdef XML_PARSING
 mxml_node_t *nftnl_mxml_build_tree(const void *data, const char *treename,
@@ -229,4 +230,75 @@ int nftnl_mxml_family_parse(mxml_node_t *tree, const char *node_name,
 
 	return family;
 }
+
+static int nftnl_mxml_udata_attr_parse(struct nftnl_udata_buf *buf,
+				       mxml_node_t *tree,
+				       uint32_t mxml_flags, uint16_t flags,
+				       struct nftnl_parse_err *err)
+{
+	uint8_t len;
+	uint8_t type;
+	const char *value_str;
+	char *value = NULL;
+
+	if (nftnl_mxml_num_parse(tree, "type", mxml_flags, BASE_DEC, &type,
+				 NFTNL_TYPE_U8, flags, err) < 0)
+		return 0;
+
+	if (nftnl_mxml_num_parse(tree, "length", mxml_flags, BASE_DEC, &len,
+				 NFTNL_TYPE_U8, flags, err) < 0)
+		return 0;
+
+	value_str = nftnl_mxml_str_parse(tree, "value", mxml_flags, flags, err);
+	if (!value_str)
+		return 0;
+
+	if (strlen(value_str) != 2 * len)
+		return 0;
+
+	value = str2value(value_str, 2 * len);
+	if (!value) {
+		err->error = NFTNL_PARSE_EBADTYPE;
+		err->node_name = "value";
+		errno = ERANGE;
+		return 0;
+	}
+
+	if (!nftnl_udata_put(buf, type, len, (void *)value))
+		return 0;
+
+	free(value);
+	return 1;
+}
+
+struct nftnl_udata_buf *nftnl_mxml_udata_parse(mxml_node_t *rootn,
+					       uint32_t mxml_flags,
+					       uint16_t flags,
+					       struct nftnl_parse_err *err)
+{
+		mxml_node_t *attrn;
+		struct nftnl_udata_buf *buf;
+
+		buf = nftnl_udata_alloc(NFT_USERDATA_MAXLEN);
+		if (!buf) {
+			free(buf);
+			return NULL;
+		}
+
+		/* Iterate over attributes */
+		for (
+			attrn = mxmlFindElement(rootn, rootn, "attr", NULL,
+						NULL, mxml_flags);
+			attrn;
+			attrn = mxmlFindElement(attrn, rootn, "attr", NULL,
+						NULL, mxml_flags)
+		) {
+			if (!nftnl_mxml_udata_attr_parse(buf, attrn, mxml_flags,
+							 flags, err))
+				return NULL;
+		}
+
+	return buf;
+}
+
 #endif
diff --git a/src/rule.c b/src/rule.c
index 3a32bf6..5ae3e0b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -28,6 +28,7 @@
 #include <libnftnl/rule.h>
 #include <libnftnl/set.h>
 #include <libnftnl/expr.h>
+#include <libnftnl/udata.h>
 
 struct nftnl_rule {
 	struct list_head head;
@@ -38,10 +39,7 @@ struct nftnl_rule {
 	const char	*chain;
 	uint64_t	handle;
 	uint64_t	position;
-	struct {
-			void		*data;
-			uint32_t	len;
-	} user;
+	struct nftnl_udata_buf	*userdata;
 	struct {
 			uint32_t	flags;
 			uint32_t	proto;
@@ -50,6 +48,15 @@ struct nftnl_rule {
 	struct list_head expr_list;
 };
 
+static size_t nftnl_rule_snprintf_data2str(char *buf, size_t size,
+					   const void *data, size_t datalen);
+static size_t nftnl_rule_snprintf_default_udata(char *buf, size_t size,
+						const struct nftnl_udata *attr);
+static size_t nftnl_rule_snprintf_xml_attr(char *buf, size_t size,
+					   const struct nftnl_udata *attr);
+static size_t nftnl_rule_snprintf_json_attr(char *buf, size_t size,
+					    const struct nftnl_udata *attr);
+
 struct nftnl_rule *nftnl_rule_alloc(void)
 {
 	struct nftnl_rule *r;
@@ -75,6 +82,8 @@ void nftnl_rule_free(struct nftnl_rule *r)
 		xfree(r->table);
 	if (r->chain != NULL)
 		xfree(r->chain);
+	if (r->flags & (1 << NFTNL_RULE_USERDATA))
+		nftnl_udata_free(r->userdata);
 
 	xfree(r);
 }
@@ -162,8 +171,14 @@ void nftnl_rule_set_data(struct nftnl_rule *r, uint16_t attr,
 		r->position = *((uint64_t *)data);
 		break;
 	case NFTNL_RULE_USERDATA:
-		r->user.data = (void *)data;
-		r->user.len = data_len;
+		if (r->flags & (1 << NFTNL_RULE_USERDATA))
+			nftnl_udata_free(r->userdata);
+		r->userdata = nftnl_udata_alloc(data_len);
+		if (!r->userdata) {
+			perror("nftnl_rule_set_data - userdata");
+			return;
+		}
+		nftnl_udata_copy_data(r->userdata, data, data_len);
 		break;
 	}
 	r->flags |= (1 << attr);
@@ -221,8 +236,8 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r, uint16_t attr,
 		*data_len = sizeof(uint64_t);
 		return &r->position;
 	case NFTNL_RULE_USERDATA:
-		*data_len = r->user.len;
-		return r->user.data;
+		*data_len = nftnl_udata_len(r->userdata);
+		return (void *)nftnl_udata_data(r->userdata);
 	}
 	return NULL;
 }
@@ -288,8 +303,9 @@ void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh, struct nftnl_rule *r)
 	if (r->flags & (1 << NFTNL_RULE_POSITION))
 		mnl_attr_put_u64(nlh, NFTA_RULE_POSITION, htobe64(r->position));
 	if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
-		mnl_attr_put(nlh, NFTA_RULE_USERDATA, r->user.len,
-			     r->user.data);
+		mnl_attr_put(nlh, NFTA_RULE_USERDATA,
+			     nftnl_udata_len(r->userdata),
+			     nftnl_udata_data(r->userdata));
 	}
 
 	if (!list_empty(&r->expr_list)) {
@@ -447,19 +463,20 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 		r->flags |= (1 << NFTNL_RULE_POSITION);
 	}
 	if (tb[NFTA_RULE_USERDATA]) {
+		uint16_t udata_size;
+
 		const void *udata =
 			mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]);
 
-		if (r->user.data)
-			xfree(r->user.data);
+		udata_size = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
 
-		r->user.len = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
-
-		r->user.data = malloc(r->user.len);
-		if (r->user.data == NULL)
+		if (r->flags & (1 << NFTNL_RULE_USERDATA))
+			nftnl_udata_free(r->userdata);
+		r->userdata = nftnl_udata_alloc(udata_size);
+		if (!r->userdata)
 			return -1;
+		nftnl_udata_copy_data(r->userdata, udata, udata_size);
 
-		memcpy(r->user.data, udata, r->user.len);
 		r->flags |= (1 << NFTNL_RULE_USERDATA);
 	}
 
@@ -481,6 +498,7 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t *tree,
 	uint64_t uval64;
 	uint32_t uval32;
 	int i, family;
+	struct nftnl_udata_buf *buf;
 
 	root = nftnl_jansson_get_node(tree, "rule", err);
 	if (root == NULL)
@@ -557,6 +575,17 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t *tree,
 		nftnl_rule_add_expr(r, e);
 	}
 
+	array = json_object_get(root, "userdata");
+	if (array) {
+		buf = nftnl_jansson_udata_parse(array, root, err, set_list);
+		if (!buf)
+			goto err;
+
+		nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+				    nftnl_udata_data(buf),
+				    nftnl_udata_len(buf));
+	}
+
 	return 0;
 err:
 	return -1;
@@ -596,6 +625,7 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
 	struct nftnl_expr *e;
 	const char *table, *chain;
 	int family;
+	struct nftnl_udata_buf *buf;
 
 	family = nftnl_mxml_family_parse(tree, "family", MXML_DESCEND_FIRST,
 				       NFTNL_XML_MAND, err);
@@ -649,6 +679,18 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
 		nftnl_rule_add_expr(r, e);
 	}
 
+	node = mxmlFindElement(tree, tree, "userdata", NULL, NULL,
+			       MXML_DESCEND);
+	if (node) {
+		buf = nftnl_mxml_udata_parse(node, MXML_DESCEND, r->flags, err);
+		if (!buf)
+			return -1;
+
+		nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+				    nftnl_udata_data(buf),
+				    nftnl_udata_len(buf));
+	}
+
 	return 0;
 }
 #endif
@@ -711,6 +753,21 @@ int nftnl_rule_parse_file(struct nftnl_rule *r, enum nftnl_parse_type type,
 }
 EXPORT_SYMBOL_ALIAS(nftnl_rule_parse_file, nft_rule_parse_file);
 
+static size_t nftnl_rule_snprintf_data2str(char *buf, size_t size,
+					   const void *data, size_t datalen)
+{
+	int i;
+	size_t ret, len = size, offset = 0;
+	const unsigned char *str = data;
+
+	for (i = 0; i < datalen; i++) {
+		ret = snprintf(buf + offset, len, "%02X", str[i]);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	return offset;
+}
+
 static int nftnl_rule_snprintf_json(char *buf, size_t size, struct nftnl_rule *r,
 					 uint32_t type, uint32_t flags)
 {
@@ -783,7 +840,34 @@ static int nftnl_rule_snprintf_json(char *buf, size_t size, struct nftnl_rule *r
 	}
 	/* Remove comma from last element */
 	offset--;
-	ret = snprintf(buf+offset, len, "]}}");
+	ret = snprintf(buf + offset, len, "]");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
+		const struct nftnl_udata *attr;
+
+		ret = snprintf(buf + offset, len, ",\"userdata\":[");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+		nftnl_udata_for_each(r->userdata, attr) {
+			ret = nftnl_rule_snprintf_json_attr(buf + offset, len,
+							    attr);
+			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+			ret = snprintf(buf + offset, len, ",");
+			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+		}
+		/* delete last comma */
+		buf[offset - 1] = '\0';
+		offset--;
+		size--;
+		len++;
+
+		ret = snprintf(buf + offset, len, "]");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	ret = snprintf(buf + offset, len, "}}");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	return offset;
@@ -849,17 +933,123 @@ static int nftnl_rule_snprintf_xml(char *buf, size_t size, struct nftnl_rule *r,
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	}
+
+	if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
+		const struct nftnl_udata *attr;
+
+		ret = snprintf(buf + offset, len, "<userdata>");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+		nftnl_udata_for_each(r->userdata, attr) {
+			ret = snprintf(buf + offset, len, "<attr>");
+			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+			ret = nftnl_rule_snprintf_xml_attr(buf + offset, len,
+							   attr);
+			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+			ret = snprintf(buf + offset, len, "</attr>");
+			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+		}
+
+		ret = snprintf(buf + offset, len, "</userdata>");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
 	ret = snprintf(buf+offset, len, "</rule>");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	return offset;
 }
 
+static size_t nftnl_rule_snprintf_xml_attr(char *buf, size_t size,
+					   const struct nftnl_udata *attr)
+{
+	size_t ret, len = size, offset = 0;
+
+	uint8_t atype = nftnl_udata_attr_type(attr);
+	uint8_t alen  = nftnl_udata_attr_len(attr);
+	void   *aval  = nftnl_udata_attr_value(attr);
+
+	/* type */
+	ret = snprintf(buf + offset, len, "<type>%d</type>", atype);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	/* len */
+	ret = snprintf(buf + offset, len, "<length>%d</length>", alen);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	/* value */
+	ret = snprintf(buf + offset, len, "<value>");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = nftnl_rule_snprintf_data2str(buf + offset, len, aval, alen);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = snprintf(buf + offset, len, "</value>");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	return offset;
+}
+
+static size_t nftnl_rule_snprintf_json_attr(char *buf, size_t size,
+					    const struct nftnl_udata *attr)
+{
+	size_t ret, len = size, offset = 0;
+
+	uint8_t atype = nftnl_udata_attr_type(attr);
+	uint8_t alen  = nftnl_udata_attr_len(attr);
+	void   *aval  = nftnl_udata_attr_value(attr);
+
+	/* type */
+	ret = snprintf(buf + offset, len, "{\"type\":%d,", atype);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	/* len */
+	ret = snprintf(buf + offset, len, "\"length\":%d,", alen);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	/* value */
+	ret = snprintf(buf + offset, len, "\"value\":\"");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = nftnl_rule_snprintf_data2str(buf + offset, len, aval, alen);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = snprintf(buf + offset, len, "\"}");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	return offset;
+}
+
+static size_t nftnl_rule_snprintf_default_udata(char *buf, size_t size,
+						const struct nftnl_udata *attr)
+{
+	size_t ret, len = size, offset = 0;
+
+	uint8_t atype = nftnl_udata_attr_type(attr);
+	uint8_t alen  = nftnl_udata_attr_len(attr);
+	void   *aval  = nftnl_udata_attr_value(attr);
+
+	/* type */
+	ret = snprintf(buf + offset, len, "{%d:\"", atype);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	/* value */
+	ret = nftnl_rule_snprintf_data2str(buf + offset, len, aval, alen);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = snprintf(buf + offset, len, "\"}");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	return offset;
+}
+
 static int nftnl_rule_snprintf_default(char *buf, size_t size, struct nftnl_rule *r,
 				     uint32_t type, uint32_t flags)
 {
 	struct nftnl_expr *expr;
-	int ret, len = size, offset = 0, i;
+	int ret, len = size, offset = 0;
 
 	if (r->flags & (1 << NFTNL_RULE_FAMILY)) {
 		ret = snprintf(buf+offset, len, "%s ",
@@ -905,21 +1095,28 @@ static int nftnl_rule_snprintf_default(char *buf, size_t size, struct nftnl_rule
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 	}
 
-	if (r->user.len) {
-		ret = snprintf(buf+offset, len, "  userdata = { ");
+	if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
+		const struct nftnl_udata *attr;
+
+		ret = snprintf(buf + offset, len, "  userdata = { ");
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-		for (i = 0; i < r->user.len; i++) {
-			char *c = r->user.data;
+		nftnl_udata_for_each(r->userdata, attr) {
+			ret = nftnl_rule_snprintf_default_udata(buf + offset,
+								len, attr);
+			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-			ret = snprintf(buf+offset, len, "%c",
-				       isalnum(c[i]) ? c[i] : 0);
+			ret = snprintf(buf + offset, len, ",");
 			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 		}
+		/* delete last comma */
+		buf[offset - 1] = '\0';
+		offset--;
+		size--;
+		len++;
 
 		ret = snprintf(buf+offset, len, " }\n");
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-
 	}
 
 	return offset;
diff --git a/src/utils.c b/src/utils.c
index ba36bc4..0e08693 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -139,6 +139,50 @@ int nftnl_strtoi(const char *string, int base, void *out, enum nftnl_type type)
 	return ret;
 }
 
+static int hex2char(char *out, char c)
+{
+	/* numbers */
+	if (c >= '0' && c <= '9')
+		*out = c - '0';
+	/* lowercase characters */
+	else if (c >= 'a' && c <= 'z')
+		*out = c - 'a' + 10;
+	/* uppercase characters */
+	else if (c >= 'A' && c <= 'Z')
+		*out = c - 'A' + 10;
+	else {
+		errno = EINVAL;
+		return 0;
+	}
+
+	return 1;
+}
+
+char *str2value(const char *str, size_t strlen)
+{
+	char *value;
+	size_t i;
+	char d0;
+	char d1;
+
+	value = malloc(strlen / 2);
+	if (!value)
+		return NULL;
+
+	for (i = 0; i < strlen / 2; i++) {
+		if (!hex2char(&d0, str[2 * i + 0]) ||
+		    !hex2char(&d1, str[2 * i + 1])
+		) {
+			free(value);
+			return NULL;
+		}
+
+		value[i] = d0 * 16 + d1;
+	}
+
+	return value;
+}
+
 const char *nftnl_verdict2str(uint32_t verdict)
 {
 	switch (verdict) {
-- 
2.7.2

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

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

* [PATCH 3/4 v5] libnftnl: test: Update test to check new nftnl_udata features of nftnl_rule
  2016-03-15 20:28 [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Carlos Falgueras García
  2016-03-15 20:28 ` [PATCH 2/4 v5] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer Carlos Falgueras García
@ 2016-03-15 20:28 ` Carlos Falgueras García
  2016-03-15 20:28 ` [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf Carlos Falgueras García
  2016-03-21 22:10 ` [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Pablo Neira Ayuso
  3 siblings, 0 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-03-15 20:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, kaber

Modify nft-rule-test.c to check TLV attribute inclusion in nftnl_rule.
Add "*-rule-udata.[json|xml]" to check parsers.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 tests/jsonfiles/71-rule-udata.json |  1 +
 tests/nft-rule-test.c              | 21 +++++++++++++++++++++
 tests/xmlfiles/82-rule-udata.xml   |  1 +
 3 files changed, 23 insertions(+)
 create mode 100644 tests/jsonfiles/71-rule-udata.json
 create mode 100644 tests/xmlfiles/82-rule-udata.xml

diff --git a/tests/jsonfiles/71-rule-udata.json b/tests/jsonfiles/71-rule-udata.json
new file mode 100644
index 0000000..02d7903
--- /dev/null
+++ b/tests/jsonfiles/71-rule-udata.json
@@ -0,0 +1 @@
+{"nftables":[{"add":[{"rule":{"family":"ip","table":"filter","chain":"input","handle":71,"expr":[{"type":"counter","pkts":135,"bytes":21655}],"userdata":[{"type":0,"length":12,"value":"68656C6C6F20776F726C6400"},{"type":1,"length":9,"value":"627920776F726C6400"}]}}]}]}
diff --git a/tests/nft-rule-test.c b/tests/nft-rule-test.c
index dff9634..eb10270 100644
--- a/tests/nft-rule-test.c
+++ b/tests/nft-rule-test.c
@@ -15,6 +15,7 @@
 #include <netinet/in.h>
 #include <linux/netfilter/nf_tables.h>
 #include <libnftnl/rule.h>
+#include <libnftnl/udata.h>
 
 static int test_ok = 1;
 
@@ -26,6 +27,9 @@ static void print_err(const char *msg)
 
 static void cmp_nftnl_rule(struct nftnl_rule *a, struct nftnl_rule *b)
 {
+	const void *udata_a, *udata_b;
+	uint32_t len_a, len_b;
+
 	if (nftnl_rule_get_u32(a, NFTNL_RULE_FAMILY) !=
 	    nftnl_rule_get_u32(b, NFTNL_RULE_FAMILY))
 		print_err("Rule family mismatches");
@@ -47,6 +51,12 @@ static void cmp_nftnl_rule(struct nftnl_rule *a, struct nftnl_rule *b)
 	if (nftnl_rule_get_u64(a, NFTNL_RULE_POSITION) !=
 	    nftnl_rule_get_u64(b, NFTNL_RULE_POSITION))
 		print_err("Rule compat_position mismatches");
+
+	udata_a = nftnl_rule_get_data(a, NFTNL_RULE_USERDATA, &len_a);
+	udata_b = nftnl_rule_get_data(b, NFTNL_RULE_USERDATA, &len_b);
+
+	if (len_a != len_b || memcmp(udata_a, udata_b, len_a) != 0)
+		print_err("Rule userdata mismatches");
 }
 
 int main(int argc, char *argv[])
@@ -54,12 +64,20 @@ int main(int argc, char *argv[])
 	struct nftnl_rule *a, *b;
 	char buf[4096];
 	struct nlmsghdr *nlh;
+	struct nftnl_udata_buf *udata;
 
 	a = nftnl_rule_alloc();
 	b = nftnl_rule_alloc();
 	if (a == NULL || b == NULL)
 		print_err("OOM");
 
+	udata = nftnl_udata_alloc(NFT_USERDATA_MAXLEN);
+	if (!udata)
+		print_err("OOM");
+
+	if (!nftnl_udata_put_strz(udata, 0, "hello world"))
+		print_err("User data too big");
+
 	nftnl_rule_set_u32(a, NFTNL_RULE_FAMILY, AF_INET);
 	nftnl_rule_set_str(a, NFTNL_RULE_TABLE, "table");
 	nftnl_rule_set_str(a, NFTNL_RULE_CHAIN, "chain");
@@ -67,6 +85,9 @@ int main(int argc, char *argv[])
 	nftnl_rule_set_u32(a, NFTNL_RULE_COMPAT_PROTO, 0x12345678);
 	nftnl_rule_set_u32(a, NFTNL_RULE_COMPAT_FLAGS, 0x12345678);
 	nftnl_rule_set_u64(a, NFTNL_RULE_POSITION, 0x1234567812345678);
+	nftnl_rule_set_data(a, NFTNL_RULE_USERDATA,
+			    nftnl_udata_data(udata),
+			    nftnl_udata_len(udata));
 
 	nlh = nftnl_rule_nlmsg_build_hdr(buf, NFT_MSG_NEWRULE, AF_INET, 0, 1234);
 	nftnl_rule_nlmsg_build_payload(nlh, a);
diff --git a/tests/xmlfiles/82-rule-udata.xml b/tests/xmlfiles/82-rule-udata.xml
new file mode 100644
index 0000000..b986926
--- /dev/null
+++ b/tests/xmlfiles/82-rule-udata.xml
@@ -0,0 +1 @@
+<nftables><add><rule><family>ip6</family><table>filter</table><chain>input</chain><handle>82</handle><expr type="counter"><pkts>3</pkts><bytes>177</bytes></expr><userdata><attr><type>0</type><length>12</length><value>68656C6C6F20776F726C6400</value></attr><attr><type>1</type><length>9</length><value>627920776F726C6400</value></attr></userdata></rule></add></nftables>
-- 
2.7.2

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

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

* [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf
  2016-03-15 20:28 [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Carlos Falgueras García
  2016-03-15 20:28 ` [PATCH 2/4 v5] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer Carlos Falgueras García
  2016-03-15 20:28 ` [PATCH 3/4 v5] libnftnl: test: Update test to check new nftnl_udata features of nftnl_rule Carlos Falgueras García
@ 2016-03-15 20:28 ` Carlos Falgueras García
  2016-03-21 22:13   ` Pablo Neira Ayuso
  2016-03-21 22:10 ` [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Pablo Neira Ayuso
  3 siblings, 1 reply; 9+ messages in thread
From: Carlos Falgueras García @ 2016-03-15 20:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, kaber

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@riseup.net>
---
 include/rule.h            |  7 +++++++
 src/netlink_delinearize.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--
 src/netlink_linearize.c   | 16 +++++++++++++--
 3 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index c848f0f..b52f0ac 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
@@ -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..5fcb5c1 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;
@@ -1746,6 +1747,54 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
 	}
 }
 
+static int 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 char *udata_get_comment(const void *data, uint32_t data_len)
+{
+	const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] = {};
+	struct nftnl_udata_buf *udata;
+	uint8_t attr_len;
+	char *comment = NULL;
+
+	udata = nftnl_udata_alloc(data_len);
+	if (!udata)
+		memory_allocation_error();
+	nftnl_udata_copy_data(udata, data, data_len);
+
+	if (nftnl_udata_parse(udata, parse_udata_cb, tb) <= 0)
+		goto exit;
+
+	if (!tb[UDATA_TYPE_COMMENT])
+		goto exit;
+
+	attr_len = nftnl_udata_attr_len(tb[UDATA_TYPE_COMMENT]);
+	comment = xmalloc(attr_len);
+	memcpy(comment, nftnl_udata_attr_value(tb[UDATA_TYPE_COMMENT]),
+	       attr_len);
+
+exit:
+	nftnl_udata_free(udata);
+	return comment;
+}
+
 struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 				      const struct nftnl_rule *nlr)
 {
@@ -1773,8 +1822,7 @@ 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->comment = udata_get_comment(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 bb51de7..ddf0cd3 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 {
@@ -1162,6 +1163,7 @@ void netlink_linearize_rule(struct netlink_ctx *ctx, struct nftnl_rule *nlr,
 			    const struct rule *rule)
 {
 	struct netlink_linearize_ctx lctx;
+	struct nftnl_udata_buf *udata;
 	const struct stmt *stmt;
 
 	memset(&lctx, 0, sizeof(lctx));
@@ -1171,9 +1173,19 @@ 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->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,
-				    rule->comment, strlen(rule->comment) + 1);
+				    nftnl_udata_data(udata),
+				    nftnl_udata_len(udata));
+	}
 
 	netlink_dump_rule(nlr);
 }
-- 
2.7.2

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

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

* Re: [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects
  2016-03-15 20:28 [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Carlos Falgueras García
                   ` (2 preceding siblings ...)
  2016-03-15 20:28 ` [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf Carlos Falgueras García
@ 2016-03-21 22:10 ` Pablo Neira Ayuso
  2016-03-22 11:36   ` Carlos Falgueras García
  3 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-03-21 22:10 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel, kaber

On Tue, Mar 15, 2016 at 09:28:04PM +0100, Carlos Falgueras García wrote:
> These functions allow to create a buffer (nftnl_udata_buf) of TLV objects
> (nftnl_udata). It is inspired by libmnl/src/attr.c. It can be used to store
> several variable length user data into an object.
> 
> Example usage:
> 	```
> 	struct nftnl_udata_buf *buf;
> 	struct nftnl_udata *attr;
> 	const char str[] = "Hello World!";
> 
> 	buf = nftnl_udata_alloc(UDATA_SIZE);
> 	if (!buf) {
> 		perror("OOM");
> 		exit(EXIT_FAILURE);
> 	}
> 
> 	if (!nftnl_udata_put_strz(buf, MY_TYPE, str)) {
> 		perror("Can't put attribute \"%s\"", str);
> 		exit(EXIT_FAILURE);
> 	}
> 
> 	nftnl_udata_for_each(buf, attr) {
> 		printf("%s\n", (char *)nftnl_udata_attr_value(attr));
> 	}
> 
u> 	nftnl_udata_free(buf);
> 	```
> 
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  include/Makefile.am          |   1 +
>  include/libnftnl/Makefile.am |   1 +
>  include/libnftnl/udata.h     |  49 ++++++++++++++++
>  include/udata.h              |  40 +++++++++++++
>  src/Makefile.am              |   1 +
>  src/libnftnl.map             |  32 +++++++++++
>  src/udata.c                  | 133 +++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 257 insertions(+)
>  create mode 100644 include/libnftnl/udata.h
>  create mode 100644 include/udata.h
>  create mode 100644 src/udata.c
> 
> diff --git a/include/Makefile.am b/include/Makefile.am
> index be9eb9b..9f55737 100644
> --- a/include/Makefile.am
> +++ b/include/Makefile.am
> @@ -12,4 +12,5 @@ noinst_HEADERS = internal.h	\
>  		 expr.h		\
>  		 json.h		\
>  		 set_elem.h	\
> +		 udata.h	\
>  		 utils.h
> diff --git a/include/libnftnl/Makefile.am b/include/libnftnl/Makefile.am
> index 84f01b6..457ec95 100644
> --- a/include/libnftnl/Makefile.am
> +++ b/include/libnftnl/Makefile.am
> @@ -7,4 +7,5 @@ pkginclude_HEADERS = batch.h		\
>  		     set.h		\
>  		     ruleset.h		\
>  		     common.h		\
> +		     udata.h		\
>  		     gen.h
> diff --git a/include/libnftnl/udata.h b/include/libnftnl/udata.h
> new file mode 100644
> index 0000000..f65a1dc
> --- /dev/null
> +++ b/include/libnftnl/udata.h
> @@ -0,0 +1,49 @@
> +#ifndef _LIBNFTNL_UDATA_H_
> +#define _LIBNFTNL_UDATA_H_
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +/*
> + * nftnl user data attributes API
> + */
> +struct nftnl_udata;
> +struct nftnl_udata_buf;
> +
> +/* nftnl_udata_buf */
> +struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size);
> +void nftnl_udata_free(struct nftnl_udata_buf *buf);

If nftnl_udata_alloc() returns struct nftnl_udata_buf, then it's
better to call this:

        nftnl_udata_buf_alloc()

instead.

> +uint32_t nftnl_udata_len(const struct nftnl_udata_buf *buf);
> +uint32_t nftnl_udata_size(const struct nftnl_udata_buf *buf);
> +void *nftnl_udata_data(const struct nftnl_udata_buf *buf);

Same thing for these functions: nftnl_udata_buf_*().

> +void nftnl_udata_copy_data(struct nftnl_udata_buf *buf, const void *data,
> +			   uint32_t len);

Please, rename this to:

        nftnl_udata_buf_put()

> +struct nftnl_udata *nftnl_udata_start(const struct nftnl_udata_buf *buf);
> +struct nftnl_udata *nftnl_udata_end(const struct nftnl_udata_buf *buf);
> +
> +/* putters */
> +bool nftnl_udata_put(struct nftnl_udata_buf *buf, uint8_t type, uint32_t len,
> +		     const void *value);
> +bool nftnl_udata_put_strz(struct nftnl_udata_buf *buf, uint8_t type,
> +			  const char *strz);
> +
> +/* nftnl_udata_attr */
> +uint8_t nftnl_udata_attr_type(const struct nftnl_udata *attr);
> +uint8_t nftnl_udata_attr_len(const struct nftnl_udata *attr);
> +void *nftnl_udata_attr_value(const struct nftnl_udata *attr);
> +
> +/* iterator */
> +struct nftnl_udata *nftnl_udata_attr_next(const struct nftnl_udata *attr);
> +
> +#define nftnl_udata_for_each(buf, attr)                       \
> +	for ((attr) = nftnl_udata_start(buf);                 \
> +	     (char *)(nftnl_udata_end(buf)) > (char *)(attr); \
> +	     (attr) = nftnl_udata_attr_next(attr))
> +
> +typedef int (*nftnl_udata_cb_t)(const struct nftnl_udata *attr,
> +				void *data);
> +int nftnl_udata_parse(const struct nftnl_udata_buf *buf, nftnl_udata_cb_t cb,
> +		      void *data);
> +
> +#endif /* _LIBNFTNL_UDATA_H_ */
> diff --git a/include/udata.h b/include/udata.h
> new file mode 100644
> index 0000000..407a3b9
> --- /dev/null
> +++ b/include/udata.h
> @@ -0,0 +1,40 @@
> +#ifndef _LIBNFTNL_UDATA_INTERNAL_H_
> +#define _LIBNFTNL_UDATA_INTERNAL_H_
> +
> +#include <stdint.h>
> +#include <stddef.h>
> +
> +/*
> + * TLV structures:
> + * nftnl_udata
> + *  <-------- HEADER --------> <------ PAYLOAD ------>
> + * +------------+-------------+- - - - - - - - - - - -+
> + * |    type    |     len     |         value         |
> + * |  (1 byte)  |   (1 byte)  |                       |
> + * +--------------------------+- - - - - - - - - - - -+
> + *  <-- sizeof(nftnl_udata) -> <-- nftnl_udata->len -->
> + */
> +struct nftnl_udata {
> +	uint8_t		type;
> +	uint8_t		len;
> +	unsigned char	value[];
> +} __attribute__((__packed__));
> +
> +/*
> + *              +---------------------------------++
> + *              | data[]                          ||
> + *              |   ||                            ||
> + *              |   \/                            \/
> + *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
> + *  | size  |  end  |  TLV  |  TLV  |     |  TLV  |    Empty    |
> + *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
> + *                  |<---- nftnl_udata_len() ---->|
> + *                  |<----------- nftnl_udata_size() ---------->|
> + */
> +struct nftnl_udata_buf {
> +	uint32_t	size;
> +	char		*end;
> +	char		data[];
> +};
> +
> +#endif /* _LIBNFTNL_UDATA_INTERNAL_H_ */
> diff --git a/src/Makefile.am b/src/Makefile.am
> index a27e292..7e580e4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -19,6 +19,7 @@ libnftnl_la_SOURCES = utils.c		\
>  		      ruleset.c		\
>  		      mxml.c		\
>  		      jansson.c		\
> +		      udata.c		\
>  		      expr.c		\
>  		      expr_ops.c	\
>  		      expr/bitwise.c	\
> diff --git a/src/libnftnl.map b/src/libnftnl.map
> index 2e193b7..329caca 100644
> --- a/src/libnftnl.map
> +++ b/src/libnftnl.map
> @@ -336,6 +336,22 @@ global:
>    nftnl_set_snprintf;
>    nftnl_set_fprintf;
>  
> +  nftnl_udata_alloc;
> +  nftnl_udata_free;
> +  nftnl_udata_len;
> +  nftnl_udata_size;
> +  nftnl_udata_data;
> +  nftnl_udata_copy_data;
> +  nftnl_udata_start;
> +  nftnl_udata_end;
> +  nftnl_udata_put;
> +  nftnl_udata_put_strz;
> +  nftnl_udata_attr_type;
> +  nftnl_udata_attr_len;
> +  nftnl_udata_attr_value;
> +  nftnl_udata_attr_next;
> +  nftnl_udata_parse;

These above, you can remove them...

>    nftnl_set_list_alloc;
>    nftnl_set_list_free;
>    nftnl_set_list_add;
> @@ -512,4 +528,20 @@ LIBNFTNL_4.1 {
>  	nftnl_trace_get_data;
>  
>  	nftnl_trace_nlmsg_parse;
> +
> +	nftnl_udata_alloc;
> +	nftnl_udata_free;
> +	nftnl_udata_len;
> +	nftnl_udata_size;
> +	nftnl_udata_data;
> +	nftnl_udata_copy_data;
> +	nftnl_udata_start;
> +	nftnl_udata_end;
> +	nftnl_udata_put;
> +	nftnl_udata_put_strz;
> +	nftnl_udata_attr_type;
> +	nftnl_udata_attr_len;
> +	nftnl_udata_attr_value;
> +	nftnl_udata_attr_next;
> +	nftnl_udata_parse;

... Since now you have placed them here in the right place, which is
the list of new symbols available under LIBNFTNL_4.1 :)

>  } LIBNFTNL_4;
> diff --git a/src/udata.c b/src/udata.c
> new file mode 100644
> index 0000000..61ee6c9
> --- /dev/null
> +++ b/src/udata.c
> @@ -0,0 +1,133 @@
> +#include <libnftnl/udata.h>
> +#include <udata.h>
> +#include <utils.h>
> +
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +
> +struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size)
> +{
> +	struct nftnl_udata_buf *buf;
> +
> +	buf = malloc(sizeof(struct nftnl_udata_buf) + data_size);
> +	if (!buf)
> +		return NULL;
> +	buf->size = data_size;
> +	buf->end = buf->data;
> +
> +	return buf;
> +}
> +EXPORT_SYMBOL(nftnl_udata_alloc);
> +
> +void nftnl_udata_free(struct nftnl_udata_buf *buf)
> +{
> +	buf->size = 0;
> +	buf->end = NULL;

No need to set this to zero and NULL given that we're just releasing
this object.
--
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

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

* Re: [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf
  2016-03-15 20:28 ` [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf Carlos Falgueras García
@ 2016-03-21 22:13   ` Pablo Neira Ayuso
  2016-03-22 11:37     ` Carlos Falgueras García
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-03-21 22:13 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel, kaber

On Tue, Mar 15, 2016 at 09:28:07PM +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@riseup.net>
> ---
>  include/rule.h            |  7 +++++++
>  src/netlink_delinearize.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--
>  src/netlink_linearize.c   | 16 +++++++++++++--
>  3 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/include/rule.h b/include/rule.h
> index c848f0f..b52f0ac 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
> @@ -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..5fcb5c1 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;
> @@ -1746,6 +1747,54 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
>  	}
>  }
>  
> +static int 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 char *udata_get_comment(const void *data, uint32_t data_len)
> +{
> +	const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] = {};
> +	struct nftnl_udata_buf *udata;
> +	uint8_t attr_len;
> +	char *comment = NULL;
> +
> +	udata = nftnl_udata_alloc(data_len);
> +	if (!udata)
> +		memory_allocation_error();
> +	nftnl_udata_copy_data(udata, data, data_len);
> +
> +	if (nftnl_udata_parse(udata, parse_udata_cb, tb) <= 0)
> +		goto exit;

I think this should be instead:

	if (nftnl_udata_parse(data, data_len, parse_udata_cb, tb) <= 0)

So you don't need to allocate the buffer then copy data into it.

I think the buffer infrastructure is only necessary to build the TLV
attributes, not to parse it.

> +	if (!tb[UDATA_TYPE_COMMENT])
> +		goto exit;
> +
> +	attr_len = nftnl_udata_attr_len(tb[UDATA_TYPE_COMMENT]);
> +	comment = xmalloc(attr_len);
> +	memcpy(comment, nftnl_udata_attr_value(tb[UDATA_TYPE_COMMENT]),
> +	       attr_len);

I'd suggest:

        comment = xstrdup(nftnl_udata_attr_get_str(tb[UDATA_TYPE_COMMENT]));

> +
> +exit:
> +	nftnl_udata_free(udata);
> +	return comment;
> +}
> +
>  struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
>  				      const struct nftnl_rule *nlr)
>  {
--
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

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

* Re: [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects
  2016-03-21 22:10 ` [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Pablo Neira Ayuso
@ 2016-03-22 11:36   ` Carlos Falgueras García
  2016-03-22 16:55     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 9+ messages in thread
From: Carlos Falgueras García @ 2016-03-22 11:36 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On 21/03/16 23:10, Pablo Neira Ayuso wrote:
> On Tue, Mar 15, 2016 at 09:28:04PM +0100, Carlos Falgueras García wrote:
>> These functions allow to create a buffer (nftnl_udata_buf) of TLV objects
>> (nftnl_udata). It is inspired by libmnl/src/attr.c. It can be used to store
>> several variable length user data into an object.
>>
>> Example usage:
>> 	```
>> 	struct nftnl_udata_buf *buf;
>> 	struct nftnl_udata *attr;
>> 	const char str[] = "Hello World!";
>>
>> 	buf = nftnl_udata_alloc(UDATA_SIZE);
>> 	if (!buf) {
>> 		perror("OOM");
>> 		exit(EXIT_FAILURE);
>> 	}
>>
>> 	if (!nftnl_udata_put_strz(buf, MY_TYPE, str)) {
>> 		perror("Can't put attribute \"%s\"", str);
>> 		exit(EXIT_FAILURE);
>> 	}
>>
>> 	nftnl_udata_for_each(buf, attr) {
>> 		printf("%s\n", (char *)nftnl_udata_attr_value(attr));
>> 	}
>>
> u> 	nftnl_udata_free(buf);
>> 	```

I don't understand what that means :/

>> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
>> ---
>>   include/Makefile.am          |   1 +
>>   include/libnftnl/Makefile.am |   1 +
>>   include/libnftnl/udata.h     |  49 ++++++++++++++++
>>   include/udata.h              |  40 +++++++++++++
>>   src/Makefile.am              |   1 +
>>   src/libnftnl.map             |  32 +++++++++++
>>   src/udata.c                  | 133 +++++++++++++++++++++++++++++++++++++++++++
>>   7 files changed, 257 insertions(+)
>>   create mode 100644 include/libnftnl/udata.h
>>   create mode 100644 include/udata.h
>>   create mode 100644 src/udata.c
>>
>> diff --git a/include/Makefile.am b/include/Makefile.am
>> index be9eb9b..9f55737 100644
>> --- a/include/Makefile.am
>> +++ b/include/Makefile.am
>> @@ -12,4 +12,5 @@ noinst_HEADERS = internal.h	\
>>   		 expr.h		\
>>   		 json.h		\
>>   		 set_elem.h	\
>> +		 udata.h	\
>>   		 utils.h
>> diff --git a/include/libnftnl/Makefile.am b/include/libnftnl/Makefile.am
>> index 84f01b6..457ec95 100644
>> --- a/include/libnftnl/Makefile.am
>> +++ b/include/libnftnl/Makefile.am
>> @@ -7,4 +7,5 @@ pkginclude_HEADERS = batch.h		\
>>   		     set.h		\
>>   		     ruleset.h		\
>>   		     common.h		\
>> +		     udata.h		\
>>   		     gen.h
>> diff --git a/include/libnftnl/udata.h b/include/libnftnl/udata.h
>> new file mode 100644
>> index 0000000..f65a1dc
>> --- /dev/null
>> +++ b/include/libnftnl/udata.h
>> @@ -0,0 +1,49 @@
>> +#ifndef _LIBNFTNL_UDATA_H_
>> +#define _LIBNFTNL_UDATA_H_
>> +
>> +#include <stdio.h>
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +
>> +/*
>> + * nftnl user data attributes API
>> + */
>> +struct nftnl_udata;
>> +struct nftnl_udata_buf;
>> +
>> +/* nftnl_udata_buf */
>> +struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size);
>> +void nftnl_udata_free(struct nftnl_udata_buf *buf);
>
> If nftnl_udata_alloc() returns struct nftnl_udata_buf, then it's
> better to call this:
>
>          nftnl_udata_buf_alloc()
>
> instead.
>
>> +uint32_t nftnl_udata_len(const struct nftnl_udata_buf *buf);
>> +uint32_t nftnl_udata_size(const struct nftnl_udata_buf *buf);
>> +void *nftnl_udata_data(const struct nftnl_udata_buf *buf);
>
> Same thing for these functions: nftnl_udata_buf_*().
>
>> +void nftnl_udata_copy_data(struct nftnl_udata_buf *buf, const void *data,
>> +			   uint32_t len);
>
> Please, rename this to:
>
>          nftnl_udata_buf_put()
>

Ok.

>> +struct nftnl_udata *nftnl_udata_start(const struct nftnl_udata_buf *buf);
>> +struct nftnl_udata *nftnl_udata_end(const struct nftnl_udata_buf *buf);
>> +
>> +/* putters */
>> +bool nftnl_udata_put(struct nftnl_udata_buf *buf, uint8_t type, uint32_t len,
>> +		     const void *value);
>> +bool nftnl_udata_put_strz(struct nftnl_udata_buf *buf, uint8_t type,
>> +			  const char *strz);
>> +
>> +/* nftnl_udata_attr */
>> +uint8_t nftnl_udata_attr_type(const struct nftnl_udata *attr);
>> +uint8_t nftnl_udata_attr_len(const struct nftnl_udata *attr);
>> +void *nftnl_udata_attr_value(const struct nftnl_udata *attr);
>> +
>> +/* iterator */
>> +struct nftnl_udata *nftnl_udata_attr_next(const struct nftnl_udata *attr);
>> +
>> +#define nftnl_udata_for_each(buf, attr)                       \
>> +	for ((attr) = nftnl_udata_start(buf);                 \
>> +	     (char *)(nftnl_udata_end(buf)) > (char *)(attr); \
>> +	     (attr) = nftnl_udata_attr_next(attr))
>> +
>> +typedef int (*nftnl_udata_cb_t)(const struct nftnl_udata *attr,
>> +				void *data);
>> +int nftnl_udata_parse(const struct nftnl_udata_buf *buf, nftnl_udata_cb_t cb,
>> +		      void *data);
>> +
>> +#endif /* _LIBNFTNL_UDATA_H_ */
>> diff --git a/include/udata.h b/include/udata.h
>> new file mode 100644
>> index 0000000..407a3b9
>> --- /dev/null
>> +++ b/include/udata.h
>> @@ -0,0 +1,40 @@
>> +#ifndef _LIBNFTNL_UDATA_INTERNAL_H_
>> +#define _LIBNFTNL_UDATA_INTERNAL_H_
>> +
>> +#include <stdint.h>
>> +#include <stddef.h>
>> +
>> +/*
>> + * TLV structures:
>> + * nftnl_udata
>> + *  <-------- HEADER --------> <------ PAYLOAD ------>
>> + * +------------+-------------+- - - - - - - - - - - -+
>> + * |    type    |     len     |         value         |
>> + * |  (1 byte)  |   (1 byte)  |                       |
>> + * +--------------------------+- - - - - - - - - - - -+
>> + *  <-- sizeof(nftnl_udata) -> <-- nftnl_udata->len -->
>> + */
>> +struct nftnl_udata {
>> +	uint8_t		type;
>> +	uint8_t		len;
>> +	unsigned char	value[];
>> +} __attribute__((__packed__));
>> +
>> +/*
>> + *              +---------------------------------++
>> + *              | data[]                          ||
>> + *              |   ||                            ||
>> + *              |   \/                            \/
>> + *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
>> + *  | size  |  end  |  TLV  |  TLV  |     |  TLV  |    Empty    |
>> + *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
>> + *                  |<---- nftnl_udata_len() ---->|
>> + *                  |<----------- nftnl_udata_size() ---------->|
>> + */
>> +struct nftnl_udata_buf {
>> +	uint32_t	size;
>> +	char		*end;
>> +	char		data[];
>> +};
>> +
>> +#endif /* _LIBNFTNL_UDATA_INTERNAL_H_ */
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index a27e292..7e580e4 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -19,6 +19,7 @@ libnftnl_la_SOURCES = utils.c		\
>>   		      ruleset.c		\
>>   		      mxml.c		\
>>   		      jansson.c		\
>> +		      udata.c		\
>>   		      expr.c		\
>>   		      expr_ops.c	\
>>   		      expr/bitwise.c	\
>> diff --git a/src/libnftnl.map b/src/libnftnl.map
>> index 2e193b7..329caca 100644
>> --- a/src/libnftnl.map
>> +++ b/src/libnftnl.map
>> @@ -336,6 +336,22 @@ global:
>>     nftnl_set_snprintf;
>>     nftnl_set_fprintf;
>>
>> +  nftnl_udata_alloc;
>> +  nftnl_udata_free;
>> +  nftnl_udata_len;
>> +  nftnl_udata_size;
>> +  nftnl_udata_data;
>> +  nftnl_udata_copy_data;
>> +  nftnl_udata_start;
>> +  nftnl_udata_end;
>> +  nftnl_udata_put;
>> +  nftnl_udata_put_strz;
>> +  nftnl_udata_attr_type;
>> +  nftnl_udata_attr_len;
>> +  nftnl_udata_attr_value;
>> +  nftnl_udata_attr_next;
>> +  nftnl_udata_parse;
>
> These above, you can remove them...
>
>>     nftnl_set_list_alloc;
>>     nftnl_set_list_free;
>>     nftnl_set_list_add;
>> @@ -512,4 +528,20 @@ LIBNFTNL_4.1 {
>>   	nftnl_trace_get_data;
>>
>>   	nftnl_trace_nlmsg_parse;
>> +
>> +	nftnl_udata_alloc;
>> +	nftnl_udata_free;
>> +	nftnl_udata_len;
>> +	nftnl_udata_size;
>> +	nftnl_udata_data;
>> +	nftnl_udata_copy_data;
>> +	nftnl_udata_start;
>> +	nftnl_udata_end;
>> +	nftnl_udata_put;
>> +	nftnl_udata_put_strz;
>> +	nftnl_udata_attr_type;
>> +	nftnl_udata_attr_len;
>> +	nftnl_udata_attr_value;
>> +	nftnl_udata_attr_next;
>> +	nftnl_udata_parse;
>
> ... Since now you have placed them here in the right place, which is
> the list of new symbols available under LIBNFTNL_4.1 :)

Of course. Sorry, i didn't realize about that.

>>   } LIBNFTNL_4;
>> diff --git a/src/udata.c b/src/udata.c
>> new file mode 100644
>> index 0000000..61ee6c9
>> --- /dev/null
>> +++ b/src/udata.c
>> @@ -0,0 +1,133 @@
>> +#include <libnftnl/udata.h>
>> +#include <udata.h>
>> +#include <utils.h>
>> +
>> +#include <stdlib.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +
>> +struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size)
>> +{
>> +	struct nftnl_udata_buf *buf;
>> +
>> +	buf = malloc(sizeof(struct nftnl_udata_buf) + data_size);
>> +	if (!buf)
>> +		return NULL;
>> +	buf->size = data_size;
>> +	buf->end = buf->data;
>> +
>> +	return buf;
>> +}
>> +EXPORT_SYMBOL(nftnl_udata_alloc);
>> +
>> +void nftnl_udata_free(struct nftnl_udata_buf *buf)
>> +{
>> +	buf->size = 0;
>> +	buf->end = NULL;
>
> No need to set this to zero and NULL given that we're just releasing
> this object.

I've put this because if you forget free the object and then you use it, 
you'll have the chance to obtain no error if the memory hasn't been 
overwritten. In this way, when you use the object after free it, you'll 
realize it is empty and something is worng.

I don't know if it is a good practise. I must get rid of this anyway?
--
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

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

* Re: [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf
  2016-03-21 22:13   ` Pablo Neira Ayuso
@ 2016-03-22 11:37     ` Carlos Falgueras García
  0 siblings, 0 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-03-22 11:37 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Netfilter Development Mailing list

On 21/03/16 23:13, Pablo Neira Ayuso wrote:
> On Tue, Mar 15, 2016 at 09:28:07PM +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@riseup.net>
>> ---
>>   include/rule.h            |  7 +++++++
>>   src/netlink_delinearize.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--
>>   src/netlink_linearize.c   | 16 +++++++++++++--
>>   3 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/rule.h b/include/rule.h
>> index c848f0f..b52f0ac 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
>> @@ -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..5fcb5c1 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;
>> @@ -1746,6 +1747,54 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
>>   	}
>>   }
>>
>> +static int 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 char *udata_get_comment(const void *data, uint32_t data_len)
>> +{
>> +	const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] = {};
>> +	struct nftnl_udata_buf *udata;
>> +	uint8_t attr_len;
>> +	char *comment = NULL;
>> +
>> +	udata = nftnl_udata_alloc(data_len);
>> +	if (!udata)
>> +		memory_allocation_error();
>> +	nftnl_udata_copy_data(udata, data, data_len);
>> +
>> +	if (nftnl_udata_parse(udata, parse_udata_cb, tb) <= 0)
>> +		goto exit;
>
> I think this should be instead:
>
> 	if (nftnl_udata_parse(data, data_len, parse_udata_cb, tb) <= 0)
>
> So you don't need to allocate the buffer then copy data into it.
>
> I think the buffer infrastructure is only necessary to build the TLV
> attributes, not to parse it.
>

Ok.

>> +	if (!tb[UDATA_TYPE_COMMENT])
>> +		goto exit;
>> +
>> +	attr_len = nftnl_udata_attr_len(tb[UDATA_TYPE_COMMENT]);
>> +	comment = xmalloc(attr_len);
>> +	memcpy(comment, nftnl_udata_attr_value(tb[UDATA_TYPE_COMMENT]),
>> +	       attr_len);
>
> I'd suggest:
>
>          comment = xstrdup(nftnl_udata_attr_get_str(tb[UDATA_TYPE_COMMENT]));

I'll change this.

>> +
>> +exit:
>> +	nftnl_udata_free(udata);
>> +	return comment;
>> +}
>> +
>>   struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
>>   				      const struct nftnl_rule *nlr)
>>   {
--
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

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

* Re: [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects
  2016-03-22 11:36   ` Carlos Falgueras García
@ 2016-03-22 16:55     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-03-22 16:55 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Tue, Mar 22, 2016 at 12:36:55PM +0100, Carlos Falgueras García wrote:
> On 21/03/16 23:10, Pablo Neira Ayuso wrote:
> >On Tue, Mar 15, 2016 at 09:28:04PM +0100, Carlos Falgueras García wrote:
> >>These functions allow to create a buffer (nftnl_udata_buf) of TLV objects
> >>(nftnl_udata). It is inspired by libmnl/src/attr.c. It can be used to store
> >>several variable length user data into an object.
> >>
> >>Example usage:
> >>	```
> >>	struct nftnl_udata_buf *buf;
> >>	struct nftnl_udata *attr;
> >>	const char str[] = "Hello World!";
> >>
> >>	buf = nftnl_udata_alloc(UDATA_SIZE);
> >>	if (!buf) {
> >>		perror("OOM");
> >>		exit(EXIT_FAILURE);
> >>	}
> >>
> >>	if (!nftnl_udata_put_strz(buf, MY_TYPE, str)) {
> >>		perror("Can't put attribute \"%s\"", str);
> >>		exit(EXIT_FAILURE);
> >>	}
> >>
> >>	nftnl_udata_for_each(buf, attr) {
> >>		printf("%s\n", (char *)nftnl_udata_attr_value(attr));
> >>	}
> >>
> >u> 	nftnl_udata_free(buf);
> >>	```
> 
> I don't understand what that means :/
> 
> >>Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> >>---
> >>  include/Makefile.am          |   1 +
> >>  include/libnftnl/Makefile.am |   1 +
> >>  include/libnftnl/udata.h     |  49 ++++++++++++++++
> >>  include/udata.h              |  40 +++++++++++++
> >>  src/Makefile.am              |   1 +
> >>  src/libnftnl.map             |  32 +++++++++++
> >>  src/udata.c                  | 133 +++++++++++++++++++++++++++++++++++++++++++
> >>  7 files changed, 257 insertions(+)
> >>  create mode 100644 include/libnftnl/udata.h
> >>  create mode 100644 include/udata.h
> >>  create mode 100644 src/udata.c
> >>
> >>diff --git a/include/Makefile.am b/include/Makefile.am
> >>index be9eb9b..9f55737 100644
> >>--- a/include/Makefile.am
> >>+++ b/include/Makefile.am
> >>@@ -12,4 +12,5 @@ noinst_HEADERS = internal.h	\
> >>  		 expr.h		\
> >>  		 json.h		\
> >>  		 set_elem.h	\
> >>+		 udata.h	\
> >>  		 utils.h
> >>diff --git a/include/libnftnl/Makefile.am b/include/libnftnl/Makefile.am
> >>index 84f01b6..457ec95 100644
> >>--- a/include/libnftnl/Makefile.am
> >>+++ b/include/libnftnl/Makefile.am
> >>@@ -7,4 +7,5 @@ pkginclude_HEADERS = batch.h		\
> >>  		     set.h		\
> >>  		     ruleset.h		\
> >>  		     common.h		\
> >>+		     udata.h		\
> >>  		     gen.h
> >>diff --git a/include/libnftnl/udata.h b/include/libnftnl/udata.h
> >>new file mode 100644
> >>index 0000000..f65a1dc
> >>--- /dev/null
> >>+++ b/include/libnftnl/udata.h
> >>@@ -0,0 +1,49 @@
> >>+#ifndef _LIBNFTNL_UDATA_H_
> >>+#define _LIBNFTNL_UDATA_H_
> >>+
> >>+#include <stdio.h>
> >>+#include <stdint.h>
> >>+#include <stdbool.h>
> >>+
> >>+/*
> >>+ * nftnl user data attributes API
> >>+ */
> >>+struct nftnl_udata;
> >>+struct nftnl_udata_buf;
> >>+
> >>+/* nftnl_udata_buf */
> >>+struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size);
> >>+void nftnl_udata_free(struct nftnl_udata_buf *buf);
> >
> >If nftnl_udata_alloc() returns struct nftnl_udata_buf, then it's
> >better to call this:
> >
> >         nftnl_udata_buf_alloc()
> >
> >instead.
> >
> >>+uint32_t nftnl_udata_len(const struct nftnl_udata_buf *buf);
> >>+uint32_t nftnl_udata_size(const struct nftnl_udata_buf *buf);
> >>+void *nftnl_udata_data(const struct nftnl_udata_buf *buf);
> >
> >Same thing for these functions: nftnl_udata_buf_*().
> >
> >>+void nftnl_udata_copy_data(struct nftnl_udata_buf *buf, const void *data,
> >>+			   uint32_t len);
> >
> >Please, rename this to:
> >
> >         nftnl_udata_buf_put()
> >
> 
> Ok.
> 
> >>+struct nftnl_udata *nftnl_udata_start(const struct nftnl_udata_buf *buf);
> >>+struct nftnl_udata *nftnl_udata_end(const struct nftnl_udata_buf *buf);
> >>+
> >>+/* putters */
> >>+bool nftnl_udata_put(struct nftnl_udata_buf *buf, uint8_t type, uint32_t len,
> >>+		     const void *value);
> >>+bool nftnl_udata_put_strz(struct nftnl_udata_buf *buf, uint8_t type,
> >>+			  const char *strz);
> >>+
> >>+/* nftnl_udata_attr */
> >>+uint8_t nftnl_udata_attr_type(const struct nftnl_udata *attr);
> >>+uint8_t nftnl_udata_attr_len(const struct nftnl_udata *attr);
> >>+void *nftnl_udata_attr_value(const struct nftnl_udata *attr);
> >>+
> >>+/* iterator */
> >>+struct nftnl_udata *nftnl_udata_attr_next(const struct nftnl_udata *attr);
> >>+
> >>+#define nftnl_udata_for_each(buf, attr)                       \
> >>+	for ((attr) = nftnl_udata_start(buf);                 \
> >>+	     (char *)(nftnl_udata_end(buf)) > (char *)(attr); \
> >>+	     (attr) = nftnl_udata_attr_next(attr))
> >>+
> >>+typedef int (*nftnl_udata_cb_t)(const struct nftnl_udata *attr,
> >>+				void *data);
> >>+int nftnl_udata_parse(const struct nftnl_udata_buf *buf, nftnl_udata_cb_t cb,
> >>+		      void *data);
> >>+
> >>+#endif /* _LIBNFTNL_UDATA_H_ */
> >>diff --git a/include/udata.h b/include/udata.h
> >>new file mode 100644
> >>index 0000000..407a3b9
> >>--- /dev/null
> >>+++ b/include/udata.h
> >>@@ -0,0 +1,40 @@
> >>+#ifndef _LIBNFTNL_UDATA_INTERNAL_H_
> >>+#define _LIBNFTNL_UDATA_INTERNAL_H_
> >>+
> >>+#include <stdint.h>
> >>+#include <stddef.h>
> >>+
> >>+/*
> >>+ * TLV structures:
> >>+ * nftnl_udata
> >>+ *  <-------- HEADER --------> <------ PAYLOAD ------>
> >>+ * +------------+-------------+- - - - - - - - - - - -+
> >>+ * |    type    |     len     |         value         |
> >>+ * |  (1 byte)  |   (1 byte)  |                       |
> >>+ * +--------------------------+- - - - - - - - - - - -+
> >>+ *  <-- sizeof(nftnl_udata) -> <-- nftnl_udata->len -->
> >>+ */
> >>+struct nftnl_udata {
> >>+	uint8_t		type;
> >>+	uint8_t		len;
> >>+	unsigned char	value[];
> >>+} __attribute__((__packed__));
> >>+
> >>+/*
> >>+ *              +---------------------------------++
> >>+ *              | data[]                          ||
> >>+ *              |   ||                            ||
> >>+ *              |   \/                            \/
> >>+ *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
> >>+ *  | size  |  end  |  TLV  |  TLV  |     |  TLV  |    Empty    |
> >>+ *  +-------+-------+-------+-------+ ... +-------+- - - - - - -+
> >>+ *                  |<---- nftnl_udata_len() ---->|
> >>+ *                  |<----------- nftnl_udata_size() ---------->|
> >>+ */
> >>+struct nftnl_udata_buf {
> >>+	uint32_t	size;
> >>+	char		*end;
> >>+	char		data[];
> >>+};
> >>+
> >>+#endif /* _LIBNFTNL_UDATA_INTERNAL_H_ */
> >>diff --git a/src/Makefile.am b/src/Makefile.am
> >>index a27e292..7e580e4 100644
> >>--- a/src/Makefile.am
> >>+++ b/src/Makefile.am
> >>@@ -19,6 +19,7 @@ libnftnl_la_SOURCES = utils.c		\
> >>  		      ruleset.c		\
> >>  		      mxml.c		\
> >>  		      jansson.c		\
> >>+		      udata.c		\
> >>  		      expr.c		\
> >>  		      expr_ops.c	\
> >>  		      expr/bitwise.c	\
> >>diff --git a/src/libnftnl.map b/src/libnftnl.map
> >>index 2e193b7..329caca 100644
> >>--- a/src/libnftnl.map
> >>+++ b/src/libnftnl.map
> >>@@ -336,6 +336,22 @@ global:
> >>    nftnl_set_snprintf;
> >>    nftnl_set_fprintf;
> >>
> >>+  nftnl_udata_alloc;
> >>+  nftnl_udata_free;
> >>+  nftnl_udata_len;
> >>+  nftnl_udata_size;
> >>+  nftnl_udata_data;
> >>+  nftnl_udata_copy_data;
> >>+  nftnl_udata_start;
> >>+  nftnl_udata_end;
> >>+  nftnl_udata_put;
> >>+  nftnl_udata_put_strz;
> >>+  nftnl_udata_attr_type;
> >>+  nftnl_udata_attr_len;
> >>+  nftnl_udata_attr_value;
> >>+  nftnl_udata_attr_next;
> >>+  nftnl_udata_parse;
> >
> >These above, you can remove them...
> >
> >>    nftnl_set_list_alloc;
> >>    nftnl_set_list_free;
> >>    nftnl_set_list_add;
> >>@@ -512,4 +528,20 @@ LIBNFTNL_4.1 {
> >>  	nftnl_trace_get_data;
> >>
> >>  	nftnl_trace_nlmsg_parse;
> >>+
> >>+	nftnl_udata_alloc;
> >>+	nftnl_udata_free;
> >>+	nftnl_udata_len;
> >>+	nftnl_udata_size;
> >>+	nftnl_udata_data;
> >>+	nftnl_udata_copy_data;
> >>+	nftnl_udata_start;
> >>+	nftnl_udata_end;
> >>+	nftnl_udata_put;
> >>+	nftnl_udata_put_strz;
> >>+	nftnl_udata_attr_type;
> >>+	nftnl_udata_attr_len;
> >>+	nftnl_udata_attr_value;
> >>+	nftnl_udata_attr_next;
> >>+	nftnl_udata_parse;
> >
> >... Since now you have placed them here in the right place, which is
> >the list of new symbols available under LIBNFTNL_4.1 :)
> 
> Of course. Sorry, i didn't realize about that.
> 
> >>  } LIBNFTNL_4;
> >>diff --git a/src/udata.c b/src/udata.c
> >>new file mode 100644
> >>index 0000000..61ee6c9
> >>--- /dev/null
> >>+++ b/src/udata.c
> >>@@ -0,0 +1,133 @@
> >>+#include <libnftnl/udata.h>
> >>+#include <udata.h>
> >>+#include <utils.h>
> >>+
> >>+#include <stdlib.h>
> >>+#include <stdint.h>
> >>+#include <string.h>
> >>+
> >>+struct nftnl_udata_buf *nftnl_udata_alloc(uint32_t data_size)
> >>+{
> >>+	struct nftnl_udata_buf *buf;
> >>+
> >>+	buf = malloc(sizeof(struct nftnl_udata_buf) + data_size);
> >>+	if (!buf)
> >>+		return NULL;
> >>+	buf->size = data_size;
> >>+	buf->end = buf->data;
> >>+
> >>+	return buf;
> >>+}
> >>+EXPORT_SYMBOL(nftnl_udata_alloc);
> >>+
> >>+void nftnl_udata_free(struct nftnl_udata_buf *buf)
> >>+{
> >>+	buf->size = 0;
> >>+	buf->end = NULL;
> >
> >No need to set this to zero and NULL given that we're just releasing
> >this object.
> 
> I've put this because if you forget free the object and then you use it,
> you'll have the chance to obtain no error if the memory hasn't been
> overwritten. In this way, when you use the object after free it, you'll
> realize it is empty and something is worng.
> 
> I don't know if it is a good practise. I must get rid of this anyway?

We should be able to catch that via valgrind and such, which should be
the way to validate this. Please, make sure you run valgrind in your
patches and that it doesn't report any unexpected behaviour.
--
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

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

end of thread, other threads:[~2016-03-22 16:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-15 20:28 [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Carlos Falgueras García
2016-03-15 20:28 ` [PATCH 2/4 v5] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer Carlos Falgueras García
2016-03-15 20:28 ` [PATCH 3/4 v5] libnftnl: test: Update test to check new nftnl_udata features of nftnl_rule Carlos Falgueras García
2016-03-15 20:28 ` [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf Carlos Falgueras García
2016-03-21 22:13   ` Pablo Neira Ayuso
2016-03-22 11:37     ` Carlos Falgueras García
2016-03-21 22:10 ` [PATCH 1/4 v5] libnftnl: Implement new buffer of TLV objects Pablo Neira Ayuso
2016-03-22 11:36   ` Carlos Falgueras García
2016-03-22 16:55     ` Pablo Neira Ayuso

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.