* [PATCH 1/4 V6 nft] Simplify parser rule_spec tree
@ 2016-08-21 21:22 Carlos Falgueras García
2016-08-21 21:22 ` [PATCH 2/4 V6 nft] Implement deleting rule by description Carlos Falgueras García
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-08-21 21:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
This patch separates the rule identification from the rule localization, so
the logic moves from the evaluator to the parser. This allows to revert the
patch "evaluate: improve rule managment checks"
(4176c7d30c2ff1b3f52468fc9c08b8df83f979a8) and saves a lot of code.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
src/evaluate.c | 68 +-----------------------------------------------------
src/parser_bison.y | 45 +++++++++++++++++-------------------
2 files changed, 22 insertions(+), 91 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 87f5a6d..2f94ac6 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -44,12 +44,6 @@ static const char *byteorder_names[] = {
__stmt_binary_error(ctx, &(s1)->location, NULL, fmt, ## args)
#define cmd_error(ctx, fmt, args...) \
__stmt_binary_error(ctx, &(ctx->cmd)->location, NULL, fmt, ## args)
-#define handle_error(ctx, fmt, args...) \
- __stmt_binary_error(ctx, &ctx->cmd->handle.handle.location, NULL, fmt, ## args)
-#define position_error(ctx, fmt, args...) \
- __stmt_binary_error(ctx, &ctx->cmd->handle.position.location, NULL, fmt, ## args)
-#define handle_position_error(ctx, fmt, args...) \
- __stmt_binary_error(ctx, &ctx->cmd->handle.handle.location, &ctx->cmd->handle.position.location, fmt, ## args)
static int __fmtstring(3, 4) set_error(struct eval_ctx *ctx,
const struct set *set,
@@ -2481,68 +2475,11 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
return 0;
}
-static int rule_evaluate_cmd(struct eval_ctx *ctx)
-{
- struct handle *handle = &ctx->cmd->handle;
-
- /* allowed:
- * - insert [position] (no handle)
- * - add [position] (no handle)
- * - replace <handle> (no position)
- * - delete <handle> (no position)
- */
-
- switch (ctx->cmd->op) {
- case CMD_INSERT:
- if (handle->handle.id && handle->position.id)
- return handle_position_error(ctx, "use only `position'"
- " instead");
-
- if (handle->handle.id)
- return handle_error(ctx, "use `position' instead");
- break;
- case CMD_ADD:
- if (handle->handle.id && handle->position.id)
- return handle_position_error(ctx, "use only `position'"
- " instead");
-
- if (handle->handle.id)
- return handle_error(ctx, "use `position' instead");
-
- break;
- case CMD_REPLACE:
- if (handle->handle.id && handle->position.id)
- return handle_position_error(ctx, "use only `handle' "
- "instead");
- if (handle->position.id)
- return position_error(ctx, "use `handle' instead");
- if (!handle->handle.id)
- return cmd_error(ctx, "missing `handle'");
- break;
- case CMD_DELETE:
- if (handle->handle.id && handle->position.id)
- return handle_position_error(ctx, "use only `handle' "
- "instead");
- if (handle->position.id)
- return position_error(ctx, "use `handle' instead");
- if (!handle->handle.id)
- return cmd_error(ctx, "missing `handle'");
- break;
- default:
- BUG("unkown command type %u\n", ctx->cmd->op);
- }
-
- return 0;
-}
-
static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule)
{
struct stmt *stmt, *tstmt = NULL;
struct error_record *erec;
- if (rule_evaluate_cmd(ctx) < 0)
- return -1;
-
proto_ctx_init(&ctx->pctx, rule->handle.family);
memset(&ctx->ectx, 0, sizeof(ctx->ectx));
@@ -2723,11 +2660,8 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
return ret;
return setelem_evaluate(ctx, &cmd->expr);
- case CMD_OBJ_RULE:
- if (rule_evaluate_cmd(ctx) < 0)
- return -1;
- /* fall through */
case CMD_OBJ_SET:
+ case CMD_OBJ_RULE:
case CMD_OBJ_CHAIN:
case CMD_OBJ_TABLE:
return 0;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index e16b8a3..2ca7eea 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -425,15 +425,12 @@ static void location_update(struct location *loc, struct location *rhs, int n)
%type <cmd> base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd
%destructor { cmd_free($$); } base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd
-%type <handle> table_spec chain_spec chain_identifier ruleid_spec ruleset_spec
-%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier ruleid_spec ruleset_spec
+%type <handle> table_spec chain_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
+%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
%type <handle> set_spec set_identifier
%destructor { handle_free(&$$); } set_spec set_identifier
%type <val> family_spec family_spec_explicit chain_policy prio_spec
-%type <handle_spec> handle_spec
-%type <position_spec> position_spec
-
%type <string> dev_spec
%destructor { xfree($$); } dev_spec
@@ -720,11 +717,11 @@ add_cmd : TABLE table_spec
close_scope(state);
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_CHAIN, &$2, &@$, $5);
}
- | RULE ruleid_spec rule
+ | RULE rule_position rule
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &$2, &@$, $3);
}
- | /* empty */ ruleid_spec rule
+ | /* empty */ rule_position rule
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &$1, &@$, $2);
}
@@ -779,7 +776,7 @@ create_cmd : TABLE table_spec
}
;
-insert_cmd : RULE ruleid_spec rule
+insert_cmd : RULE rule_position rule
{
$$ = cmd_alloc(CMD_INSERT, CMD_OBJ_RULE, &$2, &@$, $3);
}
@@ -1252,35 +1249,35 @@ set_identifier : identifier
}
;
-handle_spec : /* empty */
+handle_spec : HANDLE NUM
{
- memset(&$$, 0, sizeof($$));
+ $$.handle.location = @$;
+ $$.handle.id = $2;
}
- | HANDLE NUM
+ ;
+
+position_spec : POSITION NUM
{
- memset(&$$, 0, sizeof($$));
- $$.location = @$;
- $$.id = $2;
+ $$.position.location = @$;
+ $$.position.id = $2;
}
;
-position_spec : /* empty */
+rule_position : chain_spec
{
- memset(&$$, 0, sizeof($$));
+ $$ = $1;
}
- | POSITION NUM
+ | chain_spec position_spec
{
- memset(&$$, 0, sizeof($$));
- $$.location = @$;
- $$.id = $2;
+ $$ = $1;
+ handle_merge(&$$, &$2);
}
;
-ruleid_spec : chain_spec handle_spec position_spec
+ruleid_spec : chain_spec handle_spec
{
- $$ = $1;
- $$.handle = $2;
- $$.position = $3;
+ $$ = $1;
+ handle_merge(&$$, &$2);
}
;
--
2.8.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4 V6 nft] Implement deleting rule by description
2016-08-21 21:22 [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Carlos Falgueras García
@ 2016-08-21 21:22 ` Carlos Falgueras García
2016-08-22 16:13 ` Pablo Neira Ayuso
2016-08-21 21:22 ` [PATCH 3/4 V6 nft] test: shell: Add tests for " Carlos Falgueras García
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Carlos Falgueras García @ 2016-08-21 21:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
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 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
}
}
$ 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
}
}
The parser rule 'ruleid_spec' is now of the type 'struct rule' in order to
hold a rule description. When rule is identified with its handle a dummy
'struct rule' is allocated to hold the specified handle.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
src/evaluate.c | 6 ++++++
src/parser_bison.y | 24 ++++++++++++++++--------
src/rule.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 70 insertions(+), 10 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 2f94ac6..f7b349b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2661,7 +2661,13 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
return setelem_evaluate(ctx, &cmd->expr);
case CMD_OBJ_SET:
+ return 0;
case CMD_OBJ_RULE:
+ /* CMD_LIST force caching all ruleset */
+ ret = cache_update(CMD_LIST, ctx->msgs);
+ if (ret < 0)
+ return ret;
+ return rule_evaluate(ctx, cmd->rule);
case CMD_OBJ_CHAIN:
case CMD_OBJ_TABLE:
return 0;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 2ca7eea..beea38b 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -425,8 +425,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
%type <cmd> base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd
%destructor { cmd_free($$); } base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd
-%type <handle> table_spec chain_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
-%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
+%type <handle> table_spec chain_spec chain_identifier handle_spec position_spec rule_position ruleset_spec
+%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier handle_spec position_spec rule_position ruleset_spec
%type <handle> set_spec set_identifier
%destructor { handle_free(&$$); } set_spec set_identifier
%type <val> family_spec family_spec_explicit chain_policy prio_spec
@@ -438,7 +438,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
%destructor { close_scope(state); table_free($$); } table_block_alloc
%type <chain> chain_block_alloc chain_block
%destructor { close_scope(state); chain_free($$); } chain_block_alloc
-%type <rule> rule rule_alloc
+%type <rule> rule ruleid_spec rule_alloc
%destructor { rule_free($$); } rule
%type <val> set_flag_list set_flag
@@ -745,9 +745,10 @@ add_cmd : TABLE table_spec
}
;
-replace_cmd : RULE ruleid_spec rule
+replace_cmd : RULE chain_spec handle_spec rule
{
- $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2, &@$, $3);
+ handle_merge(&$2, &$3);
+ $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2, &@$, $4);
}
;
@@ -792,7 +793,7 @@ delete_cmd : TABLE table_spec
}
| RULE ruleid_spec
{
- $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_RULE, &$2, &@$, NULL);
+ $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_RULE, &$2->handle, &@$, $2);
}
| SET set_spec
{
@@ -1276,8 +1277,15 @@ rule_position : chain_spec
ruleid_spec : chain_spec handle_spec
{
- $$ = $1;
- handle_merge(&$$, &$2);
+ $$ = rule_alloc(&@$, NULL);
+ $$->handle = $1;
+ handle_merge(&$$->handle, &$2);
+ }
+ |
+ chain_spec rule
+ {
+ $$ = $2;
+ handle_merge(&$$->handle, &$1);
}
;
diff --git a/src/rule.c b/src/rule.c
index 14e57f2..e9672a7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -20,6 +20,7 @@
#include <rule.h>
#include <utils.h>
#include <netlink.h>
+#include <erec.h>
#include <libnftnl/common.h>
#include <libnftnl/ruleset.h>
@@ -402,6 +403,32 @@ void rule_print(const struct rule *rule)
printf(" # handle %" PRIu64, rule->handle.handle.id);
}
+static struct rule *rule_find_first(const struct rule *rule)
+{
+ struct nftnl_rule *nlr1, *nlr2;
+ struct rule *rule_idx;
+ struct table *table;
+ struct chain *chain;
+
+ table = table_lookup(&rule->handle);
+ if (!table)
+ return NULL;
+ chain = chain_lookup(table, &rule->handle);
+ if (!chain)
+ return NULL;
+
+ nlr1 = alloc_nftnl_rule(&rule->handle);
+ netlink_linearize_rule(NULL, nlr1, rule);
+
+ list_for_each_entry(rule_idx, &chain->rules, list) {
+ nlr2 = alloc_nftnl_rule(&rule_idx->handle);
+ netlink_linearize_rule(NULL, nlr2, rule_idx);
+ if (nftnl_rule_cmp(nlr1, nlr2))
+ return rule_idx;
+ }
+ return NULL;
+}
+
struct rule *rule_lookup(const struct chain *chain, uint64_t handle)
{
struct rule *rule;
@@ -1010,6 +1037,26 @@ static int do_delete_setelems(struct netlink_ctx *ctx, const struct handle *h,
return 0;
}
+static int do_delete_rule(struct netlink_ctx *ctx, const struct cmd *cmd)
+{
+ struct rule *rule;
+
+ /* Delete by handle */
+ if (cmd->handle.handle.id)
+ return netlink_del_rule_batch(ctx, &cmd->handle, &cmd->location);
+
+ /* Delete by description */
+ rule = rule_find_first(cmd->rule);
+ if (!rule) {
+ struct error_record *e = erec_create(EREC_ERROR, &cmd->location,
+ "Could not delete rule to batch: Rule not found");
+ erec_queue(e, ctx->msgs);
+ return -1;
+ }
+ return netlink_del_rule_batch(ctx, &rule->handle,
+ &rule->handle.position.location);
+}
+
static int do_command_delete(struct netlink_ctx *ctx, struct cmd *cmd)
{
switch (cmd->obj) {
@@ -1018,8 +1065,7 @@ static int do_command_delete(struct netlink_ctx *ctx, struct cmd *cmd)
case CMD_OBJ_CHAIN:
return netlink_delete_chain(ctx, &cmd->handle, &cmd->location);
case CMD_OBJ_RULE:
- return netlink_del_rule_batch(ctx, &cmd->handle,
- &cmd->location);
+ return do_delete_rule(ctx, cmd);
case CMD_OBJ_SET:
return netlink_delete_set(ctx, &cmd->handle, &cmd->location);
case CMD_OBJ_SETELEM:
--
2.8.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4 V6 nft] test: shell: Add tests for deleting rule by description
2016-08-21 21:22 [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Carlos Falgueras García
2016-08-21 21:22 ` [PATCH 2/4 V6 nft] Implement deleting rule by description Carlos Falgueras García
@ 2016-08-21 21:22 ` Carlos Falgueras García
2016-08-22 16:13 ` Pablo Neira Ayuso
[not found] ` <20160822162050.GA9973@salvia>
2016-08-21 21:22 ` [PATCH 4/4 V6 nft] parser: Improve syntax errors Carlos Falgueras García
2016-08-22 16:13 ` [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Pablo Neira Ayuso
3 siblings, 2 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-08-21 21:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
They checks if commands like "nft delete rule <table> <chain> <rule desc>"
works as is expected.
First one checks if command deletes only one of the matched rules.
Second one checks if command fails when rule did not found.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
.../testcases/rule_management/0010delete-by-desc_0 | 39 ++++++++++++++++++++++
.../testcases/rule_management/0011delete-by-desc_1 | 20 +++++++++++
2 files changed, 59 insertions(+)
create mode 100755 tests/shell/testcases/rule_management/0010delete-by-desc_0
create mode 100755 tests/shell/testcases/rule_management/0011delete-by-desc_1
diff --git a/tests/shell/testcases/rule_management/0010delete-by-desc_0 b/tests/shell/testcases/rule_management/0010delete-by-desc_0
new file mode 100755
index 0000000..6afdec1
--- /dev/null
+++ b/tests/shell/testcases/rule_management/0010delete-by-desc_0
@@ -0,0 +1,39 @@
+#!/bin/bash
+
+# positive tests for rule deletion by description:
+# $ nft delete rule <table> <chain> <rule description>
+
+RULE2DEL="ip saddr 1.1.1.1 counter"
+
+set -e
+$NFT add table t
+$NFT add chain t c
+$NFT add rule t c ip saddr 1.1.1.1
+$NFT add rule t c $RULE2DEL
+$NFT add rule t c ip saddr 1.1.1.1 accept
+$NFT add rule t c $RULE2DEL
+
+$NFT delete rule t c $RULE2DEL
+if [ $? -ne 0 ]; then
+ echo "E: unable to delete rule \"$RULE2DEL\"" >&2
+ exit 1
+fi
+
+set +e; # Next commands can return 0
+REMAINS_RULE2DEL=$($NFT list -a ruleset | grep -c "$RULE2DEL")
+REMAINS_RULES=$(( $($NFT list -a ruleset | wc -l) - 4 ))
+set -e
+
+if [ $REMAINS_RULE2DEL -eq 2 ]; then
+ echo "E: First rule \"$RULE2DEL\" should have been deleted" >&2
+ exit 1
+elif [ $REMAINS_RULE2DEL -eq 0 ]; then
+ echo "E: Second rule \"$RULE2DEL\" should not have been deleted" >&2
+ exit 1
+fi
+
+if [ $REMAINS_RULES -ne 3 ]; then
+ echo "E: Rest of rules should not have been deleted" >&2
+ $NFT list -a ruleset
+ exit 1
+fi
diff --git a/tests/shell/testcases/rule_management/0011delete-by-desc_1 b/tests/shell/testcases/rule_management/0011delete-by-desc_1
new file mode 100755
index 0000000..3ddb5ef
--- /dev/null
+++ b/tests/shell/testcases/rule_management/0011delete-by-desc_1
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# negative tests for rule deletion by description:
+# $ nft delete rule <table> <chain> <rule description>
+
+set -e
+$NFT add table t
+$NFT add chain t c
+$NFT add rule t c ip saddr 1.1.1.1
+$NFT add rule t c ip saddr 1.1.1.1 accept
+
+set +e; # Next command must fail
+$NFT delete rule t c ip saddr 2.2.2.2
+RET=$?
+if [ $RET -ne 1 ]; then
+ echo "E: Try to delete a nonexistent rule should throw an error" >&2
+ exit $RET
+fi
+
+exit $RET
--
2.8.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4 V6 nft] parser: Improve syntax errors
2016-08-21 21:22 [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Carlos Falgueras García
2016-08-21 21:22 ` [PATCH 2/4 V6 nft] Implement deleting rule by description Carlos Falgueras García
2016-08-21 21:22 ` [PATCH 3/4 V6 nft] test: shell: Add tests for " Carlos Falgueras García
@ 2016-08-21 21:22 ` Carlos Falgueras García
2016-08-22 16:13 ` [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Pablo Neira Ayuso
3 siblings, 0 replies; 9+ messages in thread
From: Carlos Falgueras García @ 2016-08-21 21:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Shows a more informative message when user commits a syntax error:
$ nft add rule t c handle 3 ...
<cmdline>:1:14-19: Error: Did you mean `position'?
add rule t c handle 3 ...
^^^^^^
$ nft delete rule t c position 3 ...
<cmdline>:1:17-24: Error: Did you mean `handle' or insert a rule description?
delete rule t c position 3 ...
^^^^^^^^
Adds function 'erec_del_last' that deletes last error from the error queue.
This is needed to do not show two error messages.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
include/erec.h | 5 +++++
src/parser_bison.y | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/erec.h b/include/erec.h
index 36e0efa..95ed24a 100644
--- a/include/erec.h
+++ b/include/erec.h
@@ -58,6 +58,11 @@ static inline void erec_queue(struct error_record *erec,
list_add_tail(&erec->list, queue);
}
+static inline void erec_del_last(struct list_head *queue)
+{
+ list_del(queue->prev);
+}
+
extern void erec_print(FILE *f, const struct error_record *erec);
extern void erec_print_list(FILE *f, struct list_head *list);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index beea38b..9739cb1 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1273,6 +1273,14 @@ rule_position : chain_spec
$$ = $1;
handle_merge(&$$, &$2);
}
+ | chain_spec HANDLE error
+ {
+ erec_del_last(state->msgs);
+ erec_queue(error(&@2, "Did you mean `position'?"),
+ state->msgs);
+ $$ = $1;
+ YYERROR;
+ }
;
ruleid_spec : chain_spec handle_spec
@@ -1287,6 +1295,15 @@ ruleid_spec : chain_spec handle_spec
$$ = $2;
handle_merge(&$$->handle, &$1);
}
+ | chain_spec POSITION error
+ {
+ erec_del_last(state->msgs);
+ erec_queue(error(&@2, "Did you mean `handle' or insert a rule description?"),
+ state->msgs);
+ $$ = rule_alloc(&@$, NULL);
+ handle_merge(&$$->handle, &$1);
+ YYERROR;
+ }
;
comment_spec : COMMENT string
--
2.8.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4 V6 nft] Simplify parser rule_spec tree
2016-08-21 21:22 [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Carlos Falgueras García
` (2 preceding siblings ...)
2016-08-21 21:22 ` [PATCH 4/4 V6 nft] parser: Improve syntax errors Carlos Falgueras García
@ 2016-08-22 16:13 ` Pablo Neira Ayuso
3 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22 16:13 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Sun, Aug 21, 2016 at 11:22:07PM +0200, Carlos Falgueras García wrote:
> This patch separates the rule identification from the rule localization, so
> the logic moves from the evaluator to the parser. This allows to revert the
> patch "evaluate: improve rule managment checks"
> (4176c7d30c2ff1b3f52468fc9c08b8df83f979a8) and saves a lot of code.
Applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4 V6 nft] Implement deleting rule by description
2016-08-21 21:22 ` [PATCH 2/4 V6 nft] Implement deleting rule by description Carlos Falgueras García
@ 2016-08-22 16:13 ` Pablo Neira Ayuso
0 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22 16:13 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Sun, Aug 21, 2016 at 11:22:08PM +0200, Carlos Falgueras García wrote:
> 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 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
> }
> }
> $ 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
> }
> }
>
> The parser rule 'ruleid_spec' is now of the type 'struct rule' in order to
> hold a rule description. When rule is identified with its handle a dummy
> 'struct rule' is allocated to hold the specified handle.
Applied, thanks Carlos.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4 V6 nft] test: shell: Add tests for deleting rule by description
2016-08-21 21:22 ` [PATCH 3/4 V6 nft] test: shell: Add tests for " Carlos Falgueras García
@ 2016-08-22 16:13 ` Pablo Neira Ayuso
[not found] ` <20160822162050.GA9973@salvia>
1 sibling, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22 16:13 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Sun, Aug 21, 2016 at 11:22:09PM +0200, Carlos Falgueras García wrote:
> They checks if commands like "nft delete rule <table> <chain> <rule desc>"
> works as is expected.
>
> First one checks if command deletes only one of the matched rules.
> Second one checks if command fails when rule did not found.
Also applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4 V6 nft] test: shell: Add tests for deleting rule by description
[not found] ` <20160822162050.GA9973@salvia>
@ 2016-08-22 16:21 ` Pablo Neira Ayuso
2016-08-22 16:27 ` Pablo Neira Ayuso
0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22 16:21 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
Hi Carlos,
One of this test fails... so please send me a follow up to fix it.
W: [FAILED] ./testcases/rule_management/0010delete-by-desc_0
This chunk also looks a bit strange to me.
set +e; # Next commands can return 0
REMAINS_RULE2DEL=$($NFT list -a ruleset | grep -c "$RULE2DEL")
REMAINS_RULES=$(( $($NFT list -a ruleset | wc -l) - 4 ))
set -e
Please, send me follow up patchset to fix this.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4 V6 nft] test: shell: Add tests for deleting rule by description
2016-08-22 16:21 ` Pablo Neira Ayuso
@ 2016-08-22 16:27 ` Pablo Neira Ayuso
0 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22 16:27 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Mon, Aug 22, 2016 at 06:21:46PM +0200, Pablo Neira Ayuso wrote:
> Hi Carlos,
>
> One of this test fails... so please send me a follow up to fix it.
>
> W: [FAILED] ./testcases/rule_management/0010delete-by-desc_0
>
> This chunk also looks a bit strange to me.
>
> set +e; # Next commands can return 0
> REMAINS_RULE2DEL=$($NFT list -a ruleset | grep -c "$RULE2DEL")
> REMAINS_RULES=$(( $($NFT list -a ruleset | wc -l) - 4 ))
> set -e
>
> Please, send me follow up patchset to fix this.
Also hitting this.
rule.c: In function ‘rule_find_first’:
rule.c:427:3: warning: implicit declaration of function
‘nftnl_rule_cmp’ [-Wimplicit-function-declaration]
if (nftnl_rule_cmp(nlr1, nlr2))
^
Sorry, I'm tossing this patchset, send a v7.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-08-22 16:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-21 21:22 [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Carlos Falgueras García
2016-08-21 21:22 ` [PATCH 2/4 V6 nft] Implement deleting rule by description Carlos Falgueras García
2016-08-22 16:13 ` Pablo Neira Ayuso
2016-08-21 21:22 ` [PATCH 3/4 V6 nft] test: shell: Add tests for " Carlos Falgueras García
2016-08-22 16:13 ` Pablo Neira Ayuso
[not found] ` <20160822162050.GA9973@salvia>
2016-08-22 16:21 ` Pablo Neira Ayuso
2016-08-22 16:27 ` Pablo Neira Ayuso
2016-08-21 21:22 ` [PATCH 4/4 V6 nft] parser: Improve syntax errors Carlos Falgueras García
2016-08-22 16:13 ` [PATCH 1/4 V6 nft] Simplify parser rule_spec tree Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).