[libnftables PATCH 0/2]

[libnftables PATCH 0/2]

[Arturo Borrero]

The following series implements:
        * A typo fix.
        * src: expr: nft_data_reg support for XML output.

Please, note that the XML output of nft_data_reg stills need some work, such as
endianess related issues and how to print the other part of the union.
This is intended to be the first basic approach.

Also, comments are welcome.

---

Arturo Borrero (2):
      Fix a typo in src/expr/match
      Basic support for printing nft_data_reg in XML format.


 src/expr/bitwise.c   |   33 ++++++++++++++-------------------
 src/expr/cmp.c       |   23 +++++++++++------------
 src/expr/data_reg.c  |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/expr/data_reg.h  |    3 +++
 src/expr/immediate.c |   43 ++++++++++++++++++++++++++++++++++++++-----
 src/expr/match.c     |    4 ++--
 6 files changed, 116 insertions(+), 38 deletions(-)

-- 
Arturo Borrero Gonzalez

[libnftables PATCH 1/2] Fix a typo in src/expr/match

[Arturo Borrero]

---
 src/expr/match.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/expr/match.c b/src/expr/match.c
index 9ae849b..f83a8ea 100644
--- a/src/expr/match.c
+++ b/src/expr/match.c
@@ -185,7 +185,7 @@ static int nft_rule_expr_match_parse(struct nft_rule_expr *e, struct nlattr *att
 }
 
 static
-int nft_rule_exp_match_snprintf_xml(char *buf, size_t len,
+int nft_rule_expr_match_snprintf_xml(char *buf, size_t len,
 				struct nft_expr_match *mt)
 {
 	int ret, size=len;
@@ -217,7 +217,7 @@ nft_rule_expr_match_snprintf(char *buf, size_t len, uint32_t type,
 
 	switch(type) {
 	case NFT_RULE_O_XML:
-		return nft_rule_exp_match_snprintf_xml(buf, len, match);
+		return nft_rule_expr_match_snprintf_xml(buf, len, match);
 	case NFT_RULE_O_DEFAULT:
 		return snprintf(buf, len, "name=%s rev=%u ",
 				match->name, match->rev);

[libnftables PATCH 2/2] Basic support for printing nft_data_reg in XML format.

[Arturo Borrero]

---
 src/expr/bitwise.c   |   33 ++++++++++++++-------------------
 src/expr/cmp.c       |   23 +++++++++++------------
 src/expr/data_reg.c  |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/expr/data_reg.h  |    3 +++
 src/expr/immediate.c |   43 ++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 114 insertions(+), 36 deletions(-)

diff --git a/src/expr/bitwise.c b/src/expr/bitwise.c
index ac89cba..249307a 100644
--- a/src/expr/bitwise.c
+++ b/src/expr/bitwise.c
@@ -199,7 +199,7 @@ static int
 nft_rule_expr_bitwise_snprintf_xml(char *buf, size_t size,
 				   struct nft_expr_bitwise *bitwise)
 {
-	int len = size, offset = 0, ret, i;
+	int len = size, offset = 0, ret;
 
 	ret = snprintf(buf, len, "\t\t<sreg>%u</sreg> "
 					"<dreg>%u</dreg> ",
@@ -209,19 +209,16 @@ nft_rule_expr_bitwise_snprintf_xml(char *buf, size_t size,
 	ret = snprintf(buf+offset, len, "<mask>");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i<bitwise->mask.len/sizeof(uint32_t); i++) {
-		ret = snprintf(buf+offset, len, "%.8x ",
-				bitwise->mask.val[i]);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
+	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->mask,
+							NFT_RULE_O_XML, 0);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	ret = snprintf(buf+offset, len, "</mask> <xor>");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i<bitwise->xor.len/sizeof(uint32_t); i++) {
-		ret = snprintf(buf+offset, len, "%.8x ", bitwise->xor.val[i]);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
+	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->xor,
+							NFT_RULE_O_XML, 0);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	ret = snprintf(buf+offset, len, "</xor> ");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
@@ -233,7 +230,7 @@ static int
 nft_rule_expr_bitwise_snprintf_default(char *buf, size_t size,
 					struct nft_expr_bitwise *bitwise)
 {
-	int len = size, offset = 0, ret, i;
+	int len = size, offset = 0, ret;
 
 	ret = snprintf(buf, len, "sreg=%u dreg=%u ",
 			bitwise->sreg, bitwise->dreg);
@@ -242,18 +239,16 @@ nft_rule_expr_bitwise_snprintf_default(char *buf, size_t size,
 	ret = snprintf(buf+offset, len, " mask=");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i<bitwise->mask.len/sizeof(uint32_t); i++) {
-		ret = snprintf(buf+offset, len, "%.8x ", bitwise->mask.val[i]);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
+	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->mask,
+							NFT_RULE_O_DEFAULT, 0);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	ret = snprintf(buf+offset, len, " xor=");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i<bitwise->xor.len/sizeof(uint32_t); i++) {
-		ret = snprintf(buf+offset, len, "%.8x ", bitwise->xor.val[i]);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
+	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->xor,
+							NFT_RULE_O_DEFAULT, 0);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	return offset;
 }
diff --git a/src/expr/cmp.c b/src/expr/cmp.c
index 429f024..6a5c4c2 100644
--- a/src/expr/cmp.c
+++ b/src/expr/cmp.c
@@ -169,18 +169,17 @@ static char *expr_cmp_str[] = {
 static int
 nft_rule_expr_cmp_snprintf_xml(char *buf, size_t size, struct nft_expr_cmp *cmp)
 {
-	int len = size, offset = 0, ret, i;
+	int len = size, offset = 0, ret;
 
-	ret = snprintf(buf, len, "\t\t<sreg>%u</sreg> <op>%s</op> <data>",
+	ret = snprintf(buf, len, "\t\t<sreg>%u</sreg> <op>%s</op> <cmpdata>",
 			cmp->sreg, expr_cmp_str[cmp->op]);
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i<cmp->data.len/sizeof(uint32_t); i++) {
-		ret = snprintf(buf+offset, len, "%.8x ", cmp->data.val[i]);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
+	ret = nft_data_reg_snprintf(buf+offset, len, &cmp->data,
+							NFT_RULE_O_XML, 0);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	ret = snprintf(buf+offset, len, "</data> ");
+	ret = snprintf(buf+offset, len, "</cmpdata> ");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	return offset;
@@ -190,16 +189,16 @@ static int
 nft_rule_expr_cmp_snprintf_default(char *buf, size_t size,
 				   struct nft_expr_cmp *cmp)
 {
-	int len = size, offset = 0, ret, i;
+	int len = size, offset = 0, ret;
 
 	ret = snprintf(buf, len, "sreg=%u op=%s data=",
 			cmp->sreg, expr_cmp_str[cmp->op]);
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i<cmp->data.len/sizeof(uint32_t); i++) {
-		ret = snprintf(buf+offset, len, "%.8x ", cmp->data.val[i]);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
+	ret = nft_data_reg_snprintf(buf+offset, len, &cmp->data,
+							NFT_RULE_O_DEFAULT, 0);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
 	return offset;
 }
 
diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
index 5b14695..b188571 100644
--- a/src/expr/data_reg.c
+++ b/src/expr/data_reg.c
@@ -18,10 +18,58 @@
 #include <linux/netfilter.h>
 #include <linux/netfilter/nf_tables.h>
 #include <libnftables/expr.h>
+#include <libnftables/rule.h>
 #include "expr_ops.h"
 #include "data_reg.h"
 #include "internal.h"
 
+static int nft_data_reg_snprintf_xml(char *buf, size_t size, uint32_t flags,
+						union nft_data_reg *reg)
+{
+	/* NOTE: The other struct in the union (the one with veredict/chain)
+	* is not supported yet */
+
+	int len = size, offset = 0, ret, i;
+
+	for (i=0; i<4; i++) {
+		ret = snprintf(buf+offset, len, "<data%d>0x%.8x</data%d>",
+							i, reg->val[i], i);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	return offset;
+}
+
+static int nft_data_reg_snprintf_default(char *buf, size_t size, uint32_t flags,
+						union nft_data_reg *reg)
+{
+	/* NOTE: The other struct in the union (the one with veredict/chain)
+	* is not supported yet */
+
+	int len = size, offset = 0, ret, i;
+
+	for (i=0; i<reg->len/sizeof(uint32_t); i++) {
+		ret = snprintf(buf+offset, len, "0x%.8x ", reg->val[i]);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	return offset;
+}
+
+int nft_data_reg_snprintf(char *buf, size_t size, union nft_data_reg *reg,
+						uint32_t type, uint32_t flags)
+{
+	switch(type) {
+	case NFT_RULE_O_XML:
+		return nft_data_reg_snprintf_xml(buf, size, flags, reg);
+	case NFT_RULE_O_DEFAULT:
+		return nft_data_reg_snprintf_default(buf, size, flags, reg);
+	default:
+		break;
+	}
+	return -1;
+}
+
 static int nft_data_parse_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
diff --git a/src/expr/data_reg.h b/src/expr/data_reg.h
index 00eab63..8441dc8 100644
--- a/src/expr/data_reg.h
+++ b/src/expr/data_reg.h
@@ -18,6 +18,9 @@ union nft_data_reg {
 	};
 };
 
+int nft_data_reg_snprintf(char *buf, size_t size, union nft_data_reg *reg,
+						uint32_t type, uint32_t flags);
+int nft_data_reg_xml_parse(union nft_data_reg *reg, char *xml);
 int nft_parse_data(union nft_data_reg *data, struct nlattr *attr, int *type);
 
 #endif
diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index 496cbfd..f244783 100644
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -196,6 +196,42 @@ nft_rule_expr_immediate_parse(struct nft_rule_expr *e, struct nlattr *attr)
 }
 
 static int
+nft_rule_expr_immediate_snprintf_xml(char *buf, size_t len,
+			struct nft_expr_immediate *imm, uint32_t flags)
+{
+	int size = len, offset = 0, ret;
+
+	ret = snprintf(buf, len, "\t\t<dreg>%u</dreg>"
+				"\n\t\t<immediatedata>", imm->dreg);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = nft_data_reg_snprintf(buf+offset, len, &imm->data,
+							NFT_RULE_O_XML, flags);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = snprintf(buf+offset, len, "</immediatedata>");
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	return offset;
+}
+
+static int
+nft_rule_expr_immediate_snprintf_default(char *buf, size_t len,
+				struct nft_expr_immediate *imm, uint32_t flags)
+{
+	int size = len, offset = 0, ret;
+
+	ret = snprintf(buf, len, "dreg=%u data=", imm->dreg);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	ret = nft_data_reg_snprintf(buf+offset, len, &imm->data,
+						NFT_RULE_O_DEFAULT, flags);
+	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+	return offset;
+}
+
+static int
 nft_rule_expr_immediate_snprintf(char *buf, size_t len, uint32_t type,
 				 uint32_t flags, struct nft_rule_expr *e)
 {
@@ -203,12 +239,9 @@ nft_rule_expr_immediate_snprintf(char *buf, size_t len, uint32_t type,
 
 	switch(type) {
 	case NFT_RULE_O_XML:
-		return snprintf(buf, len, "\t\t<dreg>%u</dreg>"
-					  " <data>%u</data> ",
-				imm->dreg, imm->data.val[0]);
+		return nft_rule_expr_immediate_snprintf_xml(buf, len, imm, flags);
 	case NFT_RULE_O_DEFAULT:
-		return snprintf(buf, len, "dreg=%u data=%u ",
-				imm->dreg, imm->data.val[0]);
+		return nft_rule_expr_immediate_snprintf_default(buf, len, imm, flags);
 	default:
 		break;
 	}

Re: [libnftables PATCH 2/2] Basic support for printing nft_data_reg in XML format.

[Pablo Neira Ayuso]

Hi Arturo,

On Fri, Mar 29, 2013 at 04:34:32PM +0100, Arturo Borrero wrote:
> 
> ---
>  src/expr/bitwise.c   |   33 ++++++++++++++-------------------
>  src/expr/cmp.c       |   23 +++++++++++------------
>  src/expr/data_reg.c  |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/expr/data_reg.h  |    3 +++
>  src/expr/immediate.c |   43 ++++++++++++++++++++++++++++++++++++++-----
>  5 files changed, 114 insertions(+), 36 deletions(-)
> 
> diff --git a/src/expr/bitwise.c b/src/expr/bitwise.c
> index ac89cba..249307a 100644
> --- a/src/expr/bitwise.c
> +++ b/src/expr/bitwise.c
> @@ -199,7 +199,7 @@ static int
>  nft_rule_expr_bitwise_snprintf_xml(char *buf, size_t size,
>  				   struct nft_expr_bitwise *bitwise)
>  {
> -	int len = size, offset = 0, ret, i;
> +	int len = size, offset = 0, ret;
>  
>  	ret = snprintf(buf, len, "\t\t<sreg>%u</sreg> "
>  					"<dreg>%u</dreg> ",
> @@ -209,19 +209,16 @@ nft_rule_expr_bitwise_snprintf_xml(char *buf, size_t size,
>  	ret = snprintf(buf+offset, len, "<mask>");
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	for (i=0; i<bitwise->mask.len/sizeof(uint32_t); i++) {
> -		ret = snprintf(buf+offset, len, "%.8x ",
> -				bitwise->mask.val[i]);
> -		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> -	}
> +	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->mask,
> +							NFT_RULE_O_XML, 0);

I think we need something similar to:

int nft_parse_data(union nft_data_reg *data, struct nlattr *attr, int *type);

We should pass a 'type' that indicates:

        DATA_VALUE,
        DATA_VERDICT,
        DATA_CHAIN,

So you know if you have print what the data_reg contains.

For bitwise, it should be DATA_VALUE.

> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
>  	ret = snprintf(buf+offset, len, "</mask> <xor>");
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	for (i=0; i<bitwise->xor.len/sizeof(uint32_t); i++) {
> -		ret = snprintf(buf+offset, len, "%.8x ", bitwise->xor.val[i]);
> -		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> -	}
> +	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->xor,
> +							NFT_RULE_O_XML, 0);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
>  	ret = snprintf(buf+offset, len, "</xor> ");
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> @@ -233,7 +230,7 @@ static int
>  nft_rule_expr_bitwise_snprintf_default(char *buf, size_t size,
>  					struct nft_expr_bitwise *bitwise)
>  {
> -	int len = size, offset = 0, ret, i;
> +	int len = size, offset = 0, ret;
>  
>  	ret = snprintf(buf, len, "sreg=%u dreg=%u ",
>  			bitwise->sreg, bitwise->dreg);
> @@ -242,18 +239,16 @@ nft_rule_expr_bitwise_snprintf_default(char *buf, size_t size,
>  	ret = snprintf(buf+offset, len, " mask=");
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	for (i=0; i<bitwise->mask.len/sizeof(uint32_t); i++) {
> -		ret = snprintf(buf+offset, len, "%.8x ", bitwise->mask.val[i]);
> -		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> -	}
> +	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->mask,
> +							NFT_RULE_O_DEFAULT, 0);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
>  	ret = snprintf(buf+offset, len, " xor=");
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	for (i=0; i<bitwise->xor.len/sizeof(uint32_t); i++) {
> -		ret = snprintf(buf+offset, len, "%.8x ", bitwise->xor.val[i]);
> -		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> -	}
> +	ret = nft_data_reg_snprintf(buf+offset, len, &bitwise->xor,
> +							NFT_RULE_O_DEFAULT, 0);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
>  	return offset;
>  }
> diff --git a/src/expr/cmp.c b/src/expr/cmp.c
> index 429f024..6a5c4c2 100644
> --- a/src/expr/cmp.c
> +++ b/src/expr/cmp.c
> @@ -169,18 +169,17 @@ static char *expr_cmp_str[] = {
>  static int
>  nft_rule_expr_cmp_snprintf_xml(char *buf, size_t size, struct nft_expr_cmp *cmp)
>  {
> -	int len = size, offset = 0, ret, i;
> +	int len = size, offset = 0, ret;
>  
> -	ret = snprintf(buf, len, "\t\t<sreg>%u</sreg> <op>%s</op> <data>",
> +	ret = snprintf(buf, len, "\t\t<sreg>%u</sreg> <op>%s</op> <cmpdata>",
>  			cmp->sreg, expr_cmp_str[cmp->op]);
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	for (i=0; i<cmp->data.len/sizeof(uint32_t); i++) {
> -		ret = snprintf(buf+offset, len, "%.8x ", cmp->data.val[i]);
> -		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> -	}
> +	ret = nft_data_reg_snprintf(buf+offset, len, &cmp->data,
> +							NFT_RULE_O_XML, 0);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	ret = snprintf(buf+offset, len, "</data> ");
> +	ret = snprintf(buf+offset, len, "</cmpdata> ");
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
>  	return offset;
> @@ -190,16 +189,16 @@ static int
>  nft_rule_expr_cmp_snprintf_default(char *buf, size_t size,
>  				   struct nft_expr_cmp *cmp)
>  {
> -	int len = size, offset = 0, ret, i;
> +	int len = size, offset = 0, ret;
>  
>  	ret = snprintf(buf, len, "sreg=%u op=%s data=",
>  			cmp->sreg, expr_cmp_str[cmp->op]);
>  	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>  
> -	for (i=0; i<cmp->data.len/sizeof(uint32_t); i++) {
> -		ret = snprintf(buf+offset, len, "%.8x ", cmp->data.val[i]);
> -		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> -	}
> +	ret = nft_data_reg_snprintf(buf+offset, len, &cmp->data,
> +							NFT_RULE_O_DEFAULT, 0);

For cmp, it should also be DATA_VALUE.

> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
>  	return offset;
>  }
>  
> diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
> index 5b14695..b188571 100644
> --- a/src/expr/data_reg.c
> +++ b/src/expr/data_reg.c
> @@ -18,10 +18,58 @@
>  #include <linux/netfilter.h>
>  #include <linux/netfilter/nf_tables.h>
>  #include <libnftables/expr.h>
> +#include <libnftables/rule.h>
>  #include "expr_ops.h"
>  #include "data_reg.h"
>  #include "internal.h"
>  
> +static int nft_data_reg_snprintf_xml(char *buf, size_t size, uint32_t flags,
> +						union nft_data_reg *reg)
> +{
> +	/* NOTE: The other struct in the union (the one with veredict/chain)
> +	* is not supported yet */
> +
> +	int len = size, offset = 0, ret, i;
> +
> +	for (i=0; i<4; i++) {
> +		ret = snprintf(buf+offset, len, "<data%d>0x%.8x</data%d>",
> +							i, reg->val[i], i);
> +		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +	}
> +
> +	return offset;
> +}
> +
> +static int nft_data_reg_snprintf_default(char *buf, size_t size, uint32_t flags,
> +						union nft_data_reg *reg)
> +{
> +	/* NOTE: The other struct in the union (the one with veredict/chain)
> +	* is not supported yet */
> +
> +	int len = size, offset = 0, ret, i;
> +
> +	for (i=0; i<reg->len/sizeof(uint32_t); i++) {
> +		ret = snprintf(buf+offset, len, "0x%.8x ", reg->val[i]);
> +		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +	}
> +
> +	return offset;
> +}
> +
> +int nft_data_reg_snprintf(char *buf, size_t size, union nft_data_reg *reg,
> +						uint32_t type, uint32_t flags)
> +{
> +	switch(type) {
> +	case NFT_RULE_O_XML:
> +		return nft_data_reg_snprintf_xml(buf, size, flags, reg);
> +	case NFT_RULE_O_DEFAULT:
> +		return nft_data_reg_snprintf_default(buf, size, flags, reg);
> +	default:
> +		break;
> +	}
> +	return -1;
> +}
> +
>  static int nft_data_parse_cb(const struct nlattr *attr, void *data)
>  {
>  	const struct nlattr **tb = data;
> diff --git a/src/expr/data_reg.h b/src/expr/data_reg.h
> index 00eab63..8441dc8 100644
> --- a/src/expr/data_reg.h
> +++ b/src/expr/data_reg.h
> @@ -18,6 +18,9 @@ union nft_data_reg {
>  	};
>  };
>  
> +int nft_data_reg_snprintf(char *buf, size_t size, union nft_data_reg *reg,
> +						uint32_t type, uint32_t flags);
> +int nft_data_reg_xml_parse(union nft_data_reg *reg, char *xml);
>  int nft_parse_data(union nft_data_reg *data, struct nlattr *attr, int *type);
>  
>  #endif
> diff --git a/src/expr/immediate.c b/src/expr/immediate.c
> index 496cbfd..f244783 100644
> --- a/src/expr/immediate.c
> +++ b/src/expr/immediate.c
> @@ -196,6 +196,42 @@ nft_rule_expr_immediate_parse(struct nft_rule_expr *e, struct nlattr *attr)
>  }
>  
>  static int
> +nft_rule_expr_immediate_snprintf_xml(char *buf, size_t len,
> +			struct nft_expr_immediate *imm, uint32_t flags)
> +{
> +	int size = len, offset = 0, ret;
> +
> +	ret = snprintf(buf, len, "\t\t<dreg>%u</dreg>"
> +				"\n\t\t<immediatedata>", imm->dreg);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> +	ret = nft_data_reg_snprintf(buf+offset, len, &imm->data,
> +							NFT_RULE_O_XML, flags);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> +	ret = snprintf(buf+offset, len, "</immediatedata>");
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> +	return offset;
> +}
> +
> +static int
> +nft_rule_expr_immediate_snprintf_default(char *buf, size_t len,
> +				struct nft_expr_immediate *imm, uint32_t flags)
> +{
> +	int size = len, offset = 0, ret;
> +
> +	ret = snprintf(buf, len, "dreg=%u data=", imm->dreg);
> +	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> +	ret = nft_data_reg_snprintf(buf+offset, len, &imm->data,
> +						NFT_RULE_O_DEFAULT, flags);

For immediate, it depends on the attribute set:

NFT_EXPR_IMM_VERDICT => DATA_VERDICT
NFT_EXPR_IMM_CHAIN => DATA_CHAIN
NFT_EXPR_IMM_VALUE => DATA_VALUE

Please, re-spin and send me a new patch.

Thanks.

Re: [libnftables PATCH 2/2] Basic support for printing nft_data_reg in XML format.

[Arturo Borrero Gonzalez]

2013/4/2 Pablo Neira Ayuso <pablo@netfilter.org>:
> I think we need something similar to:
>
> int nft_parse_data(union nft_data_reg *data, struct nlattr *attr, int *type);
>
> We should pass a 'type' that indicates:
>
>         DATA_VALUE,
>         DATA_VERDICT,
>         DATA_CHAIN,
>
> So you know if you have print what the data_reg contains.
>

If cmp and bitwise are going to always have DATA_VALUE, and imm
depending on what was set, I think we could do:

int nft_data_reg_snprintf(char *buf, union data_reg *data, int type, ...)
switch(type)
return nft_data_reg_snprintf_'type'(buf, data, ...)

And the same applies when parsing XML.

Is this true?

If this is not the case, I think I didn't understand.
--
Arturo Borrero González
--
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

Re: [libnftables PATCH 2/2] Basic support for printing nft_data_reg in XML format.

[Pablo Neira Ayuso]

On Tue, Apr 02, 2013 at 07:18:39PM +0200, Arturo Borrero Gonzalez wrote:
> 2013/4/2 Pablo Neira Ayuso <pablo@netfilter.org>:
> > I think we need something similar to:
> >
> > int nft_parse_data(union nft_data_reg *data, struct nlattr *attr, int *type);
> >
> > We should pass a 'type' that indicates:
> >
> >         DATA_VALUE,
> >         DATA_VERDICT,
> >         DATA_CHAIN,
> >
> > So you know if you have print what the data_reg contains.
> >
> 
> If cmp and bitwise are going to always have DATA_VALUE, and imm
> depending on what was set, I think we could do:
> 
> int nft_data_reg_snprintf(char *buf, union data_reg *data, int type, ...)
> switch(type)
> return nft_data_reg_snprintf_'type'(buf, data, ...)

where 'type' can be: value, verdict or chain.

> And the same applies when parsing XML.

Exactly.

Re: [libnftables PATCH 2/2] Basic support for printing nft_data_reg in XML format.

[Arturo Borrero Gonzalez]

2013/4/2 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Tue, Apr 02, 2013 at 07:18:39PM +0200, Arturo Borrero Gonzalez wrote:
>> If cmp and bitwise are going to always have DATA_VALUE, and imm
>> depending on what was set, I think we could do:
>>
>> int nft_data_reg_snprintf(char *buf, union data_reg *data, int type, ...)
>> switch(type)
>> return nft_data_reg_snprintf_'type'(buf, data, ...)
>
> where 'type' can be: value, verdict or chain.
>
>> And the same applies when parsing XML.
>
> Exactly.

I propose to use this format:

for DATA_VALUE:

<expr type=cmp>
   [...]
   <cmpdata>
     <data_reg type=value>
      <len>N</len>
      <data0>reg->val[i]</data0>
       [...]
       <dataN>reg->val[n]</dataN>
    </data_reg>
  </cmpdata>
</expr>

This way we also export/import the len of reg->val (actually reg->len)

For DATA_VERDICT or DATA_CHAIN:
<data_reg type=verdict>
    <verdict>int</verdict>
    <chain>string</chain>
</data_reg>

assuming verdict and chain are always used together, despite of node
'<verdict>' or '<chain>' being not set (but present with no value)
--
Arturo Borrero González
--
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

Re: [libnftables PATCH 2/2] Basic support for printing nft_data_reg in XML format.

[Pablo Neira Ayuso]

On Wed, Apr 03, 2013 at 02:38:38PM +0200, Arturo Borrero Gonzalez wrote:
> 2013/4/2 Pablo Neira Ayuso <pablo@netfilter.org>:
> > On Tue, Apr 02, 2013 at 07:18:39PM +0200, Arturo Borrero Gonzalez wrote:
> >> If cmp and bitwise are going to always have DATA_VALUE, and imm
> >> depending on what was set, I think we could do:
> >>
> >> int nft_data_reg_snprintf(char *buf, union data_reg *data, int type, ...)
> >> switch(type)
> >> return nft_data_reg_snprintf_'type'(buf, data, ...)
> >
> > where 'type' can be: value, verdict or chain.
> >
> >> And the same applies when parsing XML.
> >
> > Exactly.
> 
> I propose to use this format:
> 
> for DATA_VALUE:
> 
> <expr type=cmp>
>    [...]
>    <cmpdata>
>      <data_reg type=value>
>       <len>N</len>
>       <data0>reg->val[i]</data0>
>        [...]
>        <dataN>reg->val[n]</dataN>
>     </data_reg>
>   </cmpdata>
> </expr>
> 
> This way we also export/import the len of reg->val (actually reg->len)
> 
> For DATA_VERDICT or DATA_CHAIN:
> <data_reg type=verdict>
>     <verdict>int</verdict>
>     <chain>string</chain>
> </data_reg>
> 
> assuming verdict and chain are always used together, despite of node
> '<verdict>' or '<chain>' being not set (but present with no value)
>
> For DATA_VERDICT or DATA_CHAIN:
> <data_reg type=verdict>
>     <verdict>int</verdict>
>     <chain>string</chain>
> </data_reg> 

The verdict and the chain are mutually exclusive. So it has to be:

* For DATA_VERDICT:

<data_reg type=verdict>
    <verdict>string</verdict>
</data_reg>

where string can be accept, drop, return. Better use the string than
the value, it's human readable and people can edit it without knowing
the mapping between values and verdicts.

* For DATA_CHAIN:

<data_reg type=verdict>
    <chain>string</chain>
</data_reg>

where string is the chain name. This chain is the chain to jump in
case of matching (like in iptables: -j some_chain).

Regards.