[PATCH nft 00/11] revisiting protocol conflict resolution

[PATCH nft 00/11] revisiting protocol conflict resolution

[Pablo Neira Ayuso]

Hi,

This patchset revisits Florian's protocol conflict resolution to fully
support vlan matching without having to specify 'ether type vlan',
through our automatic dependency generation happening from the
evaluation step.

Patches from 1 to 7 are cleanups, then 8 to 11 deal with the problem
above.

To show you an example:

# nft --debug=netlink add rule netdev filter ingress \
	vlan id 1 ip saddr 10.0.0.0/23 udp dport 53 counter

generates the following bytecode:

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 ]
  [ payload load 2b @ link header + 16 => reg 1 ]
  [ cmp eq reg 1 0x00000008 ]
  [ payload load 4b @ network header + 12 => reg 1 ]
  [ bitwise reg 1 = (reg=1 & 0x00feffff ) ^ 0x00000000 ]
  [ cmp eq reg 1 0x0000000a ]
  [ payload load 1b @ network header + 9 => reg 1 ]
  [ cmp eq reg 1 0x00000011 ]
  [ payload load 2b @ transport header + 2 => reg 1 ]
  [ cmp eq reg 1 0x00003500 ]
  [ counter pkts 0 bytes 0 ]

So the only addition wrt. to bridge are these two new instructions:

  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]

that fetch the interface type and then check for ARPHRD_ETHER.

We can investigate later on if we can generalize the protocol context
code to deal with stackable headers in a more generic way. We can
discuss some idea during NetDev 1.1.

Thanks!

Pablo Neira Ayuso (11):
  evaluate: resolve_protocol_conflict() should return int
  evaluate: move inet/netdev protocol context supersede logic to supersede_dep()
  evaluate: check if we have to resolve a conflict in first place
  evaluate: don't adjust offset from resolve_protocol_conflict()
  evaluate: only try to replace dummy protocol from link-layer context
  evaluate: assert on invalid base in resolve_protocol_conflict()
  evaluate: wrap protocol context debunk into function
  evaluate: generate ether type payload after meta iiftype
  proto: proto_dev_type() returns interface type for base protocols too
  src: annotate follow up dependency just after killing another
  tests/py: test vlan on ingress

 src/evaluate.c                        | 133 +++++++++++--------
 src/netlink_delinearize.c             |  45 ++++---
 src/proto.c                           |  12 +-
 tests/py/bridge/vlan.t                |   2 +
 tests/py/bridge/vlan.t.payload.netdev | 235 ++++++++++++++++++++++++++++++++++
 5 files changed, 355 insertions(+), 72 deletions(-)
 create mode 100644 tests/py/bridge/vlan.t.payload.netdev

-- 
2.1.4

[PATCH nft 01/11] evaluate: resolve_protocol_conflict() should return int

[Pablo Neira Ayuso]

Instead of bool, expr_error() returns -1 if we fail to create
dependencies. We need to propagate this error value.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 5e9783d..7898ed4 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -371,8 +371,7 @@ static bool supersede_dep(const struct proto_desc *have,
 	return true;
 }
 
-static bool resolve_protocol_conflict(struct eval_ctx *ctx,
-				      struct expr *payload)
+static int resolve_protocol_conflict(struct eval_ctx *ctx, struct expr *payload)
 {
 	const struct hook_proto_desc *h = &hook_proto_desc[ctx->pctx.family];
 	enum proto_bases base = payload->payload.base;
@@ -381,14 +380,13 @@ static bool resolve_protocol_conflict(struct eval_ctx *ctx,
 	int link;
 
 	desc = ctx->pctx.protocol[base].desc;
-
 	if (desc == payload->payload.desc) {
 		payload->payload.offset += ctx->pctx.protocol[base].offset;
-		return true;
+		return 0;
 	}
 
 	if (payload->payload.base != h->base)
-		return false;
+		return 1;
 
 	if (supersede_dep(desc, payload)) {
 		uint16_t type;
@@ -405,7 +403,7 @@ static bool resolve_protocol_conflict(struct eval_ctx *ctx,
 
 		list_add_tail(&nstmt->list, &ctx->stmt->list);
 		ctx->pctx.protocol[base].desc = payload->payload.desc;
-		return true;
+		return 0;
 	}
 
 	if (base < PROTO_BASE_MAX) {
@@ -416,21 +414,21 @@ static bool resolve_protocol_conflict(struct eval_ctx *ctx,
 			ctx->pctx.protocol[base].desc = next;
 			ctx->pctx.protocol[base].offset += desc->length;
 			payload->payload.offset += desc->length;
-			return true;
+			return 0;
 		} else if (next) {
-			return false;
+			return 1;
 		}
 	}
 
 	link = proto_find_num(desc, payload->payload.desc);
 	if (link < 0 || conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
-		return false;
+		return 1;
 
 	payload->payload.offset += ctx->pctx.protocol[base].offset;
 	list_add_tail(&nstmt->list, &ctx->stmt->list);
 	ctx->pctx.protocol[base + 1].desc = NULL;
 
-	return true;
+	return 0;
 }
 
 /*
@@ -443,17 +441,21 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 	struct expr *payload = expr;
 	enum proto_bases base = payload->payload.base;
 	struct stmt *nstmt;
+	int err;
 
 	if (ctx->pctx.protocol[base].desc == NULL) {
 		if (payload_gen_dependency(ctx, payload, &nstmt) < 0)
 			return -1;
 		list_add_tail(&nstmt->list, &ctx->stmt->list);
-	} else if (!resolve_protocol_conflict(ctx, payload))
+	} else {
+		err = resolve_protocol_conflict(ctx, payload);
+		if (err <= 0)
+			return err;
 		return expr_error(ctx->msgs, payload,
 				  "conflicting protocols specified: %s vs. %s",
 				  ctx->pctx.protocol[base].desc->name,
 				  payload->payload.desc->name);
-
+	}
 	return 0;
 }
 
-- 
2.1.4

[PATCH nft 02/11] evaluate: move inet/netdev protocol context supersede logic to supersede_dep()

[Pablo Neira Ayuso]

This is a cleanup to untangle this logic a bit.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 7898ed4..93f408d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -359,16 +359,32 @@ conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
  * ip saddr adds meta dependency on ipv4 packets
  * ether saddr adds another dependeny on ethernet frames.
  */
-static bool supersede_dep(const struct proto_desc *have,
-			  struct expr *payload)
+static int supersede_dep(struct eval_ctx *ctx, const struct proto_desc *have,
+			 struct expr *payload)
 {
+	enum proto_bases base = payload->payload.base;
+	struct stmt *nstmt;
+	uint16_t type;
+
 	if (payload->payload.base != PROTO_BASE_LL_HDR || have->length)
-		return false;
+		return 1;
 
 	if (have != &proto_inet && have != &proto_netdev)
-		return false;
+		return 1;
 
-	return true;
+	if (proto_dev_type(payload->payload.desc, &type) < 0)
+		return expr_error(ctx->msgs, payload,
+				  "protocol specification is invalid "
+				  "for this family");
+
+	nstmt = meta_stmt_meta_iiftype(&payload->location, type);
+	if (stmt_evaluate(ctx, nstmt) < 0)
+		return expr_error(ctx->msgs, payload,
+				  "dependency statement is invalid");
+
+	list_add_tail(&nstmt->list, &ctx->stmt->list);
+	ctx->pctx.protocol[base].desc = payload->payload.desc;
+	return 0;
 }
 
 static int resolve_protocol_conflict(struct eval_ctx *ctx, struct expr *payload)
@@ -377,7 +393,7 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx, struct expr *payload)
 	enum proto_bases base = payload->payload.base;
 	const struct proto_desc *desc;
 	struct stmt *nstmt = NULL;
-	int link;
+	int link, err;
 
 	desc = ctx->pctx.protocol[base].desc;
 	if (desc == payload->payload.desc) {
@@ -388,23 +404,9 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx, struct expr *payload)
 	if (payload->payload.base != h->base)
 		return 1;
 
-	if (supersede_dep(desc, payload)) {
-		uint16_t type;
-
-		if (proto_dev_type(payload->payload.desc, &type) < 0)
-			return expr_error(ctx->msgs, payload,
-					  "protocol specification is invalid "
-					  "for this family");
-
-		nstmt = meta_stmt_meta_iiftype(&payload->location, type);
-		if (stmt_evaluate(ctx, nstmt) < 0)
-			return expr_error(ctx->msgs, payload,
-					  "dependency statement is invalid");
-
-		list_add_tail(&nstmt->list, &ctx->stmt->list);
-		ctx->pctx.protocol[base].desc = payload->payload.desc;
-		return 0;
-	}
+	err = supersede_dep(ctx, desc, payload);
+	if (err <= 0)
+		return err;
 
 	if (base < PROTO_BASE_MAX) {
 		const struct proto_desc *next = ctx->pctx.protocol[base + 1].desc;
-- 
2.1.4

[PATCH nft 03/11] evaluate: check if we have to resolve a conflict in first place

[Pablo Neira Ayuso]

So we enter resolve_protocol_conflict() only when we really have a
conflict that we want to try to resolve.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 93f408d..a294070 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -387,23 +387,19 @@ static int supersede_dep(struct eval_ctx *ctx, const struct proto_desc *have,
 	return 0;
 }
 
-static int resolve_protocol_conflict(struct eval_ctx *ctx, struct expr *payload)
+static int resolve_protocol_conflict(struct eval_ctx *ctx,
+				     const struct proto_desc *desc,
+				     struct expr *payload)
 {
-	const struct hook_proto_desc *h = &hook_proto_desc[ctx->pctx.family];
 	enum proto_bases base = payload->payload.base;
-	const struct proto_desc *desc;
 	struct stmt *nstmt = NULL;
 	int link, err;
 
-	desc = ctx->pctx.protocol[base].desc;
 	if (desc == payload->payload.desc) {
 		payload->payload.offset += ctx->pctx.protocol[base].offset;
 		return 0;
 	}
 
-	if (payload->payload.base != h->base)
-		return 1;
-
 	err = supersede_dep(ctx, desc, payload);
 	if (err <= 0)
 		return err;
@@ -442,17 +438,24 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 {
 	struct expr *payload = expr;
 	enum proto_bases base = payload->payload.base;
+	const struct proto_desc *desc;
 	struct stmt *nstmt;
 	int err;
 
-	if (ctx->pctx.protocol[base].desc == NULL) {
+	desc = ctx->pctx.protocol[base].desc;
+	if (desc == NULL) {
 		if (payload_gen_dependency(ctx, payload, &nstmt) < 0)
 			return -1;
 		list_add_tail(&nstmt->list, &ctx->stmt->list);
 	} else {
-		err = resolve_protocol_conflict(ctx, payload);
-		if (err <= 0)
-			return err;
+		/* If we already have context and this payload is on the same
+		 * base, try to resolve the protocol conflict.
+		 */
+		if (payload->payload.base == desc->base) {
+			err = resolve_protocol_conflict(ctx, desc, payload);
+			if (err <= 0)
+				return err;
+		}
 		return expr_error(ctx->msgs, payload,
 				  "conflicting protocols specified: %s vs. %s",
 				  ctx->pctx.protocol[base].desc->name,
-- 
2.1.4

[PATCH nft 04/11] evaluate: don't adjust offset from resolve_protocol_conflict()

[Pablo Neira Ayuso]

This is not itself a conflict, move this check out of this function.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index a294070..5ef035b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -395,11 +395,6 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 	struct stmt *nstmt = NULL;
 	int link, err;
 
-	if (desc == payload->payload.desc) {
-		payload->payload.offset += ctx->pctx.protocol[base].offset;
-		return 0;
-	}
-
 	err = supersede_dep(ctx, desc, payload);
 	if (err <= 0)
 		return err;
@@ -448,6 +443,14 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 			return -1;
 		list_add_tail(&nstmt->list, &ctx->stmt->list);
 	} else {
+		/* No conflict: Same payload protocol as context, adjust offset
+		 * if needed.
+		 */
+		if (desc == payload->payload.desc) {
+			payload->payload.offset +=
+				ctx->pctx.protocol[base].offset;
+			return 0;
+		}
 		/* If we already have context and this payload is on the same
 		 * base, try to resolve the protocol conflict.
 		 */
-- 
2.1.4

[PATCH nft 05/11] evaluate: only try to replace dummy protocol from link-layer context

[Pablo Neira Ayuso]

Add proto_is_dummy() that returns true for netdev and inet family, the
only two using a dummy link-layer protocol base definition.

Rename supersede_dep() to meta_iiftype_gen_dependency() since this is
generating the implicit meta iiftype check for netdev and inet.

This patch also gets rid of the have->length check. The tests pass fine
without this so I suspect this is superfluos.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 5ef035b..eb442d5 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -359,19 +359,14 @@ conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
  * ip saddr adds meta dependency on ipv4 packets
  * ether saddr adds another dependeny on ethernet frames.
  */
-static int supersede_dep(struct eval_ctx *ctx, const struct proto_desc *have,
-			 struct expr *payload)
+static int meta_iiftype_gen_dependency(struct eval_ctx *ctx,
+				       const struct proto_desc *have,
+				       struct expr *payload)
 {
 	enum proto_bases base = payload->payload.base;
 	struct stmt *nstmt;
 	uint16_t type;
 
-	if (payload->payload.base != PROTO_BASE_LL_HDR || have->length)
-		return 1;
-
-	if (have != &proto_inet && have != &proto_netdev)
-		return 1;
-
 	if (proto_dev_type(payload->payload.desc, &type) < 0)
 		return expr_error(ctx->msgs, payload,
 				  "protocol specification is invalid "
@@ -387,6 +382,11 @@ static int supersede_dep(struct eval_ctx *ctx, const struct proto_desc *have,
 	return 0;
 }
 
+static bool proto_is_dummy(const struct proto_desc *desc)
+{
+	return desc == &proto_inet || desc == &proto_netdev;
+}
+
 static int resolve_protocol_conflict(struct eval_ctx *ctx,
 				     const struct proto_desc *desc,
 				     struct expr *payload)
@@ -395,9 +395,12 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 	struct stmt *nstmt = NULL;
 	int link, err;
 
-	err = supersede_dep(ctx, desc, payload);
-	if (err <= 0)
-		return err;
+	if (payload->payload.base == PROTO_BASE_LL_HDR &&
+	    proto_is_dummy(desc)) {
+		err = meta_iiftype_gen_dependency(ctx, desc, payload);
+		if (err <= 0)
+			return err;
+	}
 
 	if (base < PROTO_BASE_MAX) {
 		const struct proto_desc *next = ctx->pctx.protocol[base + 1].desc;
-- 
2.1.4

[PATCH nft 06/11] evaluate: assert on invalid base in resolve_protocol_conflict()

[Pablo Neira Ayuso]

We already have similar code in the tree, we shouldn't see bases over
transport yet.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index eb442d5..b70ff07 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -392,6 +392,7 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 				     struct expr *payload)
 {
 	enum proto_bases base = payload->payload.base;
+	const struct proto_desc *next;
 	struct stmt *nstmt = NULL;
 	int link, err;
 
@@ -402,18 +403,17 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 			return err;
 	}
 
-	if (base < PROTO_BASE_MAX) {
-		const struct proto_desc *next = ctx->pctx.protocol[base + 1].desc;
+	assert(base < PROTO_BASE_MAX);
+	next = ctx->pctx.protocol[base + 1].desc;
 
-		if (payload->payload.desc == next) {
-			ctx->pctx.protocol[base + 1].desc = NULL;
-			ctx->pctx.protocol[base].desc = next;
-			ctx->pctx.protocol[base].offset += desc->length;
-			payload->payload.offset += desc->length;
-			return 0;
-		} else if (next) {
-			return 1;
-		}
+	if (payload->payload.desc == next) {
+		ctx->pctx.protocol[base + 1].desc = NULL;
+		ctx->pctx.protocol[base].desc = next;
+		ctx->pctx.protocol[base].offset += desc->length;
+		payload->payload.offset += desc->length;
+		return 0;
+	} else if (next) {
+		return 1;
 	}
 
 	link = proto_find_num(desc, payload->payload.desc);
-- 
2.1.4

[PATCH nft 07/11] evaluate: wrap protocol context debunk into function

[Pablo Neira Ayuso]

ether type vlan sets the network layer protocol context to vlan. This
function debunks the existing link layer protocol context by setting it
to vlan.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index b70ff07..e53627a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -382,6 +382,17 @@ static int meta_iiftype_gen_dependency(struct eval_ctx *ctx,
 	return 0;
 }
 
+static void proto_ctx_debunk(struct eval_ctx *ctx,
+			     const struct proto_desc *desc,
+			     const struct proto_desc *next,
+			     struct expr *payload, enum proto_bases base)
+{
+	ctx->pctx.protocol[base + 1].desc = NULL;
+	ctx->pctx.protocol[base].desc = next;
+	ctx->pctx.protocol[base].offset += desc->length;
+	payload->payload.offset += desc->length;
+}
+
 static bool proto_is_dummy(const struct proto_desc *desc)
 {
 	return desc == &proto_inet || desc == &proto_netdev;
@@ -406,16 +417,18 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 	assert(base < PROTO_BASE_MAX);
 	next = ctx->pctx.protocol[base + 1].desc;
 
+	/* ether type vlan sets vlan as network protocol, debunk ethernet if it
+	 * is already there.
+	 */
 	if (payload->payload.desc == next) {
-		ctx->pctx.protocol[base + 1].desc = NULL;
-		ctx->pctx.protocol[base].desc = next;
-		ctx->pctx.protocol[base].offset += desc->length;
-		payload->payload.offset += desc->length;
+		proto_ctx_debunk(ctx, desc, next, payload, base);
 		return 0;
-	} else if (next) {
-		return 1;
 	}
 
+	/* This payload and the existing context don't match, conflict. */
+	if (next != NULL)
+		return 1;
+
 	link = proto_find_num(desc, payload->payload.desc);
 	if (link < 0 || conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
 		return 1;
-- 
2.1.4

[PATCH nft 08/11] evaluate: generate ether type payload after meta iiftype

[Pablo Neira Ayuso]

Once the meta iiftype is generated, we shouldn't return from
resolve_protocol_conflict() since we also need to generate the ether
type payload implicit match after it.

This gets rid of the manual proto-ctx update from
meta_iiftype_gen_dependency() that we don't need since stmt_evaluate()
already handles this for us.

Moreover, skip error reporting once we verify that the protocol conflict
has been resolved.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index e53627a..ed78896 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -360,10 +360,8 @@ conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
  * ether saddr adds another dependeny on ethernet frames.
  */
 static int meta_iiftype_gen_dependency(struct eval_ctx *ctx,
-				       const struct proto_desc *have,
-				       struct expr *payload)
+				       struct expr *payload, struct stmt **res)
 {
-	enum proto_bases base = payload->payload.base;
 	struct stmt *nstmt;
 	uint16_t type;
 
@@ -377,8 +375,7 @@ static int meta_iiftype_gen_dependency(struct eval_ctx *ctx,
 		return expr_error(ctx->msgs, payload,
 				  "dependency statement is invalid");
 
-	list_add_tail(&nstmt->list, &ctx->stmt->list);
-	ctx->pctx.protocol[base].desc = payload->payload.desc;
+	*res = nstmt;
 	return 0;
 }
 
@@ -409,9 +406,11 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 
 	if (payload->payload.base == PROTO_BASE_LL_HDR &&
 	    proto_is_dummy(desc)) {
-		err = meta_iiftype_gen_dependency(ctx, desc, payload);
-		if (err <= 0)
+		err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
+		if (err < 0)
 			return err;
+
+		list_add_tail(&nstmt->list, &ctx->stmt->list);
 	}
 
 	assert(base < PROTO_BASE_MAX);
@@ -474,6 +473,10 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 			err = resolve_protocol_conflict(ctx, desc, payload);
 			if (err <= 0)
 				return err;
+
+			desc = ctx->pctx.protocol[base].desc;
+			if (desc == payload->payload.desc)
+				return 0;
 		}
 		return expr_error(ctx->msgs, payload,
 				  "conflicting protocols specified: %s vs. %s",
-- 
2.1.4

[PATCH nft 09/11] proto: proto_dev_type() returns interface type for base protocols too

[Pablo Neira Ayuso]

The device protocol definition provides a mapping between the interface
type, ie. ARPHDR_*, and the overlying protocol base definition, eg.
proto_eth.

This patch updates proto_dev_type() so it also returns a mapping for
these overlying ethernet protocol definitions, ie. ip, ip6, vlan, ip,
arp.

This patch required to resolve problems with automatic dependency
generation for vlan in the netdev and inet families.

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

diff --git a/src/proto.c b/src/proto.c
index 65ee158..0cd9fdb 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -94,13 +94,21 @@ static const struct dev_proto_desc dev_proto_desc[] = {
  */
 int proto_dev_type(const struct proto_desc *desc, uint16_t *res)
 {
-	unsigned int i;
+	const struct proto_desc *base;
+	unsigned int i, j;
 
 	for (i = 0; i < array_size(dev_proto_desc); i++) {
-		if (dev_proto_desc[i].desc == desc) {
+		base = dev_proto_desc[i].desc;
+		if (base == desc) {
 			*res = dev_proto_desc[i].type;
 			return 0;
 		}
+		for (j = 0; j < array_size(base->protocols); j++) {
+			if (base->protocols[j].desc == desc) {
+				*res = dev_proto_desc[i].type;
+				return 0;
+			}
+		}
 	}
 	return -1;
 }
-- 
2.1.4

[PATCH nft 10/11] src: annotate follow up dependency just after killing another

[Pablo Neira Ayuso]

The inet and netdev families generate two implicit dependencies to check
for the interface type, so we have to check just after killing an implicit
dependency if there is another that we should annotate to kill it as well.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/netlink_delinearize.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index cb9c3ab..bbe1876 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1031,6 +1031,30 @@ static void integer_type_postprocess(struct expr *expr)
 	}
 }
 
+static void payload_dependency_save(struct rule_pp_ctx *ctx, unsigned int base,
+				    struct stmt *nstmt, struct expr *tmp)
+{
+	unsigned int proto = mpz_get_be16(tmp->value);
+	const struct proto_desc *desc, *next;
+	bool stacked_header = false;
+
+	desc = ctx->pctx.protocol[base].desc;
+
+	assert(desc);
+	if (desc) {
+		next = proto_find_upper(desc, proto);
+		stacked_header = next && next->base == base;
+	}
+
+	if (stacked_header) {
+		ctx->pctx.protocol[base].desc = next;
+		ctx->pctx.protocol[base].offset += desc->length;
+		payload_dependency_store(ctx, nstmt, base - 1);
+	} else {
+		payload_dependency_store(ctx, nstmt, base);
+	}
+}
+
 static void payload_match_expand(struct rule_pp_ctx *ctx,
 				 struct expr *expr,
 				 struct expr *payload)
@@ -1068,26 +1092,11 @@ static void payload_match_expand(struct rule_pp_ctx *ctx,
 		 */
 		if (ctx->pbase == PROTO_BASE_INVALID &&
 		    left->flags & EXPR_F_PROTOCOL) {
-			unsigned int proto = mpz_get_be16(tmp->value);
-			const struct proto_desc *desc, *next;
-			bool stacked_header = false;
-
-			desc = ctx->pctx.protocol[base].desc;
-			assert(desc);
-			if (desc) {
-				next = proto_find_upper(desc, proto);
-				stacked_header = next && next->base == base;
-			}
-
-			if (stacked_header) {
-				ctx->pctx.protocol[base].desc = next;
-				ctx->pctx.protocol[base].offset += desc->length;
-				payload_dependency_store(ctx, nstmt, base - 1);
-			} else {
-				payload_dependency_store(ctx, nstmt, base);
-			}
+			payload_dependency_save(ctx, base, nstmt, tmp);
 		} else {
 			payload_dependency_kill(ctx, nexpr->left);
+			if (left->flags & EXPR_F_PROTOCOL)
+				payload_dependency_save(ctx, base, nstmt, tmp);
 		}
 	}
 	list_del(&ctx->stmt->list);
-- 
2.1.4

[PATCH nft 11/11] tests/py: test vlan on ingress

[Pablo Neira Ayuso]

This generates the same code as bridge does, but it includes this check
in first place.

  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 tests/py/bridge/vlan.t                |   2 +
 tests/py/bridge/vlan.t.payload.netdev | 235 ++++++++++++++++++++++++++++++++++
 2 files changed, 237 insertions(+)
 create mode 100644 tests/py/bridge/vlan.t.payload.netdev

diff --git a/tests/py/bridge/vlan.t b/tests/py/bridge/vlan.t
index e7c75bb..526d7cc 100644
--- a/tests/py/bridge/vlan.t
+++ b/tests/py/bridge/vlan.t
@@ -1,6 +1,8 @@
 :input;type filter hook input priority 0
+:ingress;type filter hook ingress device lo priority 0
 
 *bridge;test-bridge;input
+*netdev;test-netdev;ingress
 
 vlan id 4094;ok
 vlan id 0;ok
diff --git a/tests/py/bridge/vlan.t.payload.netdev b/tests/py/bridge/vlan.t.payload.netdev
new file mode 100644
index 0000000..62c7adf
--- /dev/null
+++ b/tests/py/bridge/vlan.t.payload.netdev
@@ -0,0 +1,235 @@
+# vlan id 4094
+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 0x0000fe0f ]
+
+# vlan id 0
+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 0x00000000 ]
+
+# vlan id 4094 vlan cfi 0
+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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000000 ]
+
+# vlan id 4094 vlan cfi != 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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp neq reg 1 0x00000010 ]
+
+# vlan id 4094 vlan cfi 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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000010 ]
+
+# ether type vlan vlan id 4094
+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 0x0000fe0f ]
+
+# ether type vlan vlan id 0
+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 0x00000000 ]
+
+# ether type vlan vlan id 4094 vlan cfi 0
+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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000000 ]
+
+# ether type vlan vlan id 4094 vlan cfi 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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000010 ]
+
+# vlan id 4094 tcp dport 22
+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 0x0000fe0f ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ cmp eq reg 1 0x00001600 ]
+
+# vlan id 1 ip saddr 10.0.0.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 ]
+  [ payload load 2b @ link header + 16 => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x0100000a ]
+
+# vlan id 1 ip saddr 10.0.0.0/23
+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 ]
+  [ payload load 2b @ link header + 16 => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00feffff ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x0000000a ]
+
+# vlan id 1 ip saddr 10.0.0.0/23 udp dport 53
+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 ]
+  [ payload load 2b @ link header + 16 => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00feffff ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 1b @ network header + 9 => reg 1 ]
+  [ cmp eq reg 1 0x00000011 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ cmp eq reg 1 0x00003500 ]
+
+# ether type vlan vlan id 1 ip saddr 10.0.0.0/23 udp dport 53
+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 ]
+  [ payload load 2b @ link header + 16 => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00feffff ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 1b @ network header + 9 => reg 1 ]
+  [ cmp eq reg 1 0x00000011 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ cmp eq reg 1 0x00003500 ]
+
+# vlan id 4094 vlan cfi 1 vlan pcp 7
+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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000010 ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000e0 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x000000e0 ]
+
+# vlan id 4094 vlan cfi 1 vlan pcp 3
+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 0x0000fe0f ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000010 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000010 ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000e0 ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000060 ]
+
+# vlan id { 1, 2, 4, 100, 4095 } vlan pcp 1-3
+set%d test-netdev 3
+set%d test-netdev 0
+	element 00000100  : 0 [end]	element 00000200  : 0 [end]	element 00000400  : 0 [end]	element 00006400  : 0 [end]	element 0000ff0f  : 0 [end]
+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 ]
+  [ lookup reg 1 set set%d ]
+  [ payload load 1b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000e0 ) ^ 0x00000000 ]
+  [ cmp gte reg 1 0x00000001 ]
+  [ cmp lte reg 1 0x00000003 ]
+
-- 
2.1.4