[RFC,nft] evaluate: kill anon sets with one element

Message ID 20190125155946.18259-1-fw@strlen.de
State Under Review
Delegated to: Pablo Neira
Headers show
Series
  • [RFC,nft] evaluate: kill anon sets with one element
Related show

Commit Message

Florian Westphal Jan. 25, 2019, 3:59 p.m.
pretends that
ip saddr { 1.1.1.1 }

is
ip saddr 1.1.1.1

Needs more work (breaks dumps in test cases afaics).
Is that a good idea in first place?

I see various new users adopting

{ single-value }

probably due to copying/adapting a "{ foo, bar }" rule to "{ foo }"...
---
 src/evaluate.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Phil Sutter Jan. 27, 2019, 11:40 a.m. | #1
Hi,

On Fri, Jan 25, 2019 at 04:59:46PM +0100, Florian Westphal wrote:
> pretends that
> ip saddr { 1.1.1.1 }
> 
> is
> ip saddr 1.1.1.1
> 
> Needs more work (breaks dumps in test cases afaics).
> Is that a good idea in first place?
> 
> I see various new users adopting
> 
> { single-value }
> 
> probably due to copying/adapting a "{ foo, bar }" rule to "{ foo }"...

I think it's a good idea. Just like with auto-merging of ranges,
anonymous sets may (and IMHO should) optimize themselves as much as
possible.

Cheers, Phil

Patch

diff --git a/src/evaluate.c b/src/evaluate.c
index 0bda431d5a16..e922b7f9fb26 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1251,11 +1251,13 @@  static int expr_evaluate_set_elem(struct eval_ctx *ctx, struct expr **expr)
 static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *set = *expr, *i, *next;
+	unsigned int count = 0;
 
 	list_for_each_entry_safe(i, next, &set->expressions, list) {
 		if (list_member_evaluate(ctx, &i) < 0)
 			return -1;
 
+		count++;
 		if (i->ops->type == EXPR_SET_ELEM &&
 		    i->key->ops->type == EXPR_SET_REF)
 			return expr_error(ctx->msgs, i,
@@ -1286,6 +1288,22 @@  static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
 			set->set_flags |= NFT_SET_INTERVAL;
 	}
 
+	if (!ctx->set && count == 1) {
+		i = list_first_entry(&set->expressions, struct expr, list);
+		if (i->ops->type == EXPR_SET_ELEM) {
+			switch (i->key->ops->type) {
+			case EXPR_RANGE:
+			case EXPR_VALUE:
+				*expr = i->key;
+				i->key = NULL;
+				expr_free(set);
+				return 0;
+			default:
+				break;
+			}
+		}
+	}
+
 	set->set_flags |= NFT_SET_CONSTANT;
 
 	set->dtype = ctx->ectx.dtype;