[PATCH nft 00/18] fixes and improvements for -o/--optimize

[PATCH nft 00/18] fixes and improvements for -o/--optimize

[Pablo Neira Ayuso]

Hi,

The following patchset contains a batch with fix/improvements for
-o/--optimize.

1) Fix statement comparison, leading to incorrect rule matching to be merged.

2) Do not merge rules using set reference.

3) Do not print stateful information, eg. counter packets 0 bytes 0

4) Drop comments when merging.

5) Fix reject statement comparison.

6) Do not fully compare verdict statement, otherwise statement vs rule matrix
   gets multiple occurrences of this statement.

7) Add missing expressions used in relationals: osf, xfrm, fib, numgen, hash.

8) Add binary expression support.

9) Add unsupported statement, to avoid merging rules with statements that are
   not yet supported.

10) Only merge relationals with OP_IMPLICIT and OP_EQ.

11) Assume verdict is the same when rule specifies no verdict.

12) Remove support for limit statement, not actually supported yet. Merging
    rules with the limit statement require a new class of transformation not
    yet supported.

13) Release top level scope to avoid a bogus variable redefinition error
    when using -c and -o.

And many new tests.

This infrastructure is new code, please help testing and reporting bugs
running it on your existing rulesets.

Thanks.

Pablo Neira Ayuso (18):
  optimize: do not compare relational expression rhs when collecting statements
  optimize: do not merge rules with set reference in rhs
  optimize: do not print stateful information
  optimize: remove comment after merging
  optimize: fix reject statement
  optimize: fix verdict map merging
  optimize: add osf expression support
  optimize: add xfrm expression support
  optimize: add fib expression support
  optimize: add binop expression support
  optimize: add numgen expression support
  optimize: add hash expression support
  optimize: add unsupported statement
  tests: shell: run -c -o on ruleset
  optimize: only merge OP_IMPLICIT and OP_EQ relational
  optimize: assume verdict is same when rules have no verdict
  optimize: limit statement is not supported yet
  libnftables: release top level scope

 src/libnftables.c                             |   2 +
 src/optimize.c                                | 205 ++++++++++++++----
 .../optimizations/dumps/merge_reject.nft      |  13 ++
 .../optimizations/dumps/skip_merge.nft        |  23 ++
 .../optimizations/dumps/skip_non_eq.nft       |   6 +
 .../optimizations/dumps/skip_unsupported.nft  |   7 +
 .../testcases/optimizations/merge_reject      |  26 +++
 .../shell/testcases/optimizations/merge_stmts |   6 +-
 tests/shell/testcases/optimizations/ruleset   | 168 ++++++++++++++
 .../shell/testcases/optimizations/skip_merge  |  34 +++
 .../shell/testcases/optimizations/skip_non_eq |  12 +
 .../testcases/optimizations/skip_unsupported  |  14 ++
 tests/shell/testcases/optimizations/variables |  15 ++
 13 files changed, 488 insertions(+), 43 deletions(-)
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_reject.nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_merge.nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_non_eq.nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
 create mode 100755 tests/shell/testcases/optimizations/merge_reject
 create mode 100755 tests/shell/testcases/optimizations/ruleset
 create mode 100755 tests/shell/testcases/optimizations/skip_merge
 create mode 100755 tests/shell/testcases/optimizations/skip_non_eq
 create mode 100755 tests/shell/testcases/optimizations/skip_unsupported
 create mode 100755 tests/shell/testcases/optimizations/variables

-- 
2.30.2

[PATCH nft 01/18] optimize: do not compare relational expression rhs when collecting statements

[Pablo Neira Ayuso]

When building the statement matrix, do not compare expression right hand
side, otherwise bogus mismatches might occur.

The fully compared flag is set on when comparing rules to look for
possible mergers.

Fixes: 3f36cc6c3dcd ("optimize: do not merge unsupported statement expressions")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index 3a3049d43690..a2a4e587e125 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -105,7 +105,8 @@ static bool stmt_expr_supported(const struct expr *expr)
 	return false;
 }
 
-static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
+static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
+			   bool fully_compare)
 {
 	struct expr *expr_a, *expr_b;
 
@@ -117,9 +118,11 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
 		expr_a = stmt_a->expr;
 		expr_b = stmt_b->expr;
 
-		if (!stmt_expr_supported(expr_a) ||
-		    !stmt_expr_supported(expr_b))
-			return false;
+		if (fully_compare) {
+			if (!stmt_expr_supported(expr_a) ||
+			    !stmt_expr_supported(expr_b))
+				return false;
+		}
 
 		return __expr_cmp(expr_a->left, expr_b->left);
 	case STMT_COUNTER:
@@ -237,24 +240,12 @@ static bool stmt_verdict_eq(const struct stmt *stmt_a, const struct stmt *stmt_b
 	return false;
 }
 
-static bool stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
-{
-	if (!stmt_a && !stmt_b)
-		return true;
-	else if (!stmt_a)
-		return false;
-	else if (!stmt_b)
-		return false;
-
-	return __stmt_type_eq(stmt_a, stmt_b);
-}
-
 static bool stmt_type_find(struct optimize_ctx *ctx, const struct stmt *stmt)
 {
 	uint32_t i;
 
 	for (i = 0; i < ctx->num_stmts; i++) {
-		if (__stmt_type_eq(stmt, ctx->stmt[i]))
+		if (__stmt_type_eq(stmt, ctx->stmt[i], false))
 			return true;
 	}
 
@@ -321,7 +312,7 @@ static int cmd_stmt_find_in_stmt_matrix(struct optimize_ctx *ctx, struct stmt *s
 	uint32_t i;
 
 	for (i = 0; i < ctx->num_stmts; i++) {
-		if (__stmt_type_eq(stmt, ctx->stmt[i]))
+		if (__stmt_type_eq(stmt, ctx->stmt[i], false))
 			return i;
 	}
 	/* should not ever happen. */
@@ -886,6 +877,18 @@ static void merge_rules(const struct optimize_ctx *ctx,
 	fprintf(octx->error_fp, "\n");
 }
 
+static bool stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
+{
+	if (!stmt_a && !stmt_b)
+		return true;
+	else if (!stmt_a)
+		return false;
+	else if (!stmt_b)
+		return false;
+
+	return __stmt_type_eq(stmt_a, stmt_b, true);
+}
+
 static bool rules_eq(const struct optimize_ctx *ctx, int i, int j)
 {
 	uint32_t k;
-- 
2.30.2

[PATCH nft 02/18] optimize: do not merge rules with set reference in rhs

[Pablo Neira Ayuso]

Otherwise set reference ends up included in an anonymous set, as an
element, which is not supported.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                | 10 ++++++
 .../optimizations/dumps/skip_merge.nft        | 23 +++++++++++++
 .../shell/testcases/optimizations/skip_merge  | 34 +++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_merge.nft
 create mode 100755 tests/shell/testcases/optimizations/skip_merge

diff --git a/src/optimize.c b/src/optimize.c
index a2a4e587e125..543d3ca5a9c7 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -105,6 +105,12 @@ static bool stmt_expr_supported(const struct expr *expr)
 	return false;
 }
 
+static bool expr_symbol_set(const struct expr *expr)
+{
+	return expr->right->etype == EXPR_SYMBOL &&
+	       expr->right->symtype == SYMBOL_SET;
+}
+
 static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
 			   bool fully_compare)
 {
@@ -122,6 +128,10 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
 			if (!stmt_expr_supported(expr_a) ||
 			    !stmt_expr_supported(expr_b))
 				return false;
+
+			if (expr_symbol_set(expr_a) ||
+			    expr_symbol_set(expr_b))
+				return false;
 		}
 
 		return __expr_cmp(expr_a->left, expr_b->left);
diff --git a/tests/shell/testcases/optimizations/dumps/skip_merge.nft b/tests/shell/testcases/optimizations/dumps/skip_merge.nft
new file mode 100644
index 000000000000..9c10b74b4be2
--- /dev/null
+++ b/tests/shell/testcases/optimizations/dumps/skip_merge.nft
@@ -0,0 +1,23 @@
+table inet filter {
+	set udp_accepted {
+		type inet_service
+		elements = { 500, 4500 }
+	}
+
+	set tcp_accepted {
+		type inet_service
+		elements = { 80, 443 }
+	}
+
+	chain udp_input {
+		udp dport 1-128 accept
+		udp dport @udp_accepted accept
+		udp dport 53 accept
+	}
+
+	chain tcp_input {
+		tcp dport { 1-128, 8888-9999 } accept
+		tcp dport @tcp_accepted accept
+		tcp dport 1024-65535 accept
+	}
+}
diff --git a/tests/shell/testcases/optimizations/skip_merge b/tests/shell/testcases/optimizations/skip_merge
new file mode 100755
index 000000000000..8af976cac56d
--- /dev/null
+++ b/tests/shell/testcases/optimizations/skip_merge
@@ -0,0 +1,34 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table inet filter {
+    set udp_accepted {
+        type inet_service;
+        elements = {
+            isakmp, ipsec-nat-t
+        }
+    }
+
+    set tcp_accepted {
+        type inet_service;
+        elements = {
+            http, https
+        }
+    }
+
+    chain udp_input {
+        udp dport 1-128 accept
+        udp dport @udp_accepted accept
+        udp dport domain accept
+    }
+
+    chain tcp_input {
+        tcp dport 1-128 accept
+        tcp dport 8888-9999 accept
+        tcp dport @tcp_accepted accept
+        tcp dport 1024-65535 accept
+    }
+}"
+
+$NFT -o -f - <<< $RULESET
-- 
2.30.2

[PATCH nft 03/18] optimize: do not print stateful information

[Pablo Neira Ayuso]

Do not print stateful information such as counters which are likely set
to zero.

Before this patch:

  Merging:
  packets.conf:10:3-29:                 ip protocol  4 counter drop
  packets.conf:11:3-29:                 ip protocol 41 counter drop
  packets.conf:12:3-29:                 ip protocol 47 counter drop
  into:
          ip protocol { 4, 41, 47 } counter packets 0 bytes 0 drop
                                            ^^^^^^^^^^^^^^^^^
After:

  Merging:
  packets.conf:10:3-29:                 ip protocol  4 counter drop
  packets.conf:11:3-29:                 ip protocol 41 counter drop
  packets.conf:12:3-29:                 ip protocol 47 counter drop
  into:
          ip protocol { 4, 41, 47 } counter drop

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 543d3ca5a9c7..b19a8b553555 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -873,6 +873,8 @@ static void merge_rules(const struct optimize_ctx *ctx,
 		assert(0);
 	}
 
+        octx->flags |= NFT_CTX_OUTPUT_STATELESS;
+
 	fprintf(octx->error_fp, "Merging:\n");
 	rule_optimize_print(octx, ctx->rule[from]);
 
@@ -885,6 +887,8 @@ static void merge_rules(const struct optimize_ctx *ctx,
 	fprintf(octx->error_fp, "into:\n\t");
 	rule_print(ctx->rule[from], octx);
 	fprintf(octx->error_fp, "\n");
+
+        octx->flags &= ~NFT_CTX_OUTPUT_STATELESS;
 }
 
 static bool stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
-- 
2.30.2

[PATCH nft 04/18] optimize: remove comment after merging

[Pablo Neira Ayuso]

Remove rule comment after merging rules, let the user decide if they want
to reintroduce the comment in the ruleset file.

Update optimizations/merge_stmt test.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                  | 5 +++++
 tests/shell/testcases/optimizations/merge_stmts | 6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index b19a8b553555..94242ee5f490 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -873,6 +873,11 @@ static void merge_rules(const struct optimize_ctx *ctx,
 		assert(0);
 	}
 
+	if (ctx->rule[from]->comment) {
+		xfree(ctx->rule[from]->comment);
+		ctx->rule[from]->comment = NULL;
+	}
+
         octx->flags |= NFT_CTX_OUTPUT_STATELESS;
 
 	fprintf(octx->error_fp, "Merging:\n");
diff --git a/tests/shell/testcases/optimizations/merge_stmts b/tests/shell/testcases/optimizations/merge_stmts
index 0c35636efaa9..ec7a9dd6b554 100755
--- a/tests/shell/testcases/optimizations/merge_stmts
+++ b/tests/shell/testcases/optimizations/merge_stmts
@@ -4,9 +4,9 @@ set -e
 
 RULESET="table ip x {
 	chain y {
-		ip daddr 192.168.0.1 counter accept
-		ip daddr 192.168.0.2 counter accept
-		ip daddr 192.168.0.3 counter accept
+		ip daddr 192.168.0.1 counter accept comment "test1"
+		ip daddr 192.168.0.2 counter accept comment "test2"
+		ip daddr 192.168.0.3 counter accept comment "test3"
 	}
 }"
 
-- 
2.30.2

[PATCH nft 05/18] optimize: fix reject statement

[Pablo Neira Ayuso]

Add missing code to the statement collection routine. Compare reject
expressions when available. Add tests/shell.

Fixes: fb298877ece2 ("src: add ruleset optimization infrastructure")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                | 19 ++++++++++++++++---
 .../optimizations/dumps/merge_reject.nft      |  7 +++++++
 .../testcases/optimizations/merge_reject      | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_reject.nft
 create mode 100755 tests/shell/testcases/optimizations/merge_reject

diff --git a/src/optimize.c b/src/optimize.c
index 94242ee5f490..427625846484 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -178,13 +178,19 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
 			return false;
 		break;
 	case STMT_REJECT:
-		if (stmt_a->reject.expr || stmt_b->reject.expr)
-			return false;
-
 		if (stmt_a->reject.family != stmt_b->reject.family ||
 		    stmt_a->reject.type != stmt_b->reject.type ||
 		    stmt_a->reject.icmp_code != stmt_b->reject.icmp_code)
 			return false;
+
+		if (!!stmt_a->reject.expr ^ !!stmt_b->reject.expr)
+			return false;
+
+		if (!stmt_a->reject.expr)
+			return true;
+
+		if (__expr_cmp(stmt_a->reject.expr, stmt_b->reject.expr))
+			return false;
 		break;
 	case STMT_NAT:
 		if (stmt_a->nat.type != stmt_b->nat.type ||
@@ -304,6 +310,13 @@ static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 			clone->nat.flags = stmt->nat.flags;
 			clone->nat.type_flags = stmt->nat.type_flags;
 			break;
+		case STMT_REJECT:
+			if (stmt->reject.expr)
+				clone->reject.expr = expr_get(stmt->reject.expr);
+			clone->reject.type = stmt->reject.type;
+			clone->reject.icmp_code = stmt->reject.icmp_code;
+			clone->reject.family = stmt->reject.family;
+			break;
 		default:
 			xfree(clone);
 			continue;
diff --git a/tests/shell/testcases/optimizations/dumps/merge_reject.nft b/tests/shell/testcases/optimizations/dumps/merge_reject.nft
new file mode 100644
index 000000000000..9a13e2b96faa
--- /dev/null
+++ b/tests/shell/testcases/optimizations/dumps/merge_reject.nft
@@ -0,0 +1,7 @@
+table ip x {
+	chain y {
+		ip daddr 172.30.33.70 tcp dport 3306 counter packets 0 bytes 0 drop
+		meta l4proto . ip daddr . tcp dport { tcp . 172.30.238.117 . 8080, tcp . 172.30.33.71 . 3306, tcp . 172.30.254.251 . 3306 } counter packets 0 bytes 0 reject
+		ip daddr 172.30.254.252 tcp dport 3306 counter packets 0 bytes 0 reject with tcp reset
+	}
+}
diff --git a/tests/shell/testcases/optimizations/merge_reject b/tests/shell/testcases/optimizations/merge_reject
new file mode 100755
index 000000000000..497e8f64dc5d
--- /dev/null
+++ b/tests/shell/testcases/optimizations/merge_reject
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table ip x {
+	chain y {
+		meta l4proto tcp ip daddr 172.30.33.70 tcp dport 3306 counter packets 0 bytes 0 drop
+		meta l4proto tcp ip daddr 172.30.33.71 tcp dport 3306 counter packets 0 bytes 0 reject
+		meta l4proto tcp ip daddr 172.30.238.117 tcp dport 8080 counter packets 0 bytes 0 reject
+		meta l4proto tcp ip daddr 172.30.254.251 tcp dport 3306 counter packets 0 bytes 0 reject
+		meta l4proto tcp ip daddr 172.30.254.252 tcp dport 3306 counter packets 0 bytes 0 reject with tcp reset
+	}
+}"
+
+$NFT -o -f - <<< $RULESET
-- 
2.30.2

[PATCH nft 06/18] optimize: fix verdict map merging

[Pablo Neira Ayuso]

Skip comparison when collecting the statement and building the rule vs
statement matrix. Compare verdict type when merging rules.

When infering rule mergers, honor the STMT_VERDICT with map (ie. vmap).

Fixes: 561aa3cfa8da ("optimize: merge verdict maps with same lookup key")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index 427625846484..747282b4d7f4 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -139,6 +139,9 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
 	case STMT_NOTRACK:
 		break;
 	case STMT_VERDICT:
+		if (!fully_compare)
+			break;
+
 		expr_a = stmt_a->expr;
 		expr_b = stmt_b->expr;
 
@@ -276,10 +279,6 @@ static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 		if (stmt_type_find(ctx, stmt))
 			continue;
 
-		if (stmt->ops->type == STMT_VERDICT &&
-		    stmt->expr->etype == EXPR_MAP)
-			continue;
-
 		/* No refcounter available in statement objects, clone it to
 		 * to store in the array of selectors.
 		 */
@@ -999,6 +998,10 @@ static int chain_optimize(struct nft_ctx *nft, struct list_head *rules)
 			case STMT_EXPRESSION:
 				merge[k].stmt[merge[k].num_stmts++] = m;
 				break;
+			case STMT_VERDICT:
+				if (ctx->stmt_matrix[i][m]->expr->etype == EXPR_MAP)
+					merge[k].stmt[merge[k].num_stmts++] = m;
+				break;
 			default:
 				break;
 			}
-- 
2.30.2

[PATCH nft 07/18] optimize: add osf expression support

[Pablo Neira Ayuso]

Extend expr_cmp() to compare osf expressions used in relational.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 747282b4d7f4..04c92575c4b0 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -81,6 +81,12 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 		if (expr_a->socket.level != expr_b->socket.level)
 			return false;
 		break;
+	case EXPR_OSF:
+		if (expr_a->osf.ttl != expr_b->osf.ttl)
+			return false;
+		if (expr_a->osf.flags != expr_b->osf.flags)
+			return false;
+		break;
 	default:
 		return false;
 	}
-- 
2.30.2

[PATCH nft 08/18] optimize: add xfrm expression support

[Pablo Neira Ayuso]

Extend expr_cmp() to compare xfrm expressions used in relational.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 04c92575c4b0..919f8013046b 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -87,6 +87,12 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 		if (expr_a->osf.flags != expr_b->osf.flags)
 			return false;
 		break;
+	case EXPR_XFRM:
+		if (expr_a->xfrm.key != expr_b->xfrm.key)
+			return false;
+		if (expr_a->xfrm.direction != expr_b->xfrm.direction)
+			return false;
+		break;
 	default:
 		return false;
 	}
-- 
2.30.2

[PATCH nft 09/18] optimize: add fib expression support

[Pablo Neira Ayuso]

Extend expr_cmp() to compare fib expressions used in relational.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 919f8013046b..2063761a503a 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -93,6 +93,12 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 		if (expr_a->xfrm.direction != expr_b->xfrm.direction)
 			return false;
 		break;
+	case EXPR_FIB:
+		if (expr_a->fib.flags != expr_b->fib.flags)
+			return false;
+		if (expr_a->fib.result != expr_b->fib.result)
+			return false;
+		break;
 	default:
 		return false;
 	}
-- 
2.30.2

[PATCH nft 10/18] optimize: add binop expression support

[Pablo Neira Ayuso]

Do recursive call using left expression in the binop expression tree to
search for the primary expression.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 2063761a503a..54dde948876f 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -99,6 +99,8 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 		if (expr_a->fib.result != expr_b->fib.result)
 			return false;
 		break;
+	case EXPR_BINOP:
+		return __expr_cmp(expr_a->left, expr_b->left);
 	default:
 		return false;
 	}
-- 
2.30.2

[PATCH nft 11/18] optimize: add numgen expression support

[Pablo Neira Ayuso]

Extend expr_cmp() to compare numgen expressions used in relational.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 54dde948876f..1fa46cb6a663 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -99,6 +99,14 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 		if (expr_a->fib.result != expr_b->fib.result)
 			return false;
 		break;
+	case EXPR_NUMGEN:
+		if (expr_a->numgen.type != expr_b->numgen.type)
+			return false;
+		if (expr_a->numgen.mod != expr_b->numgen.mod)
+			return false;
+		if (expr_a->numgen.offset != expr_b->numgen.offset)
+			return false;
+		break;
 	case EXPR_BINOP:
 		return __expr_cmp(expr_a->left, expr_b->left);
 	default:
-- 
2.30.2

[PATCH nft 12/18] optimize: add hash expression support

[Pablo Neira Ayuso]

Extend expr_cmp() to compare hash expressions used in relational.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 1fa46cb6a663..abd0b72f90d3 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -107,6 +107,18 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 		if (expr_a->numgen.offset != expr_b->numgen.offset)
 			return false;
 		break;
+	case EXPR_HASH:
+		if (expr_a->hash.mod != expr_b->hash.mod)
+			return false;
+		if (expr_a->hash.seed_set != expr_b->hash.seed_set)
+			return false;
+		if (expr_a->hash.seed != expr_b->hash.seed)
+			return false;
+		if (expr_a->hash.offset != expr_b->hash.offset)
+			return false;
+		if (expr_a->hash.type != expr_b->hash.type)
+			return false;
+		break;
 	case EXPR_BINOP:
 		return __expr_cmp(expr_a->left, expr_b->left);
 	default:
-- 
2.30.2

[PATCH nft 13/18] optimize: add unsupported statement

[Pablo Neira Ayuso]

Do not try to merge rules with unsupported statements. This patch adds a
dummy unsupported statement which is included in the statement
collection and the rule vs statement matrix.

When looking for possible rule mergers, rules using unsupported
statements are discarded, otherwise bogus rule mergers might occur.

Note that __stmt_type_eq() already returns false for unsupported
statements.

Add a test using meta mark statement, which is not yet supported.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                | 56 +++++++++++++++++--
 .../optimizations/dumps/skip_unsupported.nft  |  7 +++
 .../testcases/optimizations/skip_unsupported  | 14 +++++
 3 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
 create mode 100755 tests/shell/testcases/optimizations/skip_unsupported

diff --git a/src/optimize.c b/src/optimize.c
index abd0b72f90d3..e3d4bc785226 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -301,16 +301,42 @@ static bool stmt_verdict_eq(const struct stmt *stmt_a, const struct stmt *stmt_b
 
 static bool stmt_type_find(struct optimize_ctx *ctx, const struct stmt *stmt)
 {
+	bool unsupported_exists = false;
 	uint32_t i;
 
 	for (i = 0; i < ctx->num_stmts; i++) {
+		if (ctx->stmt[i]->ops->type == STMT_INVALID)
+			unsupported_exists = true;
+
 		if (__stmt_type_eq(stmt, ctx->stmt[i], false))
 			return true;
 	}
 
+	switch (stmt->ops->type) {
+	case STMT_EXPRESSION:
+	case STMT_VERDICT:
+	case STMT_COUNTER:
+	case STMT_NOTRACK:
+	case STMT_LIMIT:
+	case STMT_LOG:
+	case STMT_NAT:
+	case STMT_REJECT:
+		break;
+	default:
+		/* add unsupported statement only once to statement matrix. */
+		if (unsupported_exists)
+			return true;
+		break;
+	}
+
 	return false;
 }
 
+static struct stmt_ops unsupported_stmt_ops = {
+	.type	= STMT_INVALID,
+	.name	= "unsupported",
+};
+
 static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 {
 	struct stmt *stmt, *clone;
@@ -357,8 +383,8 @@ static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 			clone->reject.family = stmt->reject.family;
 			break;
 		default:
-			xfree(clone);
-			continue;
+			clone->ops = &unsupported_stmt_ops;
+			break;
 		}
 
 		ctx->stmt[ctx->num_stmts++] = clone;
@@ -369,6 +395,18 @@ static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 	return 0;
 }
 
+static int unsupported_in_stmt_matrix(const struct optimize_ctx *ctx)
+{
+	uint32_t i;
+
+	for (i = 0; i < ctx->num_stmts; i++) {
+		if (ctx->stmt[i]->ops->type == STMT_INVALID)
+			return i;
+	}
+	/* this should not happen. */
+	return -1;
+}
+
 static int cmd_stmt_find_in_stmt_matrix(struct optimize_ctx *ctx, struct stmt *stmt)
 {
 	uint32_t i;
@@ -377,10 +415,14 @@ static int cmd_stmt_find_in_stmt_matrix(struct optimize_ctx *ctx, struct stmt *s
 		if (__stmt_type_eq(stmt, ctx->stmt[i], false))
 			return i;
 	}
-	/* should not ever happen. */
-	return 0;
+
+	return -1;
 }
 
+static struct stmt unsupported_stmt = {
+	.ops	= &unsupported_stmt_ops,
+};
+
 static void rule_build_stmt_matrix_stmts(struct optimize_ctx *ctx,
 					 struct rule *rule, uint32_t *i)
 {
@@ -389,6 +431,12 @@ static void rule_build_stmt_matrix_stmts(struct optimize_ctx *ctx,
 
 	list_for_each_entry(stmt, &rule->stmts, list) {
 		k = cmd_stmt_find_in_stmt_matrix(ctx, stmt);
+		if (k < 0) {
+			k = unsupported_in_stmt_matrix(ctx);
+			assert(k >= 0);
+			ctx->stmt_matrix[*i][k] = &unsupported_stmt;
+			continue;
+		}
 		ctx->stmt_matrix[*i][k] = stmt;
 	}
 	ctx->rule[(*i)++] = rule;
diff --git a/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft b/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
new file mode 100644
index 000000000000..43b6578dc704
--- /dev/null
+++ b/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
@@ -0,0 +1,7 @@
+table inet x {
+	chain y {
+		ip saddr 1.2.3.4 tcp dport 80 meta mark set 0x0000000a accept
+		ip saddr 1.2.3.4 tcp dport 81 meta mark set 0x0000000b accept
+		ip saddr . tcp dport { 1.2.3.5 . 81, 1.2.3.5 . 82 } accept
+	}
+}
diff --git a/tests/shell/testcases/optimizations/skip_unsupported b/tests/shell/testcases/optimizations/skip_unsupported
new file mode 100755
index 000000000000..9313c302048c
--- /dev/null
+++ b/tests/shell/testcases/optimizations/skip_unsupported
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table inet x {
+	chain y {
+		ip saddr 1.2.3.4 tcp dport 80 meta mark set 10 accept
+		ip saddr 1.2.3.4 tcp dport 81 meta mark set 11 accept
+		ip saddr 1.2.3.5 tcp dport 81 accept comment \"test\"
+		ip saddr 1.2.3.5 tcp dport 82 accept
+	}
+}"
+
+$NFT -o -f - <<< $RULESET
-- 
2.30.2

[PATCH nft 14/18] tests: shell: run -c -o on ruleset

[Pablo Neira Ayuso]

Just run -o/--optimize on a ruleset.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 tests/shell/testcases/optimizations/ruleset | 168 ++++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100755 tests/shell/testcases/optimizations/ruleset

diff --git a/tests/shell/testcases/optimizations/ruleset b/tests/shell/testcases/optimizations/ruleset
new file mode 100755
index 000000000000..ef2652dbeae8
--- /dev/null
+++ b/tests/shell/testcases/optimizations/ruleset
@@ -0,0 +1,168 @@
+#!/bin/bash
+
+RULESET="table inet uni {
+	chain gtfo {
+		reject with icmpx type host-unreachable
+		drop
+	}
+
+	chain filter_in_tcp {
+		tcp dport vmap {
+			   80 : accept,
+			   81 : accept,
+			  443 : accept,
+			  931 : accept,
+			 5001 : accept,
+			 5201 : accept,
+		}
+		tcp dport vmap {
+			 6800-6999  : accept,
+			33434-33499 : accept,
+		}
+
+		drop
+	}
+
+	chain filter_in_udp {
+		udp dport vmap {
+			   53 : accept,
+			  123 : accept,
+			  846 : accept,
+			  849 : accept,
+			 5001 : accept,
+			 5201 : accept,
+		}
+		udp dport vmap {
+			 5300-5399  : accept,
+			 6800-6999  : accept,
+			33434-33499 : accept,
+		}
+
+		drop
+	}
+
+	chain filter_in {
+		type filter hook input priority 0; policy drop;
+
+		ct state vmap {
+			invalid     : drop,
+			established : accept,
+			related     : accept,
+			untracked   : accept,
+		}
+
+		ct status vmap {
+			dnat : accept,
+			snat : accept,
+		}
+
+		iif lo  accept
+
+		meta iifgroup {100-199}  accept
+
+		meta l4proto tcp  goto filter_in_tcp
+		meta l4proto udp  goto filter_in_udp
+
+		icmp type vmap {
+			echo-request : accept,
+		}
+		ip6 nexthdr icmpv6 icmpv6 type vmap {
+			echo-request : accept,
+		}
+	}
+
+	chain filter_fwd_ifgroup {
+		meta iifgroup . oifgroup vmap {
+		          100 .  10 : accept,
+		          100 . 100 : accept,
+		          100 . 101 : accept,
+		          101 . 101 : accept,
+		}
+		goto gtfo
+	}
+
+	chain filter_fwd {
+		type filter hook forward priority 0; policy drop;
+
+		fib daddr type broadcast  drop
+
+		ct state vmap {
+			invalid     : drop,
+			established : accept,
+			related     : accept,
+			untracked   : accept,
+		}
+
+		ct status vmap {
+			dnat : accept,
+			snat : accept,
+		}
+
+		meta iifgroup {100-199}  goto filter_fwd_ifgroup
+	}
+
+	chain nat_fwd_tun {
+		meta l4proto tcp redirect to :15
+		udp dport 53 redirect to :13
+		goto gtfo
+	}
+
+	chain nat_dns_dnstc     { meta l4proto udp redirect to :5300 ; drop ; }
+	chain nat_dns_this_5301 { meta l4proto udp redirect to :5301 ; drop ; }
+	chain nat_dns_moon_5301  { meta nfproto ipv4 meta l4proto udp dnat to 240.0.1.2:5301 ; drop ; }
+	chain nat_dns_moon_5302  { meta nfproto ipv4 meta l4proto udp dnat to 240.0.1.2:5302 ; drop ; }
+	chain nat_dns_moon_5303  { meta nfproto ipv4 meta l4proto udp dnat to 240.0.1.2:5303 ; drop ; }
+
+	chain nat_dns_acme {
+		udp length 47-63 @th,160,128 0x0e373135363130333131303735353203 \
+			goto nat_dns_dnstc
+
+		udp length 62-78 @th,160,128 0x0e31393032383939353831343037320e \
+			goto nat_dns_this_5301
+
+		udp length 62-78 @th,160,128 0x0e31363436323733373931323934300e \
+			goto nat_dns_moon_5301
+
+		udp length 62-78 @th,160,128 0x0e32393535373539353636383732310e \
+			goto nat_dns_moon_5302
+
+		udp length 62-78 @th,160,128 0x0e38353439353637323038363633390e \
+			goto nat_dns_moon_5303
+
+		drop
+	}
+
+	chain nat_prerouting {
+		type nat hook prerouting priority -100; policy accept;
+
+		iifgroup 10 udp dport 53 goto nat_dns_acme
+		iifgroup 10 accept
+
+		ip  daddr 198.19.0.0/16  goto nat_fwd_tun
+		ip6 daddr fc00::/8       goto nat_fwd_tun
+
+		tcp dport 53 redirect to :25302
+		udp dport 53 redirect to :25302
+	}
+
+	chain nat_output {
+		type nat hook output priority -100; policy accept;
+
+		ip  daddr 198.19.0.0/16  goto nat_fwd_tun
+		ip6 daddr fc00::/8       goto nat_fwd_tun
+	}
+
+	chain nat_postrouting {
+		type nat hook postrouting priority 100; policy accept;
+
+		oif != lo masquerade
+	}
+
+	chain mangle_forward {
+		type filter hook forward priority -150; policy accept;
+
+		tcp flags & (syn | rst) == syn tcp option maxseg size set rt mtu
+	}
+}"
+
+$NFT -o -c -f - <<< $RULESET
-- 
2.30.2

[PATCH nft 15/18] optimize: only merge OP_IMPLICIT and OP_EQ relational

[Pablo Neira Ayuso]

Add test to cover this case.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                       | 10 ++++++++++
 .../testcases/optimizations/dumps/skip_non_eq.nft    |  6 ++++++
 tests/shell/testcases/optimizations/skip_non_eq      | 12 ++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_non_eq.nft
 create mode 100755 tests/shell/testcases/optimizations/skip_non_eq

diff --git a/src/optimize.c b/src/optimize.c
index e3d4bc785226..e4508fa5116a 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -164,6 +164,11 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
 		expr_a = stmt_a->expr;
 		expr_b = stmt_b->expr;
 
+		if (expr_a->op != expr_b->op)
+			return false;
+		if (expr_a->op != OP_IMPLICIT && expr_a->op != OP_EQ)
+			return false;
+
 		if (fully_compare) {
 			if (!stmt_expr_supported(expr_a) ||
 			    !stmt_expr_supported(expr_b))
@@ -351,6 +356,11 @@ static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 		clone = stmt_alloc(&internal_location, stmt->ops);
 		switch (stmt->ops->type) {
 		case STMT_EXPRESSION:
+			if (stmt->expr->op != OP_IMPLICIT &&
+			    stmt->expr->op != OP_EQ) {
+				clone->ops = &unsupported_stmt_ops;
+				break;
+			}
 		case STMT_VERDICT:
 			clone->expr = expr_get(stmt->expr);
 			break;
diff --git a/tests/shell/testcases/optimizations/dumps/skip_non_eq.nft b/tests/shell/testcases/optimizations/dumps/skip_non_eq.nft
new file mode 100644
index 000000000000..6df386550357
--- /dev/null
+++ b/tests/shell/testcases/optimizations/dumps/skip_non_eq.nft
@@ -0,0 +1,6 @@
+table inet x {
+	chain y {
+		iifname "eth0" oifname != "eth0" counter packets 0 bytes 0 accept
+		iifname "eth0" oifname "eth0" counter packets 0 bytes 0 accept
+	}
+}
diff --git a/tests/shell/testcases/optimizations/skip_non_eq b/tests/shell/testcases/optimizations/skip_non_eq
new file mode 100755
index 000000000000..431ed0ad05dc
--- /dev/null
+++ b/tests/shell/testcases/optimizations/skip_non_eq
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table inet x {
+	chain y {
+		iifname "eth0" oifname != "eth0" counter packets 0 bytes 0 accept
+		iifname "eth0" oifname "eth0" counter packets 0 bytes 0 accept
+	}
+}"
+
+$NFT -o -f - <<< $RULESET
-- 
2.30.2

[PATCH nft 16/18] optimize: assume verdict is same when rules have no verdict

[Pablo Neira Ayuso]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                        |  3 ++-
 .../testcases/optimizations/dumps/merge_reject.nft    |  6 ++++++
 tests/shell/testcases/optimizations/merge_reject      | 11 +++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/optimize.c b/src/optimize.c
index e4508fa5116a..c6b85d74d302 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -944,7 +944,8 @@ static enum stmt_types merge_stmt_type(const struct optimize_ctx *ctx)
 		}
 	}
 
-	return STMT_INVALID;
+	/* actually no verdict, this assumes rules have the same verdict. */
+	return STMT_VERDICT;
 }
 
 static void merge_rules(const struct optimize_ctx *ctx,
diff --git a/tests/shell/testcases/optimizations/dumps/merge_reject.nft b/tests/shell/testcases/optimizations/dumps/merge_reject.nft
index 9a13e2b96faa..c29ad6d5648b 100644
--- a/tests/shell/testcases/optimizations/dumps/merge_reject.nft
+++ b/tests/shell/testcases/optimizations/dumps/merge_reject.nft
@@ -5,3 +5,9 @@ table ip x {
 		ip daddr 172.30.254.252 tcp dport 3306 counter packets 0 bytes 0 reject with tcp reset
 	}
 }
+table ip6 x {
+	chain y {
+		meta l4proto . ip6 daddr . tcp dport { tcp . aaaa::3 . 8080, tcp . aaaa::2 . 3306, tcp . aaaa::4 . 3306 } counter packets 0 bytes 0 reject
+		ip6 daddr aaaa::5 tcp dport 3306 counter packets 0 bytes 0 reject with tcp reset
+	}
+}
diff --git a/tests/shell/testcases/optimizations/merge_reject b/tests/shell/testcases/optimizations/merge_reject
index 497e8f64dc5d..c0ef9cacbbf0 100755
--- a/tests/shell/testcases/optimizations/merge_reject
+++ b/tests/shell/testcases/optimizations/merge_reject
@@ -13,3 +13,14 @@ RULESET="table ip x {
 }"
 
 $NFT -o -f - <<< $RULESET
+
+RULESET="table ip6 x {
+	chain y {
+		meta l4proto tcp ip6 daddr aaaa::2 tcp dport 3306 counter packets 0 bytes 0 reject
+		meta l4proto tcp ip6 daddr aaaa::3 tcp dport 8080 counter packets 0 bytes 0 reject
+		meta l4proto tcp ip6 daddr aaaa::4 tcp dport 3306 counter packets 0 bytes 0 reject
+		meta l4proto tcp ip6 daddr aaaa::5 tcp dport 3306 counter packets 0 bytes 0 reject with tcp reset
+	}
+}"
+
+$NFT -o -f - <<< $RULESET
-- 
2.30.2

[PATCH nft 17/18] optimize: limit statement is not supported yet

[Pablo Neira Ayuso]

Revert support for limit statement, the limit statement is stateful and
it applies a ratelimit per rule, transformation for merging rules with
the limit statement needs to use anonymous sets with statements.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index c6b85d74d302..2340ef466fc0 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -197,14 +197,6 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b,
 		    expr_b->etype == EXPR_MAP)
 			return __expr_cmp(expr_a->map, expr_b->map);
 		break;
-	case STMT_LIMIT:
-		if (stmt_a->limit.rate != stmt_b->limit.rate ||
-		    stmt_a->limit.unit != stmt_b->limit.unit ||
-		    stmt_a->limit.burst != stmt_b->limit.burst ||
-		    stmt_a->limit.type != stmt_b->limit.type ||
-		    stmt_a->limit.flags != stmt_b->limit.flags)
-			return false;
-		break;
 	case STMT_LOG:
 		if (stmt_a->log.snaplen != stmt_b->log.snaplen ||
 		    stmt_a->log.group != stmt_b->log.group ||
@@ -322,7 +314,6 @@ static bool stmt_type_find(struct optimize_ctx *ctx, const struct stmt *stmt)
 	case STMT_VERDICT:
 	case STMT_COUNTER:
 	case STMT_NOTRACK:
-	case STMT_LIMIT:
 	case STMT_LOG:
 	case STMT_NAT:
 	case STMT_REJECT:
@@ -367,9 +358,6 @@ static int rule_collect_stmts(struct optimize_ctx *ctx, struct rule *rule)
 		case STMT_COUNTER:
 		case STMT_NOTRACK:
 			break;
-		case STMT_LIMIT:
-			memcpy(&clone->limit, &stmt->limit, sizeof(clone->limit));
-			break;
 		case STMT_LOG:
 			memcpy(&clone->log, &stmt->log, sizeof(clone->log));
 			if (stmt->log.prefix)
-- 
2.30.2

[PATCH nft 18/18] libnftables: release top level scope

[Pablo Neira Ayuso]

Otherwise bogus variable redefinition are reported via -o/--optimize:

  redefinition.conf:5:8-21: Error: redefinition of symbol 'interface_inet'
  define interface_inet = enp5s0
         ^^^^^^^^^^^^^^

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/libnftables.c                             |  2 ++
 tests/shell/testcases/optimizations/variables | 15 +++++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100755 tests/shell/testcases/optimizations/variables

diff --git a/src/libnftables.c b/src/libnftables.c
index aac682b706ff..f2a1ef04e80b 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -708,6 +708,8 @@ err:
 	if (rc)
 		nft_cache_release(&nft->cache);
 
+	scope_release(nft->state->scopes[0]);
+
 	return rc;
 }
 
diff --git a/tests/shell/testcases/optimizations/variables b/tests/shell/testcases/optimizations/variables
new file mode 100755
index 000000000000..fa986065006b
--- /dev/null
+++ b/tests/shell/testcases/optimizations/variables
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+set -e
+
+RULESET="define addrv4_vpnnet = 10.1.0.0/16
+
+table ip nat {
+    chain postrouting {
+        type nat hook postrouting priority 0; policy accept;
+
+        ip saddr \$addrv4_vpnnet counter masquerade fully-random comment \"masquerade ipv4\"
+    }
+}"
+
+$NFT -c -o -f - <<< $RULESET
-- 
2.30.2

Re: [PATCH nft 00/18] fixes and improvements for -o/--optimize

[Pablo Neira Ayuso]

On Mon, Jun 20, 2022 at 10:31:57AM +0200, Pablo Neira Ayuso wrote:
> Hi,
> 
> The following patchset contains a batch with fix/improvements for
> -o/--optimize.

For the record: I have pushed out this batch.