Re: [PATCH 5/5, V2, nft] Implement deleting rule by description

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

On Mon, Aug 08, 2016 at 02:42:35PM +0200, Carlos Falgueras García wrote:
> 	$ nft list -a ruleset
> 	table ip t {
> 		chain c {
> 			ip saddr 1.1.1.1 counter packets 0 bytes 0 # handle 1
> 			ip saddr 1.1.1.2 counter packets 0 bytes 0 # handle 2
> 			ip saddr 1.1.1.2 counter packets 0 bytes 0 # handle 3
> 			ip saddr 1.1.1.4 counter packets 0 bytes 0 # handle 4
> 		}
> 	}
> 
> Before this patch:
> 	$ nft delete rule table chain ip saddr 1.1.1.2 counter
> 	<cmdline>:1:17-18: Error: syntax error, unexpected ip, expecting end of
> 	file or newline or semicolon
> 	delete rule t c ip saddr 1.1.1.2 counter
> 	                ^^
> After this patch:

Please, remove all this above. I suggest a description like:

This patch introduces deletion in a similar fashion as in iptables,
thus, we can delete the first rule that matches our description, for
example:

> 	$ nft delete rule table chain ip saddr 1.1.1.2 counter
> 	$ nft list -a ruleset
> 	table ip t {
> 		chain c {
> 			ip saddr 1.1.1.1 counter packets 0 bytes 0 # handle 1
> 			ip saddr 1.1.1.2 counter packets 0 bytes 0 # handle 3
> 			ip saddr 1.1.1.4 counter packets 0 bytes 0 # handle 4
> 		}
> 	}

More comments below.

> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  src/evaluate.c     |  7 +++++++
>  src/parser_bison.y | 28 +++++++++++++++++++++-------
>  src/rule.c         | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 4611969..efd5f69 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -2573,8 +2573,15 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
>  			return ret;
>  
>  		return setelem_evaluate(ctx, &cmd->expr);
> +		break;

Why this new break?

>  	case CMD_OBJ_SET:
>  	case CMD_OBJ_RULE:
> +		// CMD_LIST force caching all ruleset

Please, no C++ comment style, use /* ... */.

> +		ret = cache_update(CMD_LIST, ctx->msgs);
> +		if (ret < 0)
> +			return ret;
> +		return rule_evaluate(ctx, cmd->rule);
> +		break;

No need for break here either.

>  	case CMD_OBJ_CHAIN:
>  	case CMD_OBJ_TABLE:
>  		return 0;