[iptables (nft-compat) PATCH 0/5] Various fixes version 2

[iptables (nft-compat) PATCH 0/5] Various fixes version 2

[Tomasz Bursztyka]

Patch 1 and 2 are a resent of previous patchset.
(patch 1 is quite useful actually, at leats it was for testing patch 3)
(patch 2 is a bug fix)

Patch 3 is the full rewrite of patch 8 it previous patchset.
Much smaller, and works in all case with xtables-restore with or without --table option.

Patch 4 is applying the idea to get rid of useless error message (which clearly did not help at all).

Patch 5 is a debug message fix.

Tomasz Bursztyka (5):
  nft: Add useful debug output when a builtin table is created
  nft: A builtin chain might be created when restoring
  nft: Initialize a table only once
  nft: Remove useless error message
  nft: Pass a line after printing out a debug message

 iptables/nft.c             | 66 ++++++++++++++++++----------------------------
 iptables/nft.h             |  2 ++
 iptables/xtables-restore.c |  1 +
 3 files changed, 28 insertions(+), 41 deletions(-)

-- 
1.8.3.2

[iptables (nft-compat) PATCH 1/5] nft: Add useful debug output when a builtin table is created

[Tomasz Bursztyka]

This is useful to know if a builtin table is requested to be created.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index fc9db99..968fe8c 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -451,6 +451,14 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 	nft_table_nlmsg_build_payload(nlh, t);
 	nft_table_free(t);
 
+#ifdef NLDEBUG
+	char tmp[1024];
+
+	nft_table_snprintf(tmp, sizeof(tmp), t, 0, 0);
+	printf("DEBUG: table: %s", tmp);
+	mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len, sizeof(struct nfgenmsg));
+#endif
+
 	ret = mnl_talk(h, nlh, NULL, NULL);
 	if (ret < 0) {
 		if (errno != EEXIST)
-- 
1.8.3.2

[iptables (nft-compat) PATCH 2/5] nft: A builtin chain might be created when restoring

[Tomasz Bursztyka]

nft_chain_set() is directly used in xtables-restore.c, however at that
point no builtin chains have been created yet thus the need to request
to build it relevantly.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c             | 1 +
 iptables/nft.h             | 1 +
 iptables/xtables-restore.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 968fe8c..7031ede 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -744,6 +744,7 @@ __nft_chain_set(struct nft_handle *h, const char *table,
 	}
 
 	nlh = nft_chain_nlmsg_build_hdr(buf, NFT_MSG_NEWCHAIN, h->family,
+					h->restore ? NLM_F_ACK|NLM_F_CREATE :
 					NLM_F_ACK, h->seq);
 	nft_chain_nlmsg_build_payload(nlh, c);
 
diff --git a/iptables/nft.h b/iptables/nft.h
index 22af66e..8b64f8b 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -34,6 +34,7 @@ struct nft_handle {
 	struct mnl_nlmsg_batch	*batch;
 	struct nft_family_ops	*ops;
 	struct builtin_table	*tables;
+	bool			restore;
 };
 
 extern struct builtin_table xtables_ipv4[TABLES_MAX];
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 50d2935..0498abc 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -170,6 +170,7 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
 {
 	struct nft_handle h = {
 		.family = family,
+		.restore = true,
 	};
 	char buffer[10240];
 	int c;
-- 
1.8.3.2

[iptables (nft-compat) PATCH 3/5] nft: Initialize a table only once

[Tomasz Bursztyka]

This helps to remove some runtime overhead, especially when running
xtables-restore.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c | 10 ++++++++++
 iptables/nft.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 7031ede..ac1afa4 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -436,6 +436,9 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 	struct nft_table *t;
 	int ret;
 
+	if (_t->initialized)
+		return 0;
+
 	t = nft_table_alloc();
 	if (t == NULL)
 		return -1;
@@ -464,6 +467,10 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 		if (errno != EEXIST)
 			perror("mnl-talk:nft_table_init_one");
 	}
+
+	if (ret == 0 || errno == EEXIST)
+		_t->initialized = true;
+
 	return ret;
 }
 
@@ -2441,6 +2448,9 @@ int nft_xtables_config_load(struct nft_handle *h, const char *filename,
 	uint32_t table_family, chain_family;
 	bool found = false;
 
+	if (h->restore)
+		return 0;
+
 	if (xtables_config_parse(filename, table_list, chain_list) < 0) {
 		if (errno == ENOENT) {
 			xtables_config_perror(flags,
diff --git a/iptables/nft.h b/iptables/nft.h
index 8b64f8b..204a23f 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -22,6 +22,7 @@ struct builtin_chain {
 struct builtin_table {
 	const char *name;
 	struct builtin_chain chains[NF_INET_NUMHOOKS];
+	bool initialized;
 };
 
 struct nft_handle {
-- 
1.8.3.2

[iptables (nft-compat) PATCH 4/5] nft: Remove useless error message

[Tomasz Bursztyka]

These are not helpful.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c | 45 +++++----------------------------------------
 1 file changed, 5 insertions(+), 40 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index ac1afa4..32d3dbb 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -463,11 +463,6 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 #endif
 
 	ret = mnl_talk(h, nlh, NULL, NULL);
-	if (ret < 0) {
-		if (errno != EEXIST)
-			perror("mnl-talk:nft_table_init_one");
-	}
-
 	if (ret == 0 || errno == EEXIST)
 		_t->initialized = true;
 
@@ -513,10 +508,7 @@ nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
 	nft_chain_nlmsg_build_payload(nlh, c);
 	nft_chain_free(c);
 
-	if (mnl_talk(h, nlh, NULL, NULL) < 0) {
-		if (errno != EEXIST)
-			perror("mnl_talk:nft_chain_builtin_add");
-	}
+	mnl_talk(h, nlh, NULL, NULL);
 }
 
 /* find if built-in table already exists */
@@ -725,7 +717,6 @@ __nft_chain_set(struct nft_handle *h, const char *table,
 	struct nft_chain *c;
 	struct builtin_table *_t;
 	struct builtin_chain *_c;
-	int ret;
 
 	_t = nft_table_builtin_find(h, table);
 	/* if this built-in table does not exists, create it */
@@ -759,11 +750,7 @@ __nft_chain_set(struct nft_handle *h, const char *table,
 
 	nft_chain_free(c);
 
-	ret = mnl_talk(h, nlh, NULL, NULL);
-	if (ret < 0)
-		perror("mnl_talk:__nft_chain_policy");
-
-	return ret;
+	return mnl_talk(h, nlh, NULL, NULL);
 }
 
 int nft_chain_set(struct nft_handle *h, const char *table,
@@ -1106,7 +1093,6 @@ static struct nft_chain_list *nft_chain_list_get(struct nft_handle *h)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nlmsghdr *nlh;
-	int ret;
 	struct nft_chain_list *list;
 
 	list = nft_chain_list_alloc();
@@ -1118,9 +1104,7 @@ static struct nft_chain_list *nft_chain_list_get(struct nft_handle *h)
 	nlh = nft_chain_nlmsg_build_hdr(buf, NFT_MSG_GETCHAIN, h->family,
 					NLM_F_DUMP, h->seq);
 
-	ret = mnl_talk(h, nlh, nft_chain_list_cb, list);
-	if (ret < 0)
-		perror("mnl_talk:nft_chain_list_get");
+	mnl_talk(h, nlh, nft_chain_list_cb, list);
 
 	return list;
 }
@@ -1228,7 +1212,6 @@ static struct nft_rule_list *nft_rule_list_get(struct nft_handle *h)
 
 	ret = mnl_talk(h, nlh, nft_rule_list_cb, list);
 	if (ret < 0) {
-		perror("mnl_talk:nft_rule_save");
 		nft_rule_list_free(list);
 		return NULL;
 	}
@@ -1363,10 +1346,6 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 	nft_chain_free(c);
 
 	ret = mnl_talk(h, nlh, NULL, NULL);
-	if (ret < 0) {
-		if (errno != EEXIST)
-			perror("mnl_talk:nft_chain_add");
-	}
 
 	/* the core expects 1 for success and 0 for error */
 	return ret == 0 ? 1 : 0;
@@ -1376,19 +1355,12 @@ static int __nft_chain_del(struct nft_handle *h, struct nft_chain *c)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nlmsghdr *nlh;
-	int ret;
 
 	nlh = nft_chain_nlmsg_build_hdr(buf, NFT_MSG_DELCHAIN, h->family,
 					NLM_F_ACK, h->seq);
 	nft_chain_nlmsg_build_payload(nlh, c);
 
-	ret = mnl_talk(h, nlh, NULL, NULL);
-	if (ret < 0) {
-		if (errno != EEXIST && errno != ENOENT)
-			perror("mnl_talk:__nft_chain_del");
-	}
-
-	return ret;
+	return mnl_talk(h, nlh, NULL, NULL);
 }
 
 int nft_chain_user_del(struct nft_handle *h, const char *chain, const char *table)
@@ -1529,10 +1501,6 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 	nft_chain_free(c);
 
 	ret = mnl_talk(h, nlh, NULL, NULL);
-	if (ret < 0) {
-		if (errno != EEXIST)
-			perror("mnl_talk:nft_chain_rename");
-	}
 
 	/* the core expects 1 for success and 0 for error */
 	return ret == 0 ? 1 : 0;
@@ -1567,7 +1535,6 @@ static struct nft_table_list *nft_table_list_get(struct nft_handle *h)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nlmsghdr *nlh;
-	int ret;
 	struct nft_table_list *list;
 
 	list = nft_table_list_alloc();
@@ -1577,9 +1544,7 @@ static struct nft_table_list *nft_table_list_get(struct nft_handle *h)
 	nlh = nft_rule_nlmsg_build_hdr(buf, NFT_MSG_GETTABLE, h->family,
 					NLM_F_DUMP, h->seq);
 
-	ret = mnl_talk(h, nlh, nft_table_list_cb, list);
-	if (ret < 0)
-		perror("mnl_talk:nft_table_list_get");
+	mnl_talk(h, nlh, nft_table_list_cb, list);
 
 	return list;
 }
-- 
1.8.3.2

[iptables (nft-compat) PATCH 5/5] nft: Pass a line after printing out a debug message

[Tomasz Bursztyka]

In this specific places, libnftnl gives back a string on which iptables
should not assume any line break, thus it's up to iptables to add it.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 32d3dbb..f6d5263 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -458,7 +458,7 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 	char tmp[1024];
 
 	nft_table_snprintf(tmp, sizeof(tmp), t, 0, 0);
-	printf("DEBUG: table: %s", tmp);
+	printf("DEBUG: table: %s\n", tmp);
 	mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len, sizeof(struct nfgenmsg));
 #endif
 
@@ -702,7 +702,7 @@ static void nft_chain_print_debug(struct nft_chain *c, struct nlmsghdr *nlh)
 	char tmp[1024];
 
 	nft_chain_snprintf(tmp, sizeof(tmp), c, 0, 0);
-	printf("DEBUG: chain: %s", tmp);
+	printf("DEBUG: chain: %s\n", tmp);
 	mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len, sizeof(struct nfgenmsg));
 #endif
 }
@@ -898,7 +898,7 @@ static void nft_rule_print_debug(struct nft_rule *r, struct nlmsghdr *nlh)
 	char tmp[1024];
 
 	nft_rule_snprintf(tmp, sizeof(tmp), r, 0, 0);
-	printf("DEBUG: rule: %s", tmp);
+	printf("DEBUG: rule: %s\n", tmp);
 	mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len, sizeof(struct nfgenmsg));
 #endif
 }
-- 
1.8.3.2

Re: [iptables (nft-compat) PATCH 0/5] Various fixes version 2

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 06:36:41PM +0200, Tomasz Bursztyka wrote:
> Patch 1 and 2 are a resent of previous patchset.
> (patch 1 is quite useful actually, at leats it was for testing patch 3)
> (patch 2 is a bug fix)
> 
> Patch 3 is the full rewrite of patch 8 it previous patchset.
> Much smaller, and works in all case with xtables-restore with or without --table option.
> 
> Patch 4 is applying the idea to get rid of useless error message (which clearly did not help at all).
> 
> Patch 5 is a debug message fix.

Series applied, thanks Tomasz.