All of lore.kernel.org
 help / color / mirror / Atom feed
* [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions
@ 2020-03-07  2:26 Phil Sutter
  2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
  2020-03-09 13:32 ` [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Eric Garver
  0 siblings, 2 replies; 4+ messages in thread
From: Phil Sutter @ 2020-03-07  2:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Eric Garver, netfilter-devel

Duplicate commit 8ac2f3b2fca38's changes to bison parser into JSON
parser by introducing a new context flag signalling we're parsing
concatenated expressions.

Fixes: 8ac2f3b2fca38 ("src: Add support for concatenated set ranges")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Fix accidental reject of concat expressions on LHS.
---
 src/parser_json.c | 47 +++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index 85082ccee7ef6..480b15aeb2507 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -40,6 +40,7 @@
 #define CTX_F_MANGLE	(1 << 5)
 #define CTX_F_SES	(1 << 6)	/* set_elem_expr_stmt */
 #define CTX_F_MAP	(1 << 7)	/* LHS of map_expr */
+#define CTX_F_CONCAT	(1 << 8)	/* inside concat_expr */
 
 struct json_ctx {
 	struct input_descriptor indesc;
@@ -99,6 +100,7 @@ static struct expr *json_parse_primary_expr(struct json_ctx *ctx, json_t *root);
 static struct expr *json_parse_set_rhs_expr(struct json_ctx *ctx, json_t *root);
 static struct expr *json_parse_set_elem_expr_stmt(struct json_ctx *ctx, json_t *root);
 static struct expr *json_parse_map_lhs_expr(struct json_ctx *ctx, json_t *root);
+static struct expr *json_parse_concat_elem_expr(struct json_ctx *ctx, json_t *root);
 static struct stmt *json_parse_stmt(struct json_ctx *ctx, json_t *root);
 
 /* parsing helpers */
@@ -1058,7 +1060,7 @@ static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
 	}
 
 	json_array_foreach(root, index, value) {
-		tmp = json_parse_primary_expr(ctx, value);
+		tmp = json_parse_concat_elem_expr(ctx, value);
 		if (!tmp) {
 			json_error(ctx, "Parsing expr at index %zd failed.", index);
 			expr_free(expr);
@@ -1354,28 +1356,28 @@ static struct expr *json_parse_expr(struct json_ctx *ctx, json_t *root)
 		{ "set", json_parse_set_expr, CTX_F_RHS | CTX_F_STMT }, /* allow this as stmt expr because that allows set references */
 		{ "map", json_parse_map_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS },
 		/* below three are multiton_rhs_expr */
-		{ "prefix", json_parse_prefix_expr, CTX_F_RHS | CTX_F_STMT },
-		{ "range", json_parse_range_expr, CTX_F_RHS | CTX_F_STMT },
-		{ "payload", json_parse_payload_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP },
-		{ "exthdr", json_parse_exthdr_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
+		{ "prefix", json_parse_prefix_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_CONCAT },
+		{ "range", json_parse_range_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_CONCAT },
+		{ "payload", json_parse_payload_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "exthdr", json_parse_exthdr_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "tcp option", json_parse_tcp_option_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES },
 		{ "ip option", json_parse_ip_option_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES },
-		{ "meta", json_parse_meta_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP },
-		{ "osf", json_parse_osf_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_MAP },
-		{ "ipsec", json_parse_xfrm_expr, CTX_F_PRIMARY | CTX_F_MAP },
-		{ "socket", json_parse_socket_expr, CTX_F_PRIMARY },
-		{ "rt", json_parse_rt_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "ct", json_parse_ct_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP },
-		{ "numgen", json_parse_numgen_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
+		{ "meta", json_parse_meta_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "osf", json_parse_osf_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_MAP | CTX_F_CONCAT },
+		{ "ipsec", json_parse_xfrm_expr, CTX_F_PRIMARY | CTX_F_MAP | CTX_F_CONCAT },
+		{ "socket", json_parse_socket_expr, CTX_F_PRIMARY | CTX_F_CONCAT },
+		{ "rt", json_parse_rt_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "ct", json_parse_ct_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "numgen", json_parse_numgen_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		/* below two are hash expr */
-		{ "jhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "symhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "fib", json_parse_fib_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "|", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "^", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "&", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ ">>", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "<<", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
+		{ "jhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "symhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "fib", json_parse_fib_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "|", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "^", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "&", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ ">>", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "<<", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "accept", json_parse_verdict_expr, CTX_F_RHS | CTX_F_SET_RHS },
 		{ "drop", json_parse_verdict_expr, CTX_F_RHS | CTX_F_SET_RHS },
 		{ "continue", json_parse_verdict_expr, CTX_F_RHS | CTX_F_SET_RHS },
@@ -1500,6 +1502,11 @@ static struct expr *json_parse_map_lhs_expr(struct json_ctx *ctx, json_t *root)
 	return json_parse_flagged_expr(ctx, CTX_F_MAP, root);
 }
 
+static struct expr *json_parse_concat_elem_expr(struct json_ctx *ctx, json_t *root)
+{
+	return json_parse_flagged_expr(ctx, CTX_F_CONCAT, root);
+}
+
 static struct expr *json_parse_dtype_expr(struct json_ctx *ctx, json_t *root)
 {
 	if (json_is_string(root)) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges
  2020-03-07  2:26 [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Phil Sutter
@ 2020-03-07  2:26 ` Phil Sutter
  2020-03-07 10:16   ` Stefano Brivio
  2020-03-09 13:32 ` [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Eric Garver
  1 sibling, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2020-03-07  2:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Eric Garver, netfilter-devel

Very basic testing, just a set definition, a rule which references it
and another one with an anonymous set.

Sadly this is already enough to expose some pending issues:

* Payload dependency killing ignores the concatenated IP header
  expressions on LHS, so rule output is asymmetric.

* Anonymous sets don't accept concatenated ranges yet, so the second
  rule is manually disabled for now.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- New patch.
---
 tests/py/inet/sets.t                |  6 +++++
 tests/py/inet/sets.t.json           | 35 +++++++++++++++++++++++++++++
 tests/py/inet/sets.t.payload.bridge | 13 +++++++++++
 tests/py/inet/sets.t.payload.inet   | 11 +++++++++
 tests/py/inet/sets.t.payload.netdev | 12 ++++++++++
 5 files changed, 77 insertions(+)

diff --git a/tests/py/inet/sets.t b/tests/py/inet/sets.t
index daf8f2d6ca302..e0b0ee867f9b7 100644
--- a/tests/py/inet/sets.t
+++ b/tests/py/inet/sets.t
@@ -16,3 +16,9 @@ ip saddr != @set2 drop;fail
 
 ip6 daddr != @set2 accept;ok
 ip6 daddr @set1 drop;fail
+
+!set3 type ipv4_addr . ipv4_addr . inet_service flags interval;ok
+?set3 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535;ok
+
+ip saddr . ip daddr . tcp dport @set3 accept;ok
+-ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept;ok
diff --git a/tests/py/inet/sets.t.json b/tests/py/inet/sets.t.json
index bcb638f2664d5..58e19ef647058 100644
--- a/tests/py/inet/sets.t.json
+++ b/tests/py/inet/sets.t.json
@@ -36,3 +36,38 @@
     }
 ]
 
+# ip saddr . ip daddr . tcp dport @set3 accept
+[
+    {
+        "match": {
+            "left": {
+                "concat": [
+                    {
+                        "payload": {
+                            "field": "saddr",
+                            "protocol": "ip"
+                        }
+                    },
+                    {
+                        "payload": {
+                            "field": "daddr",
+                            "protocol": "ip"
+                        }
+                    },
+                    {
+                        "payload": {
+                            "field": "dport",
+                            "protocol": "tcp"
+                        }
+                    }
+                ]
+            },
+            "op": "==",
+            "right": "@set3"
+        }
+    },
+    {
+        "accept": null
+    }
+]
+
diff --git a/tests/py/inet/sets.t.payload.bridge b/tests/py/inet/sets.t.payload.bridge
index f5aaab1d79bc6..089d9dd7a28dd 100644
--- a/tests/py/inet/sets.t.payload.bridge
+++ b/tests/py/inet/sets.t.payload.bridge
@@ -13,3 +13,16 @@ bridge test-inet input
   [ payload load 16b @ network header + 24 => reg 1 ]
   [ lookup reg 1 set set2 0x1 ]
   [ immediate reg 0 accept ]
+
+# ip saddr . ip daddr . tcp dport @set3 accept
+bridge 
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ payload load 4b @ network header + 16 => reg 9 ]
+  [ payload load 2b @ transport header + 2 => reg 10 ]
+  [ lookup reg 1 set set3 ]
+  [ immediate reg 0 accept ]
+
diff --git a/tests/py/inet/sets.t.payload.inet b/tests/py/inet/sets.t.payload.inet
index 1584fc07451eb..c5acd6103a038 100644
--- a/tests/py/inet/sets.t.payload.inet
+++ b/tests/py/inet/sets.t.payload.inet
@@ -14,4 +14,15 @@ inet test-inet input
   [ lookup reg 1 set set2 0x1 ]
   [ immediate reg 0 accept ]
 
+# ip saddr . ip daddr . tcp dport @set3 accept
+inet 
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ payload load 4b @ network header + 16 => reg 9 ]
+  [ payload load 2b @ transport header + 2 => reg 10 ]
+  [ lookup reg 1 set set3 ]
+  [ immediate reg 0 accept ]
 
diff --git a/tests/py/inet/sets.t.payload.netdev b/tests/py/inet/sets.t.payload.netdev
index 9c94e38429fb7..82994eabf48b7 100644
--- a/tests/py/inet/sets.t.payload.netdev
+++ b/tests/py/inet/sets.t.payload.netdev
@@ -14,3 +14,15 @@ netdev test-netdev ingress
   [ lookup reg 1 set set2 0x1 ]
   [ immediate reg 0 accept ]
 
+# ip saddr . ip daddr . tcp dport @ set3 accept
+inet 
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ payload load 4b @ network header + 16 => reg 9 ]
+  [ payload load 2b @ transport header + 2 => reg 10 ]
+  [ lookup reg 1 set set3 ]
+  [ immediate reg 0 accept ]
+
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges
  2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
@ 2020-03-07 10:16   ` Stefano Brivio
  0 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2020-03-07 10:16 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Pablo Neira Ayuso, Eric Garver, netfilter-devel

On Sat,  7 Mar 2020 03:26:33 +0100
Phil Sutter <phil@nwl.cc> wrote:

> * Anonymous sets don't accept concatenated ranges yet, so the second
>   rule is manually disabled for now.

By the way, I'm looking into this these days.

-- 
Stefano


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions
  2020-03-07  2:26 [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Phil Sutter
  2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
@ 2020-03-09 13:32 ` Eric Garver
  1 sibling, 0 replies; 4+ messages in thread
From: Eric Garver @ 2020-03-09 13:32 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Pablo Neira Ayuso, netfilter-devel

On Sat, Mar 07, 2020 at 03:26:32AM +0100, Phil Sutter wrote:
> Duplicate commit 8ac2f3b2fca38's changes to bison parser into JSON
> parser by introducing a new context flag signalling we're parsing
> concatenated expressions.
> 
> Fixes: 8ac2f3b2fca38 ("src: Add support for concatenated set ranges")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Fix accidental reject of concat expressions on LHS.
> ---

Thanks Phil. This passes all my tests and the patch looks good.

Acked-by: Eric Garver <eric@garver.life>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-09 13:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-07  2:26 [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Phil Sutter
2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
2020-03-07 10:16   ` Stefano Brivio
2020-03-09 13:32 ` [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Eric Garver

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.