Re: [PATCH 2/3 v2] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer.

Carlos Falgueras García <carlosfg@xxxxxxxxxx> · Tue, 8 Mar 2016 10:58:11 +0100

On 02/03/16 19:41, Pablo Neira Ayuso wrote:
On Mon, Feb 29, 2016 at 05:25:39PM +0100, Carlos Falgueras García wrote:

Please, wrap these code below into a function, eg. nftnl_jansson_udata_parse()

+	array = json_object_get(root, "userdata");
+	if (array == NULL) {
+		err->error = NFTNL_PARSE_EMISSINGNODE;
+		err->node_name = "userdata";
+		goto err;
+	}
+
+	attrbuf = nftnl_attrbuf_alloc(NFT_USERDATA_MAXLEN);
+	if (!attrbuf) {
+		perror("nftnl_jansson_parse_rule");
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = 0; i < json_array_size(array); ++i) {
+		if (nftnl_jansson_attr_parse(attrbuf,
+					     json_array_get(array, i),
+					     err,
+					     set_list) < 0)
+			goto err;
+	}
+
+	nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+			    nftnl_attrbuf_get_data(attrbuf),
+			    nftnl_attrbuf_get_len(attrbuf));
+
  	return 0;
  err:
  	return -1;
@@ -592,7 +629,7 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
  			struct nftnl_parse_err *err,
  			struct nftnl_set_list *set_list)
  {
-	mxml_node_t *node;
+	mxml_node_t *node, *node_ud;
  	struct nftnl_expr *e;
  	const char *table, *chain;
  	int family;
@@ -649,6 +686,35 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
  		nftnl_rule_add_expr(r, e);
  	}

+	node_ud = mxmlFindElement(tree, tree, "userdata", NULL, NULL,
+				  MXML_DESCEND);
+	if (node_ud) {

Please, wrap these code below into a function, eg. nftnl_mxml_udata_parse()

+		struct nftnl_attrbuf *attrbuf;
+
+		attrbuf = nftnl_attrbuf_alloc(NFT_USERDATA_MAXLEN);
+		if (!attrbuf) {
+			perror("nftnl_mxml_rule_parse");
+			exit(EXIT_FAILURE);
+		}
+
+		/* Iterate over attributes */
+		for (
+			node = mxmlFindElement(node_ud, node_ud, "attr", NULL,
+				NULL, MXML_DESCEND);
+			node != NULL;
+			node = mxmlFindElement(node, node_ud, "attr", NULL,
+				NULL, MXML_DESCEND)
+		) {
+			if (nftnl_mxml_attr_parse(attrbuf, node,
+					MXML_DESCEND_FIRST, r->flags, err) < 0)
+				return -1;
+		}
+
+		nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+				    nftnl_attrbuf_get_data(attrbuf),
+				    nftnl_attrbuf_get_len(attrbuf));
+	}
+
  	return 0;
  }
  #endif
@@ -711,6 +777,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)
  {
@@ -757,6 +838,31 @@ static int nftnl_rule_snprintf_json(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_attr *attr;
+
+		ret = snprintf(buf+offset, len, "\"userdata\":[");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+		nftnl_attr_for_each(attr, r->userdata) {
+			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, "],\n");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	}
+
  	ret = snprintf(buf+offset, len, "\"expr\":[");
  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);

@@ -849,17 +955,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_attr *attr;
+
+		ret = snprintf(buf+offset, len, "<userdata>");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+		nftnl_attr_for_each(attr, r->userdata) {
+			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_attr *attr)
[...]
+static size_t nftnl_rule_snprintf_json_attr(char *buf, size_t size,
+					    const struct nftnl_attr *attr)

You can consolidate these two functions into one using the NFTNL_BUF_*
API.


If I wrap these functions in this patch, the new code structure will 
differ from the one already there. Maybe it is better if I do a complete 
refactor in a different patch?

diff --git a/src/utils.c b/src/utils.c
index ba36bc4..0cac4b6 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -139,6 +139,51 @@ 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 >= 0x30 && c <= 0x39)
+		*out = c - 0x30;
+	/* lowercase characters */
+	else if (c >= 0x61 && c <= 0x66)
+		*out = c - 0x61 + 10;
+	/* uppercase characters */
+	else if (c >= 0x41 && c <= 0x46)
+		*out = c - 0x41 + 10;
+	else {
+		errno = EINVAL;
+		return 0;
+	}

You can just print data in hexadecimal, so we can get rid of this.


This functions is not for print, it is for parse an hexadecimal 
character (maybe i must find a better name?). I'm printing the user data 
in hexadecimal with snprintf("%02X") and later I'm parsing it character 
by character with this function. There is other possible solutions with 
strtoul, atoi or sscanf, but these can't convert a single byte so they 
are endian dependient.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html






