[PATCH nft 0/3] vlan followup fixes

[PATCH nft 0/3] vlan followup fixes

[Florian Westphal]

vlan header matching in ip/ip6/inet families may not work, because
default behaviour is to remove the vlan header/tag information.

Update documentation to mention this.
Furthermore, dependency generation was broken so that even if the
vlan striping is disabled matching did not work, as the offset was
not computed correctly.

Add test cases for this too.

Florian Westphal (3):
  doc: mention vlan matching in ip/ip6/inet families
  evaluate: add ethernet header size offset for implicit vlan dependency
  tests: py: add vlan test case for ip/inet family

 doc/payload-expression.txt           |  8 +++++++
 src/evaluate.c                       | 20 ++++++++++++++++-
 tests/py/inet/ether.t                |  6 ++++++
 tests/py/inet/ether.t.json           | 32 ++++++++++++++++++++++++++++
 tests/py/inet/ether.t.payload        | 20 +++++++++++++++++
 tests/py/inet/ether.t.payload.bridge | 16 ++++++++++++++
 tests/py/inet/ether.t.payload.ip     | 20 +++++++++++++++++
 7 files changed, 121 insertions(+), 1 deletion(-)

-- 
2.35.1

[PATCH nft 1/3] doc: mention vlan matching in ip/ip6/inet families

[Florian Westphal]

It only works if vlan_reorder is turned off to disable the vlan tag
removal.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 doc/payload-expression.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/payload-expression.txt b/doc/payload-expression.txt
index 106ff74ce57e..113f5bfc597c 100644
--- a/doc/payload-expression.txt
+++ b/doc/payload-expression.txt
@@ -23,6 +23,14 @@ VLAN HEADER EXPRESSION
 [verse]
 *vlan* {*id* | *dei* | *pcp* | *type*}
 
+The vlan expression is used to match on the vlan header fields.
+This expression will not work in the *ip*, *ip6* and *inet* families,
+unless the vlan interface is configured with the *reorder_hdr off* setting.
+The default is *reorder_hdr on* which will automatically remove the vlan tag
+from the packet. See ip-link(8) for more information.
+For these families its easier to match the vlan interface name
+instead, using the *meta iif* or *meta iifname* expression.
+
 .VLAN header expression
 [options="header"]
 |==================
-- 
2.35.1

[PATCH nft 2/3] evaluate: add ethernet header size offset for implicit vlan dependency

[Florian Westphal]

'vlan id 1'

must also add a ethernet header dep, else nft fetches the payload from
header offset 0 instead of 14.

Reported-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index ca6e5883a1f9..a52867b33be0 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -723,7 +723,25 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 
 		rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 		desc = ctx->pctx.protocol[base].desc;
-		goto check_icmp;
+
+		if (desc == expr->payload.desc)
+			goto check_icmp;
+
+		if (base == PROTO_BASE_LL_HDR) {
+			int link;
+
+			link = proto_find_num(desc, payload->payload.desc);
+			if (link < 0 ||
+			    conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
+				return expr_error(ctx->msgs, payload,
+						  "conflicting protocols specified: %s vs. %s",
+						  desc->name,
+						  payload->payload.desc->name);
+
+			payload->payload.offset += ctx->pctx.stacked_ll[0]->length;
+			rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+			return 1;
+		}
 	}
 
 	if (payload->payload.base == desc->base &&
-- 
2.35.1

[PATCH nft 3/3] tests: py: add vlan test case for ip/inet family

[Florian Westphal]

before fixup, this failed with:

line 4: 'add rule ip test-ip4 input vlan id 1': '[ payload load 2b @ link header + 12 => reg 1 ]' mismatches '[ payload load 2b @ link header + 0 => reg 1 ]'

... because the auto-dependency did not add the preceeding ethernet
header, so vlan was using the wrong offset.

Note than vlan id match in inet input families will only work if header
removal was disabled, i.e.

... add link vethin1 name vethin1.3 type vlan id 3 reorder_hdr off

otherwise, kernel will strip the vlan tag and interface appears as
a normal ethernet interface.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/inet/ether.t                |  6 ++++++
 tests/py/inet/ether.t.json           | 32 ++++++++++++++++++++++++++++
 tests/py/inet/ether.t.payload        | 20 +++++++++++++++++
 tests/py/inet/ether.t.payload.bridge | 16 ++++++++++++++
 tests/py/inet/ether.t.payload.ip     | 20 +++++++++++++++++
 5 files changed, 94 insertions(+)

diff --git a/tests/py/inet/ether.t b/tests/py/inet/ether.t
index c4b1ced7a685..8625f70b7793 100644
--- a/tests/py/inet/ether.t
+++ b/tests/py/inet/ether.t
@@ -12,3 +12,9 @@ tcp dport 22 iiftype ether ether saddr 00:0f:54:0c:11:4 accept;ok;tcp dport 22 e
 tcp dport 22 ether saddr 00:0f:54:0c:11:04 accept;ok
 
 ether saddr 00:0f:54:0c:11:04 accept;ok
+
+vlan id 1;ok
+ether type vlan vlan id 2;ok;vlan id 2
+
+# invalid dependency
+ether type ip vlan id 1;fail
diff --git a/tests/py/inet/ether.t.json b/tests/py/inet/ether.t.json
index 84b184c71ac3..c7a7f88687f8 100644
--- a/tests/py/inet/ether.t.json
+++ b/tests/py/inet/ether.t.json
@@ -88,3 +88,35 @@
     }
 ]
 
+# vlan id 1
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "vlan"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    }
+]
+
+# ether type vlan vlan id 2
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "vlan"
+                }
+            },
+            "op": "==",
+            "right": 2
+        }
+    }
+]
+
diff --git a/tests/py/inet/ether.t.payload b/tests/py/inet/ether.t.payload
index 53648413d588..8b74a7815d8e 100644
--- a/tests/py/inet/ether.t.payload
+++ b/tests/py/inet/ether.t.payload
@@ -30,3 +30,23 @@ inet test-inet input
   [ cmp eq reg 1 0x0c540f00 0x00000411 ]
   [ immediate reg 0 accept ]
 
+# vlan id 1
+netdev test-netdev ingress
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
+
+# ether type vlan vlan id 2
+netdev test-netdev ingress
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000200 ]
+
diff --git a/tests/py/inet/ether.t.payload.bridge b/tests/py/inet/ether.t.payload.bridge
index e9208008214a..0128d5f02b97 100644
--- a/tests/py/inet/ether.t.payload.bridge
+++ b/tests/py/inet/ether.t.payload.bridge
@@ -26,3 +26,19 @@ bridge test-bridge input
   [ cmp eq reg 1 0x0c540f00 0x00000411 ]
   [ immediate reg 0 accept ]
 
+# vlan id 1
+bridge test-bridge input
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
+
+# ether type vlan vlan id 2
+bridge test-bridge input
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000200 ]
+
diff --git a/tests/py/inet/ether.t.payload.ip b/tests/py/inet/ether.t.payload.ip
index a604f603c69e..7c91f412c33e 100644
--- a/tests/py/inet/ether.t.payload.ip
+++ b/tests/py/inet/ether.t.payload.ip
@@ -30,3 +30,23 @@ ip test-ip4 input
   [ cmp eq reg 1 0x0c540f00 0x00000411 ]
   [ immediate reg 0 accept ]
 
+# vlan id 1
+ip test-ip4 input
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
+
+# ether type vlan vlan id 2
+ip test-ip4 input
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000200 ]
+
-- 
2.35.1

Re: [PATCH nft 0/3] vlan followup fixes

[Pablo Neira Ayuso]

On Thu, Sep 29, 2022 at 03:01:10PM +0200, Florian Westphal wrote:
> vlan header matching in ip/ip6/inet families may not work, because
> default behaviour is to remove the vlan header/tag information.
> 
> Update documentation to mention this.
> Furthermore, dependency generation was broken so that even if the
> vlan striping is disabled matching did not work, as the offset was
> not computed correctly.
> 
> Add test cases for this too.

LGTM