[libnftnl/iptables PATCH 0/8] Various fixes

[libnftnl/iptables PATCH 0/8] Various fixes

[Tomasz Bursztyka]

On libnftnl:

When enabling debug on iptables, I got a non proper output:
DEBUG: table: table 0L© unknown flags 0 ----------------	------------------

when it should be:
DEBUG: table: table 0L© unknown flags 0
----------------        ------------------

Same on chain. So these 2 patches below fix that.
Note: I haven't been able to fix yet this garbage printing
(instead of '0L©' there should be the proper table name)

Tomasz Bursztyka (2):
  chain: Break the line properly when printing out the chain
  table: Break the line properly when printing out the table

 src/chain.c | 2 +-
 src/table.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

On iptables (nft-compat branch):

Tomasz Bursztyka (6):
  xtables: Handle family is not related to Netlink family
  xtables: Add backward compatibility with -w option
  nft: Fix error message to output proper origin
  nft: Add useful debug output when a builtin table is created
  nft: A builtin chain might be created when restoring
  nft: Initialize according to requested table, if any

 iptables/nft.c                | 52 +++++++++++++++++++++++++++++++++++--------
 iptables/nft.h                |  7 ++++--
 iptables/xtables-arp.c        |  2 +-
 iptables/xtables-config.c     |  2 +-
 iptables/xtables-restore.c    | 24 +++++++++++---------
 iptables/xtables-save.c       | 16 ++++++-------
 iptables/xtables-standalone.c |  4 ++--
 iptables/xtables.c            | 12 +++++++++-
 8 files changed, 84 insertions(+), 35 deletions(-)

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

[libnftnl PATCH 1/8] chain: Break the line properly when printing out the chain

[Tomasz Bursztyka]

As when a rule is printed etc... It fixes a bug in iptables when
printing out the chain in debug mode.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 src/chain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/chain.c b/src/chain.c
index e26200d..86bd43a 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -849,7 +849,7 @@ static int nft_chain_snprintf_default(char *buf, size_t size,
 	if (c->flags & (1 << NFT_CHAIN_ATTR_HOOKNUM)) {
 		ret = snprintf(buf+offset, len,
 			       " type %s hook %s prio %d policy %s use %d "
-			       "packets %"PRIu64" bytes %"PRIu64"",
+			       "packets %"PRIu64" bytes %"PRIu64"\n",
 			       c->type, nft_hooknum2str(c->family, c->hooknum),
 			       c->prio, nft_verdict2str(c->policy), c->use,
 			       c->packets, c->bytes);
-- 
1.8.3.2

[libnftnl PATCH 2/8] table: Break the line properly when printing out the table

[Tomasz Bursztyka]

As when a rule is printed etc... It fixes a bug in iptables when
printing out the table in debug mode.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 src/table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/table.c b/src/table.c
index f50a968..529f568 100644
--- a/src/table.c
+++ b/src/table.c
@@ -383,7 +383,7 @@ static int nft_table_snprintf_xml(char *buf, size_t size, struct nft_table *t)
 
 static int nft_table_snprintf_default(char *buf, size_t size, struct nft_table *t)
 {
-	return snprintf(buf, size, "table %s %s flags %x",
+	return snprintf(buf, size, "table %s %s flags %x\n",
 			t->name, nft_family2str(t->family), t->table_flags);
 }
 
-- 
1.8.3.2

[iptables (nft-compat) PATCH 3/8] xtables: Handle family is not related to Netlink family

[Tomasz Bursztyka]

AF_INET vs NFPROTO_IPV4
AF_INET6 vs NFPROTO_IPV6

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

diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index c4af2c5..80bd724 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -475,12 +475,12 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
 
 int xtables_ip4_restore_main(int argc, char *argv[])
 {
-	return xtables_restore_main(NFPROTO_IPV4, "iptables-restore",
+	return xtables_restore_main(AF_INET, "iptables-restore",
 				    argc, argv);
 }
 
 int xtables_ip6_restore_main(int argc, char *argv[])
 {
-	return xtables_restore_main(NFPROTO_IPV6, "ip6tables-restore",
+	return xtables_restore_main(AF_INET6, "ip6tables-restore",
 				    argc, argv);
 }
-- 
1.8.3.2

[iptables (nft-compat) PATCH 4/8] xtables: Add backward compatibility with -w option

[Tomasz Bursztyka]

Just to keep aligned with iptables legacy tool.

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

diff --git a/iptables/nft.h b/iptables/nft.h
index 26b60b9..22af66e 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -135,7 +135,7 @@ int nft_compatible_revision(const char *name, uint8_t rev, int opt);
 const char *nft_strerror(int err);
 
 /* For xtables.c */
-int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table);
+int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table, bool restore);
 /* For xtables-arptables.c */
 int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table);
 
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 80bd724..50d2935 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -442,7 +442,8 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
 			for (a = 0; a < newargc; a++)
 				DEBUGP("argv[%u]: %s\n", a, newargv[a]);
 
-			ret = do_commandx(&h, newargc, newargv, &newargv[2]);
+			ret = do_commandx(&h, newargc, newargv,
+					  &newargv[2], true);
 			if (ret < 0) {
 				ret = nft_abort(&h);
 				if (ret < 0) {
diff --git a/iptables/xtables-standalone.c b/iptables/xtables-standalone.c
index eb13980..355a446 100644
--- a/iptables/xtables-standalone.c
+++ b/iptables/xtables-standalone.c
@@ -70,7 +70,7 @@ xtables_main(int family, const char *progname, int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	ret = do_commandx(&h, argc, argv, &table);
+	ret = do_commandx(&h, argc, argv, &table, false);
 	if (ret)
 		ret = nft_commit(&h);
 
diff --git a/iptables/xtables.c b/iptables/xtables.c
index c49b4a2..7a8ace3 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -678,7 +678,8 @@ static void command_match(struct iptables_command_state *cs)
 		xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 }
 
-int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
+int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
+		bool restore)
 {
 	struct iptables_command_state cs;
 	int verbose = 0;
@@ -1001,6 +1002,14 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
 				       prog_name, prog_vers);
 			exit(0);
 
+		case 'w':
+			if (restore) {
+				xtables_error(PARAMETER_PROBLEM,
+					      "You cannot use `-w' from "
+					      "iptables-restore");
+			}
+			break;
+
 		case '0':
 			set_option(&cs.options, OPT_LINENUMBERS,
 				   &args.invflags, cs.invert);
-- 
1.8.3.2

[iptables (nft-compat) PATCH 5/8] nft: Fix error message to output proper origin

[Tomasz Bursztyka]

So it prints "mnl_talk:__nft_chain_set" instead of
"mnl_talk:__nft_chain_policy".

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

diff --git a/iptables/nft.c b/iptables/nft.c
index fc9db99..3df0e47 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -745,7 +745,7 @@ __nft_chain_set(struct nft_handle *h, const char *table,
 
 	ret = mnl_talk(h, nlh, NULL, NULL);
 	if (ret < 0)
-		perror("mnl_talk:__nft_chain_policy");
+		perror("mnl_talk:__nft_chain_set");
 
 	return ret;
 }
-- 
1.8.3.2

[iptables (nft-compat) PATCH 6/8] 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 3df0e47..a58f9bd 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 7/8] 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 create 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 a58f9bd..0283d92 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 8/8] nft: Initialize according to requested table, if any

[Tomasz Bursztyka]

This will prevent too much overhead when initializing the table when
loading the configuration on builtin tables.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c                | 41 +++++++++++++++++++++++++++++++++--------
 iptables/nft.h                |  4 +++-
 iptables/xtables-arp.c        |  2 +-
 iptables/xtables-config.c     |  2 +-
 iptables/xtables-restore.c    | 16 ++++++++--------
 iptables/xtables-save.c       | 16 ++++++++--------
 iptables/xtables-standalone.c |  2 +-
 iptables/xtables.c            |  1 +
 8 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 0283d92..4166046 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 (h->table && h->initialized)
+		return 0;
+
 	t = nft_table_alloc();
 	if (t == NULL)
 		return -1;
@@ -463,7 +466,11 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 	if (ret < 0) {
 		if (errno != EEXIST)
 			perror("mnl-talk:nft_table_init_one");
-	}
+		else
+			h->initialized = true;
+	} else
+		h->initialized = true;
+
 	return ret;
 }
 
@@ -576,7 +583,10 @@ nft_chain_builtin_init(struct nft_handle *h, const char *table,
 		       const char *chain, int policy)
 {
 	int ret = 0;
-	struct builtin_table *t;
+	struct builtin_table *t = NULL;
+
+	if (strcmp(table, h->table) == 0 && h->initialized)
+		goto builtin_chain;
 
 	t = nft_table_builtin_find(h, table);
 	if (t == NULL) {
@@ -588,6 +598,8 @@ nft_chain_builtin_init(struct nft_handle *h, const char *table,
 		if (errno == EEXIST)
 			goto out;
 	}
+	h->initialized = true;
+builtin_chain:
 	__nft_chain_builtin_init(h, t, chain, policy);
 out:
 	return ret;
@@ -601,7 +613,7 @@ static bool nft_chain_builtin(struct nft_chain *c)
 	return nft_chain_attr_get(c, NFT_CHAIN_ATTR_HOOKNUM) != NULL;
 }
 
-int nft_init(struct nft_handle *h, struct builtin_table *t)
+int nft_init(struct nft_handle *h, struct builtin_table *t, const char *table)
 {
 	h->nl = mnl_socket_open(NETLINK_NETFILTER);
 	if (h->nl == NULL) {
@@ -615,6 +627,8 @@ int nft_init(struct nft_handle *h, struct builtin_table *t)
 	}
 	h->portid = mnl_socket_get_portid(h->nl);
 	h->tables = t;
+	h->table = table;
+	h->initialized = false;
 
 	INIT_LIST_HEAD(&h->rule_list);
 
@@ -2439,8 +2453,12 @@ int nft_xtables_config_load(struct nft_handle *h, const char *filename,
 	struct nft_table *table;
 	struct nft_chain *chain;
 	uint32_t table_family, chain_family;
+	char *table_name;
 	bool found = false;
 
+	if (h->initialized)
+		return 0;
+
 	if (xtables_config_parse(filename, table_list, chain_list) < 0) {
 		if (errno == ENOENT) {
 			xtables_config_perror(flags,
@@ -2462,24 +2480,29 @@ int nft_xtables_config_load(struct nft_handle *h, const char *filename,
 		if (h->family != table_family)
 			continue;
 
+		table_name = (char *)nft_table_attr_get(table,
+							NFT_TABLE_ATTR_NAME);
+		if (h->table && strcmp(h->table, table_name))
+				continue;
+
 		found = true;
 
 		if (nft_table_add(h, table) < 0) {
 			if (errno == EEXIST) {
 				xtables_config_perror(flags,
 					"table `%s' already exists, skipping\n",
-					(char *)nft_table_attr_get(table, NFT_TABLE_ATTR_NAME));
+					table_name);
 			} else {
 				xtables_config_perror(flags,
-					"table `%s' cannot be create, reason `%s'. Exitting\n",
-					(char *)nft_table_attr_get(table, NFT_TABLE_ATTR_NAME),
-					strerror(errno));
+					"table `%s' cannot be create, "
+					"reason `%s'. Exitting\n",
+					table_name, strerror(errno));
 				goto err;
 			}
 			continue;
 		}
 		xtables_config_perror(flags, "table `%s' has been created\n",
-			(char *)nft_table_attr_get(table, NFT_TABLE_ATTR_NAME));
+				      table_name);
 	}
 	nft_table_list_iter_destroy(titer);
 	nft_table_list_free(table_list);
@@ -2519,6 +2542,8 @@ int nft_xtables_config_load(struct nft_handle *h, const char *filename,
 	nft_chain_list_iter_destroy(citer);
 	nft_chain_list_free(chain_list);
 
+	h->initialized = true;
+
 	return 0;
 
 err:
diff --git a/iptables/nft.h b/iptables/nft.h
index 8b64f8b..c38516c 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -34,7 +34,9 @@ struct nft_handle {
 	struct mnl_nlmsg_batch	*batch;
 	struct nft_family_ops	*ops;
 	struct builtin_table	*tables;
+	const char		*table;
 	bool			restore;
+	bool			initialized;
 };
 
 extern struct builtin_table xtables_ipv4[TABLES_MAX];
@@ -43,7 +45,7 @@ extern struct builtin_table xtables_arp[TABLES_MAX];
 int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
 	     int (*cb)(const struct nlmsghdr *nlh, void *data),
 	     void *data);
-int nft_init(struct nft_handle *h, struct builtin_table *t);
+int nft_init(struct nft_handle *h, struct builtin_table *t, const char *table);
 void nft_fini(struct nft_handle *h);
 
 /*
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 0c79a38..4acf012 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -1377,7 +1377,7 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table)
 				"chain name `%s' too long (must be under %i chars)",
 				chain, ARPT_FUNCTION_MAXNAMELEN);
 
-	if (nft_init(h, xtables_arp) < 0)
+	if (nft_init(h, xtables_arp, *table) < 0)
 		xtables_error(OTHER_PROBLEM,
 			      "Could not initialize nftables layer.");
 
diff --git a/iptables/xtables-config.c b/iptables/xtables-config.c
index b7cf609..807a9d7 100644
--- a/iptables/xtables-config.c
+++ b/iptables/xtables-config.c
@@ -35,7 +35,7 @@ int xtables_config_main(int argc, char *argv[])
 	else
 		filename = argv[1];
 
-	if (nft_init(&h, xtables_ipv4) < 0) {
+	if (nft_init(&h, xtables_ipv4, NULL) < 0) {
                 fprintf(stderr, "Failed to initialize nft: %s\n",
 			strerror(errno));
 		return EXIT_FAILURE;
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 0498abc..56bcf78 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -197,14 +197,6 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
 	init_extensions4();
 #endif
 
-	if (nft_init(&h, xtables_ipv4) < 0) {
-		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
-				xtables_globals.program_name,
-				xtables_globals.program_version,
-				strerror(errno));
-		exit(EXIT_FAILURE);
-	}
-
 	while ((c = getopt_long(argc, argv, "bcvthnM:T:46", options, NULL)) != -1) {
 		switch (c) {
 			case 'b':
@@ -256,6 +248,14 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
 	}
 	else in = stdin;
 
+	if (nft_init(&h, xtables_ipv4, tablename) < 0) {
+		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
+				xtables_globals.program_name,
+				xtables_globals.program_version,
+				strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+
 	chain_list = nft_chain_dump(&h);
 	if (chain_list == NULL)
 		xtables_error(OTHER_PROBLEM, "cannot retrieve chain list\n");
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 93065cf..65d279e 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -97,14 +97,6 @@ xtables_save_main(int family, const char *progname, int argc, char *argv[])
 	init_extensions();
 	init_extensions4();
 #endif
-	if (nft_init(&h, xtables_ipv4) < 0) {
-		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
-				xtables_globals.program_name,
-				xtables_globals.program_version,
-				strerror(errno));
-		exit(EXIT_FAILURE);
-	}
-
 	while ((c = getopt_long(argc, argv, "bcdt:46", options, NULL)) != -1) {
 		switch (c) {
 		case 'c':
@@ -136,6 +128,14 @@ xtables_save_main(int family, const char *progname, int argc, char *argv[])
 		exit(1);
 	}
 
+	if (nft_init(&h, xtables_ipv4, tablename) < 0) {
+		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
+				xtables_globals.program_name,
+				xtables_globals.program_version,
+				strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+
 	if (dump) {
 		do_output(&h, tablename, show_counters);
 		exit(0);
diff --git a/iptables/xtables-standalone.c b/iptables/xtables-standalone.c
index 355a446..f7d7d95 100644
--- a/iptables/xtables-standalone.c
+++ b/iptables/xtables-standalone.c
@@ -61,7 +61,7 @@ xtables_main(int family, const char *progname, int argc, char *argv[])
 	init_extensions4();
 #endif
 
-	if (nft_init(&h, xtables_ipv4) < 0) {
+	if (nft_init(&h, xtables_ipv4, table) < 0) {
 		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
 				xtables_globals.program_name,
 				xtables_globals.program_version,
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 7a8ace3..051f2fa 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -987,6 +987,7 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 				xtables_error(PARAMETER_PROBLEM,
 					   "unexpected ! flag before --table");
 			*table = optarg;
+			h->table = *table;
 			break;
 
 		case 'x':
-- 
1.8.3.2

Re: [libnftnl PATCH 1/8] chain: Break the line properly when printing out the chain

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 12:46:41PM +0200, Tomasz Bursztyka wrote:
> As when a rule is printed etc... It fixes a bug in iptables when
> printing out the chain in debug mode.

All the example in libnftnl rely on that missing \n, including nft.
Better fix this in iptables-compat?

Re: [iptables (nft-compat) PATCH 3/8] xtables: Handle family is not related to Netlink family

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 12:46:43PM +0200, Tomasz Bursztyka wrote:
> AF_INET vs NFPROTO_IPV4
> AF_INET6 vs NFPROTO_IPV6

I think we should stick to NFPROTO_*

I can see many references to AF_ in the compat layer, you can send me
a patch to rename all those instead.

Re: [iptables (nft-compat) PATCH 5/8] nft: Fix error message to output proper origin

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 12:46:45PM +0200, Tomasz Bursztyka wrote:
> So it prints "mnl_talk:__nft_chain_set" instead of
> "mnl_talk:__nft_chain_policy".

I think this is a good chance to silence / remove all these debugging
messages that were added in the very early initial stage.

Re: [iptables (nft-compat) PATCH 4/8] xtables: Add backward compatibility with -w option

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 12:46:44PM +0200, Tomasz Bursztyka wrote:
> Just to keep aligned with iptables legacy tool.

Applied, thanks Tomasz.

Re: [iptables (nft-compat) PATCH 3/8] xtables: Handle family is not related to Netlink family

[Tomasz Bursztyka]

>> AF_INET vs NFPROTO_IPV4
>> AF_INET6 vs NFPROTO_IPV6
> I think we should stick to NFPROTO_*
>
> I can see many references to AF_ in the compat layer, you can send me
> a patch to rename all those instead.

Actually we have always used AF_*
I will do the change

Tomasz

Re: [iptables (nft-compat) PATCH 3/8] xtables: Handle family is not related to Netlink family

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 01:13:17PM +0200, Tomasz Bursztyka wrote:
> >>AF_INET vs NFPROTO_IPV4
> >>AF_INET6 vs NFPROTO_IPV6
> >I think we should stick to NFPROTO_*
> >
> >I can see many references to AF_ in the compat layer, you can send me
> >a patch to rename all those instead.
> 
> Actually we have always used AF_*
> I will do the change

Beware that there is no AF_ARP, that's why we have NFPROTO_ARP and
also the new NFPROTO_INET for mixed ipv4/ipv6 tables in nft.

Re: [iptables (nft-compat) PATCH 3/8] xtables: Handle family is not related to Netlink family

[Tomasz Bursztyka]

>> >Actually we have always used AF_*
>> >I will do the change
> Beware that there is no AF_ARP, that's why we have NFPROTO_ARP and
> also the new NFPROTO_INET for mixed ipv4/ipv6 tables in nft.

Yup

Re: [libnftnl PATCH 1/8] chain: Break the line properly when printing out the chain

[Tomasz Bursztyka]

Le 11/02/2014 12:58, Pablo Neira Ayuso a écrit :
>> As when a rule is printed etc... It fixes a bug in iptables when
>> >printing out the chain in debug mode.
> All the example in libnftnl rely on that missing \n, including nft.
> Better fix this in iptables-compat?

I followed what have been done in src/rule.c

Actually a fix has to be done there as well:

removing the '\n' in "%s %s %s %"PRIu64" %"PRIu64"\n"

and changing the expression printing loop so:
it does "\n  [ %s " and "]" instead of "]\n".

And fixing iptables-compat so it add the relevant '\n' at the end.

Tomasz
--
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: [libnftnl PATCH 1/8] chain: Break the line properly when printing out the chain

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 01:29:32PM +0200, Tomasz Bursztyka wrote:
> Le 11/02/2014 12:58, Pablo Neira Ayuso a écrit :
> >>As when a rule is printed etc... It fixes a bug in iptables when
> >>>printing out the chain in debug mode.
> >All the example in libnftnl rely on that missing \n, including nft.
> >Better fix this in iptables-compat?
> 
> I followed what have been done in src/rule.c
> 
> Actually a fix has to be done there as well:
> 
> removing the '\n' in "%s %s %s %"PRIu64" %"PRIu64"\n"
> 
> and changing the expression printing loop so:
> it does "\n  [ %s " and "]" instead of "]\n".
> 
> And fixing iptables-compat so it add the relevant '\n' at the end.

I see, that's inconsistent. Let's add the \n to nft and any other
caller first and wait until next nft utility is released to update the
library not to break the output with different library versions (ie.
just to keep a very short term backward compatibility). I think we
should also nul-terminated the strings for all _snprintf functions.
--
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: [iptables (nft-compat) PATCH 8/8] nft: Initialize according to requested table, if any

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 12:46:48PM +0200, Tomasz Bursztyka wrote:
> This will prevent too much overhead when initializing the table when
> loading the configuration on builtin tables.
> 
> Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
> ---
>  iptables/nft.c                | 41 +++++++++++++++++++++++++++++++++--------
>  iptables/nft.h                |  4 +++-
>  iptables/xtables-arp.c        |  2 +-
>  iptables/xtables-config.c     |  2 +-
>  iptables/xtables-restore.c    | 16 ++++++++--------
>  iptables/xtables-save.c       | 16 ++++++++--------
>  iptables/xtables-standalone.c |  2 +-
>  iptables/xtables.c            |  1 +
>  8 files changed, 56 insertions(+), 28 deletions(-)
> 
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 0283d92..4166046 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 (h->table && h->initialized)
> +		return 0;

Does it work the autochain load if you reload with rules in different
tables? eg. filter and raw.

This h->initialized field is global, I think we need one per table.

Re: [iptables (nft-compat) PATCH 8/8] nft: Initialize according to requested table, if any

[Tomasz Bursztyka]

Hi Pablo,

> Does it work the autochain load if you reload with rules in different
> tables? eg. filter and raw.

Indeed, no... While chasing a bug (patch 7 is the result) I forgot to do 
this simple test.

I could overload struct builtin_table {} for that.
But I just figured out another issue here, it will work if only we don't 
have /etc/xtables.conf file.

If you have such file, when xtables-restore will insert rules, it will 
always call nft_xtables_config_load() thus always trying to initialize 
tables that might be already there.

So there is a bigger fix here: I will add necessary stuff in handle to 
keep track of parsed file like tables and store which one has been 
initialized and not.

Will come back with a better patch then.

Tomasz

Re: [iptables (nft-compat) PATCH 8/8] nft: Initialize according to requested table, if any

[Pablo Neira Ayuso]

On Tue, Feb 11, 2014 at 03:37:21PM +0200, Tomasz Bursztyka wrote:
> Hi Pablo,
> 
> >Does it work the autochain load if you reload with rules in different
> >tables? eg. filter and raw.
> 
> Indeed, no... While chasing a bug (patch 7 is the result) I forgot
> to do this simple test.
> 
> I could overload struct builtin_table {} for that.
> But I just figured out another issue here, it will work if only we
> don't have /etc/xtables.conf file.

This is the way it should work with the file. That file provides a way
to enforce a chain configuration that will bypass the default chain
configuration.

> If you have such file, when xtables-restore will insert rules, it
> will always call nft_xtables_config_load() thus always trying to
> initialize tables that might be already there.

For xtables-restore, I think we should just initialize the chains once
when handling the ':' marker that we get from the standard input that
refers to the chain. With patch 7/8, we know if we're in the context
of a restore or not, thus you can use that to skip any sort of
chain initialization from the functions defined in nft.c.

> So there is a bigger fix here: I will add necessary stuff in handle
> to keep track of parsed file like tables and store which one has
> been initialized and not.

I think with the approach I'm describing above the patch should be
relatively small.