Re: [nft PATCH 2/2] src: add list ruleset command

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

On Wed, Sep 24, 2014 at 11:13:57AM +0200, Arturo Borrero Gonzalez wrote:
> This patch adds a new command to nft:
> 	% nft list ruleset [family]
> 
> Which list the entire ruleset.
> If no family is specified, all tables of all families are listed.
> 
> Users can now make several operations at ruleset level:
> 
>  % nft list ruleset > ruleset.nft
>  % nft -f ruleset.nft
>  % nft flush ruleset
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  src/parser.y |    4 ++++
>  src/rule.c   |   29 +++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/src/parser.y b/src/parser.y
> index e35eede..8f58e3a 100644
> --- a/src/parser.y
> +++ b/src/parser.y
> @@ -753,6 +753,10 @@ list_cmd		:	TABLE		table_spec
>  			{
>  				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_SET, &$2, &@$, NULL);
>  			}
> +			|	RULESET		ruleset_spec
> +			{
> +				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_RULESET, &$2, &@$, NULL);
> +			}
>  			;
>  
>  flush_cmd		:	TABLE		table_spec
> diff --git a/src/rule.c b/src/rule.c
> index ac72f12..9c13474 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -782,6 +782,33 @@ err:
>  	return -1;
>  }
>  
> +static int do_command_list_ruleset(struct netlink_ctx *ctx, struct cmd *cmd)

I'd suggest do_list_ruleset.

So we leave the do_command_ prefix for functions that are called from
do_command().

> +{
> +	struct table *table, *nt;
                              ^
please, use 'next' whenever possible.

> +	struct netlink_ctx inner_ctx;
> +	LIST_HEAD(msgs);
> +
> +	if (netlink_list_tables(ctx, &cmd->handle, &cmd->location) < 0)
> +		return -1;
> +
> +	memset(&inner_ctx, 0, sizeof(inner_ctx));
> +	init_list_head(&inner_ctx.list);
> +	init_list_head(&msgs);
> +	inner_ctx.msgs = &msgs;

I think you can skip this inner_ctx if you declare:

        LIST_HEAD(table_list);

and then you use list_splice_tail_init() to migrate the temporary
ctx->list to table_list.

> +
> +	list_for_each_entry_safe(table, nt, &ctx->list, list) {
                                            ^^^^^^^^^^
Then, use table_list here to iterate over it.

> +		table_add_hash(table);
> +
> +		cmd->handle.family = table->handle.family;
> +		cmd->handle.table = table->handle.table;
> +
> +		if (do_command_list_table_content(&inner_ctx, cmd, table) < 0)
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
>  {
>  	struct table *table = NULL;
> @@ -837,6 +864,8 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
>  			set_print(set);
>  		}
>  		return 0;
> +	case CMD_OBJ_RULESET:
> +		return do_command_list_ruleset(ctx, cmd);
>  	default:
>  		BUG("invalid command object type %u\n", cmd->obj);
>  	}
>