Re: [iptables PATCH 1/2] nft: Use verbose flag to toggle debug output

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Jan 28, 2022 at 09:36:59PM +0100, Phil Sutter wrote:
> Copy legacy iptables' behaviour, printing debug output if verbose flag
> is given more than once.
> 
> Since nft debug output applies to netlink messages which are not created
> until nft_action() phase, carrying verbose value is non-trivial -
> introduce a field in struct nft_handle for that.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  iptables/nft-shared.h |  1 -
>  iptables/nft.c        | 38 ++++++++++++++++++++------------------
>  iptables/nft.h        |  1 +
>  iptables/xtables.c    |  1 +
>  4 files changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
> index c3241f4b8c726..d4222ae05adc3 100644
> --- a/iptables/nft-shared.h
> +++ b/iptables/nft-shared.h
> @@ -13,7 +13,6 @@
>  #include "xshared.h"
>  
>  #ifdef DEBUG
> -#define NLDEBUG
>  #define DEBUG_DEL
>  #endif
>  
> diff --git a/iptables/nft.c b/iptables/nft.c
> index b5de687c5c4cd..ae41384fe8180 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -926,15 +926,16 @@ void nft_fini(struct nft_handle *h)
>  	mnl_socket_close(h->nl);
>  }
>  
> -static void nft_chain_print_debug(struct nftnl_chain *c, struct nlmsghdr *nlh)
> +static void nft_chain_print_debug(struct nft_handle *h,
> +				  struct nftnl_chain *c, struct nlmsghdr *nlh)
>  {
> -#ifdef NLDEBUG
> -	char tmp[1024];
> -
> -	nftnl_chain_snprintf(tmp, sizeof(tmp), c, 0, 0);
> -	printf("DEBUG: chain: %s\n", tmp);
> -	mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len, sizeof(struct nfgenmsg));
> -#endif
> +	if (h->verbose > 1) {
> +		nftnl_chain_fprintf(stdout, c, 0, 0);
> +		fprintf(stdout, "\n");
> +	}
> +	if (h->verbose > 2)
> +		mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len,
> +				  sizeof(struct nfgenmsg));

OK, so -v is netlink byte printing and -vv means print netlink message
too. LGTM.

I'd mention this in the commit description before applying.

>  }
>  
>  static struct nftnl_chain *nft_chain_new(struct nft_handle *h,
> @@ -1386,15 +1387,16 @@ int add_log(struct nftnl_rule *r, struct iptables_command_state *cs)
>  	return 0;
>  }
>  
> -static void nft_rule_print_debug(struct nftnl_rule *r, struct nlmsghdr *nlh)
> +static void nft_rule_print_debug(struct nft_handle *h,
> +				 struct nftnl_rule *r, struct nlmsghdr *nlh)
>  {
> -#ifdef NLDEBUG
> -	char tmp[1024];
> -
> -	nftnl_rule_snprintf(tmp, sizeof(tmp), r, 0, 0);
> -	printf("DEBUG: rule: %s\n", tmp);
> -	mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len, sizeof(struct nfgenmsg));
> -#endif
> +	if (h->verbose > 1) {
> +		nftnl_rule_fprintf(stdout, r, 0, 0);
> +		fprintf(stdout, "\n");
> +	}
> +	if (h->verbose > 2)
> +		mnl_nlmsg_fprintf(stdout, nlh, nlh->nlmsg_len,
> +				  sizeof(struct nfgenmsg));
>  }
>  
>  int add_counters(struct nftnl_rule *r, uint64_t packets, uint64_t bytes)
> @@ -2698,7 +2700,7 @@ static void nft_compat_chain_batch_add(struct nft_handle *h, uint16_t type,
>  	nlh = nftnl_chain_nlmsg_build_hdr(nftnl_batch_buffer(h->batch),
>  					type, h->family, flags, seq);
>  	nftnl_chain_nlmsg_build_payload(nlh, chain);
> -	nft_chain_print_debug(chain, nlh);
> +	nft_chain_print_debug(h, chain, nlh);
>  }
>  
>  static void nft_compat_rule_batch_add(struct nft_handle *h, uint16_t type,
> @@ -2710,7 +2712,7 @@ static void nft_compat_rule_batch_add(struct nft_handle *h, uint16_t type,
>  	nlh = nftnl_rule_nlmsg_build_hdr(nftnl_batch_buffer(h->batch),
>  				       type, h->family, flags, seq);
>  	nftnl_rule_nlmsg_build_payload(nlh, rule);
> -	nft_rule_print_debug(rule, nlh);
> +	nft_rule_print_debug(h, rule, nlh);
>  }
>  
>  static void batch_obj_del(struct nft_handle *h, struct obj_update *o)
> diff --git a/iptables/nft.h b/iptables/nft.h
> index 4c78f761e1c4b..fd116c2e3e198 100644
> --- a/iptables/nft.h
> +++ b/iptables/nft.h
> @@ -109,6 +109,7 @@ struct nft_handle {
>  	int8_t			config_done;
>  	struct list_head	cmd_list;
>  	bool			cache_init;
> +	int			verbose;
>  
>  	/* meta data, for error reporting */
>  	struct {
> diff --git a/iptables/xtables.c b/iptables/xtables.c
> index 051d5c7b7f98b..c44b39acdcd97 100644
> --- a/iptables/xtables.c
> +++ b/iptables/xtables.c
> @@ -163,6 +163,7 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
>  		h->ops->init_cs(&cs);
>  
>  	do_parse(argc, argv, &p, &cs, &args);
> +	h->verbose = p.verbose;
>  
>  	if (!nft_table_builtin_find(h, p.table))
>  		xtables_error(VERSION_PROBLEM,
> -- 
> 2.34.1
>