[PATCH nft] cache: include set elements in "nft set list"

[PATCH nft] cache: include set elements in "nft set list"

[Florian Westphal]

Make "nft list sets" include set elements in listing by default.
In nftables 1.0.0, "nft list sets" did not include the set elements,
but with "--json" they were included.

1.0.1 and newer never include them.
This causes a problem for people updating from 1.0.0 and relying
on the presence of the set elements.

Change nftables to always include the set elements.
The "--terse" option is honored to get the "no elements" behaviour.

Fixes: a1a6b0a5c3c4 ("cache: finer grain cache population for list commands")
Link: https://marc.info/?l=netfilter&m=168704941828372&w=2
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/cache.c | 2 ++
 src/rule.c  | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 95adee7f8ac1..becfa57fc335 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -235,6 +235,8 @@ static unsigned int evaluate_cache_list(struct nft_ctx *nft, struct cmd *cmd,
 	case CMD_OBJ_SETS:
 	case CMD_OBJ_MAPS:
 		flags |= NFT_CACHE_TABLE | NFT_CACHE_SET;
+		if (!nft_output_terse(&nft->output))
+			flags |= NFT_CACHE_SETELEM;
 		break;
 	case CMD_OBJ_FLOWTABLE:
 		if (filter &&
diff --git a/src/rule.c b/src/rule.c
index 633a5a12486d..305322ea7cc3 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1601,8 +1601,7 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
 			if (cmd->obj == CMD_OBJ_MAPS &&
 			    !map_is_literal(set->flags))
 				continue;
-			set_print_declaration(set, &opts, &ctx->nft->output);
-			nft_print(&ctx->nft->output, "%s}%s", opts.tab, opts.nl);
+			set_print(set, &ctx->nft->output);
 		}
 
 		nft_print(&ctx->nft->output, "}\n");
-- 
2.41.0

Re: [PATCH nft] cache: include set elements in "nft set list"

[Arturo Borrero Gonzalez]

On 6/18/23 18:39, Florian Westphal wrote:
> Make "nft list sets" include set elements in listing by default.
> In nftables 1.0.0, "nft list sets" did not include the set elements,
> but with "--json" they were included.
> 
> 1.0.1 and newer never include them.
> This causes a problem for people updating from 1.0.0 and relying
> on the presence of the set elements.
> 
> Change nftables to always include the set elements.
> The "--terse" option is honored to get the "no elements" behaviour.
> 

Hi,

Would you recommend the debian package backports this fix for 1.0.6/1.0.7 ?

let me know, regards

Re: [PATCH nft] cache: include set elements in "nft set list"

[Pablo Neira Ayuso]

On Sun, Jun 18, 2023 at 06:39:45PM +0200, Florian Westphal wrote:
> Make "nft list sets" include set elements in listing by default.
> In nftables 1.0.0, "nft list sets" did not include the set elements,
> but with "--json" they were included.
> 
> 1.0.1 and newer never include them.
> This causes a problem for people updating from 1.0.0 and relying
> on the presence of the set elements.
> 
> Change nftables to always include the set elements.
> The "--terse" option is honored to get the "no elements" behaviour.

LGTM.

> Fixes: a1a6b0a5c3c4 ("cache: finer grain cache population for list commands")
> Link: https://marc.info/?l=netfilter&m=168704941828372&w=2
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/cache.c | 2 ++
>  src/rule.c  | 3 +--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cache.c b/src/cache.c
> index 95adee7f8ac1..becfa57fc335 100644
> --- a/src/cache.c
> +++ b/src/cache.c
> @@ -235,6 +235,8 @@ static unsigned int evaluate_cache_list(struct nft_ctx *nft, struct cmd *cmd,
>  	case CMD_OBJ_SETS:
>  	case CMD_OBJ_MAPS:
>  		flags |= NFT_CACHE_TABLE | NFT_CACHE_SET;
> +		if (!nft_output_terse(&nft->output))
> +			flags |= NFT_CACHE_SETELEM;
>  		break;
>  	case CMD_OBJ_FLOWTABLE:
>  		if (filter &&
> diff --git a/src/rule.c b/src/rule.c
> index 633a5a12486d..305322ea7cc3 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1601,8 +1601,7 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
>  			if (cmd->obj == CMD_OBJ_MAPS &&
>  			    !map_is_literal(set->flags))
>  				continue;
> -			set_print_declaration(set, &opts, &ctx->nft->output);
> -			nft_print(&ctx->nft->output, "%s}%s", opts.tab, opts.nl);
> +			set_print(set, &ctx->nft->output);
>  		}
>  
>  		nft_print(&ctx->nft->output, "}\n");
> -- 
> 2.41.0
> 

Re: [PATCH nft] cache: include set elements in "nft set list"

[Pablo Neira Ayuso]

On Mon, Jun 19, 2023 at 09:59:03AM +0200, Arturo Borrero Gonzalez wrote:
> On 6/18/23 18:39, Florian Westphal wrote:
> > Make "nft list sets" include set elements in listing by default.
> > In nftables 1.0.0, "nft list sets" did not include the set elements,
> > but with "--json" they were included.
> > 
> > 1.0.1 and newer never include them.
> > This causes a problem for people updating from 1.0.0 and relying
> > on the presence of the set elements.
> > 
> > Change nftables to always include the set elements.
> > The "--terse" option is honored to get the "no elements" behaviour.
> > 
> 
> Hi,
> 
> Would you recommend the debian package backports this fix for 1.0.6/1.0.7 ?

fine with me.

Re: [PATCH nft] cache: include set elements in "nft set list"

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sun, Jun 18, 2023 at 06:39:45PM +0200, Florian Westphal wrote:
> > Make "nft list sets" include set elements in listing by default.
> > In nftables 1.0.0, "nft list sets" did not include the set elements,
> > but with "--json" they were included.
> > 
> > 1.0.1 and newer never include them.
> > This causes a problem for people updating from 1.0.0 and relying
> > on the presence of the set elements.
> > 
> > Change nftables to always include the set elements.
> > The "--terse" option is honored to get the "no elements" behaviour.

I pushed this patch to master, with a minor change (removal of no-longer
needed fmt struct).