[PATCH nftables 0/8] Support for shifted port-ranges in NAT

[PATCH nftables 0/8] Support for shifted port-ranges in NAT

[Jeremy Sowden]

Support for shifted port-ranges was added to iptables for DNAT in 2018.
This allows one to redirect packets intended for one port to another in
a range in such a way that the new port chosen has the same offset in
the range as the original port had from a specified base value.

For example, by using the base value 2000, one could redirect packets
intended for 10.0.0.1:2000-3000 to 10.10.0.1:12000-13000 so that the old
and new ports were at the same offset in their respective ranges, i.e.:

  10.0.0.1:2345 -> 10.10.0.1:12345

This patch-set adds support for doing likewise to nftables.  In contrast
to iptables, this works for `snat`, `redirect` and `masquerade`
statements as well as well as `dnat`.

Patches 1-3 add support for shifted ranges to the NAT statements.
Patches 4-5 add JSON support for shifted ranges.
Patches 6-7 update the NAT documentation to cover shifted ranges.
Patch 8 adds some Python test-cases for shifted ranges.

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=970672
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1501

libnftnl & kernel patch-sets to follow.

Jeremy Sowden (8):
  nat: add support for shifted port-ranges
  masq: add support for shifted port-ranges
  redir: add support for shifted port-ranges
  json: formatting fixes
  json: add support for shifted nat port-ranges
  doc: correct NAT statement description
  doc: add shifted port-ranges to nat statements
  test: py: add tests for shifted nat port-ranges

 doc/statements.txt                    | 11 +++-
 include/statement.h                   |  1 +
 src/evaluate.c                        | 10 +++
 src/json.c                            |  4 ++
 src/netlink_delinearize.c             | 48 +++++++++++++-
 src/netlink_linearize.c               | 29 ++++++---
 src/parser_bison.y                    | 55 +++++++++++++++-
 src/parser_json.c                     | 49 ++++++++-------
 src/statement.c                       |  4 ++
 tests/py/inet/dnat.t                  |  3 +
 tests/py/inet/dnat.t.json             | 91 +++++++++++++++++++++++++++
 tests/py/inet/dnat.t.payload          | 33 ++++++++++
 tests/py/inet/snat.t                  |  3 +
 tests/py/inet/snat.t.json             | 91 +++++++++++++++++++++++++++
 tests/py/inet/snat.t.payload          | 34 ++++++++++
 tests/py/ip/masquerade.t              |  1 +
 tests/py/ip/masquerade.t.json         | 26 ++++++++
 tests/py/ip/masquerade.t.payload      |  8 +++
 tests/py/ip/redirect.t                |  1 +
 tests/py/ip/redirect.t.json           | 26 ++++++++
 tests/py/ip/redirect.t.payload        |  8 +++
 tests/py/ip6/masquerade.t             |  1 +
 tests/py/ip6/masquerade.t.json        | 25 ++++++++
 tests/py/ip6/masquerade.t.payload.ip6 |  8 +++
 tests/py/ip6/redirect.t               |  1 +
 tests/py/ip6/redirect.t.json          | 26 ++++++++
 tests/py/ip6/redirect.t.payload.ip6   |  8 +++
 27 files changed, 569 insertions(+), 36 deletions(-)

-- 
2.39.2

[PATCH nftables 1/8] nat: add support for shifted port-ranges

[Jeremy Sowden]

Support for shifted port-ranges was added to iptables for DNAT in 2018.
This allows one to redirect packets intended for one port to another in
a range in such a way that the new port chosen has the same offset in
the range as the original port had from a specified base value.

For example, by using the base value 2000, one could redirect packets
intended for 10.0.0.1:2000-3000 to 10.10.0.1:12000-13000 so that the old
and new ports were at the same offset in their respective ranges, i.e.:

  10.0.0.1:2345 -> 10.10.0.1:12345

Make this functionality available in nftables:

  add rule t c ip daddr 10.0.0.1 tcp dport 2000-3000 dnat to 10.10.0.1:12000-13000/2000 persistent

In contrast to iptables, where shifting is only available for DNAT, both DNAT
and SNAT are supported.

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=970672
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1501
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/statement.h       |  1 +
 src/evaluate.c            | 10 ++++++++++
 src/netlink_delinearize.c | 16 +++++++++++++++-
 src/netlink_linearize.c   | 19 +++++++++++++++----
 src/parser_bison.y        | 33 +++++++++++++++++++++++++++++++--
 src/statement.c           |  4 ++++
 6 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/include/statement.h b/include/statement.h
index 720a6ac2c754..762ea45d4b89 100644
--- a/include/statement.h
+++ b/include/statement.h
@@ -144,6 +144,7 @@ struct nat_stmt {
 	enum nft_nat_etypes	type;
 	struct expr		*addr;
 	struct expr		*proto;
+	struct expr		*proto_base;
 	uint32_t		flags;
 	uint8_t			family;
 	uint32_t		type_flags;
diff --git a/src/evaluate.c b/src/evaluate.c
index 47caf3b0d716..339c428e5aa9 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3772,6 +3772,16 @@ static int stmt_evaluate_nat(struct eval_ctx *ctx, struct stmt *stmt)
 			return err;
 
 		stmt->nat.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
+
+		if (stmt->nat.proto_base != NULL) {
+			err = stmt_evaluate_arg(ctx, stmt,
+						&inet_service_type,
+						sizeof(uint16_t) * BITS_PER_BYTE,
+						BYTEORDER_BIG_ENDIAN,
+						&stmt->nat.proto_base);
+			if (err < 0)
+				return err;
+		}
 	}
 
 	stmt->flags |= STMT_F_TERMINAL;
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 60350cd6cd96..bdfd37870b50 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1257,7 +1257,7 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
 {
 	struct stmt *stmt;
 	struct expr *addr, *proto;
-	enum nft_registers reg1, reg2;
+	enum nft_registers reg1, reg2, reg3;
 	int family;
 
 	stmt = nat_stmt_alloc(loc,
@@ -1352,6 +1352,20 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
 		if (stmt->nat.proto != NULL)
 			proto = range_expr_alloc(loc, stmt->nat.proto, proto);
 		stmt->nat.proto = proto;
+
+		reg3 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_PROTO_BASE);
+		if (reg3) {
+			proto = netlink_get_register(ctx, loc, reg3);
+			if (proto == NULL) {
+				netlink_error(ctx, loc,
+					      "NAT statement has no proto offset expression");
+				goto out_err;
+			}
+
+			expr_set_type(proto, &inet_service_type,
+				      BYTEORDER_BIG_ENDIAN);
+			stmt->nat.proto_base = proto;
+		}
 	}
 
 	ctx->stmt = stmt;
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 11cf48a3f9d0..72a38341e39e 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -1195,11 +1195,11 @@ static void netlink_gen_nat_stmt(struct netlink_linearize_ctx *ctx,
 {
 	struct nftnl_expr *nle;
 	enum nft_registers amin_reg, amax_reg;
-	enum nft_registers pmin_reg, pmax_reg;
+	enum nft_registers pmin_reg, pmax_reg, pbase_reg;
 	uint8_t family = 0;
 	int registers = 0;
 	int nftnl_flag_attr;
-	int nftnl_reg_pmin, nftnl_reg_pmax;
+	int nftnl_reg_pmin, nftnl_reg_pmax, nftnl_reg_pbase;
 
 	switch (stmt->nat.type) {
 	case NFT_NAT_SNAT:
@@ -1211,8 +1211,9 @@ static void netlink_gen_nat_stmt(struct netlink_linearize_ctx *ctx,
 		nftnl_expr_set_u32(nle, NFTNL_EXPR_NAT_FAMILY, family);
 
 		nftnl_flag_attr = NFTNL_EXPR_NAT_FLAGS;
-		nftnl_reg_pmin = NFTNL_EXPR_NAT_REG_PROTO_MIN;
-		nftnl_reg_pmax = NFTNL_EXPR_NAT_REG_PROTO_MAX;
+		nftnl_reg_pmin  = NFTNL_EXPR_NAT_REG_PROTO_MIN;
+		nftnl_reg_pmax  = NFTNL_EXPR_NAT_REG_PROTO_MAX;
+		nftnl_reg_pbase = NFTNL_EXPR_NAT_REG_PROTO_BASE;
 		break;
 	case NFT_NAT_MASQ:
 		nle = alloc_nft_expr("masq");
@@ -1308,6 +1309,16 @@ static void netlink_gen_nat_stmt(struct netlink_linearize_ctx *ctx,
 			netlink_gen_expr(ctx, stmt->nat.proto->right, pmax_reg);
 			netlink_put_register(nle, nftnl_reg_pmin, pmin_reg);
 			netlink_put_register(nle, nftnl_reg_pmax, pmax_reg);
+
+			if (stmt->nat.proto_base) {
+				pbase_reg = get_register(ctx, NULL);
+				registers++;
+
+				netlink_gen_expr(ctx, stmt->nat.proto_base,
+						 pbase_reg);
+				netlink_put_register(nle, nftnl_reg_pbase,
+						     pbase_reg);
+			}
 		} else {
 			netlink_gen_expr(ctx, stmt->nat.proto, pmin_reg);
 			netlink_put_register(nle, nftnl_reg_pmin, pmin_reg);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index b1b67623cf66..c4e274544355 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3833,24 +3833,53 @@ nat_stmt_args		:	stmt_expr
 				$<stmt>0->nat.addr = $1;
 				$<stmt>0->nat.proto = $3;
 			}
+			|	stmt_expr	COLON	range_stmt_expr	SLASH	primary_stmt_expr
+			{
+				$<stmt>0->nat.addr = $1;
+				$<stmt>0->nat.proto = $3;
+				$<stmt>0->nat.proto_base = $5;
+			}
 			|	TO	stmt_expr	COLON	stmt_expr
 			{
 				$<stmt>0->nat.addr = $2;
 				$<stmt>0->nat.proto = $4;
 			}
+			|	TO	stmt_expr	COLON	range_stmt_expr	SLASH	primary_stmt_expr
+			{
+				$<stmt>0->nat.addr = $2;
+				$<stmt>0->nat.proto = $4;
+				$<stmt>0->nat.proto_base = $6;
+			}
 			|	nf_key_proto	TO	 stmt_expr	COLON	stmt_expr
 			{
 				$<stmt>0->nat.family = $1;
 				$<stmt>0->nat.addr = $3;
 				$<stmt>0->nat.proto = $5;
 			}
-			|	COLON		stmt_expr
+			|	nf_key_proto	TO	stmt_expr	COLON	range_stmt_expr	SLASH	primary_stmt_expr
+			{
+				$<stmt>0->nat.family = $1;
+				$<stmt>0->nat.addr = $3;
+				$<stmt>0->nat.proto = $5;
+				$<stmt>0->nat.proto_base = $7;
+			}
+			|	COLON	stmt_expr
+			{
+				$<stmt>0->nat.proto = $2;
+			}
+			|	COLON	range_stmt_expr	SLASH	primary_stmt_expr
 			{
 				$<stmt>0->nat.proto = $2;
+				$<stmt>0->nat.proto_base = $4;
+			}
+			|	TO	COLON	stmt_expr
+			{
+				$<stmt>0->nat.proto = $3;
 			}
-			|	TO	COLON		stmt_expr
+			|	TO	COLON	range_stmt_expr	SLASH	primary_stmt_expr
 			{
 				$<stmt>0->nat.proto = $3;
+				$<stmt>0->nat.proto_base = $5;
 			}
 			|       nat_stmt_args   nf_nat_flags
 			{
diff --git a/src/statement.c b/src/statement.c
index 72455522c2c9..23eee84eb4dc 100644
--- a/src/statement.c
+++ b/src/statement.c
@@ -733,6 +733,10 @@ static void nat_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 			nft_print(octx, " ");
 		nft_print(octx, ":");
 		expr_print(stmt->nat.proto, octx);
+		if (stmt->nat.proto_base) {
+			nft_print(octx, "/");
+			expr_print(stmt->nat.proto_base, octx);
+		}
 	}
 
 	print_nf_nat_flags(stmt->nat.flags, octx);
-- 
2.39.2

[PATCH nftables 2/8] masq: add support for shifted port-ranges

[Jeremy Sowden]

Support for shifted port-ranges was recently added for nat statements.
Extend this to masq statements.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 16 +++++++++++++++-
 src/netlink_linearize.c   |  5 +++--
 src/parser_bison.y        | 11 +++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index bdfd37870b50..867ca914cf96 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1442,7 +1442,7 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
 			       const struct location *loc,
 			       const struct nftnl_expr *nle)
 {
-	enum nft_registers reg1, reg2;
+	enum nft_registers reg1, reg2, reg3;
 	struct expr *proto;
 	struct stmt *stmt;
 	uint32_t flags = 0;
@@ -1477,6 +1477,20 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
 		if (stmt->nat.proto != NULL)
 			proto = range_expr_alloc(loc, stmt->nat.proto, proto);
 		stmt->nat.proto = proto;
+
+		reg3 = netlink_parse_register(nle, NFTNL_EXPR_MASQ_REG_PROTO_BASE);
+		if (reg3) {
+			proto = netlink_get_register(ctx, loc, reg3);
+			if (proto == NULL) {
+				netlink_error(ctx, loc,
+					      "MASQUERADE statement has no base proto expression");
+				goto out_err;
+			}
+
+			expr_set_type(proto, &inet_service_type,
+				      BYTEORDER_BIG_ENDIAN);
+			stmt->nat.proto_base = proto;
+		}
 	}
 
 	ctx->stmt = stmt;
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 72a38341e39e..a018290a7f56 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -1219,8 +1219,9 @@ static void netlink_gen_nat_stmt(struct netlink_linearize_ctx *ctx,
 		nle = alloc_nft_expr("masq");
 
 		nftnl_flag_attr = NFTNL_EXPR_MASQ_FLAGS;
-		nftnl_reg_pmin = NFTNL_EXPR_MASQ_REG_PROTO_MIN;
-		nftnl_reg_pmax = NFTNL_EXPR_MASQ_REG_PROTO_MAX;
+		nftnl_reg_pmin  = NFTNL_EXPR_MASQ_REG_PROTO_MIN;
+		nftnl_reg_pmax  = NFTNL_EXPR_MASQ_REG_PROTO_MAX;
+		nftnl_reg_pbase = NFTNL_EXPR_MASQ_REG_PROTO_BASE;
 		break;
 	case NFT_NAT_REDIR:
 		nle = alloc_nft_expr("redir");
diff --git a/src/parser_bison.y b/src/parser_bison.y
index c4e274544355..8a7c5f066daa 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3928,11 +3928,22 @@ masq_stmt_args		:	TO 	COLON	stmt_expr
 			{
 				$<stmt>0->nat.proto = $3;
 			}
+			|	TO	COLON	range_stmt_expr	SLASH	primary_stmt_expr
+			{
+				$<stmt>0->nat.proto = $3;
+				$<stmt>0->nat.proto_base = $5;
+			}
 			|	TO 	COLON	stmt_expr	nf_nat_flags
 			{
 				$<stmt>0->nat.proto = $3;
 				$<stmt>0->nat.flags = $4;
 			}
+			|	TO	COLON	range_stmt_expr	SLASH	primary_stmt_expr	nf_nat_flags
+			{
+				$<stmt>0->nat.proto = $3;
+				$<stmt>0->nat.proto_base = $5;
+				$<stmt>0->nat.flags = $6;
+			}
 			|	nf_nat_flags
 			{
 				$<stmt>0->nat.flags = $1;
-- 
2.39.2

[PATCH nftables 3/8] redir: add support for shifted port-ranges

[Jeremy Sowden]

Support for shifted port-ranges was recently added to nat statements.
Extend this to redir statements.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 16 +++++++++++++++-
 src/netlink_linearize.c   |  5 +++--
 src/parser_bison.y        | 11 +++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 867ca914cf96..0c48cdd70428 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1505,7 +1505,7 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,
 {
 	struct stmt *stmt;
 	struct expr *proto;
-	enum nft_registers reg1, reg2;
+	enum nft_registers reg1, reg2, reg3;
 	uint32_t flags;
 
 	stmt = nat_stmt_alloc(loc, NFT_NAT_REDIR);
@@ -1542,6 +1542,20 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,
 			proto = range_expr_alloc(loc, stmt->nat.proto,
 						 proto);
 		stmt->nat.proto = proto;
+
+		reg3 = netlink_parse_register(nle, NFTNL_EXPR_REDIR_REG_PROTO_BASE);
+		if (reg3) {
+			proto = netlink_get_register(ctx, loc, reg3);
+			if (proto == NULL) {
+				netlink_error(ctx, loc,
+					      "redirect statement has no base proto expression");
+				goto out_err;
+			}
+
+			expr_set_type(proto, &inet_service_type,
+				      BYTEORDER_BIG_ENDIAN);
+			stmt->nat.proto_base = proto;
+		}
 	}
 
 	ctx->stmt = stmt;
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index a018290a7f56..684cfdcaf91c 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -1227,8 +1227,9 @@ static void netlink_gen_nat_stmt(struct netlink_linearize_ctx *ctx,
 		nle = alloc_nft_expr("redir");
 
 		nftnl_flag_attr = NFTNL_EXPR_REDIR_FLAGS;
-		nftnl_reg_pmin = NFTNL_EXPR_REDIR_REG_PROTO_MIN;
-		nftnl_reg_pmax = NFTNL_EXPR_REDIR_REG_PROTO_MAX;
+		nftnl_reg_pmin  = NFTNL_EXPR_REDIR_REG_PROTO_MIN;
+		nftnl_reg_pmax  = NFTNL_EXPR_REDIR_REG_PROTO_MAX;
+		nftnl_reg_pbase = NFTNL_EXPR_REDIR_REG_PROTO_BASE;
 		break;
 	default:
 		BUG("unknown nat type %d\n", stmt->nat.type);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 8a7c5f066daa..5b8e48363233 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3965,6 +3965,11 @@ redir_stmt_arg		:	TO	stmt_expr
 			{
 				$<stmt>0->nat.proto = $3;
 			}
+			|	TO	COLON	range_stmt_expr	SLASH	primary_stmt_expr
+			{
+				$<stmt>0->nat.proto = $3;
+				$<stmt>0->nat.proto_base = $5;
+			}
 			|	nf_nat_flags
 			{
 				$<stmt>0->nat.flags = $1;
@@ -3979,6 +3984,12 @@ redir_stmt_arg		:	TO	stmt_expr
 				$<stmt>0->nat.proto = $3;
 				$<stmt>0->nat.flags = $4;
 			}
+			|	TO	COLON	range_stmt_expr	SLASH	primary_stmt_expr	nf_nat_flags
+			{
+				$<stmt>0->nat.proto = $3;
+				$<stmt>0->nat.proto_base = $5;
+				$<stmt>0->nat.flags = $6;
+			}
 			;
 
 dup_stmt		:	DUP	TO	stmt_expr
-- 
2.39.2

[PATCH nftables 4/8] json: formatting fixes

[Jeremy Sowden]

A few indentation tweaks for the JSON parser.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/parser_json.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index ec0c02a044e2..d8d4f1b79e6e 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -610,7 +610,7 @@ static struct expr *json_parse_tcp_option_expr(struct json_ctx *ctx,
 	struct expr *expr;
 
 	if (!json_unpack(root, "{s:i, s:i, s:i}",
-			"base", &kind, "offset", &offset, "len", &len)) {
+			 "base", &kind, "offset", &offset, "len", &len)) {
 		uint32_t flag = 0;
 
 		if (kind < 0 || kind > 255)
@@ -681,7 +681,7 @@ static int json_parse_ip_option_field(int type, const char *name, int *val)
 }
 
 static struct expr *json_parse_ip_option_expr(struct json_ctx *ctx,
-					       const char *type, json_t *root)
+					      const char *type, json_t *root)
 {
 	const char *desc, *field;
 	int descval, fieldval;
@@ -697,7 +697,7 @@ static struct expr *json_parse_ip_option_expr(struct json_ctx *ctx,
 
 	if (json_unpack(root, "{s:s}", "field", &field)) {
 		expr = ipopt_expr_alloc(int_loc, descval,
-					 IPOPT_FIELD_TYPE);
+					IPOPT_FIELD_TYPE);
 		expr->exthdr.flags = NFT_EXTHDR_F_PRESENT;
 
 		return expr;
@@ -1084,13 +1084,13 @@ static struct expr *json_parse_fib_expr(struct json_ctx *ctx,
 	}
 
 	if ((flagval & (NFTA_FIB_F_SADDR|NFTA_FIB_F_DADDR)) ==
-			(NFTA_FIB_F_SADDR|NFTA_FIB_F_DADDR)) {
+	    (NFTA_FIB_F_SADDR|NFTA_FIB_F_DADDR)) {
 		json_error(ctx, "fib: saddr and daddr are mutually exclusive");
 		return NULL;
 	}
 
 	if ((flagval & (NFTA_FIB_F_IIF|NFTA_FIB_F_OIF)) ==
-			(NFTA_FIB_F_IIF|NFTA_FIB_F_OIF)) {
+	    (NFTA_FIB_F_IIF|NFTA_FIB_F_OIF)) {
 		json_error(ctx, "fib: iif and oif are mutually exclusive");
 		return NULL;
 	}
@@ -1686,7 +1686,7 @@ static struct stmt *json_parse_match_stmt(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_counter_stmt(struct json_ctx *ctx,
-					  const char *key, json_t *value)
+					    const char *key, json_t *value)
 {
 	uint64_t packets, bytes;
 	struct stmt *stmt;
@@ -1695,8 +1695,8 @@ static struct stmt *json_parse_counter_stmt(struct json_ctx *ctx,
 		return counter_stmt_alloc(int_loc);
 
 	if (!json_unpack(value, "{s:I, s:I}",
-			    "packets", &packets,
-			    "bytes", &bytes)) {
+			 "packets", &packets,
+			 "bytes", &bytes)) {
 		stmt = counter_stmt_alloc(int_loc);
 		stmt->counter.packets = packets;
 		stmt->counter.bytes = bytes;
@@ -1727,14 +1727,14 @@ static struct stmt *json_parse_verdict_stmt(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_mangle_stmt(struct json_ctx *ctx,
-					const char *type, json_t *root)
+					   const char *type, json_t *root)
 {
 	json_t *jkey, *jvalue;
 	struct expr *key, *value;
 	struct stmt *stmt;
 
 	if (json_unpack_err(ctx, root, "{s:o, s:o}",
-			   "key", &jkey, "value", &jvalue))
+			    "key", &jkey, "value", &jvalue))
 		return NULL;
 
 	key = json_parse_mangle_lhs_expr(ctx, jkey);
@@ -1787,7 +1787,7 @@ static uint64_t rate_to_bytes(uint64_t val, const char *unit)
 }
 
 static struct stmt *json_parse_quota_stmt(struct json_ctx *ctx,
-					const char *key, json_t *value)
+					  const char *key, json_t *value)
 {
 	struct stmt *stmt;
 	int inv = 0;
@@ -1937,7 +1937,7 @@ static struct stmt *json_parse_flow_offload_stmt(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_notrack_stmt(struct json_ctx *ctx,
-					const char *key, json_t *value)
+					    const char *key, json_t *value)
 {
 	return notrack_stmt_alloc(int_loc);
 }
@@ -1975,7 +1975,7 @@ static struct stmt *json_parse_dup_stmt(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_secmark_stmt(struct json_ctx *ctx,
-					     const char *key, json_t *value)
+					    const char *key, json_t *value)
 {
 	struct stmt *stmt;
 
@@ -2047,7 +2047,7 @@ static int json_parse_nat_flags(struct json_ctx *ctx, json_t *root)
 }
 
 static int json_parse_nat_type_flag(struct json_ctx *ctx,
-			       json_t *root, int *flags)
+				    json_t *root, int *flags)
 {
 	const struct {
 		const char *flag;
@@ -2162,7 +2162,6 @@ static struct stmt *json_parse_nat_stmt(struct json_ctx *ctx,
 		}
 		stmt->nat.flags = flags;
 	}
-
 	if (!json_unpack(value, "{s:o}", "type_flags", &tmp)) {
 		int flags = json_parse_nat_type_flags(ctx, tmp);
 
@@ -2177,7 +2176,7 @@ static struct stmt *json_parse_nat_stmt(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_tproxy_stmt(struct json_ctx *ctx,
-					const char *key, json_t *value)
+					   const char *key, json_t *value)
 {
 	json_t *jaddr, *tmp;
 	struct stmt *stmt;
@@ -2213,7 +2212,7 @@ out_free:
 }
 
 static struct stmt *json_parse_reject_stmt(struct json_ctx *ctx,
-					  const char *key, json_t *value)
+					   const char *key, json_t *value)
 {
 	struct stmt *stmt = reject_stmt_alloc(int_loc);
 	const struct datatype *dtype = NULL;
@@ -2256,8 +2255,8 @@ static struct stmt *json_parse_reject_stmt(struct json_ctx *ctx,
 }
 
 static void json_parse_set_stmt_list(struct json_ctx *ctx,
-				      struct list_head *stmt_list,
-				      json_t *stmt_json)
+				     struct list_head *stmt_list,
+				     json_t *stmt_json)
 {
 	struct list_head *head;
 	struct stmt *tmp;
@@ -2279,7 +2278,7 @@ static void json_parse_set_stmt_list(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_set_stmt(struct json_ctx *ctx,
-					  const char *key, json_t *value)
+					const char *key, json_t *value)
 {
 	const char *opstr, *set;
 	struct expr *expr, *expr2;
@@ -2562,7 +2561,7 @@ static struct stmt *json_parse_cthelper_stmt(struct json_ctx *ctx,
 }
 
 static struct stmt *json_parse_cttimeout_stmt(struct json_ctx *ctx,
-					     const char *key, json_t *value)
+					      const char *key, json_t *value)
 {
 	struct stmt *stmt = objref_stmt_alloc(int_loc);
 
-- 
2.39.2

[PATCH nftables 5/8] json: add support for shifted nat port-ranges

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/json.c        | 4 ++++
 src/parser_json.c | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/src/json.c b/src/json.c
index f15461d33894..f6874b94c7ec 100644
--- a/src/json.c
+++ b/src/json.c
@@ -1407,6 +1407,10 @@ json_t *nat_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
 		json_object_set_new(root, "port",
 				    expr_print_json(stmt->nat.proto, octx));
 
+	if (stmt->nat.proto_base)
+		json_object_set_new(root, "base_port",
+				    expr_print_json(stmt->nat.proto_base, octx));
+
 	nat_stmt_add_array(root, "flags", array);
 
 	if (stmt->nat.type_flags) {
diff --git a/src/parser_json.c b/src/parser_json.c
index d8d4f1b79e6e..fca9645c7e57 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2153,6 +2153,14 @@ static struct stmt *json_parse_nat_stmt(struct json_ctx *ctx,
 			return NULL;
 		}
 	}
+	if (!json_unpack(value, "{s:o}", "base_port", &tmp)) {
+		stmt->nat.proto_base = json_parse_stmt_expr(ctx, tmp);
+		if (!stmt->nat.proto) {
+			json_error(ctx, "Invalid nat base port.");
+			stmt_free(stmt);
+			return NULL;
+		}
+	}
 	if (!json_unpack(value, "{s:o}", "flags", &tmp)) {
 		int flags = json_parse_nat_flags(ctx, tmp);
 
-- 
2.39.2

[PATCH nftables 6/8] doc: correct NAT statement description

[Jeremy Sowden]

Specifying a port specifies that a port, not an address, should be
modified.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 doc/statements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/statements.txt b/doc/statements.txt
index 0532b2b16c7d..b2794bcd6821 100644
--- a/doc/statements.txt
+++ b/doc/statements.txt
@@ -405,7 +405,7 @@ You may specify a mapping to relate a list of tuples composed of arbitrary
 expression key with address value. |
 ipv4_addr, ipv6_addr, e.g. abcd::1234, or you can use a mapping, e.g. meta mark map { 10 : 192.168.1.2, 20 : 192.168.1.3 }
 |port|
-Specifies that the source/destination address of the packet should be modified. |
+Specifies that the source/destination port of the packet should be modified. |
 port number (16 bit)
 |===============================
 
-- 
2.39.2

[PATCH nftables 7/8] doc: add shifted port-ranges to nat statements

[Jeremy Sowden]

Extend the description of ports to cover ranges and shifted ranges, and
add an example of the latter.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 doc/statements.txt | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/doc/statements.txt b/doc/statements.txt
index b2794bcd6821..3dd3b98b6cb1 100644
--- a/doc/statements.txt
+++ b/doc/statements.txt
@@ -362,7 +362,7 @@ ____
 *redirect* [*to :*'PORT_SPEC'] ['FLAGS']
 
 'ADDR_SPEC' := 'address' | 'address' *-* 'address'
-'PORT_SPEC' := 'port' | 'port' *-* 'port'
+'PORT_SPEC' := 'port' | 'port' *-* 'port' | 'port' *-* 'port' */* 'port'
 
 'FLAGS'  := 'FLAG' [*,* 'FLAGS']
 'FLAG'  := *persistent* | *random* | *fully-random*
@@ -405,7 +405,10 @@ You may specify a mapping to relate a list of tuples composed of arbitrary
 expression key with address value. |
 ipv4_addr, ipv6_addr, e.g. abcd::1234, or you can use a mapping, e.g. meta mark map { 10 : 192.168.1.2, 20 : 192.168.1.3 }
 |port|
-Specifies that the source/destination port of the packet should be modified. |
+Specifies that the source/destination port of the packet should be modified.  If
+a range is given, the new port will be chosen from within that range.  If a base
+offset is also given, the offset of the new port in the range will match the
+offset of the old port from the specified base.|
 port number (16 bit)
 |===============================
 
@@ -437,6 +440,10 @@ add rule nat postrouting oif eth0 snat to 1.2.3.4
 # redirect all traffic entering via eth0 to destination address 192.168.1.120
 add rule nat prerouting iif eth0 dnat to 192.168.1.120
 
+# redirect all traffic for address 10.0.0.1 and ports 2000-3000 to destination
+# address 10.10.0.1 and the port at the matching offset in 12000-13000
+add rule nat prerouting ip daddr 10.0.0.1 tcp dport 2000-3000 dnat to 10.10.0.1:12000-13000/2000
+
 # translate source addresses of all packets leaving via eth0 to whatever
 # locally generated packets would use as source to reach the same destination
 add rule nat postrouting oif eth0 masquerade
-- 
2.39.2

[PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/py/inet/dnat.t                  |  3 +
 tests/py/inet/dnat.t.json             | 91 +++++++++++++++++++++++++++
 tests/py/inet/dnat.t.payload          | 33 ++++++++++
 tests/py/inet/snat.t                  |  3 +
 tests/py/inet/snat.t.json             | 91 +++++++++++++++++++++++++++
 tests/py/inet/snat.t.payload          | 34 ++++++++++
 tests/py/ip/masquerade.t              |  1 +
 tests/py/ip/masquerade.t.json         | 26 ++++++++
 tests/py/ip/masquerade.t.payload      |  8 +++
 tests/py/ip/redirect.t                |  1 +
 tests/py/ip/redirect.t.json           | 26 ++++++++
 tests/py/ip/redirect.t.payload        |  8 +++
 tests/py/ip6/masquerade.t             |  1 +
 tests/py/ip6/masquerade.t.json        | 25 ++++++++
 tests/py/ip6/masquerade.t.payload.ip6 |  8 +++
 tests/py/ip6/redirect.t               |  1 +
 tests/py/ip6/redirect.t.json          | 26 ++++++++
 tests/py/ip6/redirect.t.payload.ip6   |  8 +++
 18 files changed, 394 insertions(+)

diff --git a/tests/py/inet/dnat.t b/tests/py/inet/dnat.t
index e4e169f2bc3e..9c47f51cfc71 100644
--- a/tests/py/inet/dnat.t
+++ b/tests/py/inet/dnat.t
@@ -20,3 +20,6 @@ meta l4proto { tcp, udp } dnat ip to 1.1.1.1:80;ok;meta l4proto { 6, 17} dnat ip
 ip protocol { tcp, udp } dnat ip to 1.1.1.1:80;ok;ip protocol { 6, 17} dnat ip to 1.1.1.1:80
 meta l4proto { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80;fail
 ip protocol { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80;fail
+
+ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
+ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
diff --git a/tests/py/inet/dnat.t.json b/tests/py/inet/dnat.t.json
index c341a0455fea..58d0ed4b76da 100644
--- a/tests/py/inet/dnat.t.json
+++ b/tests/py/inet/dnat.t.json
@@ -239,3 +239,94 @@
     }
 ]
 
+# ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "daddr",
+                    "protocol": "ip"
+                }
+            },
+            "op": "==",
+            "right": "10.0.0.1"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "dport",
+                    "protocol": "tcp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "range": [
+                    55900,
+                    55910
+                ]
+            }
+        }
+    },
+    {
+        "dnat": {
+            "addr": "192.168.127.1",
+            "family": "ip",
+            "port": {
+                "range": [
+                    5900,
+                    5910
+                ]
+            },
+	    "base_port": 55900
+        }
+    }
+]
+
+# ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "daddr",
+                    "protocol": "ip6"
+                }
+            },
+            "op": "==",
+            "right": "10::1"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "dport",
+                    "protocol": "tcp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "range": [
+                    55900,
+                    55910
+                ]
+            }
+        }
+    },
+    {
+        "dnat": {
+            "addr": "::c0:a8:7f:1",
+            "family": "ip6",
+            "port": {
+                "range": [
+                    5900,
+                    5910
+                ]
+            },
+	    "base_port": 55900
+        }
+    }
+]
diff --git a/tests/py/inet/dnat.t.payload b/tests/py/inet/dnat.t.payload
index ce1601ab5c9e..9747018ae89c 100644
--- a/tests/py/inet/dnat.t.payload
+++ b/tests/py/inet/dnat.t.payload
@@ -84,3 +84,36 @@ inet
   [ immediate reg 1 0x00005000 ]
   [ nat dnat inet proto_min reg 1 flags 0x2 ]
 
+# ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900
+inet test-inet prerouting
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 4b @ network header + 16 => reg 1 ]
+  [ cmp eq reg 1 0x0100000a ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ cmp gte reg 1 0x00005cda ]
+  [ cmp lte reg 1 0x000066da ]
+  [ immediate reg 1 0x017fa8c0 ]
+  [ immediate reg 2 0x00000c17 ]
+  [ immediate reg 3 0x00001617 ]
+  [ immediate reg 4 0x00005cda ]
+  [ nat dnat ip addr_min reg 1 proto_min reg 2 proto_max reg 3 proto_base reg 4 flags 0x2 ]
+
+# ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900
+inet test-inet prerouting
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 16b @ network header + 24 => reg 1 ]
+  [ cmp eq reg 1 0x00001000 0x00000000 0x00000000 0x01000000 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ cmp gte reg 1 0x00005cda ]
+  [ cmp lte reg 1 0x000066da ]
+  [ immediate reg 1 0x00000000 0x00000000 0xa800c000 0x01007f00 ]
+  [ immediate reg 2 0x00000c17 ]
+  [ immediate reg 3 0x00001617 ]
+  [ immediate reg 4 0x00005cda ]
+  [ nat dnat ip6 addr_min reg 1 proto_min reg 2 proto_max reg 3 proto_base reg 4 flags 0x2 ]
diff --git a/tests/py/inet/snat.t b/tests/py/inet/snat.t
index cf23b5cff1bb..1276145918f5 100644
--- a/tests/py/inet/snat.t
+++ b/tests/py/inet/snat.t
@@ -19,3 +19,6 @@ snat ip to dead::beef;fail
 snat ip daddr 1.2.3.4 to dead::beef;fail
 snat ip daddr 1.2.3.4 ip6 to dead::beef;fail
 snat ip6 saddr dead::beef to 1.2.3.4;fail
+
+ip saddr 10.0.0.1 tcp sport 55900-55910 snat ip to 192.168.127.1:5900-5910/55900;ok
+ip6 saddr 10::1 tcp sport 55900-55910 snat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
diff --git a/tests/py/inet/snat.t.json b/tests/py/inet/snat.t.json
index 4671625dc06d..03e5823d4258 100644
--- a/tests/py/inet/snat.t.json
+++ b/tests/py/inet/snat.t.json
@@ -129,3 +129,94 @@
     }
 ]
 
+# ip saddr 10.0.0.1 tcp sport 55900-55910 snat ip to 192.168.127.1:5900-5910/55900
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr",
+                    "protocol": "ip"
+                }
+            },
+            "op": "==",
+            "right": "10.0.0.1"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sport",
+                    "protocol": "tcp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "range": [
+                    55900,
+                    55910
+                ]
+            }
+        }
+    },
+    {
+        "snat": {
+            "addr": "192.168.127.1",
+            "family": "ip",
+            "port": {
+                "range": [
+                    5900,
+                    5910
+                ]
+            },
+	    "base_port": 55900
+        }
+    }
+]
+
+# ip6 saddr 10::1 tcp sport 55900-55910 snat ip6 to [::c0:a8:7f:1]:5900-5910/55900
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr",
+                    "protocol": "ip6"
+                }
+            },
+            "op": "==",
+            "right": "10::1"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sport",
+                    "protocol": "tcp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "range": [
+                    55900,
+                    55910
+                ]
+            }
+        }
+    },
+    {
+        "snat": {
+            "addr": "::c0:a8:7f:1",
+            "family": "ip6",
+            "port": {
+                "range": [
+                    5900,
+                    5910
+                ]
+            },
+	    "base_port": 55900
+        }
+    }
+]
diff --git a/tests/py/inet/snat.t.payload b/tests/py/inet/snat.t.payload
index 50519c6b6bb6..c2b5e5884b89 100644
--- a/tests/py/inet/snat.t.payload
+++ b/tests/py/inet/snat.t.payload
@@ -40,3 +40,37 @@ inet test-inet postrouting
   [ meta load iifname => reg 1 ]
   [ cmp eq reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
   [ masq flags 0x4 ]
+
+# ip saddr 10.0.0.1 tcp sport 55900-55910 snat ip to 192.168.127.1:5900-5910/55900
+inet test-inet postrouting
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x0100000a ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 0 => reg 1 ]
+  [ cmp gte reg 1 0x00005cda ]
+  [ cmp lte reg 1 0x000066da ]
+  [ immediate reg 1 0x017fa8c0 ]
+  [ immediate reg 2 0x00000c17 ]
+  [ immediate reg 3 0x00001617 ]
+  [ immediate reg 4 0x00005cda ]
+  [ nat snat ip addr_min reg 1 proto_min reg 2 proto_max reg 3 proto_base reg 4 flags 0x2 ]
+
+# ip6 saddr 10::1 tcp sport 55900-55910 snat ip6 to [::c0:a8:7f:1]:5900-5910/55900
+inet test-inet postrouting
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 16b @ network header + 8 => reg 1 ]
+  [ cmp eq reg 1 0x00001000 0x00000000 0x00000000 0x01000000 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 0 => reg 1 ]
+  [ cmp gte reg 1 0x00005cda ]
+  [ cmp lte reg 1 0x000066da ]
+  [ immediate reg 1 0x00000000 0x00000000 0xa800c000 0x01007f00 ]
+  [ immediate reg 2 0x00000c17 ]
+  [ immediate reg 3 0x00001617 ]
+  [ immediate reg 4 0x00005cda ]
+  [ nat snat ip6 addr_min reg 1 proto_min reg 2 proto_max reg 3 proto_base reg 4 flags 0x2 ]
diff --git a/tests/py/ip/masquerade.t b/tests/py/ip/masquerade.t
index 384ac72a15f0..98858149dfed 100644
--- a/tests/py/ip/masquerade.t
+++ b/tests/py/ip/masquerade.t
@@ -18,6 +18,7 @@ udp dport 53 masquerade persistent,fully-random,random;ok;udp dport 53 masquerad
 # using ports
 ip protocol 6 masquerade to :1024;ok
 ip protocol 6 masquerade to :1024-2048;ok
+ip protocol 6 masquerade to :1024-2048/4096;ok
 
 # masquerade is a terminal statement
 tcp dport 22 masquerade counter packets 0 bytes 0 accept;fail
diff --git a/tests/py/ip/masquerade.t.json b/tests/py/ip/masquerade.t.json
index 4a90c7062d47..29d16dd75a02 100644
--- a/tests/py/ip/masquerade.t.json
+++ b/tests/py/ip/masquerade.t.json
@@ -427,3 +427,29 @@
     }
 ]
 
+# ip protocol 6 masquerade to :1024-2048/4096
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "protocol",
+                    "protocol": "ip"
+                }
+            },
+            "op": "==",
+            "right": 6
+        }
+    },
+    {
+        "masquerade": {
+            "base_port": 4096,
+            "port": {
+                "range": [
+                    1024,
+                    2048
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip/masquerade.t.payload b/tests/py/ip/masquerade.t.payload
index 79e52856a22d..804d35377f56 100644
--- a/tests/py/ip/masquerade.t.payload
+++ b/tests/py/ip/masquerade.t.payload
@@ -140,3 +140,11 @@ ip test-ip4 postrouting
   [ immediate reg 2 0x00000008 ]
   [ masq proto_min reg 1 proto_max reg 2 flags 0x2 ]
 
+# ip protocol 6 masquerade to :1024-2048/4096
+ip test-ip4 postrouting
+  [ payload load 1b @ network header + 9 => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ immediate reg 1 0x00000004 ]
+  [ immediate reg 2 0x00000008 ]
+  [ immediate reg 3 0x00000010 ]
+  [ masq proto_min reg 1 proto_max reg 2 proto_base reg 3 flags 0x2 ]
diff --git a/tests/py/ip/redirect.t b/tests/py/ip/redirect.t
index d2991ce288b0..5321396fc079 100644
--- a/tests/py/ip/redirect.t
+++ b/tests/py/ip/redirect.t
@@ -23,6 +23,7 @@ udp dport 1234 redirect to :4321;ok
 ip daddr 172.16.0.1 udp dport 9998 redirect to :6515;ok
 tcp dport 39128 redirect to :993;ok
 ip protocol tcp redirect to :100-200;ok;ip protocol 6 redirect to :100-200
+ip protocol tcp redirect to :100-200/1000;ok;ip protocol 6 redirect to :100-200/1000
 redirect to :1234;fail
 redirect to :12341111;fail
 
diff --git a/tests/py/ip/redirect.t.json b/tests/py/ip/redirect.t.json
index 3544e7f1b9c5..41a4be95a2ee 100644
--- a/tests/py/ip/redirect.t.json
+++ b/tests/py/ip/redirect.t.json
@@ -635,3 +635,29 @@
     }
 ]
 
+# ip protocol tcp redirect to :100-200/1000
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "protocol",
+                    "protocol": "ip"
+                }
+            },
+            "op": "==",
+            "right": 6
+        }
+    },
+    {
+        "redirect": {
+            "base_port": 1000,
+            "port": {
+                "range": [
+                    100,
+                    200
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip/redirect.t.payload b/tests/py/ip/redirect.t.payload
index 424ad7b4f7ec..d4935c695ff3 100644
--- a/tests/py/ip/redirect.t.payload
+++ b/tests/py/ip/redirect.t.payload
@@ -218,3 +218,11 @@ ip test-ip4 output
   [ lookup reg 1 set __map%d dreg 1 ]
   [ redir proto_min reg 1 flags 0x2 ]
 
+# ip protocol tcp redirect to :100-200/1000
+ip test-ip4 output
+  [ payload load 1b @ network header + 9 => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ immediate reg 1 0x00006400 ]
+  [ immediate reg 2 0x0000c800 ]
+  [ immediate reg 3 0x0000e803 ]
+  [ redir proto_min reg 1 proto_max reg 2 proto_base reg 3 flags 0x2 ]
diff --git a/tests/py/ip6/masquerade.t b/tests/py/ip6/masquerade.t
index 4eb0467c362e..3d87fa1d71bb 100644
--- a/tests/py/ip6/masquerade.t
+++ b/tests/py/ip6/masquerade.t
@@ -18,6 +18,7 @@ udp dport 53 masquerade persistent,fully-random,random;ok;udp dport 53 masquerad
 # using ports
 meta l4proto 6 masquerade to :1024;ok
 meta l4proto 6 masquerade to :1024-2048;ok
+meta l4proto 6 masquerade to :1024-2048/4096;ok
 
 # masquerade is a terminal statement
 tcp dport 22 masquerade counter packets 0 bytes 0 accept;fail
diff --git a/tests/py/ip6/masquerade.t.json b/tests/py/ip6/masquerade.t.json
index 824b44f8a5f5..a56c4372e101 100644
--- a/tests/py/ip6/masquerade.t.json
+++ b/tests/py/ip6/masquerade.t.json
@@ -421,3 +421,28 @@
     }
 ]
 
+# meta l4proto 6 masquerade to :1024-2048/4096
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "l4proto"
+                }
+            },
+            "op": "==",
+            "right": 6
+        }
+    },
+    {
+        "masquerade": {
+            "base_port": 4096,
+            "port": {
+                "range": [
+                    1024,
+                    2048
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip6/masquerade.t.payload.ip6 b/tests/py/ip6/masquerade.t.payload.ip6
index 43ae2ae48244..bf64313b6b60 100644
--- a/tests/py/ip6/masquerade.t.payload.ip6
+++ b/tests/py/ip6/masquerade.t.payload.ip6
@@ -140,3 +140,11 @@ ip6 test-ip6 postrouting
   [ immediate reg 2 0x00000008 ]
   [ masq proto_min reg 1 proto_max reg 2 flags 0x2 ]
 
+# meta l4proto 6 masquerade to :1024-2048/4096
+ip6 test-ip6 postrouting
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ immediate reg 1 0x00000004 ]
+  [ immediate reg 2 0x00000008 ]
+  [ immediate reg 3 0x00000010 ]
+  [ masq proto_min reg 1 proto_max reg 2 proto_base reg 3 flags 0x2 ]
diff --git a/tests/py/ip6/redirect.t b/tests/py/ip6/redirect.t
index 778d53f33ce6..9e8747f50185 100644
--- a/tests/py/ip6/redirect.t
+++ b/tests/py/ip6/redirect.t
@@ -23,6 +23,7 @@ udp dport 53 redirect persistent,fully-random,random;ok;udp dport 53 redirect ra
 udp dport 1234 redirect to :1234;ok
 ip6 daddr fe00::cafe udp dport 9998 redirect to :6515;ok
 ip6 nexthdr tcp redirect to :100-200;ok;ip6 nexthdr 6 redirect to :100-200
+ip6 nexthdr tcp redirect to :100-200/1000;ok;ip6 nexthdr 6 redirect to :100-200/1000
 tcp dport 39128 redirect to :993;ok
 redirect to :1234;fail
 redirect to :12341111;fail
diff --git a/tests/py/ip6/redirect.t.json b/tests/py/ip6/redirect.t.json
index 0059c7accc06..4689b0c71c8b 100644
--- a/tests/py/ip6/redirect.t.json
+++ b/tests/py/ip6/redirect.t.json
@@ -599,3 +599,29 @@
     }
 ]
 
+# ip6 nexthdr tcp redirect to :100-200/1000
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "nexthdr",
+                    "protocol": "ip6"
+                }
+            },
+            "op": "==",
+            "right": 6
+        }
+    },
+    {
+        "redirect": {
+            "base_port": 1000,
+            "port": {
+                "range": [
+                    100,
+                    200
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip6/redirect.t.payload.ip6 b/tests/py/ip6/redirect.t.payload.ip6
index e9a203161485..4a19df99a3cd 100644
--- a/tests/py/ip6/redirect.t.payload.ip6
+++ b/tests/py/ip6/redirect.t.payload.ip6
@@ -202,3 +202,11 @@ ip6 test-ip6 output
   [ lookup reg 1 set __map%d dreg 1 ]
   [ redir proto_min reg 1 flags 0x2 ]
 
+# ip6 nexthdr tcp redirect to :100-200/1000
+ip6 test-ip6 output
+  [ payload load 1b @ network header + 6 => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ immediate reg 1 0x00006400 ]
+  [ immediate reg 2 0x0000c800 ]
+  [ immediate reg 3 0x0000e803 ]
+  [ redir proto_min reg 1 proto_max reg 2 proto_base reg 3 flags 0x2 ]
-- 
2.39.2

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Florian Westphal]

Jeremy Sowden <jeremy@azazel.net> wrote:
> +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok

This syntax is horrible (yes, I know, xtables fault).

Do you think this series could be changed to grab the offset register from the
left edge of the range rather than requiring the user to specify it a
second time?  Something like:

ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910

I'm open to other suggestions of course.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Phil Sutter]

On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> 
> This syntax is horrible (yes, I know, xtables fault).
> 
> Do you think this series could be changed to grab the offset register from the
> left edge of the range rather than requiring the user to specify it a
> second time?  Something like:
> 
> ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> 
> I'm open to other suggestions of course.

Initially, a map came to mind. Something like:

| dnat to : tcp dport map { 1000-2000 : 5000-6000 }

To my surprise, nft accepts the syntax (listing is broken, though). But
IIUC, it means "return 5000-6000 for any port in [1000;2000]" and dnat
does round-robin? At least it's not what one would expect. Maybe one
could control the lookup behaviour somehow via a flag?

Cheers, Phil

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On 2023-03-25, at 11:35:47 +0100, Phil Sutter wrote:
> On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> >
> > This syntax is horrible (yes, I know, xtables fault).
> >
> > Do you think this series could be changed to grab the offset register from the
> > left edge of the range rather than requiring the user to specify it a
> > second time?  Something like:
> >
> > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> >
> > I'm open to other suggestions of course.
>
> Initially, a map came to mind. Something like:
>
> | dnat to : tcp dport map { 1000-2000 : 5000-6000 }
>
> To my surprise, nft accepts the syntax (listing is broken, though). But
> IIUC, it means "return 5000-6000 for any port in [1000;2000]" and dnat
> does round-robin?

That does ring a bell.  IIRC, when I initially looked into this, I did
have a look at maps to see if they might already offer analogous func-
tionality.

> At least it's not what one would expect. Maybe one could control the
> lookup behaviour somehow via a flag?

Thanks for the suggestion.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Sat, Mar 25, 2023 at 11:10:17AM +0000, Jeremy Sowden wrote:
> On 2023-03-25, at 11:35:47 +0100, Phil Sutter wrote:
> > On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> > >
> > > This syntax is horrible (yes, I know, xtables fault).
> > >
> > > Do you think this series could be changed to grab the offset register from the
> > > left edge of the range rather than requiring the user to specify it a
> > > second time?  Something like:
> > >
> > > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> > >
> > > I'm open to other suggestions of course.
> >
> > Initially, a map came to mind. Something like:
> >
> > | dnat to : tcp dport map { 1000-2000 : 5000-6000 }
> >
> > To my surprise, nft accepts the syntax (listing is broken, though). But
> > IIUC, it means "return 5000-6000 for any port in [1000;2000]" and dnat
> > does round-robin?
> 
> That does ring a bell.  IIRC, when I initially looked into this, I did
> have a look at maps to see if they might already offer analogous func-
> tionality.
> 
> > At least it's not what one would expect. Maybe one could control the
> > lookup behaviour somehow via a flag?
> 
> Thanks for the suggestion.

Yes, one possibility would be to explore a new flag in the NAT engine.

As said in previous email, this really has to work with NAT maps in nftables.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> 
> This syntax is horrible (yes, I know, xtables fault).
> 
> Do you think this series could be changed to grab the offset register from the
> left edge of the range rather than requiring the user to specify it a
> second time?  Something like:
> 
> ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> 
> I'm open to other suggestions of course.

Not only syntax, main problema is that this port shift support has to
work with NAT maps, otherwise this needs N rules for different
mappings which takes us back to linear rule inspection.

Jeremy, may I suggest you pick up on the bitwise _SREG2 support?
I will post a v4 with small updates for ("mark statement support for
non-constant expression") tomorrow. Probably you don't need the new
AND and OR operations for this? Only the a new _SREG2 to specify that
input comes from non-constant?

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

On 2023-03-26, at 22:39:42 +0200, Pablo Neira Ayuso wrote:
> On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> > 
> > This syntax is horrible (yes, I know, xtables fault).
> > 
> > Do you think this series could be changed to grab the offset register from the
> > left edge of the range rather than requiring the user to specify it a
> > second time?  Something like:
> > 
> > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> > 
> > I'm open to other suggestions of course.
> 
> Not only syntax, main problema is that this port shift support has to
> work with NAT maps, otherwise this needs N rules for different
> mappings which takes us back to linear rule inspection.
> 
> Jeremy, may I suggest you pick up on the bitwise _SREG2 support?  I
> will post a v4 with small updates for ("mark statement support for
> non-constant expression") tomorrow. Probably you don't need the new
> AND and OR operations for this? Only the a new _SREG2 to specify that
> input comes from non-constant?

Cool, I will focus on this for the moment.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

On 2023-03-26, at 22:39:42 +0200, Pablo Neira Ayuso wrote:
> Jeremy, may I suggest you pick up on the bitwise _SREG2 support?  I
> will post a v4 with small updates for ("mark statement support for
> non-constant expression") tomorrow. Probably you don't need the new
> AND and OR operations for this? Only the a new _SREG2 to specify that
> input comes from non-constant?

Just to clarify, do you want just the `_SREG2` infrastructure from the
last patch series but without the new bitwise ops?  That is to say it
would be possible to send two operands to the kernel in registers, but
no use would be made of it (yet).  Or are you proposing to update the
existing mask-and-xor ops to send right hand operands via registers?

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Tue, Apr 11, 2023 at 01:21:40PM +0100, Jeremy Sowden wrote:
> On 2023-03-26, at 22:39:42 +0200, Pablo Neira Ayuso wrote:
> > Jeremy, may I suggest you pick up on the bitwise _SREG2 support?  I
> > will post a v4 with small updates for ("mark statement support for
> > non-constant expression") tomorrow. Probably you don't need the new
> > AND and OR operations for this? Only the a new _SREG2 to specify that
> > input comes from non-constant?
> 
> Just to clarify, do you want just the `_SREG2` infrastructure from the
> last patch series but without the new bitwise ops?  That is to say it
> would be possible to send two operands to the kernel in registers, but
> no use would be made of it (yet).  Or are you proposing to update the
> existing mask-and-xor ops to send right hand operands via registers?

I mean, would it be possible to add a NFT_BITWISE_BOOL variant that
takes _SREG2 via select_ops?

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]

On 2023-04-12, at 13:06:02 +0200, Pablo Neira Ayuso wrote:
> On Tue, Apr 11, 2023 at 01:21:40PM +0100, Jeremy Sowden wrote:
> > On 2023-03-26, at 22:39:42 +0200, Pablo Neira Ayuso wrote:
> > > Jeremy, may I suggest you pick up on the bitwise _SREG2 support?
> > > I will post a v4 with small updates for ("mark statement support
> > > for non-constant expression") tomorrow. Probably you don't need
> > > the new AND and OR operations for this? Only the a new _SREG2 to
> > > specify that input comes from non-constant?
> > 
> > Just to clarify, do you want just the `_SREG2` infrastructure from
> > the last patch series but without the new bitwise ops?  That is to
> > say it would be possible to send two operands to the kernel in
> > registers, but no use would be made of it (yet).  Or are you
> > proposing to update the existing mask-and-xor ops to send right hand
> > operands via registers?
> 
> I mean, would it be possible to add a NFT_BITWISE_BOOL variant that
> takes _SREG2 via select_ops?

In an earlier version, instead of adding new boolean ops, I added
support for passing the mask and xor arguments in registers:

  https://lore.kernel.org/netfilter-devel/20200224124931.512416-1-jeremy@azazel.net/

Doing the same thing with one extra register is straightforward for AND
and XOR:
        
  AND(x, y) = (x & y) ^ 0
  XOR(x, y) = (x & 1) ^ y

since we can pass y in _SREG2 and 0 in _XOR for AND, and 1 in _MASK and
y in _SREG2 for XOR.  For OR:

  OR(x, y) = (x & ~y) ^ y

it's a bit more complicated.  Instead of getting both the mask and xor
arguments from user space, we need to do something like passing y in
_SREG2 alone, and then constructing the bitwise negation in the kernel.

Obviously, this means that the kernel is no longer completely agnostic
about the sorts of mask-and-xor expressions user space may send.  Since
that is the case, we could go further and just perform the original ope-
rations.  Thus if we get an boolean op with an _SREG2 argument:

  * if there is an _XOR of 0, compute:

    _SREG & _SREG2

  * if there is a _MASK of 1, compute:

    _SREG ^ _SREG2

  * if there are no _MASK or _XOR arguments, compute:

    _SREG | _SREG2

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Tue, Apr 25, 2023 at 08:51:43PM +0100, Jeremy Sowden wrote:
> On 2023-04-12, at 13:06:02 +0200, Pablo Neira Ayuso wrote:
> > On Tue, Apr 11, 2023 at 01:21:40PM +0100, Jeremy Sowden wrote:
> > > On 2023-03-26, at 22:39:42 +0200, Pablo Neira Ayuso wrote:
> > > > Jeremy, may I suggest you pick up on the bitwise _SREG2 support?
> > > > I will post a v4 with small updates for ("mark statement support
> > > > for non-constant expression") tomorrow. Probably you don't need
> > > > the new AND and OR operations for this? Only the a new _SREG2 to
> > > > specify that input comes from non-constant?
> > > 
> > > Just to clarify, do you want just the `_SREG2` infrastructure from
> > > the last patch series but without the new bitwise ops?  That is to
> > > say it would be possible to send two operands to the kernel in
> > > registers, but no use would be made of it (yet).  Or are you
> > > proposing to update the existing mask-and-xor ops to send right hand
> > > operands via registers?
> > 
> > I mean, would it be possible to add a NFT_BITWISE_BOOL variant that
> > takes _SREG2 via select_ops?
> 
> In an earlier version, instead of adding new boolean ops, I added
> support for passing the mask and xor arguments in registers:
> 
>   https://lore.kernel.org/netfilter-devel/20200224124931.512416-1-jeremy@azazel.net/
> 
> Doing the same thing with one extra register is straightforward for AND
> and XOR:
> 
>   AND(x, y) = (x & y) ^ 0
>   XOR(x, y) = (x & 1) ^ y
> 
> since we can pass y in _SREG2 and 0 in _XOR for AND, and 1 in _MASK and
> y in _SREG2 for XOR.  For OR:
> 
>   OR(x, y) = (x & ~y) ^ y
> 
> it's a bit more complicated.  Instead of getting both the mask and xor
> arguments from user space, we need to do something like passing y in
> _SREG2 alone, and then constructing the bitwise negation in the kernel.
>
> Obviously, this means that the kernel is no longer completely agnostic
> about the sorts of mask-and-xor expressions user space may send.
>
> Since that is the case, we could go further and just perform the
> original ope- rations.  Thus if we get an boolean op with an _SREG2
> argument:
> 
>   * if there is an _XOR of 0, compute:
> 
>     _SREG & _SREG2
> 
>   * if there is a _MASK of 1, compute:
> 
>     _SREG ^ _SREG2
> 
>   * if there are no _MASK or _XOR arguments, compute:
> 
>     _SREG | _SREG2

OK, if my understanding is correct, these are the two options:

1) Infer from arguments the type of operation.
2) Have explicit NFT_BITWISE_{AND,OR,XOR} operations.

If so, I think it is better to stick to your original patch, where
explicit bitwise operations NFT_BITWISE_{_AND,_OR,_XOR} are added
(which is what you proposed last time IIRC).

Thanks for explaining.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2105 bytes --]

On 2023-05-03, at 22:54:11 +0200, Pablo Neira Ayuso wrote:
> On Tue, Apr 25, 2023 at 08:51:43PM +0100, Jeremy Sowden wrote:
> > On 2023-04-12, at 13:06:02 +0200, Pablo Neira Ayuso wrote:
> > > I mean, would it be possible to add a NFT_BITWISE_BOOL variant that
> > > takes _SREG2 via select_ops?
> > 
> > In an earlier version, instead of adding new boolean ops, I added
> > support for passing the mask and xor arguments in registers:
> > 
> >   https://lore.kernel.org/netfilter-devel/20200224124931.512416-1-jeremy@azazel.net/
> > 
> > Doing the same thing with one extra register is straightforward for AND
> > and XOR:
> > 
> >   AND(x, y) = (x & y) ^ 0
> >   XOR(x, y) = (x & 1) ^ y
> > 
> > since we can pass y in _SREG2 and 0 in _XOR for AND, and 1 in _MASK and
> > y in _SREG2 for XOR.  For OR:
> > 
> >   OR(x, y) = (x & ~y) ^ y
> > 
> > it's a bit more complicated.  Instead of getting both the mask and xor
> > arguments from user space, we need to do something like passing y in
> > _SREG2 alone, and then constructing the bitwise negation in the kernel.
> >
> > Obviously, this means that the kernel is no longer completely agnostic
> > about the sorts of mask-and-xor expressions user space may send.
> >
> > Since that is the case, we could go further and just perform the
> > original ope- rations.  Thus if we get an boolean op with an _SREG2
> > argument:
> > 
> >   * if there is an _XOR of 0, compute:
> > 
> >     _SREG & _SREG2
> > 
> >   * if there is a _MASK of 1, compute:
> > 
> >     _SREG ^ _SREG2
> > 
> >   * if there are no _MASK or _XOR arguments, compute:
> > 
> >     _SREG | _SREG2
> 
> OK, if my understanding is correct, these are the two options:
> 
> 1) Infer from arguments the type of operation.
> 2) Have explicit NFT_BITWISE_{AND,OR,XOR} operations.
> 
> If so, I think it is better to stick to your original patch, where
> explicit bitwise operations NFT_BITWISE_{_AND,_OR,_XOR} are added
> (which is what you proposed last time IIRC).
> 
> Thanks for explaining.

No problem.  I'll get rebasing.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Mon, May 08, 2023 at 06:58:23PM +0100, Jeremy Sowden wrote:
> On 2023-05-03, at 22:54:11 +0200, Pablo Neira Ayuso wrote:
> > On Tue, Apr 25, 2023 at 08:51:43PM +0100, Jeremy Sowden wrote:
> > > On 2023-04-12, at 13:06:02 +0200, Pablo Neira Ayuso wrote:
> > > > I mean, would it be possible to add a NFT_BITWISE_BOOL variant that
> > > > takes _SREG2 via select_ops?
> > > 
> > > In an earlier version, instead of adding new boolean ops, I added
> > > support for passing the mask and xor arguments in registers:
> > > 
> > >   https://lore.kernel.org/netfilter-devel/20200224124931.512416-1-jeremy@azazel.net/
> > > 
> > > Doing the same thing with one extra register is straightforward for AND
> > > and XOR:
> > > 
> > >   AND(x, y) = (x & y) ^ 0
> > >   XOR(x, y) = (x & 1) ^ y
> > > 
> > > since we can pass y in _SREG2 and 0 in _XOR for AND, and 1 in _MASK and
> > > y in _SREG2 for XOR.  For OR:
> > > 
> > >   OR(x, y) = (x & ~y) ^ y
> > > 
> > > it's a bit more complicated.  Instead of getting both the mask and xor
> > > arguments from user space, we need to do something like passing y in
> > > _SREG2 alone, and then constructing the bitwise negation in the kernel.
> > >
> > > Obviously, this means that the kernel is no longer completely agnostic
> > > about the sorts of mask-and-xor expressions user space may send.
> > >
> > > Since that is the case, we could go further and just perform the
> > > original ope- rations.  Thus if we get an boolean op with an _SREG2
> > > argument:
> > > 
> > >   * if there is an _XOR of 0, compute:
> > > 
> > >     _SREG & _SREG2
> > > 
> > >   * if there is a _MASK of 1, compute:
> > > 
> > >     _SREG ^ _SREG2
> > > 
> > >   * if there are no _MASK or _XOR arguments, compute:
> > > 
> > >     _SREG | _SREG2
> > 
> > OK, if my understanding is correct, these are the two options:
> > 
> > 1) Infer from arguments the type of operation.
> > 2) Have explicit NFT_BITWISE_{AND,OR,XOR} operations.
> > 
> > If so, I think it is better to stick to your original patch, where
> > explicit bitwise operations NFT_BITWISE_{_AND,_OR,_XOR} are added
> > (which is what you proposed last time IIRC).
> > 
> > Thanks for explaining.
> 
> No problem.  I'll get rebasing.

Please, small batch, not larger than 10 patches if possible.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

Hi,

On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> 
> This syntax is horrible (yes, I know, xtables fault).
> 
> Do you think this series could be changed to grab the offset register from the
> left edge of the range rather than requiring the user to specify it a
> second time?  Something like:
> 
> ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> 
> I'm open to other suggestions of course.

To allow to mix this with maps, I think the best approach is to add a
new flag (port-shift) and then allow the user to specify the
port-shift 'delta'.

ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
        192.168.127.0-129.168.127.128 : 1.2.3.4 . -55000 } port-shift

where -55000 means, subtract -55000 to the tcp dport in the packet, it
is an incremental update.

This requires a kernel patch to add the new port-shift flag.

It should be possible to add a new netlink attribute

NFTA_NAT_REG_PROTO_SHIFT

which allows for -2^16 .. +2^16 to express the (positive/negative)
delta offset.

Parser would need to be taught to deal with negative and positive
offset, we probably need a new special type for named maps too
(port-shift).

Florian, this is based on your idea to support 'add' command, which is
still needed for other usecases. I think nat is special in the sense
that the goal is to feed the registers that instruct the NAT engine
what kind of mangling is needed.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> > 
> > This syntax is horrible (yes, I know, xtables fault).
> > 
> > Do you think this series could be changed to grab the offset register from the
> > left edge of the range rather than requiring the user to specify it a
> > second time?  Something like:
> > 
> > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> > 
> > I'm open to other suggestions of course.
> 
> To allow to mix this with maps, I think the best approach is to add a
> new flag (port-shift) and then allow the user to specify the
> port-shift 'delta'.
> 
> ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
>         192.168.127.0-129.168.127.128 : 1.2.3.4 . -55000 } port-shift

Sorry, I don't see the usecase for different deltas.
But even if we assume that, kernel already takes the dnat target port
number from a register.

> where -55000 means, subtract -55000 to the tcp dport in the packet, it
> is an incremental update.
> 
> This requires a kernel patch to add the new port-shift flag.

... so I don't see why we need a new port-shift flag at all.
I think best approach is to provide the actual new dport in a register,
like we already do right now.

So we need an 'add' operation in kernel to compute

portreg = sreg_with_port + sreg_with_offset
> 
> Florian, this is based on your idea to support 'add' command, which is
> still needed for other usecases. I think nat is special in the sense
> that the goal is to feed the registers that instruct the NAT engine
> what kind of mangling is needed.

See above.  I don't think we should go with the existing NAT flag,
its very much a hack to overcome iptables design limitations.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Tue, Apr 11, 2023 at 12:25:32PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> > > 
> > > This syntax is horrible (yes, I know, xtables fault).
> > > 
> > > Do you think this series could be changed to grab the offset register from the
> > > left edge of the range rather than requiring the user to specify it a
> > > second time?  Something like:
> > > 
> > > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> > > 
> > > I'm open to other suggestions of course.
> > 
> > To allow to mix this with maps, I think the best approach is to add a
> > new flag (port-shift) and then allow the user to specify the
> > port-shift 'delta'.
> > 
> > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
> >         192.168.127.0-129.168.127.128 : 1.2.3.4 . -55000 } port-shift
> 
> Sorry, I don't see the usecase for different deltas.

Then, users will more than one single rule for different port-shift
mappings?

> But even if we assume that, kernel already takes the dnat target port
> number from a register.

In my proposal, kernel would take the delta from register, the flag
tells the nat core how to interpret this.

> > where -55000 means, subtract -55000 to the tcp dport in the packet, it
> > is an incremental update.
> > 
> > This requires a kernel patch to add the new port-shift flag.
> 
> ... so I don't see why we need a new port-shift flag at all.
> I think best approach is to provide the actual new dport in a register,
> like we already do right now.
>
> So we need an 'add' operation in kernel to compute

This is an 'add' operation built-in into the NAT engine.

How would a generic 'add' operation in the kernel will work with
concatenations?

> portreg = sreg_with_port + sreg_with_offset
> > 
> > Florian, this is based on your idea to support 'add' command, which is
> > still needed for other usecases. I think nat is special in the sense
> > that the goal is to feed the registers that instruct the NAT engine
> > what kind of mangling is needed.
> 
> See above.  I don't think we should go with the existing NAT flag,
> its very much a hack to overcome iptables design limitations.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Sorry, I don't see the usecase for different deltas.
> 
> Then, users will more than one single rule for different port-shift
> mappings?

Hmm, why?  Can't the variable offset be stored in the map itself
(instead of the new dport value)?

so assuming a map that has

  typeof ip saddr . ip daddr : ip daddr . tcp dport

... but the map content stores the delta to use, e.g.

  { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . 10000 }

... where 10000 isn't the new dport but a delta that has to be added.

  [ payload load 4b @ network header + 12 => reg 1 ] # saddr
  [ payload load 4b @ network header + 16 => reg 9 ] # daddr
  [ lookup reg 1 set m dreg 1 0x0 ]	# now we have reg1: dnat addr, reg 9: delta to add
  [ payload load 2b @ transport header + 2 => reg 10 ]
  [ math add reg 9 + reg 10 => reg 9 ]		# real port value from packet added with delta
  [ nat dnat ip addr_min reg 1 addr_max reg 1 proto_min reg 9 proto_max reg 9 flags 0x3 ]

add operation should probably also take a modulus (fixed immediate value)
so we can make a defined result for things like:

  65532 + 10000

... without a need to wrap implicitly back to "0 + remainder".

> In my proposal, kernel would take the delta from register, the flag
> tells the nat core how to interpret this.

Yep, understood.  This is mapping the existing iptables approach,
but using register instead of immediate to pass data.

> > So we need an 'add' operation in kernel to compute
> 
> This is an 'add' operation built-in into the NAT engine.
> 
> How would a generic 'add' operation in the kernel will work with
> concatenations?

Its not needed for concatentation case, the port value (in packet)
is always a fixed value, so it can be (re)loaded at runtime.

But maybe i'm missing something that the nat engone is already offering
 that this approach can't handle, or some other limitation.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Tue, Apr 11, 2023 at 01:20:01PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Sorry, I don't see the usecase for different deltas.
> > 
> > Then, users will more than one single rule for different port-shift
> > mappings?
> 
> Hmm, why?  Can't the variable offset be stored in the map itself
> (instead of the new dport value)?
> 
> so assuming a map that has
> 
>   typeof ip saddr . ip daddr : ip daddr . tcp dport

I am not sure we can use . tcp dport here, we might need a specific
datatype for offset.

> ... but the map content stores the delta to use, e.g.
> 
>   { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . 10000 }
>
> ... where 10000 isn't the new dport but a delta that has to be added.
> 
>   [ payload load 4b @ network header + 12 => reg 1 ] # saddr
>   [ payload load 4b @ network header + 16 => reg 9 ] # daddr
>   [ lookup reg 1 set m dreg 1 0x0 ]	# now we have reg1: dnat addr, reg 9: delta to add
>   [ payload load 2b @ transport header + 2 => reg 10 ]
>   [ math add reg 9 + reg 10 => reg 9 ]		# real port value from packet added with delta
>   [ nat dnat ip addr_min reg 1 addr_max reg 1 proto_min reg 9 proto_max reg 9 flags 0x3 ]

It is very similar to my proposal, but using an explicit: payload +
math.

How are you going to express this in syntax? Maybe this:

   { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . +10000 }

or

   { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . -10000 }

so + or - tells this is an offset. Parser will need this notation, so
the evaluation step infers the map datatype from the element.

For explicit maps, we need the datatype to interpret that this is an
offset accordingly.

> add operation should probably also take a modulus (fixed immediate value)
> so we can make a defined result for things like:
> 
>   65532 + 10000
> 
> ... without a need to wrap implicitly back to "0 + remainder".

not sure I follow this modulus idea.

> > In my proposal, kernel would take the delta from register, the flag
> > tells the nat core how to interpret this.
> 
> Yep, understood.  This is mapping the existing iptables approach,
> but using register instead of immediate to pass data.
> 
> > > So we need an 'add' operation in kernel to compute
> > 
> > This is an 'add' operation built-in into the NAT engine.
> > 
> > How would a generic 'add' operation in the kernel will work with
> > concatenations?
> 
> Its not needed for concatentation case, the port value (in packet)
> is always a fixed value, so it can be (re)loaded at runtime.
> 
> But maybe i'm missing something that the nat engone is already offering
> that this approach can't handle, or some other limitation.

Your proposal is not a deal breaker to me, I think it will be more
work to explore than my proposal, but this delta datatype might be
useful in the future for generic delta add/sub in other payload / meta
fields.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > so assuming a map that has
> > 
> >   typeof ip saddr . ip daddr : ip daddr . tcp dport
> 
> I am not sure we can use . tcp dport here, we might need a specific
> datatype for offset.

Right.  integer will work fine.  We will need a pseudotype
for 'typeof', tcp dport won't work as-is because nftables won't
know it needs to do the offset thing under the hood (math op or
flag or whatever).

> > ... but the map content stores the delta to use, e.g.
> > 
> >   { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . 10000 }
> >
> > ... where 10000 isn't the new dport but a delta that has to be added.
> > 
> >   [ payload load 4b @ network header + 12 => reg 1 ] # saddr
> >   [ payload load 4b @ network header + 16 => reg 9 ] # daddr
> >   [ lookup reg 1 set m dreg 1 0x0 ]	# now we have reg1: dnat addr, reg 9: delta to add
> >   [ payload load 2b @ transport header + 2 => reg 10 ]
> >   [ math add reg 9 + reg 10 => reg 9 ]		# real port value from packet added with delta
> >   [ nat dnat ip addr_min reg 1 addr_max reg 1 proto_min reg 9 proto_max reg 9 flags 0x3 ]
> 
> It is very similar to my proposal, but using an explicit: payload +
> math.

Yes.

> How are you going to express this in syntax? Maybe this:
> 
>    { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . +10000 }
> 
> or
> 
>    { 192.168.7.1 . 10.2.2.2 : 10.2.2.1 . -10000 }
> 
> so + or - tells this is an offset. Parser will need this notation, so
> the evaluation step infers the map datatype from the element.
> 
> For explicit maps, we need the datatype to interpret that this is an
> offset accordingly.

Yes.  This will also mean you can't mix real port value with offsets.
(which i don't think is a problem).

> > add operation should probably also take a modulus (fixed immediate value)
> > so we can make a defined result for things like:
> > 
> >   65532 + 10000
> > 
> > ... without a need to wrap implicitly back to "0 + remainder".
> 
> not sure I follow this modulus idea.

What should happen if you add, say, 20k, but the packet dport is larger
than (0xffff - 20k) ?

If I undertand correctly, with current iptables this will be placed
in the desired offset range, rather than wrap back to 0.

> > But maybe i'm missing something that the nat engone is already offering
> > that this approach can't handle, or some other limitation.
> 
> Your proposal is not a deal breaker to me, I think it will be more
> work to explore than my proposal, but this delta datatype might be
> useful in the future for generic delta add/sub in other payload / meta
> fields.

Ok, right, I don't think there is anything bad with your proposal
either.

Even Jeremys rebased kernel patchset looks fine to me, I just dislike
the proposed syntax (since it follows the iptables one which I don't
like either :-) )

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:

Circling back to this.

> On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> > 
> > This syntax is horrible (yes, I know, xtables fault).
> > 
> > Do you think this series could be changed to grab the offset register from the
> > left edge of the range rather than requiring the user to specify it a
> > second time?  Something like:
> > 
> > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> > 
> > I'm open to other suggestions of course.
> 
> To allow to mix this with maps, I think the best approach is to add a
> new flag (port-shift) and then allow the user to specify the
> port-shift 'delta'.
> 
> ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
>         192.168.127.0-129.168.127.128 : 1.2.3.4 . -55000 } port-shift
> 
> where -55000 means, subtract -55000 to the tcp dport in the packet, it
> is an incremental update.
> 
> This requires a kernel patch to add the new port-shift flag.

Where is this new port-shift flag needed?  NAT engine?
I'm a bit confused, are you proposing new/different syntax for Jeremys
kernel-patchset or something else?

AFAICS, for what you want do to, Jeremys kernel patches should
already work as-is?

Just to be clear again, I have no objects to the kernel patches
that Jeremy proposed.  I just dislike the iptables-inspired userspace
syntax with a need to explicitly state the left edge of the range.

> It should be possible to add a new netlink attribute
> 
> NFTA_NAT_REG_PROTO_SHIFT
> 
> which allows for -2^16 .. +2^16 to express the (positive/negative)
> delta offset.

Isn't that essentially what Jeremys patchset is already doing, i.e.
adding a new register to store the offset?

You can't use an immediate, else maps with different deltas don't work.

> Parser would need to be taught to deal with negative and positive
> offset, we probably need a new special type for named maps too
> (port-shift).

You mean a pseudotype to work with 'typeof'? We alreay do this for
verdicts so this should work.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Tue, Apr 11, 2023 at 02:36:04PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 
> Circling back to this.
> 
> > On Fri, Mar 24, 2023 at 11:59:04PM +0100, Florian Westphal wrote:
> > > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > > +ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910/55900;ok
> > > > +ip6 daddr 10::1 tcp dport 55900-55910 dnat ip6 to [::c0:a8:7f:1]:5900-5910/55900;ok
> > > 
> > > This syntax is horrible (yes, I know, xtables fault).
> > > 
> > > Do you think this series could be changed to grab the offset register from the
> > > left edge of the range rather than requiring the user to specify it a
> > > second time?  Something like:
> > > 
> > > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to 192.168.127.1:5900-5910
> > > 
> > > I'm open to other suggestions of course.
> > 
> > To allow to mix this with maps, I think the best approach is to add a
> > new flag (port-shift) and then allow the user to specify the
> > port-shift 'delta'.
> > 
> > ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
> >         192.168.127.0-129.168.127.128 : 1.2.3.4 . -55000 } port-shift
> > 
> > where -55000 means, subtract -55000 to the tcp dport in the packet, it
> > is an incremental update.
> > 
> > This requires a kernel patch to add the new port-shift flag.
> 
> Where is this new port-shift flag needed?  NAT engine?

My proposal was based on adding a new port-shift flag to the NAT
engine, for simplicity. So the NAT engine knows it has to fetch + add
delta to the existing port, instead of the explicit math + add, ie.
just let the NAT engine handle this port shift.

> I'm a bit confused, are you proposing new/different syntax for Jeremys
> kernel-patchset or something else?

I'm proposing an alternative approach. Jeremy's approach is based on
iptables way of doing things, which is backwards for nftables because
we need to integrate new things with map.

I think we can just set on the new flag in the NAT engine to say: "hey
NAT engine, I instruct you to do a port shift on the packet, and this
is the delta".

ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
         192.168.127.0-129.168.127.128 : 1.2.3.4 . -55000,
         192.168.130/24 : 1.2.3.5 . -40000 } port-shift

and all the flexibility this provides, where one might need mappings.

> AFAICS, for what you want do to, Jeremys kernel patches should
> already work as-is?
> 
> Just to be clear again, I have no objects to the kernel patches
> that Jeremy proposed.  I just dislike the iptables-inspired userspace
> syntax with a need to explicitly state the left edge of the range.

I think it is a no-go: it will require multiple rules for more than
one mapping. Then, we could extend userspace to support for:

ip daddr 10.0.0.1 tcp dport 55900-55910 dnat ip to ip saddr map { \
         192.168.127.0-129.168.127.128 : 1.2.3.4 . 45900-45910 . 55900 } port-shift
                                                                 ^^^^^

where this is the base, or even infer it, but it is a bit of
complexity to handle that from userspace, even if this is not exposed
to the syntax (ie. infered from context), that would still need to be
exposed through datatype in an explicit map definition. That will be
awkward because datatype definition of the map will have something
like:

        ipv4_addr . inet_service . base

while listing will not show the base (I am talking about the
possibility of infering the base from context).

> > It should be possible to add a new netlink attribute
> > 
> > NFTA_NAT_REG_PROTO_SHIFT
> > 
> > which allows for -2^16 .. +2^16 to express the (positive/negative)
> > delta offset.
> 
> Isn't that essentially what Jeremys patchset is already doing, i.e.
> adding a new register to store the offset?

Jeremy uses the register to store the _BASE port.

> You can't use an immediate, else maps with different deltas don't work.
> 
> > Parser would need to be taught to deal with negative and positive
> > offset, we probably need a new special type for named maps too
> > (port-shift).
> 
> You mean a pseudotype to work with 'typeof'? We alreay do this for
> verdicts so this should work.

Yes, adding a new type should not be a problem.

I think my proposal provides a simple way to support this, it just a
new flag in the NAT engine, few lines to handle it and new userspace
code to handle -/+offset in a map.

Your idea of doing it via payload + math is also good, but it would
just require more work to support this NAT port-shift feature in
userspace.

Does this help clarify? I am talking about a completely different
design for this feature, not so iptablish.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I think my proposal provides a simple way to support this, it just a
> new flag in the NAT engine, few lines to handle it and new userspace
> code to handle -/+offset in a map.

Yes, thanks for clarifying this. I'm fine with your proposal.

I think it might even be possible to rework the iptables target
(the only user of the current shift/offset infra) to work with
the 'new' delta approach, to avoid cluttering the NAT engine with
both appraoaches.

> Your idea of doing it via payload + math is also good, but it would
> just require more work to support this NAT port-shift feature in
> userspace.

Indeed, its a lot more work.

> Does this help clarify? I am talking about a completely different
> design for this feature, not so iptablish.

Yes, it does.  Agree its better solution compared to the existing
one.

Re: [PATCH nftables 8/8] test: py: add tests for shifted nat port-ranges

[Pablo Neira Ayuso]

On Wed, Apr 12, 2023 at 01:43:51PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I think my proposal provides a simple way to support this, it just a
> > new flag in the NAT engine, few lines to handle it and new userspace
> > code to handle -/+offset in a map.
> 
> Yes, thanks for clarifying this. I'm fine with your proposal.
> 
> I think it might even be possible to rework the iptables target
> (the only user of the current shift/offset infra) to work with
> the 'new' delta approach, to avoid cluttering the NAT engine with
> both appraoaches.

Agreed.

> > Your idea of doing it via payload + math is also good, but it would
> > just require more work to support this NAT port-shift feature in
> > userspace.
> 
> Indeed, its a lot more work.
>
> > Does this help clarify? I am talking about a completely different
> > design for this feature, not so iptablish.
> 
> Yes, it does.  Agree its better solution compared to the existing
> one.

OK, let's move on then :)

Re: [PATCH nftables 0/8] Support for shifted port-ranges in NAT

[Florian Westphal]

Jeremy Sowden <jeremy@azazel.net> wrote:
> Support for shifted port-ranges was added to iptables for DNAT in 2018.
> This allows one to redirect packets intended for one port to another in
> a range in such a way that the new port chosen has the same offset in
> the range as the original port had from a specified base value.
> 
> For example, by using the base value 2000, one could redirect packets
> intended for 10.0.0.1:2000-3000 to 10.10.0.1:12000-13000 so that the old
> and new ports were at the same offset in their respective ranges, i.e.:
>
>   10.0.0.1:2345 -> 10.10.0.1:12345
> 
> This patch-set adds support for doing likewise to nftables.  In contrast
> to iptables, this works for `snat`, `redirect` and `masquerade`
> statements as well as well as `dnat`.

Could you rebase and resend the kernel patches now that the
refactoring patches have been merged?

I'd like to have another look at it now that the fixes and
refactoring ones are in.

Background: I wonder if going with NF_NAT_RANGE_PROTO_OFFSET
is really a good idea or not, because it seems rather iptables-kludgy.

But if its not much work it might be simpler to jsut go along with it.
An alternate approach would be to support addition in nft, so one
could do:

dnat to tcp dport + 2000

... to get such a 'shift effect'.

[ yes, the bison parser might not like this syntax, I made it up for
illustrative purposes. ]

Something like this would also allow to emulate TTL/HL target of
iptables, ATM we can set a fixed value but cannot add or decrement
them.

Thanks.

Re: [PATCH nftables 0/8] Support for shifted port-ranges in NAT

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 1725 bytes --]

On 2023-03-24, at 15:18:56 +0100, Florian Westphal wrote:
> Jeremy Sowden wrote:
> > Support for shifted port-ranges was added to iptables for DNAT in
> > 2018.  This allows one to redirect packets intended for one port to
> > another in a range in such a way that the new port chosen has the
> > same offset in the range as the original port had from a specified
> > base value.
> > 
> > For example, by using the base value 2000, one could redirect
> > packets intended for 10.0.0.1:2000-3000 to 10.10.0.1:12000-13000 so
> > that the old and new ports were at the same offset in their
> > respective ranges, i.e.:
> >
> >   10.0.0.1:2345 -> 10.10.0.1:12345
> > 
> > This patch-set adds support for doing likewise to nftables.  In
> > contrast to iptables, this works for `snat`, `redirect` and
> > `masquerade` statements as well as well as `dnat`.
> 
> Could you rebase and resend the kernel patches now that the
> refactoring patches have been merged?
> 
> I'd like to have another look at it now that the fixes and refactoring
> ones are in.

Yup, no problem.

> Background: I wonder if going with NF_NAT_RANGE_PROTO_OFFSET is really
> a good idea or not, because it seems rather iptables-kludgy.
> 
> But if its not much work it might be simpler to jsut go along with it.
> An alternate approach would be to support addition in nft, so one
> could do:
> 
> dnat to tcp dport + 2000
> 
> ... to get such a 'shift effect'.
> 
> [ yes, the bison parser might not like this syntax, I made it up for
> illustrative purposes. ]
> 
> Something like this would also allow to emulate TTL/HL target of
> iptables, ATM we can set a fixed value but cannot add or decrement
> them.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]