Skip to contentSkip to conversation

expr: masq: optional printing of flags attr in snprintf_default

PATCH Applied Arturo Borrero Gonzalez 2 participants 6 messages Oct 2, 2014 – Oct 3, 2014 netfilter

lore
Share & tools
lore

Arturo Borrero Gonzalez

author
#0 lore raw

PATCH 1/3 expr: masq: optional printing of flags attr in snprintf_default

The flags attribute is optional. Thus we should print only if it was originally set.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---

 0 files changed
src/expr/masq.c +4 -2
index 6a1c609..c8a6a8d 100644
@@ -169,8 +169,10 @@ static int nft_rule_expr_masq_snprintf_default(char *buf, size_t len,
 {
 	struct nft_expr_masq *masq = nft_expr_data(e);
 
-	return snprintf(buf, len, " flags %u ",
-			 masq->flags);
+	if (e->flags & (1 << NFT_EXPR_MASQ_FLAGS))
+		return snprintf(buf, len, "flags %u", masq->flags);
+
+	return 0;
 }
 
 static int nft_rule_expr_masq_snprintf(char *buf, size_t len, uint32_t type,
author
#1 lore raw

PATCH 2/3 tests: add tests for the masq expression

The masq expression is lacking of tests. Let's add some.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>
---
 tests/Makefile.am                 |    4 ++
 tests/jsonfiles/68-rule-masq.json |    2 +
 tests/nft-expr_masq-test.c        |   89 +++++++++++++++++++++++++++++++++++++
 tests/test-script.sh              |    1 
 tests/xmlfiles/79-rule-masq.xml   |    2 +
 5 files changed, 98 insertions(+)
 create mode 100644 tests/jsonfiles/68-rule-masq.json
 create mode 100644 tests/nft-expr_masq-test.c
 create mode 100644 tests/xmlfiles/79-rule-masq.xml
tests/Makefile.am +4 -0
index 7942bc0..d4f44af 100644
@@ -20,6 +20,7 @@ check_PROGRAMS = 	nft-parsing-test		\
 			nft-expr_lookup-test		\
 			nft-expr_log-test		\
 			nft-expr_match-test		\
+			nft-expr_masq-test		\
 			nft-expr_meta-test		\
 			nft-expr_nat-test		\
 			nft-expr_payload-test		\

@@ -75,6 +76,9 @@ nft_expr_log_test_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
 nft_expr_match_test_SOURCES = nft-expr_match-test.c
 nft_expr_match_test_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
 
+nft_expr_masq_test_SOURCES = nft-expr_masq-test.c
+nft_expr_masq_test_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
+
 nft_expr_meta_test_SOURCES = nft-expr_meta-test.c
 nft_expr_meta_test_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
tests/jsonfiles/68-rule-masq.json new file +2 -0
new file mode 100644
index 0000000..cfaed4c
@@ -0,0 +1,2 @@
+{"nftables":[{"rule":{"family":"ip6","table":"nat","chain":"postrouting","handle":4,"expr":[{"type":"masq","flags":12}]}}]}
+
tests/nft-expr_masq-test.c new file +89 -0

89 lines not on this page · raw message

tests/test-script.sh +1 -0
index 44725d8..93caeb8 100755
@@ -10,6 +10,7 @@
 ./nft-expr_log-test
 ./nft-expr_lookup-test
 ./nft-expr_match-test
+./nft-expr_masq-test
 ./nft-expr_meta-test
 ./nft-expr_nat-test
 ./nft-expr_payload-test
tests/xmlfiles/79-rule-masq.xml new file +2 -0
new file mode 100644
index 0000000..b5c5948
@@ -0,0 +1,2 @@
+<nftables><rule><family>ip6</family><table>nat</table><chain>postrouting</chain><handle>4</handle><expr type="masq"><flags>12</flags></expr></rule></nftables>
+
maintainer Applied
#2 lore raw

Re: [libnftnl PATCH 2/3] tests: add tests for the masq expression

Quoted 1 line · Arturo Borrero Gonzalez

The masq expression is lacking of tests. Let's add some.

Applied, thanks.

Please, make sure git am doesn't complain with trailing whitespace and empty lines at the end of the file.

git am reported two in this patch.

tests/jsonfiles/68-rule-masq.json line 2

Quoted 8 lines
tests/jsonfiles/68-rule-masq.json new file +2 -0
new file mode 100644
index 0000000..cfaed4c
@@ -0,0 +1,2 @@
+{"nftables":[{"rule":{"family":"ip6","table":"nat","chain":"postrouting","handle":4,"expr":[{"type":"masq","flags":12}]}}]}
+

Here.

Quoted 1 line

+++ b/tests/xmlfiles/79-rule-masq.xml

                       ^^

BTW, I wonder when the file numbering diverted (in xml, this 79 but in json this is 68), it seems we have more tests for XML than json.

For similar mirror tests, we should be using the same number.

author
#3 lore raw

PATCH 3/3 tests: also test nat's flags attribute

In reply to Arturo Borrero Gonzalez

The nat expression has a new attribute. Let's give some testing.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>
---
 tests/jsonfiles/33-rule-nat6.json |    2 +-
 tests/nft-expr_nat-test.c         |    4 ++++
 tests/xmlfiles/33-rule-nat4.xml   |    2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)
tests/jsonfiles/33-rule-nat6.json +1 -1
index e5d8c8f..a76eb71 100644
@@ -1 +1 @@
-{"nftables":[{"rule":{"family":"ip6","table":"nat","chain":"output","handle":33,"expr":[{"type":"nat","nat_type":"snat","family":"ip6","sreg_addr_min":1,"sreg_addr_max":2,"sreg_proto_min":3,"sreg_proto_max":4}]}}]}
+{"nftables":[{"rule":{"family":"ip6","table":"nat","chain":"output","handle":33,"expr":[{"type":"nat","nat_type":"snat","family":"ip6","sreg_addr_min":1,"sreg_addr_max":2,"sreg_proto_min":3,"sreg_proto_max":4,"flags":12}]}}]}
tests/nft-expr_nat-test.c +4 -0
index 64966b2..8982b98 100644
@@ -49,6 +49,9 @@ static void cmp_nft_rule_expr(struct nft_rule_expr *rule_a,
 	if (nft_rule_expr_get_u32(rule_a, NFT_EXPR_NAT_REG_PROTO_MAX) !=
 	    nft_rule_expr_get_u32(rule_b, NFT_EXPR_NAT_REG_PROTO_MAX))
 		print_err("Expr NFT_EXPR_NAT_REG_PROTO_MAX mismatches");
+	if (nft_rule_expr_get_u32(rule_a, NFT_EXPR_NAT_FLAGS) !=
+	    nft_rule_expr_get_u32(rule_b, NFT_EXPR_NAT_FLAGS))
+		print_err("Expr NFT_EXPR_NAT_FLAGS mismatches");
 }
 
 int main(int argc, char *argv[])

@@ -74,6 +77,7 @@ int main(int argc, char *argv[])
 	nft_rule_expr_set_u32(ex, NFT_EXPR_NAT_REG_ADDR_MAX, 0x1234568);
 	nft_rule_expr_set_u32(ex, NFT_EXPR_NAT_REG_PROTO_MIN, 0x1234568);
 	nft_rule_expr_set_u32(ex, NFT_EXPR_NAT_REG_PROTO_MAX, 0x1234568);
+	nft_rule_expr_set_u32(ex, NFT_EXPR_NAT_FLAGS, 0x1234568);
 
 	nft_rule_add_expr(a, ex);
tests/xmlfiles/33-rule-nat4.xml +1 -1
index 73c5b56..233c44e 100644
@@ -1 +1 @@
-<nftables><rule><family>ip</family><table>filter</table><chain>INPUT</chain><handle>100</handle><expr type="nat"><type>dnat</type><family>ip</family><sreg_addr_min>1</sreg_addr_min><sreg_addr_max>2</sreg_addr_max><sreg_proto_min>3</sreg_proto_min><sreg_proto_max>4</sreg_proto_max></expr></rule></nftables>
+<nftables><rule><family>ip</family><table>filter</table><chain>INPUT</chain><handle>100</handle><expr type="nat"><type>dnat</type><family>ip</family><sreg_addr_min>1</sreg_addr_min><sreg_addr_max>2</sreg_addr_max><sreg_proto_min>3</sreg_proto_min><sreg_proto_max>4</sreg_proto_max><flags>12</flags></expr></rule></nftables>
maintainer
#4 lore raw

Re: [libnftnl PATCH 3/3] tests: also test nat's flags attribute

Quoted 1 line · Arturo Borrero Gonzalez

The nat expression has a new attribute. Let's give some testing.

Also applied, thanks.

maintainer
#5 lore raw
In reply to Arturo Borrero Gonzalez
Quoted 2 lines · Arturo Borrero Gonzalez

The flags attribute is optional. Thus we should print only if it was originally set.

Applied with nitpick.

Quoted 9 lines
Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---

 0 files changed
src/expr/masq.c +0 -0
index 6a1c609..c8a6a8d 100644

src/expr/masq.c static int nft_rule_expr_masq_snprintf_default(char *buf, size_t len, · line 173

Quoted 7 lines
src/expr/masq.c +2 -2
@@ -169,8 +169,10 @@ static int nft_rule_expr_masq_snprintf_default(char *buf, size_t len,
 {
 	struct nft_expr_masq *masq = nft_expr_data(e);
 
-	return snprintf(buf, len, " flags %u ",
-			 masq->flags);
+	if (e->flags & (1 << NFT_EXPR_MASQ_FLAGS))
+		return snprintf(buf, len, "flags %u", masq->flags);

^
missing space there.

Otherwise the output shows flags 0].

And change it to use %x. Hex is better for flags.