[iptables PATCH 1/4] nft: Set NFTNL_CHAIN_FAMILY in new chains

[iptables PATCH 1/4] nft: Set NFTNL_CHAIN_FAMILY in new chains

[Phil Sutter]

Kernel doesn't need it, but debug output improves significantly. Before
this patch:

| # iptables-nft -vv -A INPUT
| [...]
| unknown filter INPUT use 0 type filter hook unknown prio 0 policy accept packets 0 bytes 0
| [...]

and after:

| # iptables-nft -vv -A INPUT
| [...]
| ip filter INPUT use 0 type filter hook input prio 0 policy accept packets 0 bytes 0
| [...]

While being at it, make nft_chain_builtin_alloc() take only the builtin
table's name as parameter - it's the only field it accesses.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 7cc6ca5258150..301d6c342f982 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -665,7 +665,7 @@ static int nft_table_builtin_add(struct nft_handle *h,
 }
 
 static struct nftnl_chain *
-nft_chain_builtin_alloc(const struct builtin_table *table,
+nft_chain_builtin_alloc(int family, const char *tname,
 			const struct builtin_chain *chain, int policy)
 {
 	struct nftnl_chain *c;
@@ -674,7 +674,8 @@ nft_chain_builtin_alloc(const struct builtin_table *table,
 	if (c == NULL)
 		return NULL;
 
-	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table->name);
+	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, family);
+	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, tname);
 	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain->name);
 	nftnl_chain_set_u32(c, NFTNL_CHAIN_HOOKNUM, chain->hook);
 	nftnl_chain_set_u32(c, NFTNL_CHAIN_PRIO, chain->prio);
@@ -693,7 +694,7 @@ static void nft_chain_builtin_add(struct nft_handle *h,
 {
 	struct nftnl_chain *c;
 
-	c = nft_chain_builtin_alloc(table, chain, NF_ACCEPT);
+	c = nft_chain_builtin_alloc(h->family, table->name, chain, NF_ACCEPT);
 	if (c == NULL)
 		return;
 
@@ -959,7 +960,7 @@ static struct nftnl_chain *nft_chain_new(struct nft_handle *h,
 	_c = nft_chain_builtin_find(_t, chain);
 	if (_c != NULL) {
 		/* This is a built-in chain */
-		c = nft_chain_builtin_alloc(_t, _c, policy);
+		c = nft_chain_builtin_alloc(h->family, _t->name, _c, policy);
 		if (c == NULL)
 			return NULL;
 	} else {
@@ -1999,6 +2000,7 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 	if (c == NULL)
 		return 0;
 
+	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, h->family);
 	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
 	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
 	if (h->family == NFPROTO_BRIDGE)
@@ -2029,6 +2031,7 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 		if (!c)
 			return 0;
 
+		nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, h->family);
 		nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
 		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
 		created = true;
@@ -2190,6 +2193,7 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 	if (c == NULL)
 		return 0;
 
+	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, h->family);
 	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
 	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, newname);
 	nftnl_chain_set_u64(c, NFTNL_CHAIN_HANDLE, handle);
-- 
2.34.1

[iptables PATCH 2/4] ebtables: Support verbose mode

[Phil Sutter]

Accept '-v' flag in both ebtables-nft and ebtables-nft-restore. Mostly
interesting because it allows for netlink debug output when specified
multiple times.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/ebtables-nft.8    |  6 ++++++
 iptables/xtables-eb.c      | 25 ++++++++++++++++++-------
 iptables/xtables-restore.c |  8 ++++++--
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/iptables/ebtables-nft.8 b/iptables/ebtables-nft.8
index 08e9766f2cc74..d75aae240bc05 100644
--- a/iptables/ebtables-nft.8
+++ b/iptables/ebtables-nft.8
@@ -307,6 +307,12 @@ of the ebtables kernel table.
 Replace the current table data by the initial table data.
 .SS MISCELLANOUS COMMANDS
 .TP
+.B "-v, --verbose"
+Verbose mode.
+For appending, insertion, deletion and replacement, this causes
+detailed information on the rule or rules to be printed. \fB\-v\fP may be
+specified multiple times to possibly emit more detailed debug statements.
+.TP
 .B "-V, --version"
 Show the version of the ebtables userspace program.
 .TP
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 060e06c57a481..1e5b50ba5b0ab 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -195,6 +195,7 @@ struct option ebt_original_options[] =
 	{ "out-interface"  , required_argument, 0, 'o' },
 	{ "out-if"         , required_argument, 0, 'o' },
 	{ "version"        , no_argument      , 0, 'V' },
+	{ "verbose"        , no_argument      , 0, 'v' },
 	{ "help"           , no_argument      , 0, 'h' },
 	{ "jump"           , required_argument, 0, 'j' },
 	{ "set-counters"   , required_argument, 0, 'c' },
@@ -219,7 +220,7 @@ struct option ebt_original_options[] =
 struct xtables_globals ebtables_globals = {
 	.option_offset 		= 0,
 	.program_version	= PACKAGE_VERSION " (nf_tables)",
-	.optstring		= OPTSTRING_COMMON "h",
+	.optstring		= OPTSTRING_COMMON "hv",
 	.orig_opts		= ebt_original_options,
 	.compat_rev		= nft_compatible_revision,
 };
@@ -325,6 +326,7 @@ static void print_help(const struct xtables_target *t,
 "          pcnt bcnt           : set the counters of the to be added rule\n"
 "--modprobe -M program         : try to insert modules using this program\n"
 "--concurrent                  : use a file lock to support concurrent scripts\n"
+"--verbose -v                  : verbose mode\n"
 "--version -V                  : print package version\n\n"
 "Environment variable:\n"
 /*ATOMIC_ENV_VARIABLE "          : if set <FILE> (see above) will equal its value"*/
@@ -726,6 +728,9 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 	struct ebt_match *match;
 	bool table_set = false;
 
+	/* avoid cumulating verbosity with ebtables-restore */
+	h->verbose = 0;
+
 	/* prevent getopt to spoil our error reporting */
 	optind = 0;
 	opterr = false;
@@ -854,6 +859,10 @@ print_zero:
 				optind++;
 			}
 			break;
+		case 'v': /* verbose */
+			flags |= OPT_VERBOSE;
+			h->verbose++;
+			break;
 		case 'V': /* Version */
 			if (OPT_COMMANDS)
 				xtables_error(PARAMETER_PROBLEM,
@@ -1146,24 +1155,26 @@ print_zero:
 		}
 	} else if (command == 'L') {
 		ret = list_rules(h, chain, *table, rule_nr,
-				 0,
+				 flags & OPT_VERBOSE,
 				 0,
 				 /*flags&OPT_EXPANDED*/0,
 				 flags&LIST_N,
 				 flags&LIST_C);
 	}
 	if (flags & OPT_ZERO) {
-		ret = nft_cmd_chain_zero_counters(h, chain, *table, 0);
+		ret = nft_cmd_chain_zero_counters(h, chain, *table,
+						  flags & OPT_VERBOSE);
 	} else if (command == 'F') {
-		ret = nft_cmd_rule_flush(h, chain, *table, 0);
+		ret = nft_cmd_rule_flush(h, chain, *table, flags & OPT_VERBOSE);
 	} else if (command == 'A') {
-		ret = append_entry(h, chain, *table, &cs, 0, 0, true);
+		ret = append_entry(h, chain, *table, &cs, 0,
+				   flags & OPT_VERBOSE, true);
 	} else if (command == 'I') {
 		ret = append_entry(h, chain, *table, &cs, rule_nr - 1,
-				   0, false);
+				   flags & OPT_VERBOSE, false);
 	} else if (command == 'D') {
 		ret = delete_entry(h, chain, *table, &cs, rule_nr - 1,
-				   rule_nr_end, 0);
+				   rule_nr_end, flags & OPT_VERBOSE);
 	} /*else if (replace->command == 'C') {
 		ebt_change_counters(replace, new_entry, rule_nr, rule_nr_end, &(new_entry->cnt_surplus), chcounter);
 		if (ebt_errormsg[0] != '\0')
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index f5aabf3cc1944..81b25a43c9a04 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -417,6 +417,7 @@ static const struct nft_xt_restore_cb ebt_restore_cb = {
 
 static const struct option ebt_restore_options[] = {
 	{.name = "noflush", .has_arg = 0, .val = 'n'},
+	{.name = "verbose", .has_arg = 0, .val = 'v'},
 	{ 0 }
 };
 
@@ -430,15 +431,18 @@ int xtables_eb_restore_main(int argc, char *argv[])
 	struct nft_handle h;
 	int c;
 
-	while ((c = getopt_long(argc, argv, "n",
+	while ((c = getopt_long(argc, argv, "nv",
 				ebt_restore_options, NULL)) != -1) {
 		switch(c) {
 		case 'n':
 			noflush = 1;
 			break;
+		case 'v':
+			verbose++;
+			break;
 		default:
 			fprintf(stderr,
-				"Usage: ebtables-restore [ --noflush ]\n");
+				"Usage: ebtables-restore [ --verbose ] [ --noflush ]\n");
 			exit(1);
 			break;
 		}
-- 
2.34.1

[iptables PATCH 3/4] nft: Add debug output to table creation

[Phil Sutter]

This at least allows to inspect how tables are created on demand.
Also requires setting NFTNL_TABLE_FAMILY for clean output.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 301d6c342f982..041e1b8ccd3e5 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -657,6 +657,7 @@ static int nft_table_builtin_add(struct nft_handle *h,
 	if (t == NULL)
 		return -1;
 
+	nftnl_table_set_u32(t, NFTNL_TABLE_FAMILY, h->family);
 	nftnl_table_set_str(t, NFTNL_TABLE_NAME, _t->name);
 
 	ret = batch_table_add(h, NFT_COMPAT_TABLE_ADD, t) ? 0 : - 1;
@@ -2242,6 +2243,7 @@ static int __nft_table_flush(struct nft_handle *h, const char *table, bool exist
 	if (t == NULL)
 		return -1;
 
+	nftnl_table_set_u32(t, NFTNL_TABLE_FAMILY, h->family);
 	nftnl_table_set_str(t, NFTNL_TABLE_NAME, table);
 
 	obj = batch_table_add(h, NFT_COMPAT_TABLE_FLUSH, t);
@@ -2832,6 +2834,18 @@ error:
 	return ret;
 }
 
+static void nft_table_print_debug(struct nft_handle *h,
+				  struct nftnl_table *t, struct nlmsghdr *nlh)
+{
+	if (h->verbose > 1) {
+		nftnl_table_fprintf(stdout, t, 0, 0);
+		fprintf(stdout, "\n");
+	}
+	if (h->verbose > 2)
+		mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len,
+				  sizeof(struct nfgenmsg));
+}
+
 static void nft_compat_table_batch_add(struct nft_handle *h, uint16_t type,
 				       uint16_t flags, uint32_t seq,
 				       struct nftnl_table *table)
@@ -2841,6 +2855,7 @@ static void nft_compat_table_batch_add(struct nft_handle *h, uint16_t type,
 	nlh = nftnl_table_nlmsg_build_hdr(nftnl_batch_buffer(h->batch),
 					type, h->family, flags, seq);
 	nftnl_table_nlmsg_build_payload(nlh, table);
+	nft_table_print_debug(h, table, nlh);
 }
 
 static void nft_compat_set_batch_add(struct nft_handle *h, uint16_t type,
-- 
2.34.1

[iptables PATCH 4/4] nft: cache: Dump rules if debugging

[Phil Sutter]

If verbose flag was given twice, dump rules while populating the cache.
This not only applies to list commands, but all requiring a rule cache -
e.g. insert with position.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 43ac291ec84b2..608e42a7aa01b 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -538,9 +538,15 @@ static int fetch_chain_cache(struct nft_handle *h,
 	return ret;
 }
 
+struct rule_list_cb_data {
+	struct nftnl_chain *chain;
+	int verbose;
+};
+
 static int nftnl_rule_list_cb(const struct nlmsghdr *nlh, void *data)
 {
-	struct nftnl_chain *c = data;
+	struct rule_list_cb_data *rld = data;
+	struct nftnl_chain *c = rld->chain;
 	struct nftnl_rule *r;
 
 	r = nftnl_rule_alloc();
@@ -552,6 +558,10 @@ static int nftnl_rule_list_cb(const struct nlmsghdr *nlh, void *data)
 		return MNL_CB_OK;
 	}
 
+	if (rld->verbose > 1) {
+		nftnl_rule_fprintf(stdout, r, 0, 0);
+		fprintf(stdout, "\n");
+	}
 	nftnl_chain_rule_add_tail(r, c);
 	return MNL_CB_OK;
 }
@@ -560,6 +570,10 @@ static int nft_rule_list_update(struct nft_chain *nc, void *data)
 {
 	struct nftnl_chain *c = nc->nftnl;
 	struct nft_handle *h = data;
+	struct rule_list_cb_data rld = {
+		.chain = c,
+		.verbose = h->verbose,
+	};
 	char buf[16536];
 	struct nlmsghdr *nlh;
 	struct nftnl_rule *rule;
@@ -581,7 +595,7 @@ static int nft_rule_list_update(struct nft_chain *nc, void *data)
 					NLM_F_DUMP, h->seq);
 	nftnl_rule_nlmsg_build_payload(nlh, rule);
 
-	ret = mnl_talk(h, nlh, nftnl_rule_list_cb, c);
+	ret = mnl_talk(h, nlh, nftnl_rule_list_cb, &rld);
 	if (ret < 0 && errno == EINTR)
 		assert(nft_restart(h) >= 0);
 
-- 
2.34.1

Re: [iptables PATCH 1/4] nft: Set NFTNL_CHAIN_FAMILY in new chains

[Pablo Neira Ayuso]

Series LGTM, thanks

On Fri, Feb 04, 2022 at 05:59:58PM +0100, Phil Sutter wrote:
> Kernel doesn't need it, but debug output improves significantly. Before
> this patch:
> 
> | # iptables-nft -vv -A INPUT
> | [...]
> | unknown filter INPUT use 0 type filter hook unknown prio 0 policy accept packets 0 bytes 0
> | [...]
> 
> and after:
> 
> | # iptables-nft -vv -A INPUT
> | [...]
> | ip filter INPUT use 0 type filter hook input prio 0 policy accept packets 0 bytes 0
> | [...]
> 
> While being at it, make nft_chain_builtin_alloc() take only the builtin
> table's name as parameter - it's the only field it accesses.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  iptables/nft.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 7cc6ca5258150..301d6c342f982 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -665,7 +665,7 @@ static int nft_table_builtin_add(struct nft_handle *h,
>  }
>  
>  static struct nftnl_chain *
> -nft_chain_builtin_alloc(const struct builtin_table *table,
> +nft_chain_builtin_alloc(int family, const char *tname,
>  			const struct builtin_chain *chain, int policy)
>  {
>  	struct nftnl_chain *c;
> @@ -674,7 +674,8 @@ nft_chain_builtin_alloc(const struct builtin_table *table,
>  	if (c == NULL)
>  		return NULL;
>  
> -	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table->name);
> +	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, family);
> +	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, tname);
>  	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain->name);
>  	nftnl_chain_set_u32(c, NFTNL_CHAIN_HOOKNUM, chain->hook);
>  	nftnl_chain_set_u32(c, NFTNL_CHAIN_PRIO, chain->prio);
> @@ -693,7 +694,7 @@ static void nft_chain_builtin_add(struct nft_handle *h,
>  {
>  	struct nftnl_chain *c;
>  
> -	c = nft_chain_builtin_alloc(table, chain, NF_ACCEPT);
> +	c = nft_chain_builtin_alloc(h->family, table->name, chain, NF_ACCEPT);
>  	if (c == NULL)
>  		return;
>  
> @@ -959,7 +960,7 @@ static struct nftnl_chain *nft_chain_new(struct nft_handle *h,
>  	_c = nft_chain_builtin_find(_t, chain);
>  	if (_c != NULL) {
>  		/* This is a built-in chain */
> -		c = nft_chain_builtin_alloc(_t, _c, policy);
> +		c = nft_chain_builtin_alloc(h->family, _t->name, _c, policy);
>  		if (c == NULL)
>  			return NULL;
>  	} else {
> @@ -1999,6 +2000,7 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
>  	if (c == NULL)
>  		return 0;
>  
> +	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, h->family);
>  	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
>  	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
>  	if (h->family == NFPROTO_BRIDGE)
> @@ -2029,6 +2031,7 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
>  		if (!c)
>  			return 0;
>  
> +		nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, h->family);
>  		nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
>  		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
>  		created = true;
> @@ -2190,6 +2193,7 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
>  	if (c == NULL)
>  		return 0;
>  
> +	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, h->family);
>  	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
>  	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, newname);
>  	nftnl_chain_set_u64(c, NFTNL_CHAIN_HANDLE, handle);
> -- 
> 2.34.1
>