[iptables-nftables PATCH 1/5] nft: let nft_handle struct own the builtin table pointer

[iptables-nftables PATCH 1/5] nft: let nft_handle struct own the builtin table pointer

[Giuseppe Longo]

The following patch adds own builtin_table pointer used by future tool (like arptables)
to have own tables and not tables declared in nft.c

Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
---
 iptables/nft.c                |   22 +++-------------------
 iptables/nft.h                |   24 +++++++++++++++++++++++-
 iptables/xtables-config.c     |    2 +-
 iptables/xtables-restore.c    |    2 +-
 iptables/xtables-save.c       |    2 +-
 iptables/xtables-standalone.c |    2 +-
 6 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 9a857b9..5665148 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -80,24 +80,7 @@ static int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
 	return 0;
 }
 
-#define FILTER		0
-#define MANGLE		1
-#define RAW		2
-#define SECURITY	3
-#define NAT		4
-#define TABLES_MAX	5
-
-struct builtin_chain {
-	const char *name;
-	const char *type;
-	uint32_t prio;
-	uint32_t hook;
-};
-
-static struct builtin_table {
-	const char *name;
-	struct builtin_chain chains[NF_INET_NUMHOOKS];
-} tables[TABLES_MAX] = {
+struct builtin_table xtables_ipv4[TABLES_MAX] = {
 	[RAW] = {
 		.name	= "raw",
 		.chains = {
@@ -389,7 +372,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)
+int nft_init(struct nft_handle *h, struct builtin_table *t)
 {
 	h->nl = mnl_socket_open(NETLINK_NETFILTER);
 	if (h->nl == NULL) {
@@ -402,6 +385,7 @@ int nft_init(struct nft_handle *h)
 		return -1;
 	}
 	h->portid = mnl_socket_get_portid(h->nl);
+	h->tables = t;
 
 	return 0;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index 7a6351b..f3317c9 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -4,6 +4,25 @@
 #include "xshared.h"
 #include "nft-shared.h"
 
+#define FILTER         0
+#define MANGLE         1
+#define RAW            2
+#define SECURITY       3
+#define NAT            4
+#define TABLES_MAX     5
+
+struct builtin_chain {
+	const char *name;
+	const char *type;
+	uint32_t prio;
+	uint32_t hook;
+};
+
+struct builtin_table {
+	const char *name;
+	struct builtin_chain chains[NF_INET_NUMHOOKS];
+};
+
 struct nft_handle {
 	int			family;
 	struct mnl_socket	*nl;
@@ -11,9 +30,12 @@ struct nft_handle {
 	uint32_t		seq;
 	bool			commit;
 	struct nft_family_ops	*ops;
+	struct builtin_table	*tables;
 };
 
-int nft_init(struct nft_handle *h);
+extern struct builtin_table xtables_ipv4[TABLES_MAX];
+
+int nft_init(struct nft_handle *h, struct builtin_table *t);
 void nft_fini(struct nft_handle *h);
 
 /*
diff --git a/iptables/xtables-config.c b/iptables/xtables-config.c
index 515b18b..b7cf609 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) < 0) {
+	if (nft_init(&h, xtables_ipv4) < 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 8469ba1..608e189 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -193,7 +193,7 @@ xtables_restore_main(int argc, char *argv[])
 	init_extensions4();
 #endif
 
-	if (nft_init(&h) < 0) {
+	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,
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 41ceaf5..db03090 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -96,7 +96,7 @@ xtables_save_main(int argc, char *argv[])
 	init_extensions();
 	init_extensions4();
 #endif
-	if (nft_init(&h) < 0) {
+	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,
diff --git a/iptables/xtables-standalone.c b/iptables/xtables-standalone.c
index 3f8b981..9d5a667 100644
--- a/iptables/xtables-standalone.c
+++ b/iptables/xtables-standalone.c
@@ -61,7 +61,7 @@ xtables_main(int argc, char *argv[])
 	init_extensions4();
 #endif
 
-	if (nft_init(&h) < 0) {
+	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,
-- 
1.7.8.6

[iptables-nftables PATCH 2/5] nft: search builtin tables via nft_handle tables pointer

[Giuseppe Longo]

This finds built-in tables via tables pointer in nft_handle

Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
---
 iptables/nft.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 5665148..0b45c93 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -288,20 +288,21 @@ nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
 }
 
 /* find if built-in table already exists */
-static struct builtin_table *nft_table_builtin_find(const char *table)
+static struct builtin_table
+*nft_table_builtin_find(struct nft_handle *h, const char *table)
 {
 	int i;
 	bool found = false;
 
 	for (i=0; i<TABLES_MAX; i++) {
-		if (strcmp(tables[i].name, table) != 0)
+		if (strcmp(h->tables[i].name, table) != 0)
 			continue;
 
 		found = true;
 		break;
 	}
 
-	return found ? &tables[i] : NULL;
+	return found ? &h->tables[i] : NULL;
 }
 
 /* find if built-in chain already exists */
@@ -349,7 +350,7 @@ nft_chain_builtin_init(struct nft_handle *h, const char *table,
 	int ret = 0;
 	struct builtin_table *t;
 
-	t = nft_table_builtin_find(table);
+	t = nft_table_builtin_find(h, table);
 	if (t == NULL) {
 		ret = -1;
 		goto out;
@@ -424,7 +425,7 @@ int nft_table_set_dormant(struct nft_handle *h, const char *table)
 	int ret = 0, i;
 	struct builtin_table *t;
 
-	t = nft_table_builtin_find(table);
+	t = nft_table_builtin_find(h, table);
 	if (t == NULL) {
 		ret = -1;
 		goto out;
@@ -485,7 +486,7 @@ __nft_chain_set(struct nft_handle *h, const char *table,
 	struct builtin_chain *_c;
 	int ret;
 
-	_t = nft_table_builtin_find(table);
+	_t = nft_table_builtin_find(h, table);
 	/* if this built-in table does not exists, create it */
 	if (_t != NULL)
 		nft_table_builtin_add(h, _t, false);
-- 
1.7.8.6

[iptables-nftables PATCH 3/5] nft: nft_xtables_config_load() called only in nft_init()

[Giuseppe Longo]

This makes nft_xtables_config_load called at only one unique place
instead of multiple ones but for xtables-config since it tries to load
a different file than default one.

Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
---
 iptables/nft.c                |   29 +++++++++--------------------
 iptables/xtables-config.c     |   15 +++++++++++----
 iptables/xtables-restore.c    |   16 ++++++++--------
 iptables/xtables-standalone.c |   14 +++-----------
 iptables/xtables.c            |    4 ++++
 5 files changed, 35 insertions(+), 43 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 0b45c93..bb1a1da 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -388,6 +388,15 @@ int nft_init(struct nft_handle *h, struct builtin_table *t)
 	h->portid = mnl_socket_get_portid(h->nl);
 	h->tables = t;
 
+	/* If built-in chains don't exist for this table, create them */
+	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0) {
+		int i;
+
+		for (i = 0; i < TABLES_MAX; i++)
+			nft_chain_builtin_init(h, h->tables[i].name,
+					       NULL, NF_ACCEPT);
+	}
+
 	return 0;
 }
 
@@ -742,10 +751,6 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 	uint16_t flags = NLM_F_ACK|NLM_F_CREATE;
 	int ret = 1;
 
-	/* If built-in chains don't exist for this table, create them */
-	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
-		nft_chain_builtin_init(h, table, chain, NF_ACCEPT);
-
 	nft_fn = nft_rule_append;
 
 	r = nft_rule_new(h, chain, table, cs);
@@ -1322,10 +1327,6 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 	struct nft_chain *c;
 	int ret;
 
-	/* If built-in chains don't exist for this table, create them */
-	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
-		nft_chain_builtin_init(h, table, NULL, NF_ACCEPT);
-
 	c = nft_chain_alloc();
 	if (c == NULL)
 		return 0;
@@ -1478,10 +1479,6 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 	uint64_t handle;
 	int ret;
 
-	/* If built-in chains don't exist for this table, create them */
-	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
-		nft_chain_builtin_init(h, table, NULL, NF_ACCEPT);
-
 	/* Find the old chain to be renamed */
 	c = nft_chain_find(h, table, chain);
 	if (c == NULL) {
@@ -2177,10 +2174,6 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 	struct nft_rule *r;
 	uint64_t handle = 0;
 
-	/* If built-in chains don't exist for this table, create them */
-	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
-		nft_chain_builtin_init(h, table, chain, NF_ACCEPT);
-
 	nft_fn = nft_rule_insert;
 
 	if (rulenum > 0) {
@@ -2525,10 +2518,6 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 	struct nft_chain *c;
 	bool found = false;
 
-	/* If built-in chains don't exist for this table, create them */
-	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
-		nft_chain_builtin_init(h, table, NULL, NF_ACCEPT);
-
 	list = nft_chain_dump(h);
 
 	iter = nft_chain_list_iter_create(list);
diff --git a/iptables/xtables-config.c b/iptables/xtables-config.c
index b7cf609..d61b762 100644
--- a/iptables/xtables-config.c
+++ b/iptables/xtables-config.c
@@ -15,6 +15,7 @@
 #include <stdbool.h>
 #include <string.h>
 #include <errno.h>
+#include <libmnl/libmnl.h>
 
 #include "xtables-multi.h"
 #include "nft.h"
@@ -35,11 +36,17 @@ int xtables_config_main(int argc, char *argv[])
 	else
 		filename = argv[1];
 
-	if (nft_init(&h, xtables_ipv4) < 0) {
-                fprintf(stderr, "Failed to initialize nft: %s\n",
-			strerror(errno));
-		return EXIT_FAILURE;
+	h.nl = mnl_socket_open(NETLINK_NETFILTER);
+	if (h.nl == NULL) {
+		perror("mnl_socket_open");
+		return -1;
 	}
+	if (mnl_socket_bind(h.nl, 0, MNL_SOCKET_AUTOPID) < 0) {
+		perror("mnl_socket_bind");
+		return -1;
+	}
+	h.portid = mnl_socket_get_portid(h.nl);
+	h.tables = xtables_ipv4;
 
 	return nft_xtables_config_load(&h, filename, NFT_LOAD_VERBOSE) == 0 ?
 						    EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 608e189..dda314d 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -193,14 +193,6 @@ xtables_restore_main(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':
@@ -252,6 +244,14 @@ xtables_restore_main(int argc, char *argv[])
 	}
 	else in = stdin;
 
+	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);
+	}
+
 	chain_list = nft_chain_dump(&h);
 	if (chain_list == NULL)
 		xtables_error(OTHER_PROBLEM, "cannot retrieve chain list\n");
diff --git a/iptables/xtables-standalone.c b/iptables/xtables-standalone.c
index 9d5a667..ccce5ba 100644
--- a/iptables/xtables-standalone.c
+++ b/iptables/xtables-standalone.c
@@ -44,9 +44,9 @@ xtables_main(int argc, char *argv[])
 {
 	int ret;
 	char *table = "filter";
-	struct nft_handle h;
-
-	memset(&h, 0, sizeof(h));
+	struct nft_handle h = {
+		.family = AF_INET
+	};
 
 	xtables_globals.program_name = "xtables";
 	ret = xtables_init_all(&xtables_globals, NFPROTO_IPV4);
@@ -61,14 +61,6 @@ xtables_main(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);
-	}
-
 	ret = do_commandx(&h, argc, argv, &table);
 	if (!ret) {
 		if (errno == EINVAL) {
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 7a6509a..59f38a9 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1114,6 +1114,10 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
 	if (h->ops == NULL)
 		xtables_error(PARAMETER_PROBLEM, "Unknown family");
 
+	if (nft_init(h, xtables_ipv4) < 0)
+		xtables_error(OTHER_PROBLEM,
+			      "Could not initialize nftables layer.");
+
 	h->ops->post_parse(command, &cs, &args);
 
 	if (command == CMD_REPLACE &&
-- 
1.7.8.6

[iptables-nftables PATCH 4/5] nft: export functions reusability

[Giuseppe Longo]

This permit to reuse some functions in other tool (like arptables)

Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
---
 iptables/nft.c |   20 ++++++++++----------
 iptables/nft.h |   25 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index bb1a1da..f124419 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -53,9 +53,9 @@
 
 static void *nft_fn;
 
-static int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
-		    int (*cb)(const struct nlmsghdr *nlh, void *data),
-		    void *data)
+int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
+	     int (*cb)(const struct nlmsghdr *nlh, void *data),
+	     void *data)
 {
 	int ret;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
@@ -210,7 +210,7 @@ struct builtin_table xtables_ipv4[TABLES_MAX] = {
 	},
 };
 
-static int
+int
 nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 			bool dormant)
 {
@@ -242,7 +242,7 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 	return ret;
 }
 
-static struct nft_chain *
+struct nft_chain *
 nft_chain_builtin_alloc(struct builtin_table *table,
 			struct builtin_chain *chain, int policy)
 {
@@ -262,7 +262,7 @@ nft_chain_builtin_alloc(struct builtin_table *table,
 	return c;
 }
 
-static void
+void
 nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
 		      struct builtin_chain *chain, int policy)
 {
@@ -288,8 +288,8 @@ nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
 }
 
 /* find if built-in table already exists */
-static struct builtin_table 
-*nft_table_builtin_find(struct nft_handle *h, const char *table)
+struct builtin_table *
+nft_table_builtin_find(struct nft_handle *h, const char *table)
 {
 	int i;
 	bool found = false;
@@ -306,7 +306,7 @@ static struct builtin_table
 }
 
 /* find if built-in chain already exists */
-static struct builtin_chain *
+struct builtin_chain *
 nft_chain_builtin_find(struct builtin_table *t, const char *chain)
 {
 	int i;
@@ -343,7 +343,7 @@ __nft_chain_builtin_init(struct nft_handle *h,
 	}
 }
 
-static int
+int
 nft_chain_builtin_init(struct nft_handle *h, const char *table,
 		       const char *chain, int policy)
 {
diff --git a/iptables/nft.h b/iptables/nft.h
index f3317c9..734b852 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -35,6 +35,31 @@ struct nft_handle {
 
 extern struct builtin_table xtables_ipv4[TABLES_MAX];
 
+int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
+	     int (*cb)(const struct nlmsghdr *nlh, void *data),
+	     void *data);
+
+int nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
+			  bool dormant);
+
+struct nft_chain *
+nft_chain_builtin_alloc(struct builtin_table *table,
+			struct builtin_chain *chain, int policy);
+
+void
+nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
+		      struct builtin_chain *chain, int policy);
+
+struct builtin_table *
+nft_table_builtin_find(struct nft_handle *h, const char *table);
+
+struct builtin_chain *
+nft_chain_builtin_find(struct builtin_table *t, const char *chain);
+
+int
+nft_chain_builtin_init(struct nft_handle *h, const char *table,
+		       const char *chain, int policy);
+
 int nft_init(struct nft_handle *h, struct builtin_table *t);
 void nft_fini(struct nft_handle *h);
 
-- 
1.7.8.6

[iptables-nftables PATCH 5/5] nft: fix family operation lookup

[Giuseppe Longo]

This fixes the family operations lookup: handle's family is the one to use at this point

Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
---
 iptables/xtables.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/iptables/xtables.c b/iptables/xtables.c
index 59f38a9..eb614c6 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1110,7 +1110,7 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
 	if (h->family == AF_UNSPEC)
 		h->family = args.family;
 
-	h->ops = nft_family_ops_lookup(args.family);
+	h->ops = nft_family_ops_lookup(h->family);
 	if (h->ops == NULL)
 		xtables_error(PARAMETER_PROBLEM, "Unknown family");
 
-- 
1.7.8.6

Re: [iptables-nftables PATCH 1/5] nft: let nft_handle struct own the builtin table pointer

[Pablo Neira Ayuso]

On Fri, Jul 26, 2013 at 01:05:15PM +0200, Giuseppe Longo wrote:
> The following patch adds own builtin_table pointer used by future tool (like arptables)
> to have own tables and not tables declared in nft.c

If I apply this patch and if I type 'make', it says:

nft.c: In function ‘nft_table_builtin_find’:
nft.c:297:7: error: ‘tables’ undeclared (first use in this function)
nft.c:297:7: note: each undeclared identifier is reported only once for each function it appears in
nft.c:297:7: warning: left-hand operand of comma expression has no effect [-Wunused-value]
nft.c:297:7: warning: value computed is not used [-Wunused-value]
nft.c:297:7: warning: left-hand operand of comma expression has no effect [-Wunused-value]
mv -f .deps/xtables_multi-iptables.Tpo .deps/xtables_multi-iptables.Po
mv -f .deps/xtables_multi-xtables-config-parser.Tpo
.deps/xtables_multi-xtables-config-parser.Po
nft.c:305:1: warning: control reaches end of non-void function [-Wreturn-type]
make[2]: *** [xtables_multi-nft.o] Error 1

Patches have to leave the repository in a consistency state, ie.
compilation/operational should not break between patches, at least not
voluntarily. This usually means that you're incorrectly splitting the
patches. I have merged 1/5 and 2/5 into one single patch and push it
into master. Thanks Giuseppe.
--
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-nftables PATCH 3/5] nft: nft_xtables_config_load() called only in nft_init()

[Pablo Neira Ayuso]

Hi Giuseppe,

On Fri, Jul 26, 2013 at 01:05:17PM +0200, Giuseppe Longo wrote:
[...]
> diff --git a/iptables/xtables-config.c b/iptables/xtables-config.c
> index b7cf609..d61b762 100644
> --- a/iptables/xtables-config.c
> +++ b/iptables/xtables-config.c
> @@ -15,6 +15,7 @@
>  #include <stdbool.h>
>  #include <string.h>
>  #include <errno.h>
> +#include <libmnl/libmnl.h>
>  
>  #include "xtables-multi.h"
>  #include "nft.h"
> @@ -35,11 +36,17 @@ int xtables_config_main(int argc, char *argv[])
>  	else
>  		filename = argv[1];
>  
> -	if (nft_init(&h, xtables_ipv4) < 0) {
> -                fprintf(stderr, "Failed to initialize nft: %s\n",
> -			strerror(errno));
> -		return EXIT_FAILURE;
> +	h.nl = mnl_socket_open(NETLINK_NETFILTER);
> +	if (h.nl == NULL) {
> +		perror("mnl_socket_open");
> +		return -1;
>  	}
> +	if (mnl_socket_bind(h.nl, 0, MNL_SOCKET_AUTOPID) < 0) {
> +		perror("mnl_socket_bind");
> +		return -1;
> +	}
> +	h.portid = mnl_socket_get_portid(h.nl);
> +	h.tables = xtables_ipv4;

Hm, why do we need this here?

Re: [iptables-nftables PATCH 3/5] nft: nft_xtables_config_load() called only in nft_init()

[Giuseppe Longo]

Hi Pablo,

2013/7/26 Pablo Neira Ayuso <pablo@netfilter.org>:
> Hi Giuseppe,
>
> On Fri, Jul 26, 2013 at 01:05:17PM +0200, Giuseppe Longo wrote:
> [...]
>> diff --git a/iptables/xtables-config.c b/iptables/xtables-config.c
>> index b7cf609..d61b762 100644
>> --- a/iptables/xtables-config.c
>> +++ b/iptables/xtables-config.c
>> @@ -15,6 +15,7 @@
>>  #include <stdbool.h>
>>  #include <string.h>
>>  #include <errno.h>
>> +#include <libmnl/libmnl.h>
>>
>>  #include "xtables-multi.h"
>>  #include "nft.h"
>> @@ -35,11 +36,17 @@ int xtables_config_main(int argc, char *argv[])
>>       else
>>               filename = argv[1];
>>
>> -     if (nft_init(&h, xtables_ipv4) < 0) {
>> -                fprintf(stderr, "Failed to initialize nft: %s\n",
>> -                     strerror(errno));
>> -             return EXIT_FAILURE;
>> +     h.nl = mnl_socket_open(NETLINK_NETFILTER);
>> +     if (h.nl == NULL) {
>> +             perror("mnl_socket_open");
>> +             return -1;
>>       }
>> +     if (mnl_socket_bind(h.nl, 0, MNL_SOCKET_AUTOPID) < 0) {
>> +             perror("mnl_socket_bind");
>> +             return -1;
>> +     }
>> +     h.portid = mnl_socket_get_portid(h.nl);
>> +     h.tables = xtables_ipv4;
>
> Hm, why do we need this here?

The idea is to initialize nft_handle h without nft_init and after load
the file, otherwise using nft_init the file is load 2 times. (First in
nft_init and after with nft_xtables_config_load).

This should make code more cleaner.

Regards

Re: [iptables-nftables PATCH 3/5] nft: nft_xtables_config_load() called only in nft_init()

[Tomasz Bursztyka]

Hi Giuseppe,

> The idea is to initialize nft_handle h without nft_init and after load
> the file, otherwise using nft_init the file is load 2 times. (First in
> nft_init and after with nft_xtables_config_load).

Indeed, and if given filename is different than default one then it's 
worse yes, you load the default one and the given one.

Tomasz

[iptables-nftables PATCH 4/5] nft: export functions reusability

[Tomasz Bursztyka]

From: Giuseppe Longo <giuseppelng@gmail.com>

This permit to reuse some functions in other tool (like arptables)

Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
---

Hi Giuseppe,

For some reason patch 4 does not apply properly. And the conflict is a non-sense, I mean:
I don't see any conflict. With git am --reject <your patch>,  the iptables/nft.c.rej looks nothing special.
So I regenerated your patch. Please Pablo use this one.

Tomasz

 iptables/nft.c | 20 ++++++++++----------
 iptables/nft.h | 25 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 1b91691..16380d1 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -53,9 +53,9 @@
 
 static void *nft_fn;
 
-static int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
-		    int (*cb)(const struct nlmsghdr *nlh, void *data),
-		    void *data)
+int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
+	     int (*cb)(const struct nlmsghdr *nlh, void *data),
+	     void *data)
 {
 	int ret;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
@@ -210,7 +210,7 @@ struct builtin_table xtables_ipv4[TABLES_MAX] = {
 	},
 };
 
-static int
+int
 nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 			bool dormant)
 {
@@ -242,7 +242,7 @@ nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
 	return ret;
 }
 
-static struct nft_chain *
+struct nft_chain *
 nft_chain_builtin_alloc(struct builtin_table *table,
 			struct builtin_chain *chain, int policy)
 {
@@ -262,7 +262,7 @@ nft_chain_builtin_alloc(struct builtin_table *table,
 	return c;
 }
 
-static void
+void
 nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
 		      struct builtin_chain *chain, int policy)
 {
@@ -288,8 +288,8 @@ nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
 }
 
 /* find if built-in table already exists */
-static struct builtin_table
-*nft_table_builtin_find(struct nft_handle *h, const char *table)
+struct builtin_table *
+nft_table_builtin_find(struct nft_handle *h, const char *table)
 {
 	int i;
 	bool found = false;
@@ -306,7 +306,7 @@ static struct builtin_table
 }
 
 /* find if built-in chain already exists */
-static struct builtin_chain *
+struct builtin_chain *
 nft_chain_builtin_find(struct builtin_table *t, const char *chain)
 {
 	int i;
@@ -343,7 +343,7 @@ __nft_chain_builtin_init(struct nft_handle *h,
 	}
 }
 
-static int
+int
 nft_chain_builtin_init(struct nft_handle *h, const char *table,
 		       const char *chain, int policy)
 {
diff --git a/iptables/nft.h b/iptables/nft.h
index f3317c9..734b852 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -35,6 +35,31 @@ struct nft_handle {
 
 extern struct builtin_table xtables_ipv4[TABLES_MAX];
 
+int mnl_talk(struct nft_handle *h, struct nlmsghdr *nlh,
+	     int (*cb)(const struct nlmsghdr *nlh, void *data),
+	     void *data);
+
+int nft_table_builtin_add(struct nft_handle *h, struct builtin_table *_t,
+			  bool dormant);
+
+struct nft_chain *
+nft_chain_builtin_alloc(struct builtin_table *table,
+			struct builtin_chain *chain, int policy);
+
+void
+nft_chain_builtin_add(struct nft_handle *h, struct builtin_table *table,
+		      struct builtin_chain *chain, int policy);
+
+struct builtin_table *
+nft_table_builtin_find(struct nft_handle *h, const char *table);
+
+struct builtin_chain *
+nft_chain_builtin_find(struct builtin_table *t, const char *chain);
+
+int
+nft_chain_builtin_init(struct nft_handle *h, const char *table,
+		       const char *chain, int policy);
+
 int nft_init(struct nft_handle *h, struct builtin_table *t);
 void nft_fini(struct nft_handle *h);
 
-- 
1.8.3.2

Re: [iptables-nftables PATCH 3/5] nft: nft_xtables_config_load() called only in nft_init()

[Tomasz Bursztyka]

Hi Giuseppe,

> +	/* If built-in chains don't exist for this table, create them */
> +	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0) {
> +		int i;
> +
> +		for (i = 0; i < TABLES_MAX; i++)
> +			nft_chain_builtin_init(h, h->tables[i].name,
> +					       NULL, NF_ACCEPT);

There is a bug here. While testing your arpatbles bootstrap: arp own 
only filter talbe, so here you should check if h->tables[i].name is not 
NULL.
Keep in mind that the loop should continue, one might not have a MANGLE 
table but still own a RAW table for instance.

Fix this, and resend the patches 3-4-5  (take the patch 4 I sent yesterday)

Tomasz

Re: [iptables-nftables PATCH 4/5] nft: export functions reusability

[Pablo Neira Ayuso]

On Fri, Jul 26, 2013 at 01:05:18PM +0200, Giuseppe Longo wrote:
> This permit to reuse some functions in other tool (like arptables)

Please, add this patch to the arptables bootstrap patch, as we start
needing it there.

Thanks.

Re: [iptables-nftables PATCH 5/5] nft: fix family operation lookup

[Pablo Neira Ayuso]

On Fri, Jul 26, 2013 at 01:05:19PM +0200, Giuseppe Longo wrote:
> This fixes the family operations lookup: handle's family is the one
> to use at this point

I have applied this with a couple more fixes for the IPv6 side. I have
also reworked the description.

Thanks Giuseppe.