* [PATCH 2/5, V2, libnftnl] rule: Implement internal expression iterator
2016-08-08 12:42 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of " Carlos Falgueras García
@ 2016-08-08 12:42 ` Carlos Falgueras García
2016-08-08 12:42 ` [PATCH 3/5, V2, libnftnl] Implement rule comparison Carlos Falgueras García
` (3 subsequent siblings)
4 siblings, 0 replies; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-08 12:42 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Introduce nftnl_expr_iter_init() to allow stack allocated iterators for
internal use.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
src/rule.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/rule.c b/src/rule.c
index 30b8b5e..35b0f29 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -29,6 +29,14 @@
#include <libnftnl/set.h>
#include <libnftnl/expr.h>
+struct nftnl_expr_iter {
+ const struct nftnl_rule *r;
+ struct nftnl_expr *cur;
+};
+
+static void nftnl_expr_iter_init(const struct nftnl_rule *r,
+ struct nftnl_expr_iter *iter);
+
struct nftnl_rule {
struct list_head head;
@@ -1025,10 +1033,16 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
}
EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
-struct nftnl_expr_iter {
- const struct nftnl_rule *r;
- struct nftnl_expr *cur;
-};
+static void nftnl_expr_iter_init(const struct nftnl_rule *r,
+ struct nftnl_expr_iter *iter)
+{
+ iter->r = r;
+ if (list_empty(&r->expr_list))
+ iter->cur = NULL;
+ else
+ iter->cur = list_entry(r->expr_list.next, struct nftnl_expr,
+ head);
+}
struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r)
{
@@ -1037,13 +1051,7 @@ struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r)
iter = calloc(1, sizeof(struct nftnl_expr_iter));
if (iter == NULL)
return NULL;
-
- iter->r = r;
- if (list_empty(&r->expr_list))
- iter->cur = NULL;
- else
- iter->cur = list_entry(r->expr_list.next, struct nftnl_expr,
- head);
+ nftnl_expr_iter_init(r, iter);
return iter;
}
--
2.8.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 3/5, V2, libnftnl] Implement rule comparison
2016-08-08 12:42 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of " Carlos Falgueras García
2016-08-08 12:42 ` [PATCH 2/5, V2, libnftnl] rule: Implement internal " Carlos Falgueras García
@ 2016-08-08 12:42 ` Carlos Falgueras García
2016-08-08 14:50 ` Pablo Neira Ayuso
2016-08-08 12:42 ` [PATCH 4/5, V2, nft] Simplify parser rule_spec tree Carlos Falgueras García
` (2 subsequent siblings)
4 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-08 12:42 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
This patch implements the function 'bool nftnl_rule_cmp(const struct
nftnl_rule *r, const struct nftnl_rule *r2)' for rule comparison.
Expressions within rules need to be compared, so also has been created the
function 'nftnl_expr_cmp' which calls new field within
'nfntl_expr_<expression>': a function pointer to a comparator. The
expressions that can be compared with memcmp have this new field set to
NULL, otherwise they have implemented a comparator.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
include/data_reg.h | 3 +++
include/expr_ops.h | 1 +
include/libnftnl/expr.h | 2 ++
include/libnftnl/rule.h | 2 ++
src/expr.c | 14 ++++++++++++++
src/expr/data_reg.c | 16 ++++++++++++++++
src/expr/dynset.c | 16 ++++++++++++++++
src/expr/immediate.c | 17 +++++++++++++++++
src/expr/log.c | 15 +++++++++++++++
src/expr/lookup.c | 14 ++++++++++++++
src/expr/match.c | 13 +++++++++++++
src/expr/target.c | 13 +++++++++++++
src/libnftnl.map | 3 +++
src/rule.c | 30 ++++++++++++++++++++++++++++++
14 files changed, 159 insertions(+)
diff --git a/include/data_reg.h b/include/data_reg.h
index e749b5b..3fec7cd 100644
--- a/include/data_reg.h
+++ b/include/data_reg.h
@@ -3,6 +3,7 @@
#include <linux/netfilter/nf_tables.h>
#include <stdint.h>
+#include <stdbool.h>
#include <unistd.h>
enum {
@@ -27,6 +28,8 @@ int nftnl_data_reg_snprintf(char *buf, size_t size,
const union nftnl_data_reg *reg,
uint32_t output_format, uint32_t flags,
int reg_type);
+bool nftnl_data_reg_cmp(const union nftnl_data_reg *r1,
+ const union nftnl_data_reg *r2, int reg_type);
struct nlattr;
int nftnl_parse_data(union nftnl_data_reg *data, struct nlattr *attr, int *type);
diff --git a/include/expr_ops.h b/include/expr_ops.h
index 3c0cb18..a334732 100644
--- a/include/expr_ops.h
+++ b/include/expr_ops.h
@@ -13,6 +13,7 @@ struct expr_ops {
uint32_t alloc_len;
int max_attr;
void (*free)(const struct nftnl_expr *e);
+ bool (*cmp)(const struct nftnl_expr *e1, const struct nftnl_expr *e2);
int (*set)(struct nftnl_expr *e, uint16_t type, const void *data, uint32_t data_len);
const void *(*get)(const struct nftnl_expr *e, uint16_t type, uint32_t *data_len);
int (*parse)(struct nftnl_expr *e, struct nlattr *attr);
diff --git a/include/libnftnl/expr.h b/include/libnftnl/expr.h
index 17f60bd..8ae6f57 100644
--- a/include/libnftnl/expr.h
+++ b/include/libnftnl/expr.h
@@ -36,6 +36,8 @@ uint32_t nftnl_expr_get_u32(const struct nftnl_expr *expr, uint16_t type);
uint64_t nftnl_expr_get_u64(const struct nftnl_expr *expr, uint16_t type);
const char *nftnl_expr_get_str(const struct nftnl_expr *expr, uint16_t type);
+bool nftnl_expr_cmp(const struct nftnl_expr *e1, const struct nftnl_expr *e2);
+
int nftnl_expr_snprintf(char *buf, size_t buflen, const struct nftnl_expr *expr, uint32_t type, uint32_t flags);
enum {
diff --git a/include/libnftnl/rule.h b/include/libnftnl/rule.h
index 09af96c..5b9612a 100644
--- a/include/libnftnl/rule.h
+++ b/include/libnftnl/rule.h
@@ -50,6 +50,8 @@ uint64_t nftnl_rule_get_u64(const struct nftnl_rule *r, uint16_t attr);
void nftnl_rule_add_expr(struct nftnl_rule *r, struct nftnl_expr *expr);
+bool nftnl_rule_cmp(const struct nftnl_rule *r1, const struct nftnl_rule *r2);
+
struct nlmsghdr;
void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh, struct nftnl_rule *t);
diff --git a/src/expr.c b/src/expr.c
index e5c1dd3..7f32055 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -203,6 +203,20 @@ const char *nftnl_expr_get_str(const struct nftnl_expr *expr, uint16_t type)
}
EXPORT_SYMBOL_ALIAS(nftnl_expr_get_str, nft_rule_expr_get_str);
+bool nftnl_expr_cmp(const struct nftnl_expr *e1, const struct nftnl_expr *e2)
+{
+ if (e1->flags != e2->flags)
+ return false;
+
+ if (strcmp(e1->ops->name, e2->ops->name))
+ return false;
+ if (e1->ops->cmp)
+ return e1->ops->cmp(e1, e2);
+ else
+ return !memcmp(e1->data, e2->data, e1->ops->alloc_len);
+}
+EXPORT_SYMBOL(nftnl_expr_cmp);
+
void
nftnl_expr_build_payload(struct nlmsghdr *nlh, struct nftnl_expr *expr)
{
diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
index 688823b..a954e95 100644
--- a/src/expr/data_reg.c
+++ b/src/expr/data_reg.c
@@ -379,6 +379,22 @@ int nftnl_data_reg_snprintf(char *buf, size_t size,
return -1;
}
+bool nftnl_data_reg_cmp(const union nftnl_data_reg *r1,
+ const union nftnl_data_reg *r2, int reg_type)
+{
+ switch (reg_type) {
+ case DATA_VALUE:
+ return r1->len == r2->len &&
+ !memcmp(r1->val, r2->val, r1->len);
+ case DATA_VERDICT:
+ case DATA_CHAIN:
+ return r1->verdict == r2->verdict &&
+ !strcmp(r1->chain, r2->chain);
+ default:
+ return false;
+ }
+}
+
static int nftnl_data_parse_cb(const struct nlattr *attr, void *data)
{
const struct nlattr **tb = data;
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index 0eaa409..83311f1 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -370,11 +370,27 @@ static void nftnl_expr_dynset_free(const struct nftnl_expr *e)
xfree(dynset->set_name);
}
+static bool nftnl_expr_dynset_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_dynset *d1 = nftnl_expr_data(e1);
+ struct nftnl_expr_dynset *d2 = nftnl_expr_data(e2);
+
+ return d1->sreg_key == d2->sreg_key &&
+ d1->sreg_data == d2->sreg_data &&
+ d1->op == d2->op &&
+ d1->timeout == d2->timeout &&
+ nftnl_expr_cmp(d1->expr, d2->expr) &&
+ !strcmp(d1->set_name, d2->set_name) &&
+ d1->set_id == d2->set_id;
+}
+
struct expr_ops expr_ops_dynset = {
.name = "dynset",
.alloc_len = sizeof(struct nftnl_expr_dynset),
.max_attr = NFTA_DYNSET_MAX,
.free = nftnl_expr_dynset_free,
+ .cmp = nftnl_expr_dynset_cmp,
.set = nftnl_expr_dynset_set,
.get = nftnl_expr_dynset_get,
.parse = nftnl_expr_dynset_parse,
diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index 22ec864..ec0613d 100644
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -320,11 +320,28 @@ static void nftnl_expr_immediate_free(const struct nftnl_expr *e)
nftnl_free_verdict(&imm->data);
}
+static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_immediate *i1 = nftnl_expr_data(e1);
+ struct nftnl_expr_immediate *i2 = nftnl_expr_data(e2);
+ int reg_type;
+
+ if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
+ reg_type = DATA_VERDICT;
+ else
+ reg_type = DATA_VALUE;
+
+ return i1->dreg == i2->dreg &&
+ nftnl_data_reg_cmp(&i1->data, &i2->data, reg_type);
+}
+
struct expr_ops expr_ops_immediate = {
.name = "immediate",
.alloc_len = sizeof(struct nftnl_expr_immediate),
.max_attr = NFTA_IMMEDIATE_MAX,
.free = nftnl_expr_immediate_free,
+ .cmp = nftnl_expr_immediate_cmp,
.set = nftnl_expr_immediate_set,
.get = nftnl_expr_immediate_get,
.parse = nftnl_expr_immediate_parse,
diff --git a/src/expr/log.c b/src/expr/log.c
index b9b3951..d8e2572 100644
--- a/src/expr/log.c
+++ b/src/expr/log.c
@@ -336,11 +336,26 @@ static void nftnl_expr_log_free(const struct nftnl_expr *e)
xfree(log->prefix);
}
+static bool nftnl_expr_log_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_log *l1 = nftnl_expr_data(e1);
+ struct nftnl_expr_log *l2 = nftnl_expr_data(e2);
+
+ return l1->snaplen == l2->snaplen &&
+ l1->group == l2->group &&
+ l1->qthreshold == l2->qthreshold &&
+ l1->level == l2->level &&
+ l1->flags == l2->flags &&
+ !strcmp(l1->prefix, l2->prefix);
+}
+
struct expr_ops expr_ops_log = {
.name = "log",
.alloc_len = sizeof(struct nftnl_expr_log),
.max_attr = NFTA_LOG_MAX,
.free = nftnl_expr_log_free,
+ .cmp = nftnl_expr_log_cmp,
.set = nftnl_expr_log_set,
.get = nftnl_expr_log_get,
.parse = nftnl_expr_log_parse,
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index 97478c2..5e5d3d4 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -296,11 +296,25 @@ static void nftnl_expr_lookup_free(const struct nftnl_expr *e)
xfree(lookup->set_name);
}
+static bool nftnl_expr_lookup_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_lookup *l1 = nftnl_expr_data(e1);
+ struct nftnl_expr_lookup *l2 = nftnl_expr_data(e2);
+
+ return l1->sreg == l2->sreg &&
+ l1->dreg == l2->dreg &&
+ !strcmp(l1->set_name, l2->set_name) &&
+ l1->set_id == l2->set_id &&
+ l1->flags == l2->flags;
+}
+
struct expr_ops expr_ops_lookup = {
.name = "lookup",
.alloc_len = sizeof(struct nftnl_expr_lookup),
.max_attr = NFTA_LOOKUP_MAX,
.free = nftnl_expr_lookup_free,
+ .cmp = nftnl_expr_lookup_cmp,
.set = nftnl_expr_lookup_set,
.get = nftnl_expr_lookup_get,
.parse = nftnl_expr_lookup_parse,
diff --git a/src/expr/match.c b/src/expr/match.c
index 3342e2c..8d9001f 100644
--- a/src/expr/match.c
+++ b/src/expr/match.c
@@ -240,11 +240,24 @@ static void nftnl_expr_match_free(const struct nftnl_expr *e)
xfree(match->data);
}
+static bool nftnl_expr_match_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_match *m1 = nftnl_expr_data(e1);
+ struct nftnl_expr_match *m2 = nftnl_expr_data(e2);
+
+ return !strcmp(m1->name, m2->name) &&
+ m1->rev == m2->rev &&
+ m1->data_len == m2->data_len &&
+ !memcmp(m1->data, m2->data, m1->data_len);
+}
+
struct expr_ops expr_ops_match = {
.name = "match",
.alloc_len = sizeof(struct nftnl_expr_match),
.max_attr = NFTA_MATCH_MAX,
.free = nftnl_expr_match_free,
+ .cmp = nftnl_expr_match_cmp,
.set = nftnl_expr_match_set,
.get = nftnl_expr_match_get,
.parse = nftnl_expr_match_parse,
diff --git a/src/expr/target.c b/src/expr/target.c
index d4c0091..55a0af3 100644
--- a/src/expr/target.c
+++ b/src/expr/target.c
@@ -241,11 +241,24 @@ static void nftnl_expr_target_free(const struct nftnl_expr *e)
xfree(target->data);
}
+static bool nftnl_expr_target_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_target *t1 = nftnl_expr_data(e1);
+ struct nftnl_expr_target *t2 = nftnl_expr_data(e2);
+
+ return !strcmp(t1->name, t2->name) &&
+ t1->rev == t2->rev &&
+ t1->data_len == t2->data_len &&
+ !memcmp(t1->data, t2->data, t1->data_len);
+}
+
struct expr_ops expr_ops_target = {
.name = "target",
.alloc_len = sizeof(struct nftnl_expr_target),
.max_attr = NFTA_TARGET_MAX,
.free = nftnl_expr_target_free,
+ .cmp = nftnl_expr_target_cmp,
.set = nftnl_expr_target_set,
.get = nftnl_expr_target_get,
.parse = nftnl_expr_target_parse,
diff --git a/src/libnftnl.map b/src/libnftnl.map
index c38e081..faaa90d 100644
--- a/src/libnftnl.map
+++ b/src/libnftnl.map
@@ -527,4 +527,7 @@ LIBNFTNL_4.1 {
nftnl_udata_get;
nftnl_udata_next;
nftnl_udata_parse;
+
+ nftnl_rule_cmp;
+ nftnl_expr_cmp;
} LIBNFTNL_4;
diff --git a/src/rule.c b/src/rule.c
index 35b0f29..32a780c 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -339,6 +339,36 @@ void nftnl_rule_add_expr(struct nftnl_rule *r, struct nftnl_expr *expr)
}
EXPORT_SYMBOL_ALIAS(nftnl_rule_add_expr, nft_rule_add_expr);
+bool nftnl_rule_cmp(const struct nftnl_rule *r1, const struct nftnl_rule *r2)
+{
+ struct nftnl_expr_iter it1, it2;
+ struct nftnl_expr *e1, *e2;
+ unsigned int eq = 1;
+
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_TABLE))
+ eq &= !strcmp(r1->table, r2->table);
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_CHAIN))
+ eq &= !strcmp(r1->chain, r2->chain);
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_COMPAT_FLAGS))
+ eq &= r1->compat.flags == r2->compat.flags;
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_COMPAT_PROTO))
+ eq &= r1->compat.proto == r2->compat.proto;
+
+ nftnl_expr_iter_init(r1, &it1);
+ nftnl_expr_iter_init(r2, &it2);
+ e1 = nftnl_expr_iter_next(&it1);
+ e2 = nftnl_expr_iter_next(&it2);
+ while (eq && e1 && e2) {
+ eq = nftnl_expr_cmp(e1, e2);
+
+ e1 = nftnl_expr_iter_next(&it1);
+ e2 = nftnl_expr_iter_next(&it2);
+ }
+
+ return eq;
+}
+EXPORT_SYMBOL(nftnl_rule_cmp);
+
static int nftnl_rule_parse_attr_cb(const struct nlattr *attr, void *data)
{
const struct nlattr **tb = data;
--
2.8.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 3/5, V2, libnftnl] Implement rule comparison
2016-08-08 12:42 ` [PATCH 3/5, V2, libnftnl] Implement rule comparison Carlos Falgueras García
@ 2016-08-08 14:50 ` Pablo Neira Ayuso
0 siblings, 0 replies; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-08 14:50 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Mon, Aug 08, 2016 at 02:42:33PM +0200, Carlos Falgueras García wrote:
> diff --git a/src/libnftnl.map b/src/libnftnl.map
> index c38e081..faaa90d 100644
> --- a/src/libnftnl.map
> +++ b/src/libnftnl.map
> @@ -527,4 +527,7 @@ LIBNFTNL_4.1 {
> nftnl_udata_get;
> nftnl_udata_next;
> nftnl_udata_parse;
> +
> + nftnl_rule_cmp;
> + nftnl_expr_cmp;
You have to place this in LIBNFTNL_4.2, but LIBNFTNL_4.2 should
inherit from LIBNFTNL_4.1.
> } LIBNFTNL_4;
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 4/5, V2, nft] Simplify parser rule_spec tree
2016-08-08 12:42 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of " Carlos Falgueras García
2016-08-08 12:42 ` [PATCH 2/5, V2, libnftnl] rule: Implement internal " Carlos Falgueras García
2016-08-08 12:42 ` [PATCH 3/5, V2, libnftnl] Implement rule comparison Carlos Falgueras García
@ 2016-08-08 12:42 ` Carlos Falgueras García
2016-08-08 14:54 ` Pablo Neira Ayuso
2016-08-08 12:42 ` [PATCH 5/5, V2, nft] Implement deleting rule by description Carlos Falgueras García
2016-08-08 14:48 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of expression iterator Pablo Neira Ayuso
4 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-08 12:42 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 | 51 +++++++++++++++++++++-------------------
2 files changed, 28 insertions(+), 91 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index f24e5f3..4611969 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -65,12 +65,6 @@ static int __fmtstring(4, 5) __stmt_binary_error(struct eval_ctx *ctx,
__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,
@@ -2394,68 +2388,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));
@@ -2636,11 +2573,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 d7cba23..8e83970 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
@@ -718,11 +715,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);
}
@@ -777,7 +774,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);
}
@@ -1250,35 +1247,41 @@ 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);
+ }
+ | chain_spec error
+ {
+ erec_queue(error(&@2, "Expected `position' or nothing"),
+ state->msgs);
+ $$ = $1;
}
;
-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] 32+ messages in thread* Re: [PATCH 4/5, V2, nft] Simplify parser rule_spec tree
2016-08-08 12:42 ` [PATCH 4/5, V2, nft] Simplify parser rule_spec tree Carlos Falgueras García
@ 2016-08-08 14:54 ` Pablo Neira Ayuso
2016-08-08 17:02 ` Carlos Falgueras García
0 siblings, 1 reply; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-08 14:54 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Mon, Aug 08, 2016 at 02:42:34PM +0200, Carlos Falgueras García wrote:
> @@ -1250,35 +1247,41 @@ 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);
> + }
> + | chain_spec error
> + {
> + erec_queue(error(&@2, "Expected `position' or nothing"),
In what case we display this error message after this patch?
Please, include examples in your patch description.
You can also explicitly indicate that tests pass succesfully.
> + state->msgs);
> + $$ = $1;
> }
> ;
>
> -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 [flat|nested] 32+ messages in thread* Re: [PATCH 4/5, V2, nft] Simplify parser rule_spec tree
2016-08-08 14:54 ` Pablo Neira Ayuso
@ 2016-08-08 17:02 ` Carlos Falgueras García
0 siblings, 0 replies; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-08 17:02 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On 08/08/2016 04:54 PM, Pablo Neira Ayuso wrote:
> On Mon, Aug 08, 2016 at 02:42:34PM +0200, Carlos Falgueras García wrote:
>> -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);
>> + }
>> + | chain_spec error
>> + {
>> + erec_queue(error(&@2, "Expected `position' or nothing"),
>
> In what case we display this error message after this patch?
When user try to use 'handle' with CMD_INSERT or CMD_ADD. The error was
"use only `position' instead"
> Please, include examples in your patch description.
Ok, I'll do.
> You can also explicitly indicate that tests pass succesfully.
At V3 all tests will pass succesfully.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 5/5, V2, nft] Implement deleting rule by description
2016-08-08 12:42 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of " Carlos Falgueras García
` (2 preceding siblings ...)
2016-08-08 12:42 ` [PATCH 4/5, V2, nft] Simplify parser rule_spec tree Carlos Falgueras García
@ 2016-08-08 12:42 ` Carlos Falgueras García
2016-08-08 14:46 ` Pablo Neira Ayuso
2016-08-08 14:48 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of expression iterator Pablo Neira Ayuso
4 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-08 12:42 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
$ 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:
$ 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
}
}
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;
case CMD_OBJ_SET:
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);
+ break;
case CMD_OBJ_CHAIN:
case CMD_OBJ_TABLE:
return 0;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 8e83970..096a22a 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,7 +745,7 @@ add_cmd : TABLE table_spec
replace_cmd : RULE ruleid_spec rule
{
- $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2, &@$, $3);
+ $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2->handle, &@$, $3);
}
;
@@ -790,7 +790,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
{
@@ -1280,8 +1280,22 @@ 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);
+ }
+ | chain_spec error
+ {
+ erec_queue(error(&@2, "Expected `handle' or rule description"),
+ state->msgs);
+ $$ = rule_alloc(&@$, NULL);
+ handle_merge(&$$->handle, &$1);
}
;
diff --git a/src/rule.c b/src/rule.c
index 14e57f2..3f15ee5 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -402,6 +402,32 @@ void rule_print(const struct rule *rule)
printf(" # handle %" PRIu64, rule->handle.handle.id);
}
+static struct rule *search_first_rule(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 +1036,22 @@ 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 = search_first_rule(cmd->rule);
+ if (!rule)
+ return 0;
+ 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 +1060,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] 32+ messages in thread* Re: [PATCH 5/5, V2, nft] Implement deleting rule by description
2016-08-08 12:42 ` [PATCH 5/5, V2, nft] Implement deleting rule by description Carlos Falgueras García
@ 2016-08-08 14:46 ` Pablo Neira Ayuso
0 siblings, 0 replies; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-08 14:46 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
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;
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of expression iterator
2016-08-08 12:42 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of " Carlos Falgueras García
` (3 preceding siblings ...)
2016-08-08 12:42 ` [PATCH 5/5, V2, nft] Implement deleting rule by description Carlos Falgueras García
@ 2016-08-08 14:48 ` Pablo Neira Ayuso
2016-08-08 18:10 ` [PATCH] rule: Constify rule iterators Carlos Falgueras García
2016-08-10 9:48 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Carlos Falgueras García
4 siblings, 2 replies; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-08 14:48 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Mon, Aug 08, 2016 at 02:42:31PM +0200, Carlos Falgueras García wrote:
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
> include/libnftnl/rule.h | 2 +-
> src/rule.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/libnftnl/rule.h b/include/libnftnl/rule.h
> index 2776a77..09af96c 100644
> --- a/include/libnftnl/rule.h
> +++ b/include/libnftnl/rule.h
> @@ -70,7 +70,7 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
>
> struct nftnl_expr_iter;
>
> -struct nftnl_expr_iter *nftnl_expr_iter_create(struct nftnl_rule *r);
> +struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r);
Good point, other iterators need this too.
Some please send a patch constifying them all these nftnl_*_iter
functions.
Once this patch is applied, then you send a v3 of this patches. Try to
deliver updates that have dependencies in smaller batches.
Thanks Carlos.
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH] rule: Constify rule iterators
2016-08-08 14:48 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of expression iterator Pablo Neira Ayuso
@ 2016-08-08 18:10 ` Carlos Falgueras García
2016-08-09 7:17 ` Pablo Neira Ayuso
2016-08-10 9:48 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Carlos Falgueras García
1 sibling, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-08 18:10 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Iterators do not modify objects which they iterate, so input pointer must
be const.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
include/libnftnl/rule.h | 8 ++++----
src/rule.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/libnftnl/rule.h b/include/libnftnl/rule.h
index 2776a77..e3bd6b8 100644
--- a/include/libnftnl/rule.h
+++ b/include/libnftnl/rule.h
@@ -70,7 +70,7 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
struct nftnl_expr_iter;
-struct nftnl_expr_iter *nftnl_expr_iter_create(struct nftnl_rule *r);
+struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r);
struct nftnl_expr *nftnl_expr_iter_next(struct nftnl_expr_iter *iter);
void nftnl_expr_iter_destroy(struct nftnl_expr_iter *iter);
@@ -86,7 +86,7 @@ int nftnl_rule_list_foreach(struct nftnl_rule_list *rule_list, int (*cb)(struct
struct nftnl_rule_list_iter;
-struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(struct nftnl_rule_list *l);
+struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(const struct nftnl_rule_list *l);
struct nftnl_rule *nftnl_rule_list_iter_cur(struct nftnl_rule_list_iter *iter);
struct nftnl_rule *nftnl_rule_list_iter_next(struct nftnl_rule_list_iter *iter);
void nftnl_rule_list_iter_destroy(const struct nftnl_rule_list_iter *iter);
@@ -153,7 +153,7 @@ int nft_rule_expr_foreach(struct nft_rule *r,
struct nft_rule_expr_iter;
-struct nft_rule_expr_iter *nft_rule_expr_iter_create(struct nft_rule *r);
+struct nft_rule_expr_iter *nft_rule_expr_iter_create(const struct nft_rule *r);
struct nft_rule_expr *nft_rule_expr_iter_next(struct nft_rule_expr_iter *iter);
void nft_rule_expr_iter_destroy(struct nft_rule_expr_iter *iter);
@@ -169,7 +169,7 @@ int nft_rule_list_foreach(struct nft_rule_list *rule_list, int (*cb)(struct nft_
struct nft_rule_list_iter;
-struct nft_rule_list_iter *nft_rule_list_iter_create(struct nft_rule_list *l);
+struct nft_rule_list_iter *nft_rule_list_iter_create(const struct nft_rule_list *l);
struct nft_rule *nft_rule_list_iter_cur(struct nft_rule_list_iter *iter);
struct nft_rule *nft_rule_list_iter_next(struct nft_rule_list_iter *iter);
void nft_rule_list_iter_destroy(struct nft_rule_list_iter *iter);
diff --git a/src/rule.c b/src/rule.c
index a0edca7..11fceca 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1026,11 +1026,11 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
struct nftnl_expr_iter {
- struct nftnl_rule *r;
+ const struct nftnl_rule *r;
struct nftnl_expr *cur;
};
-struct nftnl_expr_iter *nftnl_expr_iter_create(struct nftnl_rule *r)
+struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r)
{
struct nftnl_expr_iter *iter;
@@ -1142,11 +1142,11 @@ int nftnl_rule_list_foreach(struct nftnl_rule_list *rule_list,
EXPORT_SYMBOL_ALIAS(nftnl_rule_list_foreach, nft_rule_list_foreach);
struct nftnl_rule_list_iter {
- struct nftnl_rule_list *list;
+ const struct nftnl_rule_list *list;
struct nftnl_rule *cur;
};
-struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(struct nftnl_rule_list *l)
+struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(const struct nftnl_rule_list *l)
{
struct nftnl_rule_list_iter *iter;
--
2.8.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH] rule: Constify rule iterators
2016-08-08 18:10 ` [PATCH] rule: Constify rule iterators Carlos Falgueras García
@ 2016-08-09 7:17 ` Pablo Neira Ayuso
2016-08-09 11:42 ` [PATCH, v2] Constify iterators Carlos Falgueras García
0 siblings, 1 reply; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-09 7:17 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Mon, Aug 08, 2016 at 08:10:16PM +0200, Carlos Falgueras García wrote:
> Iterators do not modify objects which they iterate, so input pointer must
> be const.
Please, constify other iterators: chain, set, set_elem, table, and so
on. So we get this code in sync too.
Thanks.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH, v2] Constify iterators
2016-08-09 7:17 ` Pablo Neira Ayuso
@ 2016-08-09 11:42 ` Carlos Falgueras García
2016-08-10 8:30 ` Pablo Neira Ayuso
0 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-09 11:42 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Iterators do not modify objects which they iterate, so input pointer must
be const.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
include/libnftnl/chain.h | 4 ++--
include/libnftnl/rule.h | 8 ++++----
include/libnftnl/set.h | 16 ++++++++--------
include/libnftnl/table.h | 4 ++--
src/chain.c | 6 +++---
src/rule.c | 8 ++++----
src/set.c | 6 +++---
src/set_elem.c | 10 +++++-----
src/table.c | 6 +++---
9 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/include/libnftnl/chain.h b/include/libnftnl/chain.h
index ed21e48..26460a6 100644
--- a/include/libnftnl/chain.h
+++ b/include/libnftnl/chain.h
@@ -81,7 +81,7 @@ void nftnl_chain_list_del(struct nftnl_chain *c);
struct nftnl_chain_list_iter;
-struct nftnl_chain_list_iter *nftnl_chain_list_iter_create(struct nftnl_chain_list *l);
+struct nftnl_chain_list_iter *nftnl_chain_list_iter_create(const struct nftnl_chain_list *l);
struct nftnl_chain *nftnl_chain_list_iter_next(struct nftnl_chain_list_iter *iter);
void nftnl_chain_list_iter_destroy(struct nftnl_chain_list_iter *iter);
@@ -158,7 +158,7 @@ void nft_chain_list_del(struct nft_chain *c);
struct nft_chain_list_iter;
-struct nft_chain_list_iter *nft_chain_list_iter_create(struct nft_chain_list *l);
+struct nft_chain_list_iter *nft_chain_list_iter_create(const struct nft_chain_list *l);
struct nft_chain *nft_chain_list_iter_next(struct nft_chain_list_iter *iter);
void nft_chain_list_iter_destroy(struct nft_chain_list_iter *iter);
diff --git a/include/libnftnl/rule.h b/include/libnftnl/rule.h
index 2776a77..e3bd6b8 100644
--- a/include/libnftnl/rule.h
+++ b/include/libnftnl/rule.h
@@ -70,7 +70,7 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
struct nftnl_expr_iter;
-struct nftnl_expr_iter *nftnl_expr_iter_create(struct nftnl_rule *r);
+struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r);
struct nftnl_expr *nftnl_expr_iter_next(struct nftnl_expr_iter *iter);
void nftnl_expr_iter_destroy(struct nftnl_expr_iter *iter);
@@ -86,7 +86,7 @@ int nftnl_rule_list_foreach(struct nftnl_rule_list *rule_list, int (*cb)(struct
struct nftnl_rule_list_iter;
-struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(struct nftnl_rule_list *l);
+struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(const struct nftnl_rule_list *l);
struct nftnl_rule *nftnl_rule_list_iter_cur(struct nftnl_rule_list_iter *iter);
struct nftnl_rule *nftnl_rule_list_iter_next(struct nftnl_rule_list_iter *iter);
void nftnl_rule_list_iter_destroy(const struct nftnl_rule_list_iter *iter);
@@ -153,7 +153,7 @@ int nft_rule_expr_foreach(struct nft_rule *r,
struct nft_rule_expr_iter;
-struct nft_rule_expr_iter *nft_rule_expr_iter_create(struct nft_rule *r);
+struct nft_rule_expr_iter *nft_rule_expr_iter_create(const struct nft_rule *r);
struct nft_rule_expr *nft_rule_expr_iter_next(struct nft_rule_expr_iter *iter);
void nft_rule_expr_iter_destroy(struct nft_rule_expr_iter *iter);
@@ -169,7 +169,7 @@ int nft_rule_list_foreach(struct nft_rule_list *rule_list, int (*cb)(struct nft_
struct nft_rule_list_iter;
-struct nft_rule_list_iter *nft_rule_list_iter_create(struct nft_rule_list *l);
+struct nft_rule_list_iter *nft_rule_list_iter_create(const struct nft_rule_list *l);
struct nft_rule *nft_rule_list_iter_cur(struct nft_rule_list_iter *iter);
struct nft_rule *nft_rule_list_iter_next(struct nft_rule_list_iter *iter);
void nft_rule_list_iter_destroy(struct nft_rule_list_iter *iter);
diff --git a/include/libnftnl/set.h b/include/libnftnl/set.h
index 5266b6f..adeb16c 100644
--- a/include/libnftnl/set.h
+++ b/include/libnftnl/set.h
@@ -71,8 +71,8 @@ void nftnl_set_list_del(struct nftnl_set *s);
int nftnl_set_list_foreach(struct nftnl_set_list *set_list, int (*cb)(struct nftnl_set *t, void *data), void *data);
struct nftnl_set_list_iter;
-struct nftnl_set_list_iter *nftnl_set_list_iter_create(struct nftnl_set_list *l);
-struct nftnl_set *nftnl_set_list_iter_cur(struct nftnl_set_list_iter *iter);
+struct nftnl_set_list_iter *nftnl_set_list_iter_create(const struct nftnl_set_list *l);
+struct nftnl_set *nftnl_set_list_iter_cur(const struct nftnl_set_list_iter *iter);
struct nftnl_set *nftnl_set_list_iter_next(struct nftnl_set_list_iter *iter);
void nftnl_set_list_iter_destroy(const struct nftnl_set_list_iter *iter);
@@ -133,8 +133,8 @@ int nftnl_set_elem_fprintf(FILE *fp, struct nftnl_set_elem *se, uint32_t type, u
int nftnl_set_elem_foreach(struct nftnl_set *s, int (*cb)(struct nftnl_set_elem *e, void *data), void *data);
struct nftnl_set_elems_iter;
-struct nftnl_set_elems_iter *nftnl_set_elems_iter_create(struct nftnl_set *s);
-struct nftnl_set_elem *nftnl_set_elems_iter_cur(struct nftnl_set_elems_iter *iter);
+struct nftnl_set_elems_iter *nftnl_set_elems_iter_create(const struct nftnl_set *s);
+struct nftnl_set_elem *nftnl_set_elems_iter_cur(const struct nftnl_set_elems_iter *iter);
struct nftnl_set_elem *nftnl_set_elems_iter_next(struct nftnl_set_elems_iter *iter);
void nftnl_set_elems_iter_destroy(struct nftnl_set_elems_iter *iter);
@@ -207,8 +207,8 @@ void nft_set_list_del(struct nft_set *s);
int nft_set_list_foreach(struct nft_set_list *set_list, int (*cb)(struct nft_set *t, void *data), void *data);
struct nft_set_list_iter;
-struct nft_set_list_iter *nft_set_list_iter_create(struct nft_set_list *l);
-struct nft_set *nft_set_list_iter_cur(struct nft_set_list_iter *iter);
+struct nft_set_list_iter *nft_set_list_iter_create(const struct nft_set_list *l);
+struct nft_set *nft_set_list_iter_cur(const struct nft_set_list_iter *iter);
struct nft_set *nft_set_list_iter_next(struct nft_set_list_iter *iter);
void nft_set_list_iter_destroy(struct nft_set_list_iter *iter);
@@ -269,8 +269,8 @@ int nft_set_elem_fprintf(FILE *fp, struct nft_set_elem *se, uint32_t type, uint3
int nft_set_elem_foreach(struct nft_set *s, int (*cb)(struct nft_set_elem *e, void *data), void *data);
struct nft_set_elems_iter;
-struct nft_set_elems_iter *nft_set_elems_iter_create(struct nft_set *s);
-struct nft_set_elem *nft_set_elems_iter_cur(struct nft_set_elems_iter *iter);
+struct nft_set_elems_iter *nft_set_elems_iter_create(const struct nft_set *s);
+struct nft_set_elem *nft_set_elems_iter_cur(const struct nft_set_elems_iter *iter);
struct nft_set_elem *nft_set_elems_iter_next(struct nft_set_elems_iter *iter);
void nft_set_elems_iter_destroy(struct nft_set_elems_iter *iter);
diff --git a/include/libnftnl/table.h b/include/libnftnl/table.h
index 1f9ab1c..296667b 100644
--- a/include/libnftnl/table.h
+++ b/include/libnftnl/table.h
@@ -69,7 +69,7 @@ void nftnl_table_list_del(struct nftnl_table *r);
struct nftnl_table_list_iter;
-struct nftnl_table_list_iter *nftnl_table_list_iter_create(struct nftnl_table_list *l);
+struct nftnl_table_list_iter *nftnl_table_list_iter_create(const struct nftnl_table_list *l);
struct nftnl_table *nftnl_table_list_iter_next(struct nftnl_table_list_iter *iter);
void nftnl_table_list_iter_destroy(const struct nftnl_table_list_iter *iter);
@@ -134,7 +134,7 @@ void nft_table_list_del(struct nft_table *r);
struct nft_table_list_iter;
-struct nft_table_list_iter *nft_table_list_iter_create(struct nft_table_list *l);
+struct nft_table_list_iter *nft_table_list_iter_create(const struct nft_table_list *l);
struct nft_table *nft_table_list_iter_next(struct nft_table_list_iter *iter);
void nft_table_list_iter_destroy(struct nft_table_list_iter *iter);
diff --git a/src/chain.c b/src/chain.c
index 4c562fe..2c6ae70 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -1039,11 +1039,11 @@ int nftnl_chain_list_foreach(struct nftnl_chain_list *chain_list,
EXPORT_SYMBOL_ALIAS(nftnl_chain_list_foreach, nft_chain_list_foreach);
struct nftnl_chain_list_iter {
- struct nftnl_chain_list *list;
- struct nftnl_chain *cur;
+ const struct nftnl_chain_list *list;
+ struct nftnl_chain *cur;
};
-struct nftnl_chain_list_iter *nftnl_chain_list_iter_create(struct nftnl_chain_list *l)
+struct nftnl_chain_list_iter *nftnl_chain_list_iter_create(const struct nftnl_chain_list *l)
{
struct nftnl_chain_list_iter *iter;
diff --git a/src/rule.c b/src/rule.c
index a0edca7..11fceca 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1026,11 +1026,11 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
struct nftnl_expr_iter {
- struct nftnl_rule *r;
+ const struct nftnl_rule *r;
struct nftnl_expr *cur;
};
-struct nftnl_expr_iter *nftnl_expr_iter_create(struct nftnl_rule *r)
+struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r)
{
struct nftnl_expr_iter *iter;
@@ -1142,11 +1142,11 @@ int nftnl_rule_list_foreach(struct nftnl_rule_list *rule_list,
EXPORT_SYMBOL_ALIAS(nftnl_rule_list_foreach, nft_rule_list_foreach);
struct nftnl_rule_list_iter {
- struct nftnl_rule_list *list;
+ const struct nftnl_rule_list *list;
struct nftnl_rule *cur;
};
-struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(struct nftnl_rule_list *l)
+struct nftnl_rule_list_iter *nftnl_rule_list_iter_create(const struct nftnl_rule_list *l)
{
struct nftnl_rule_list_iter *iter;
diff --git a/src/set.c b/src/set.c
index 8a592db..19ab371 100644
--- a/src/set.c
+++ b/src/set.c
@@ -1175,11 +1175,11 @@ int nftnl_set_list_foreach(struct nftnl_set_list *set_list,
EXPORT_SYMBOL_ALIAS(nftnl_set_list_foreach, nft_set_list_foreach);
struct nftnl_set_list_iter {
- struct nftnl_set_list *list;
+ const struct nftnl_set_list *list;
struct nftnl_set *cur;
};
-struct nftnl_set_list_iter *nftnl_set_list_iter_create(struct nftnl_set_list *l)
+struct nftnl_set_list_iter *nftnl_set_list_iter_create(const struct nftnl_set_list *l)
{
struct nftnl_set_list_iter *iter;
@@ -1197,7 +1197,7 @@ struct nftnl_set_list_iter *nftnl_set_list_iter_create(struct nftnl_set_list *l)
}
EXPORT_SYMBOL_ALIAS(nftnl_set_list_iter_create, nft_set_list_iter_create);
-struct nftnl_set *nftnl_set_list_iter_cur(struct nftnl_set_list_iter *iter)
+struct nftnl_set *nftnl_set_list_iter_cur(const struct nftnl_set_list_iter *iter)
{
return iter->cur;
}
diff --git a/src/set_elem.c b/src/set_elem.c
index 7908661..ecda18f 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -274,7 +274,7 @@ void nftnl_set_elem_nlmsg_build_payload(struct nlmsghdr *nlh,
}
static void nftnl_set_elem_nlmsg_build_def(struct nlmsghdr *nlh,
- struct nftnl_set *s)
+ const struct nftnl_set *s)
{
if (s->flags & (1 << NFTNL_SET_NAME))
mnl_attr_put_strz(nlh, NFTA_SET_ELEM_LIST_SET, s->name);
@@ -840,12 +840,12 @@ int nftnl_set_elem_foreach(struct nftnl_set *s,
EXPORT_SYMBOL_ALIAS(nftnl_set_elem_foreach, nft_set_elem_foreach);
struct nftnl_set_elems_iter {
- struct nftnl_set *set;
- struct list_head *list;
+ const struct nftnl_set *set;
+ const struct list_head *list;
struct nftnl_set_elem *cur;
};
-struct nftnl_set_elems_iter *nftnl_set_elems_iter_create(struct nftnl_set *s)
+struct nftnl_set_elems_iter *nftnl_set_elems_iter_create(const struct nftnl_set *s)
{
struct nftnl_set_elems_iter *iter;
@@ -865,7 +865,7 @@ struct nftnl_set_elems_iter *nftnl_set_elems_iter_create(struct nftnl_set *s)
}
EXPORT_SYMBOL_ALIAS(nftnl_set_elems_iter_create, nft_set_elems_iter_create);
-struct nftnl_set_elem *nftnl_set_elems_iter_cur(struct nftnl_set_elems_iter *iter)
+struct nftnl_set_elem *nftnl_set_elems_iter_cur(const struct nftnl_set_elems_iter *iter)
{
return iter->cur;
}
diff --git a/src/table.c b/src/table.c
index 3d4d7b9..55ec4e1 100644
--- a/src/table.c
+++ b/src/table.c
@@ -544,11 +544,11 @@ int nftnl_table_list_foreach(struct nftnl_table_list *table_list,
EXPORT_SYMBOL_ALIAS(nftnl_table_list_foreach, nft_table_list_foreach);
struct nftnl_table_list_iter {
- struct nftnl_table_list *list;
- struct nftnl_table *cur;
+ const struct nftnl_table_list *list;
+ struct nftnl_table *cur;
};
-struct nftnl_table_list_iter *nftnl_table_list_iter_create(struct nftnl_table_list *l)
+struct nftnl_table_list_iter *nftnl_table_list_iter_create(const struct nftnl_table_list *l)
{
struct nftnl_table_list_iter *iter;
--
2.8.3
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator
2016-08-08 14:48 ` [PATCH 1/5, V2, libnftnl] rule: Add const modifier to rule field of expression iterator Pablo Neira Ayuso
2016-08-08 18:10 ` [PATCH] rule: Constify rule iterators Carlos Falgueras García
@ 2016-08-10 9:48 ` Carlos Falgueras García
2016-08-10 9:48 ` [PATCH 2/4, V3, libnftnl] Implement rule comparison Carlos Falgueras García
` (3 more replies)
1 sibling, 4 replies; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-10 9:48 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Introduce nftnl_expr_iter_init() to allow stack allocated iterators for
internal use.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
src/rule.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/rule.c b/src/rule.c
index 11fceca..69ffc7e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -29,6 +29,14 @@
#include <libnftnl/set.h>
#include <libnftnl/expr.h>
+struct nftnl_expr_iter {
+ const struct nftnl_rule *r;
+ struct nftnl_expr *cur;
+};
+
+static void nftnl_expr_iter_init(const struct nftnl_rule *r,
+ struct nftnl_expr_iter *iter);
+
struct nftnl_rule {
struct list_head head;
@@ -1025,10 +1033,16 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
}
EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
-struct nftnl_expr_iter {
- const struct nftnl_rule *r;
- struct nftnl_expr *cur;
-};
+static void nftnl_expr_iter_init(const struct nftnl_rule *r,
+ struct nftnl_expr_iter *iter)
+{
+ iter->r = r;
+ if (list_empty(&r->expr_list))
+ iter->cur = NULL;
+ else
+ iter->cur = list_entry(r->expr_list.next, struct nftnl_expr,
+ head);
+}
struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r)
{
@@ -1037,13 +1051,7 @@ struct nftnl_expr_iter *nftnl_expr_iter_create(const struct nftnl_rule *r)
iter = calloc(1, sizeof(struct nftnl_expr_iter));
if (iter == NULL)
return NULL;
-
- iter->r = r;
- if (list_empty(&r->expr_list))
- iter->cur = NULL;
- else
- iter->cur = list_entry(r->expr_list.next, struct nftnl_expr,
- head);
+ nftnl_expr_iter_init(r, iter);
return iter;
}
--
2.8.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 2/4, V3, libnftnl] Implement rule comparison
2016-08-10 9:48 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Carlos Falgueras García
@ 2016-08-10 9:48 ` Carlos Falgueras García
2016-08-10 11:35 ` Pablo Neira Ayuso
2016-08-10 9:48 ` [PATCH 3/4, V3, nft] Simplify parser rule_spec tree Carlos Falgueras García
` (2 subsequent siblings)
3 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-10 9:48 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
This patch implements the function 'bool nftnl_rule_cmp(const struct
nftnl_rule *r, const struct nftnl_rule *r2)' for rule comparison.
Expressions within rules need to be compared, so also has been created the
function 'nftnl_expr_cmp' which calls new field within
'nfntl_expr_<expression>': a function pointer to a comparator. The
expressions that can be compared with memcmp have this new field set to
NULL, otherwise they have implemented a comparator.
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
include/data_reg.h | 3 +++
include/expr_ops.h | 1 +
include/libnftnl/expr.h | 2 ++
include/libnftnl/rule.h | 2 ++
src/expr.c | 14 ++++++++++++++
src/expr/data_reg.c | 16 ++++++++++++++++
src/expr/dynset.c | 16 ++++++++++++++++
src/expr/immediate.c | 17 +++++++++++++++++
src/expr/log.c | 15 +++++++++++++++
src/expr/lookup.c | 14 ++++++++++++++
src/expr/match.c | 13 +++++++++++++
src/expr/target.c | 13 +++++++++++++
src/libnftnl.map | 5 +++++
src/rule.c | 30 ++++++++++++++++++++++++++++++
14 files changed, 161 insertions(+)
diff --git a/include/data_reg.h b/include/data_reg.h
index e749b5b..3fec7cd 100644
--- a/include/data_reg.h
+++ b/include/data_reg.h
@@ -3,6 +3,7 @@
#include <linux/netfilter/nf_tables.h>
#include <stdint.h>
+#include <stdbool.h>
#include <unistd.h>
enum {
@@ -27,6 +28,8 @@ int nftnl_data_reg_snprintf(char *buf, size_t size,
const union nftnl_data_reg *reg,
uint32_t output_format, uint32_t flags,
int reg_type);
+bool nftnl_data_reg_cmp(const union nftnl_data_reg *r1,
+ const union nftnl_data_reg *r2, int reg_type);
struct nlattr;
int nftnl_parse_data(union nftnl_data_reg *data, struct nlattr *attr, int *type);
diff --git a/include/expr_ops.h b/include/expr_ops.h
index 3c0cb18..a334732 100644
--- a/include/expr_ops.h
+++ b/include/expr_ops.h
@@ -13,6 +13,7 @@ struct expr_ops {
uint32_t alloc_len;
int max_attr;
void (*free)(const struct nftnl_expr *e);
+ bool (*cmp)(const struct nftnl_expr *e1, const struct nftnl_expr *e2);
int (*set)(struct nftnl_expr *e, uint16_t type, const void *data, uint32_t data_len);
const void *(*get)(const struct nftnl_expr *e, uint16_t type, uint32_t *data_len);
int (*parse)(struct nftnl_expr *e, struct nlattr *attr);
diff --git a/include/libnftnl/expr.h b/include/libnftnl/expr.h
index 17f60bd..8ae6f57 100644
--- a/include/libnftnl/expr.h
+++ b/include/libnftnl/expr.h
@@ -36,6 +36,8 @@ uint32_t nftnl_expr_get_u32(const struct nftnl_expr *expr, uint16_t type);
uint64_t nftnl_expr_get_u64(const struct nftnl_expr *expr, uint16_t type);
const char *nftnl_expr_get_str(const struct nftnl_expr *expr, uint16_t type);
+bool nftnl_expr_cmp(const struct nftnl_expr *e1, const struct nftnl_expr *e2);
+
int nftnl_expr_snprintf(char *buf, size_t buflen, const struct nftnl_expr *expr, uint32_t type, uint32_t flags);
enum {
diff --git a/include/libnftnl/rule.h b/include/libnftnl/rule.h
index e3bd6b8..adeedf2 100644
--- a/include/libnftnl/rule.h
+++ b/include/libnftnl/rule.h
@@ -50,6 +50,8 @@ uint64_t nftnl_rule_get_u64(const struct nftnl_rule *r, uint16_t attr);
void nftnl_rule_add_expr(struct nftnl_rule *r, struct nftnl_expr *expr);
+bool nftnl_rule_cmp(const struct nftnl_rule *r1, const struct nftnl_rule *r2);
+
struct nlmsghdr;
void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh, struct nftnl_rule *t);
diff --git a/src/expr.c b/src/expr.c
index e5c1dd3..7f32055 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -203,6 +203,20 @@ const char *nftnl_expr_get_str(const struct nftnl_expr *expr, uint16_t type)
}
EXPORT_SYMBOL_ALIAS(nftnl_expr_get_str, nft_rule_expr_get_str);
+bool nftnl_expr_cmp(const struct nftnl_expr *e1, const struct nftnl_expr *e2)
+{
+ if (e1->flags != e2->flags)
+ return false;
+
+ if (strcmp(e1->ops->name, e2->ops->name))
+ return false;
+ if (e1->ops->cmp)
+ return e1->ops->cmp(e1, e2);
+ else
+ return !memcmp(e1->data, e2->data, e1->ops->alloc_len);
+}
+EXPORT_SYMBOL(nftnl_expr_cmp);
+
void
nftnl_expr_build_payload(struct nlmsghdr *nlh, struct nftnl_expr *expr)
{
diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
index 688823b..a954e95 100644
--- a/src/expr/data_reg.c
+++ b/src/expr/data_reg.c
@@ -379,6 +379,22 @@ int nftnl_data_reg_snprintf(char *buf, size_t size,
return -1;
}
+bool nftnl_data_reg_cmp(const union nftnl_data_reg *r1,
+ const union nftnl_data_reg *r2, int reg_type)
+{
+ switch (reg_type) {
+ case DATA_VALUE:
+ return r1->len == r2->len &&
+ !memcmp(r1->val, r2->val, r1->len);
+ case DATA_VERDICT:
+ case DATA_CHAIN:
+ return r1->verdict == r2->verdict &&
+ !strcmp(r1->chain, r2->chain);
+ default:
+ return false;
+ }
+}
+
static int nftnl_data_parse_cb(const struct nlattr *attr, void *data)
{
const struct nlattr **tb = data;
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index 0eaa409..83311f1 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -370,11 +370,27 @@ static void nftnl_expr_dynset_free(const struct nftnl_expr *e)
xfree(dynset->set_name);
}
+static bool nftnl_expr_dynset_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_dynset *d1 = nftnl_expr_data(e1);
+ struct nftnl_expr_dynset *d2 = nftnl_expr_data(e2);
+
+ return d1->sreg_key == d2->sreg_key &&
+ d1->sreg_data == d2->sreg_data &&
+ d1->op == d2->op &&
+ d1->timeout == d2->timeout &&
+ nftnl_expr_cmp(d1->expr, d2->expr) &&
+ !strcmp(d1->set_name, d2->set_name) &&
+ d1->set_id == d2->set_id;
+}
+
struct expr_ops expr_ops_dynset = {
.name = "dynset",
.alloc_len = sizeof(struct nftnl_expr_dynset),
.max_attr = NFTA_DYNSET_MAX,
.free = nftnl_expr_dynset_free,
+ .cmp = nftnl_expr_dynset_cmp,
.set = nftnl_expr_dynset_set,
.get = nftnl_expr_dynset_get,
.parse = nftnl_expr_dynset_parse,
diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index 22ec864..ec0613d 100644
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -320,11 +320,28 @@ static void nftnl_expr_immediate_free(const struct nftnl_expr *e)
nftnl_free_verdict(&imm->data);
}
+static bool nftnl_expr_immediate_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_immediate *i1 = nftnl_expr_data(e1);
+ struct nftnl_expr_immediate *i2 = nftnl_expr_data(e2);
+ int reg_type;
+
+ if (e1->flags & (1 << NFTNL_EXPR_IMM_VERDICT))
+ reg_type = DATA_VERDICT;
+ else
+ reg_type = DATA_VALUE;
+
+ return i1->dreg == i2->dreg &&
+ nftnl_data_reg_cmp(&i1->data, &i2->data, reg_type);
+}
+
struct expr_ops expr_ops_immediate = {
.name = "immediate",
.alloc_len = sizeof(struct nftnl_expr_immediate),
.max_attr = NFTA_IMMEDIATE_MAX,
.free = nftnl_expr_immediate_free,
+ .cmp = nftnl_expr_immediate_cmp,
.set = nftnl_expr_immediate_set,
.get = nftnl_expr_immediate_get,
.parse = nftnl_expr_immediate_parse,
diff --git a/src/expr/log.c b/src/expr/log.c
index b9b3951..d8e2572 100644
--- a/src/expr/log.c
+++ b/src/expr/log.c
@@ -336,11 +336,26 @@ static void nftnl_expr_log_free(const struct nftnl_expr *e)
xfree(log->prefix);
}
+static bool nftnl_expr_log_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_log *l1 = nftnl_expr_data(e1);
+ struct nftnl_expr_log *l2 = nftnl_expr_data(e2);
+
+ return l1->snaplen == l2->snaplen &&
+ l1->group == l2->group &&
+ l1->qthreshold == l2->qthreshold &&
+ l1->level == l2->level &&
+ l1->flags == l2->flags &&
+ !strcmp(l1->prefix, l2->prefix);
+}
+
struct expr_ops expr_ops_log = {
.name = "log",
.alloc_len = sizeof(struct nftnl_expr_log),
.max_attr = NFTA_LOG_MAX,
.free = nftnl_expr_log_free,
+ .cmp = nftnl_expr_log_cmp,
.set = nftnl_expr_log_set,
.get = nftnl_expr_log_get,
.parse = nftnl_expr_log_parse,
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index 97478c2..5e5d3d4 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -296,11 +296,25 @@ static void nftnl_expr_lookup_free(const struct nftnl_expr *e)
xfree(lookup->set_name);
}
+static bool nftnl_expr_lookup_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_lookup *l1 = nftnl_expr_data(e1);
+ struct nftnl_expr_lookup *l2 = nftnl_expr_data(e2);
+
+ return l1->sreg == l2->sreg &&
+ l1->dreg == l2->dreg &&
+ !strcmp(l1->set_name, l2->set_name) &&
+ l1->set_id == l2->set_id &&
+ l1->flags == l2->flags;
+}
+
struct expr_ops expr_ops_lookup = {
.name = "lookup",
.alloc_len = sizeof(struct nftnl_expr_lookup),
.max_attr = NFTA_LOOKUP_MAX,
.free = nftnl_expr_lookup_free,
+ .cmp = nftnl_expr_lookup_cmp,
.set = nftnl_expr_lookup_set,
.get = nftnl_expr_lookup_get,
.parse = nftnl_expr_lookup_parse,
diff --git a/src/expr/match.c b/src/expr/match.c
index 3342e2c..8d9001f 100644
--- a/src/expr/match.c
+++ b/src/expr/match.c
@@ -240,11 +240,24 @@ static void nftnl_expr_match_free(const struct nftnl_expr *e)
xfree(match->data);
}
+static bool nftnl_expr_match_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_match *m1 = nftnl_expr_data(e1);
+ struct nftnl_expr_match *m2 = nftnl_expr_data(e2);
+
+ return !strcmp(m1->name, m2->name) &&
+ m1->rev == m2->rev &&
+ m1->data_len == m2->data_len &&
+ !memcmp(m1->data, m2->data, m1->data_len);
+}
+
struct expr_ops expr_ops_match = {
.name = "match",
.alloc_len = sizeof(struct nftnl_expr_match),
.max_attr = NFTA_MATCH_MAX,
.free = nftnl_expr_match_free,
+ .cmp = nftnl_expr_match_cmp,
.set = nftnl_expr_match_set,
.get = nftnl_expr_match_get,
.parse = nftnl_expr_match_parse,
diff --git a/src/expr/target.c b/src/expr/target.c
index d4c0091..55a0af3 100644
--- a/src/expr/target.c
+++ b/src/expr/target.c
@@ -241,11 +241,24 @@ static void nftnl_expr_target_free(const struct nftnl_expr *e)
xfree(target->data);
}
+static bool nftnl_expr_target_cmp(const struct nftnl_expr *e1,
+ const struct nftnl_expr *e2)
+{
+ struct nftnl_expr_target *t1 = nftnl_expr_data(e1);
+ struct nftnl_expr_target *t2 = nftnl_expr_data(e2);
+
+ return !strcmp(t1->name, t2->name) &&
+ t1->rev == t2->rev &&
+ t1->data_len == t2->data_len &&
+ !memcmp(t1->data, t2->data, t1->data_len);
+}
+
struct expr_ops expr_ops_target = {
.name = "target",
.alloc_len = sizeof(struct nftnl_expr_target),
.max_attr = NFTA_TARGET_MAX,
.free = nftnl_expr_target_free,
+ .cmp = nftnl_expr_target_cmp,
.set = nftnl_expr_target_set,
.get = nftnl_expr_target_get,
.parse = nftnl_expr_target_parse,
diff --git a/src/libnftnl.map b/src/libnftnl.map
index c38e081..abed8b9 100644
--- a/src/libnftnl.map
+++ b/src/libnftnl.map
@@ -528,3 +528,8 @@ LIBNFTNL_4.1 {
nftnl_udata_next;
nftnl_udata_parse;
} LIBNFTNL_4;
+
+LIBNFTNL_4.2 {
+ nftnl_rule_cmp;
+ nftnl_expr_cmp;
+} LIBNFTNL_4.1;
diff --git a/src/rule.c b/src/rule.c
index 69ffc7e..d4c3a6b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -339,6 +339,36 @@ void nftnl_rule_add_expr(struct nftnl_rule *r, struct nftnl_expr *expr)
}
EXPORT_SYMBOL_ALIAS(nftnl_rule_add_expr, nft_rule_add_expr);
+bool nftnl_rule_cmp(const struct nftnl_rule *r1, const struct nftnl_rule *r2)
+{
+ struct nftnl_expr_iter it1, it2;
+ struct nftnl_expr *e1, *e2;
+ unsigned int eq = 1;
+
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_TABLE))
+ eq &= !strcmp(r1->table, r2->table);
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_CHAIN))
+ eq &= !strcmp(r1->chain, r2->chain);
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_COMPAT_FLAGS))
+ eq &= r1->compat.flags == r2->compat.flags;
+ if (r1->flags & r1->flags & (1 << NFTNL_RULE_COMPAT_PROTO))
+ eq &= r1->compat.proto == r2->compat.proto;
+
+ nftnl_expr_iter_init(r1, &it1);
+ nftnl_expr_iter_init(r2, &it2);
+ e1 = nftnl_expr_iter_next(&it1);
+ e2 = nftnl_expr_iter_next(&it2);
+ while (eq && e1 && e2) {
+ eq = nftnl_expr_cmp(e1, e2);
+
+ e1 = nftnl_expr_iter_next(&it1);
+ e2 = nftnl_expr_iter_next(&it2);
+ }
+
+ return eq;
+}
+EXPORT_SYMBOL(nftnl_rule_cmp);
+
static int nftnl_rule_parse_attr_cb(const struct nlattr *attr, void *data)
{
const struct nlattr **tb = data;
--
2.8.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 2/4, V3, libnftnl] Implement rule comparison
2016-08-10 9:48 ` [PATCH 2/4, V3, libnftnl] Implement rule comparison Carlos Falgueras García
@ 2016-08-10 11:35 ` Pablo Neira Ayuso
0 siblings, 0 replies; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-10 11:35 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Wed, Aug 10, 2016 at 11:48:55AM +0200, Carlos Falgueras García wrote:
> This patch implements the function 'bool nftnl_rule_cmp(const struct
> nftnl_rule *r, const struct nftnl_rule *r2)' for rule comparison.
>
> Expressions within rules need to be compared, so also has been created the
> function 'nftnl_expr_cmp' which calls new field within
> 'nfntl_expr_<expression>': a function pointer to a comparator. The
> expressions that can be compared with memcmp have this new field set to
> NULL, otherwise they have implemented a comparator.
Could you send me a follow up patch to extend what we have under
tests/ so it covers these new nftnl_.*_cmp() functions? I would like
to have this tested before applying this.
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/4, V3, nft] Simplify parser rule_spec tree
2016-08-10 9:48 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Carlos Falgueras García
2016-08-10 9:48 ` [PATCH 2/4, V3, libnftnl] Implement rule comparison Carlos Falgueras García
@ 2016-08-10 9:48 ` Carlos Falgueras García
2016-08-12 12:11 ` Pablo Neira Ayuso
2016-08-10 9:48 ` [PATCH 4/4, V3, nft] Implement deleting rule by description Carlos Falgueras García
2016-08-10 11:41 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Pablo Neira Ayuso
3 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-10 9:48 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.
An specific error message is shown when user commits a syntax error, as
before this patch:
$ nft add rule t c handle ip saddr 1.1.1.1 counter
<cmdline>:1:14-19: Error: syntax error, unexpected handle
add rule t c handle ip saddr 1.1.1.1 counter
^^^^^^
<cmdline>:1:14-19: Error: Expected `position' or nothing
add rule t c handle ip saddr 1.1.1.1 counter
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
src/evaluate.c | 68 +-----------------------------------------------------
src/parser_bison.y | 51 +++++++++++++++++++++-------------------
2 files changed, 28 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..8dce416 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,41 @@ 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);
+ }
+ | chain_spec error
+ {
+ erec_queue(error(&@2, "Expected `position' or nothing"),
+ state->msgs);
+ $$ = $1;
}
;
-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] 32+ messages in thread* Re: [PATCH 3/4, V3, nft] Simplify parser rule_spec tree
2016-08-10 9:48 ` [PATCH 3/4, V3, nft] Simplify parser rule_spec tree Carlos Falgueras García
@ 2016-08-12 12:11 ` Pablo Neira Ayuso
0 siblings, 0 replies; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-12 12:11 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Wed, Aug 10, 2016 at 11:48:56AM +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.
>
> An specific error message is shown when user commits a syntax error, as
> before this patch:
>
> $ nft add rule t c handle ip saddr 1.1.1.1 counter
> <cmdline>:1:14-19: Error: syntax error, unexpected handle
> add rule t c handle ip saddr 1.1.1.1 counter
> ^^^^^^
> <cmdline>:1:14-19: Error: Expected `position' or nothing
> add rule t c handle ip saddr 1.1.1.1 counter
I get a double error with this, which is odd:
# nft add rule t c handle ip saddr 1.1.1.1 counter
<cmdline>:1:14-19: Error: syntax error, unexpected handle
add rule t c handle ip saddr 1.1.1.1 counter
^^^^^^
<cmdline>:1:14-19: Error: Expected `position' or nothing
add rule t c handle ip saddr 1.1.1.1 counter
^^^^^^
We don't have any similar behaviour in our existing code that I can
remember.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 4/4, V3, nft] Implement deleting rule by description
2016-08-10 9:48 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Carlos Falgueras García
2016-08-10 9:48 ` [PATCH 2/4, V3, libnftnl] Implement rule comparison Carlos Falgueras García
2016-08-10 9:48 ` [PATCH 3/4, V3, nft] Simplify parser rule_spec tree Carlos Falgueras García
@ 2016-08-10 9:48 ` Carlos Falgueras García
2016-08-10 11:41 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Pablo Neira Ayuso
3 siblings, 0 replies; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-10 9:48 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
}
}
Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
src/evaluate.c | 6 ++++++
src/parser_bison.y | 28 +++++++++++++++++++++-------
src/rule.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 70 insertions(+), 9 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 8dce416..31ce35b 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
@@ -747,7 +747,7 @@ add_cmd : TABLE table_spec
replace_cmd : RULE ruleid_spec rule
{
- $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2, &@$, $3);
+ $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2->handle, &@$, $3);
}
;
@@ -792,7 +792,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
{
@@ -1282,8 +1282,22 @@ 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);
+ }
+ | chain_spec error
+ {
+ erec_queue(error(&@2, "Expected `handle' or rule description"),
+ state->msgs);
+ $$ = rule_alloc(&@$, NULL);
+ handle_merge(&$$->handle, &$1);
}
;
diff --git a/src/rule.c b/src/rule.c
index 14e57f2..3f15ee5 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -402,6 +402,32 @@ void rule_print(const struct rule *rule)
printf(" # handle %" PRIu64, rule->handle.handle.id);
}
+static struct rule *search_first_rule(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 +1036,22 @@ 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 = search_first_rule(cmd->rule);
+ if (!rule)
+ return 0;
+ 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 +1060,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] 32+ messages in thread* Re: [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator
2016-08-10 9:48 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Carlos Falgueras García
` (2 preceding siblings ...)
2016-08-10 9:48 ` [PATCH 4/4, V3, nft] Implement deleting rule by description Carlos Falgueras García
@ 2016-08-10 11:41 ` Pablo Neira Ayuso
2016-08-10 11:56 ` Carlos Falgueras García
3 siblings, 1 reply; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-10 11:41 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Wed, Aug 10, 2016 at 11:48:54AM +0200, Carlos Falgueras García wrote:
> Introduce nftnl_expr_iter_init() to allow stack allocated iterators for
> internal use.
Applied with minor changes, see below.
>
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
> src/rule.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/src/rule.c b/src/rule.c
> index 11fceca..69ffc7e 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -29,6 +29,14 @@
> #include <libnftnl/set.h>
> #include <libnftnl/expr.h>
>
> +struct nftnl_expr_iter {
> + const struct nftnl_rule *r;
> + struct nftnl_expr *cur;
> +};
> +
> +static void nftnl_expr_iter_init(const struct nftnl_rule *r,
> + struct nftnl_expr_iter *iter);
No need for this forward declaration. I have removed this.
> +
> struct nftnl_rule {
> struct list_head head;
>
> @@ -1025,10 +1033,16 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
> }
> EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
>
> -struct nftnl_expr_iter {
> - const struct nftnl_rule *r;
> - struct nftnl_expr *cur;
> -};
I have kept this structure definition here, we don't have to scroll up
and down to get the context when looking at the iterators code.
After my change, the patch looks even smaller.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator
2016-08-10 11:41 ` [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator Pablo Neira Ayuso
@ 2016-08-10 11:56 ` Carlos Falgueras García
2016-08-10 12:08 ` Pablo Neira Ayuso
0 siblings, 1 reply; 32+ messages in thread
From: Carlos Falgueras García @ 2016-08-10 11:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On 08/10/2016 01:41 PM, Pablo Neira Ayuso wrote:
> On Wed, Aug 10, 2016 at 11:48:54AM +0200, Carlos Falgueras García wrote:
>> Introduce nftnl_expr_iter_init() to allow stack allocated iterators for
>> internal use.
>
> Applied with minor changes, see below.
>>
>> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
>> ---
>> src/rule.c | 30 +++++++++++++++++++-----------
>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/rule.c b/src/rule.c
>> index 11fceca..69ffc7e 100644
>> --- a/src/rule.c
>> +++ b/src/rule.c
>> @@ -29,6 +29,14 @@
>> #include <libnftnl/set.h>
>> #include <libnftnl/expr.h>
>>
>> +struct nftnl_expr_iter {
>> + const struct nftnl_rule *r;
>> + struct nftnl_expr *cur;
>> +};
>> +
>> +static void nftnl_expr_iter_init(const struct nftnl_rule *r,
>> + struct nftnl_expr_iter *iter);
>
> No need for this forward declaration. I have removed this.
>
>> +
>> struct nftnl_rule {
>> struct list_head head;
>>
>> @@ -1025,10 +1033,16 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
>> }
>> EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
>>
>> -struct nftnl_expr_iter {
>> - const struct nftnl_rule *r;
>> - struct nftnl_expr *cur;
>> -};
>
> I have kept this structure definition here, we don't have to scroll up
> and down to get the context when looking at the iterators code.
>
> After my change, the patch looks even smaller.
>
But we will need it at the next patch, so I will include these changes
in the patch : 2/4 - "Implement rule comparison". It is ok?
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 1/4, V3, libnftnl] rule: Implement internal expression iterator
2016-08-10 11:56 ` Carlos Falgueras García
@ 2016-08-10 12:08 ` Pablo Neira Ayuso
0 siblings, 0 replies; 32+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-10 12:08 UTC (permalink / raw)
To: Carlos Falgueras García; +Cc: netfilter-devel
On Wed, Aug 10, 2016 at 01:56:32PM +0200, Carlos Falgueras García wrote:
> On 08/10/2016 01:41 PM, Pablo Neira Ayuso wrote:
> >>@@ -1025,10 +1033,16 @@ int nftnl_expr_foreach(struct nftnl_rule *r,
> >> }
> >> EXPORT_SYMBOL_ALIAS(nftnl_expr_foreach, nft_rule_expr_foreach);
> >>
> >>-struct nftnl_expr_iter {
> >>- const struct nftnl_rule *r;
> >>- struct nftnl_expr *cur;
> >>-};
> >
> >I have kept this structure definition here, we don't have to scroll up
> >and down to get the context when looking at the iterators code.
> >
> >After my change, the patch looks even smaller.
>
> But we will need it at the next patch, so I will include these changes in
> the patch : 2/4 - "Implement rule comparison". It is ok?
You can place nftnl_rule_cmp() after the iterators, so you don't need
this forward declaration.
^ permalink raw reply [flat|nested] 32+ messages in thread