[PATCH nftables 0/8] add typeof keyword

[PATCH nftables 0/8] add typeof keyword

[Florian Westphal]

This patch series adds the typeof keyword.

The only dependency is a small change to libnftnl to add two new
UDATA_SET_TYPEOF enum values.

named set can be configured as follows:

set os {
   type typeof(osf name)
   elements = { "Linux", "Windows" }
}

or
nft add set ip filter allowed "{ type typeof(ip daddr) . typeof(tcp dport); }"

... which is the same as the "old" 'type ipv4_addr . inet_service".

The type is stored in the kernel via the udata set infrastructure,
on listing -- if a udata type is present -- nft will validate that this
type matches the set key length.

This initial submission doesn't include a documentation update because
I'd like to get feedback on the chosen syntax first.

Florian Westphal (8):
      src: libnftnl: run single-initcalls only once
      src: libnftnl: split nft_ctx_new/free
      src: store expr, not dtype to track data in sets
      src: parser: add syntax to provide bitsize for non-spcific types
      src: add "typeof" keyword
      src: add "typeof" print support
      src: netlink: remove assertion
      tests: add typeof test cases

 include/datatype.h                                 |    1 
 include/netlink.h                                  |    1 
 include/nftables.h                                 |    3 
 include/rule.h                                     |    6 
 src/datatype.c                                     |    5 
 src/evaluate.c                                     |   58 ++++--
 src/expression.c                                   |    2 
 src/json.c                                         |    4 
 src/libnftables.c                                  |   48 +++--
 src/mnl.c                                          |   39 ++++
 src/monitor.c                                      |    2 
 src/netlink.c                                      |  176 ++++++++++++++++++---
 src/netlink_delinearize.c                          |   15 +
 src/parser_bison.y                                 |   26 ++-
 src/parser_json.c                                  |    8 
 src/rule.c                                         |   35 +++-
 src/scanner.l                                      |    1 
 src/segtree.c                                      |    8 
 tests/shell/testcases/maps/dumps/typeof_maps_0.nft |   16 +
 tests/shell/testcases/maps/typeof_maps_0           |   26 +++
 tests/shell/testcases/sets/dumps/typeof_sets_0.nft |   31 +++
 tests/shell/testcases/sets/typeof_sets_0           |   40 ++++
 22 files changed, 459 insertions(+), 92 deletions(-)

[PATCH nftables 1/8] src: libnftnl: run single-initcalls only once

[Florian Westphal]

---
 src/libnftables.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/libnftables.c b/src/libnftables.c
index a693c0c69075..b169dd2f2afe 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -90,11 +90,6 @@ static void nft_init(struct nft_ctx *ctx)
 	realm_table_rt_init(ctx);
 	devgroup_table_init(ctx);
 	ct_label_table_init(ctx);
-
-	gmp_init();
-#ifdef HAVE_LIBXTABLES
-	xt_init();
-#endif
 }
 
 static void nft_exit(struct nft_ctx *ctx)
@@ -142,8 +137,17 @@ static void nft_ctx_netlink_init(struct nft_ctx *ctx)
 EXPORT_SYMBOL(nft_ctx_new);
 struct nft_ctx *nft_ctx_new(uint32_t flags)
 {
+	static bool init_once;
 	struct nft_ctx *ctx;
 
+	if (!init_once) {
+		init_once = true;
+		gmp_init();
+#ifdef HAVE_LIBXTABLES
+		xt_init();
+#endif
+	}
+
 	ctx = xzalloc(sizeof(struct nft_ctx));
 	nft_init(ctx);
 
-- 
2.21.0

[PATCH nftables 2/8] src: libnftnl: split nft_ctx_new/free

[Florian Westphal]

a followup patch doesn't need e.g. symbol files to be loaded,
so split those functions to create a helper that can be used
internally by nft to create a more minimal nft context.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/nftables.h |  3 +++
 src/libnftables.c  | 32 ++++++++++++++++++++++----------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/include/nftables.h b/include/nftables.h
index ef737c839b2e..b7fbcf5726d4 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -201,6 +201,9 @@ int nft_print(struct output_ctx *octx, const char *fmt, ...)
 int nft_gmp_print(struct output_ctx *octx, const char *fmt, ...)
 	__attribute__((format(printf, 2, 0)));
 
+struct nft_ctx *__nft_ctx_new(void);
+void __nft_ctx_free(struct nft_ctx *ctx);
+
 #define __NFT_OUTPUT_NOTSUPP	UINT_MAX
 
 #endif /* NFTABLES_NFTABLES_H */
diff --git a/src/libnftables.c b/src/libnftables.c
index b169dd2f2afe..a1e2fd662a7a 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -134,8 +134,7 @@ static void nft_ctx_netlink_init(struct nft_ctx *ctx)
 	ctx->nf_sock = nft_mnl_socket_open();
 }
 
-EXPORT_SYMBOL(nft_ctx_new);
-struct nft_ctx *nft_ctx_new(uint32_t flags)
+struct nft_ctx *__nft_ctx_new(void)
 {
 	static bool init_once;
 	struct nft_ctx *ctx;
@@ -149,18 +148,26 @@ struct nft_ctx *nft_ctx_new(uint32_t flags)
 	}
 
 	ctx = xzalloc(sizeof(struct nft_ctx));
-	nft_init(ctx);
 
 	ctx->state = xzalloc(sizeof(struct parser_state));
 	nft_ctx_add_include_path(ctx, DEFAULT_INCLUDE_PATH);
-	ctx->parser_max_errors	= 10;
 	init_list_head(&ctx->cache.list);
-	ctx->flags = flags;
 	ctx->output.output_fp = stdout;
 	ctx->output.error_fp = stderr;
 
-	if (flags == NFT_CTX_DEFAULT)
-		nft_ctx_netlink_init(ctx);
+	return ctx;
+}
+
+EXPORT_SYMBOL(nft_ctx_new);
+struct nft_ctx *nft_ctx_new(uint32_t flags)
+{
+	struct nft_ctx *ctx = __nft_ctx_new();
+
+	nft_init(ctx);
+
+	ctx->parser_max_errors	= 10;
+	ctx->flags = flags;
+	nft_ctx_netlink_init(ctx);
 
 	return ctx;
 }
@@ -281,20 +288,25 @@ const char *nft_ctx_get_error_buffer(struct nft_ctx *ctx)
 	return get_cookie_buffer(&ctx->output.error_cookie);
 }
 
-EXPORT_SYMBOL(nft_ctx_free);
-void nft_ctx_free(struct nft_ctx *ctx)
+void __nft_ctx_free(struct nft_ctx *ctx)
 {
 	if (ctx->nf_sock)
 		mnl_socket_close(ctx->nf_sock);
 
 	exit_cookie(&ctx->output.output_cookie);
 	exit_cookie(&ctx->output.error_cookie);
-	iface_cache_release();
 	cache_release(&ctx->cache);
 	nft_ctx_clear_include_paths(ctx);
 	xfree(ctx->state);
 	xfree(ctx);
+}
+
+EXPORT_SYMBOL(nft_ctx_free);
+void nft_ctx_free(struct nft_ctx *ctx)
+{
 	nft_exit(ctx);
+	__nft_ctx_free(ctx);
+	iface_cache_release();
 }
 
 EXPORT_SYMBOL(nft_ctx_set_output);
-- 
2.21.0

[PATCH nftables 3/8] src: store expr, not dtype to track data in sets

[Florian Westphal]

This will be needed once we add support for the 'typeof' keyword to
handle maps that could e.g. store 'ct helper' "type" values.

Instead of:

set foo {
	type ipv4_addr . mark;

this would allow

set foo {
	typeof(ip saddr) . typeof(ct mark);

(exact syntax TBD).

This would be needed to allow sets that store variable-sized data types
(string, integer and the like) that can't be used at at the moment.

Adding special data types for everything is problematic due to the
large amount of different types needed.

For anonymous sets, e.g. "string" can be used because the needed size can
be inferred from the statement, e.g.  'osf name { "Windows", "Linux }',
but in case of named sets that won't work because 'type string' lacks the
context needed to derive the size information.

With 'typeof(osf name)' the context is there, but at the moment it won't
help because the expression is discarded instantly and only the data
type is retained.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/datatype.h |  1 -
 include/netlink.h  |  1 -
 include/rule.h     |  6 ++---
 src/datatype.c     |  5 ----
 src/evaluate.c     | 58 ++++++++++++++++++++++++++++++++--------------
 src/expression.c   |  2 +-
 src/json.c         |  4 ++--
 src/mnl.c          |  6 ++---
 src/monitor.c      |  2 +-
 src/netlink.c      | 32 ++++++++++++-------------
 src/parser_bison.y |  3 +--
 src/parser_json.c  |  8 +++++--
 src/rule.c         |  8 +++----
 src/segtree.c      |  8 +++++--
 14 files changed, 81 insertions(+), 63 deletions(-)

diff --git a/include/datatype.h b/include/datatype.h
index c1d08cc29e47..87fef59dd1a6 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -287,7 +287,6 @@ concat_subtype_lookup(uint32_t type, unsigned int n)
 
 extern const struct datatype *
 set_datatype_alloc(const struct datatype *orig_dtype, unsigned int byteorder);
-extern void set_datatype_destroy(const struct datatype *dtype);
 
 extern void time_print(uint64_t msec, struct output_ctx *octx);
 extern struct error_record *time_parse(const struct location *loc,
diff --git a/include/netlink.h b/include/netlink.h
index 279723f33d31..6aead9a8c5e2 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -190,6 +190,5 @@ int netlink_events_trace_cb(const struct nlmsghdr *nlh, int type,
 			    struct netlink_mon_handler *monh);
 
 enum nft_data_types dtype_map_to_kernel(const struct datatype *dtype);
-const struct datatype *dtype_map_from_kernel(enum nft_data_types type);
 
 #endif /* NFTABLES_NETLINK_H */
diff --git a/include/rule.h b/include/rule.h
index 0ef6aacdd71b..331815643362 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -274,8 +274,7 @@ extern struct rule *rule_lookup_by_index(const struct chain *chain,
  * @gc_int:	garbage collection interval
  * @timeout:	default timeout value
  * @key:	key expression (data type, length))
- * @datatype:	mapping data type
- * @datalen:	mapping data len
+ * @data:	mapping data expression
  * @objtype:	mapping object type
  * @init:	initializer
  * @rg_cache:	cached range element (left)
@@ -292,8 +291,7 @@ struct set {
 	uint32_t		gc_int;
 	uint64_t		timeout;
 	struct expr		*key;
-	const struct datatype	*datatype;
-	unsigned int		datalen;
+	struct expr		*data;
 	uint32_t		objtype;
 	struct expr		*init;
 	struct expr		*rg_cache;
diff --git a/src/datatype.c b/src/datatype.c
index c5a013463eb9..2111a72d4bed 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1187,11 +1187,6 @@ const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype,
 	return dtype;
 }
 
-void set_datatype_destroy(const struct datatype *dtype)
-{
-	datatype_free(dtype);
-}
-
 static struct error_record *time_unit_parse(const struct location *loc,
 					    const char *str, uint64_t *unit)
 {
diff --git a/src/evaluate.c b/src/evaluate.c
index 831eb7c25c5c..bbae6220a2cd 100755
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1338,6 +1338,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr_ctx ectx = ctx->ectx;
 	struct expr *map = *expr, *mappings;
+	const struct datatype *dtype;
 	struct expr *key;
 
 	expr_set_context(&ctx->ectx, NULL, 0);
@@ -1360,10 +1361,14 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
 		mappings = implicit_set_declaration(ctx, "__map%d",
 						    key,
 						    mappings);
-		mappings->set->datatype =
-			datatype_get(set_datatype_alloc(ectx.dtype,
-							ectx.byteorder));
-		mappings->set->datalen  = ectx.len;
+
+		dtype = set_datatype_alloc(ectx.dtype, ectx.byteorder);
+
+		mappings->set->data = constant_expr_alloc(&netlink_location,
+							  dtype, dtype->byteorder,
+							  ectx.len, NULL);
+		if (ectx.len && mappings->set->data->len != ectx.len)
+			BUG("%d vs %d\n", mappings->set->data->len, ectx.len);
 
 		map->mappings = mappings;
 
@@ -1399,7 +1404,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
 					 map->mappings->set->key->dtype->desc,
 					 map->map->dtype->desc);
 
-	datatype_set(map, map->mappings->set->datatype);
+	datatype_set(map, map->mappings->set->data->dtype);
 	map->flags |= EXPR_F_CONSTANT;
 
 	/* Data for range lookups needs to be in big endian order */
@@ -1429,7 +1434,12 @@ static int expr_evaluate_mapping(struct eval_ctx *ctx, struct expr **expr)
 				  "Key must be a constant");
 	mapping->flags |= mapping->left->flags & EXPR_F_SINGLETON;
 
-	expr_set_context(&ctx->ectx, set->datatype, set->datalen);
+	if (set->data) {
+		expr_set_context(&ctx->ectx, set->data->dtype, set->data->len);
+	} else {
+		assert((set->flags & NFT_SET_MAP) == 0);
+	}
+
 	if (expr_evaluate(ctx, &mapping->right) < 0)
 		return -1;
 	if (!expr_is_constant(mapping->right))
@@ -1999,7 +2009,7 @@ static int stmt_evaluate_arg(struct eval_ctx *ctx, struct stmt *stmt,
 					 (*expr)->len);
 	else if ((*expr)->dtype->type != TYPE_INTEGER &&
 		 !datatype_equal((*expr)->dtype, dtype))
-		return stmt_binary_error(ctx, *expr, stmt,
+		return stmt_binary_error(ctx, *expr, stmt,		/* verdict vs invalid? */
 					 "datatype mismatch: expected %s, "
 					 "expression has type %s",
 					 dtype->desc, (*expr)->dtype->desc);
@@ -2983,9 +2993,9 @@ static int stmt_evaluate_map(struct eval_ctx *ctx, struct stmt *stmt)
 				  "Key expression comments are not supported");
 
 	if (stmt_evaluate_arg(ctx, stmt,
-			      stmt->map.set->set->datatype,
-			      stmt->map.set->set->datalen,
-			      stmt->map.set->set->datatype->byteorder,
+			      stmt->map.set->set->data->dtype,
+			      stmt->map.set->set->data->len,
+			      stmt->map.set->set->data->byteorder,
 			      &stmt->map.data->key) < 0)
 		return -1;
 	if (expr_is_constant(stmt->map.data))
@@ -3031,8 +3041,12 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt)
 
 		mappings = implicit_set_declaration(ctx, "__objmap%d",
 						    key, mappings);
-		mappings->set->datatype = &string_type;
-		mappings->set->datalen  = NFT_OBJ_MAXNAMELEN * BITS_PER_BYTE;
+
+		mappings->set->data = constant_expr_alloc(&netlink_location,
+							  &string_type,
+							  BYTEORDER_HOST_ENDIAN,
+							  NFT_OBJ_MAXNAMELEN * BITS_PER_BYTE,
+							  NULL);
 		mappings->set->objtype  = stmt->objref.type;
 
 		map->mappings = mappings;
@@ -3067,7 +3081,7 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt)
 					 map->mappings->set->key->dtype->desc,
 					 map->map->dtype->desc);
 
-	datatype_set(map, map->mappings->set->datatype);
+	datatype_set(map, map->mappings->set->data->dtype);
 	map->flags |= EXPR_F_CONSTANT;
 
 	/* Data for range lookups needs to be in big endian order */
@@ -3212,17 +3226,25 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 		return set_error(ctx, set, "concatenated types not supported in interval sets");
 
 	if (set_is_datamap(set->flags)) {
-		if (set->datatype == NULL)
+		if (set->data == NULL)
 			return set_error(ctx, set, "map definition does not "
 					 "specify mapping data type");
 
-		set->datalen = set->datatype->size;
-		if (set->datalen == 0 && set->datatype->type != TYPE_VERDICT)
+		if (set->data->len == 0 && set->data->dtype->type != TYPE_VERDICT)
 			return set_error(ctx, set, "unqualified mapping data "
 					 "type specified in map definition");
 	} else if (set_is_objmap(set->flags)) {
-		set->datatype = &string_type;
-		set->datalen  = NFT_OBJ_MAXNAMELEN * BITS_PER_BYTE;
+		if (set->data) {
+			assert(set->data->etype == EXPR_VALUE);
+			assert(set->data->dtype == &string_type);
+		}
+
+		assert(set->data == NULL);
+		set->data = constant_expr_alloc(&netlink_location, &string_type,
+						BYTEORDER_HOST_ENDIAN,
+						NFT_OBJ_MAXNAMELEN * BITS_PER_BYTE,
+						NULL);
+
 	}
 
 	ctx->set = set;
diff --git a/src/expression.c b/src/expression.c
index cb49e0b73f5a..09005c650ede 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -994,7 +994,7 @@ static void map_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	expr_print(expr->map, octx);
 	if (expr->mappings->etype == EXPR_SET_REF &&
-	    expr->mappings->set->datatype->type == TYPE_VERDICT)
+	    expr->mappings->set->data->dtype->type == TYPE_VERDICT)
 		nft_print(octx, " vmap ");
 	else
 		nft_print(octx, " map ");
diff --git a/src/json.c b/src/json.c
index 55ce05315d37..3481f2e0677f 100644
--- a/src/json.c
+++ b/src/json.c
@@ -82,7 +82,7 @@ static json_t *set_print_json(struct output_ctx *octx, const struct set *set)
 
 	if (set_is_datamap(set->flags)) {
 		type = "map";
-		datatype_ext = set->datatype->name;
+		datatype_ext = set->data->dtype->name;
 	} else if (set_is_objmap(set->flags)) {
 		type = "map";
 		datatype_ext = obj_type_name(set->objtype);
@@ -617,7 +617,7 @@ json_t *map_expr_json(const struct expr *expr, struct output_ctx *octx)
 	const char *type = "map";
 
 	if (expr->mappings->etype == EXPR_SET_REF &&
-	    expr->mappings->set->datatype->type == TYPE_VERDICT)
+	    expr->mappings->set->data->dtype->type == TYPE_VERDICT)
 		type = "vmap";
 
 	return json_pack("{s:{s:o, s:o}}", type,
diff --git a/src/mnl.c b/src/mnl.c
index 9c1f5356c9b9..034f21709a19 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -815,9 +815,9 @@ int mnl_nft_set_add(struct netlink_ctx *ctx, const struct cmd *cmd,
 			  div_round_up(set->key->len, BITS_PER_BYTE));
 	if (set_is_datamap(set->flags)) {
 		nftnl_set_set_u32(nls, NFTNL_SET_DATA_TYPE,
-				  dtype_map_to_kernel(set->datatype));
+				  dtype_map_to_kernel(set->data->dtype));
 		nftnl_set_set_u32(nls, NFTNL_SET_DATA_LEN,
-				  set->datalen / BITS_PER_BYTE);
+				  set->data->len / BITS_PER_BYTE);
 	}
 	if (set_is_objmap(set->flags))
 		nftnl_set_set_u32(nls, NFTNL_SET_OBJ_TYPE, set->objtype);
@@ -849,7 +849,7 @@ int mnl_nft_set_add(struct netlink_ctx *ctx, const struct cmd *cmd,
 
 	if (set_is_datamap(set->flags) &&
 	    !nftnl_udata_put_u32(udbuf, NFTNL_UDATA_SET_DATABYTEORDER,
-				 set->datatype->byteorder))
+				 set->data->byteorder))
 		memory_allocation_error();
 
 	if (set->automerge &&
diff --git a/src/monitor.c b/src/monitor.c
index 40c381149cda..d90bbfd4d505 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -401,7 +401,7 @@ static int netlink_events_setelem_cb(const struct nlmsghdr *nlh, int type,
 	 */
 	dummyset = set_alloc(monh->loc);
 	dummyset->key = expr_clone(set->key);
-	dummyset->datatype = set->datatype;
+	dummyset->data = set->data;
 	dummyset->flags = set->flags;
 	dummyset->init = set_expr_alloc(monh->loc, set);
 
diff --git a/src/netlink.c b/src/netlink.c
index f8e1120447d9..773b98cec61a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -520,7 +520,7 @@ enum nft_data_types dtype_map_to_kernel(const struct datatype *dtype)
 	}
 }
 
-const struct datatype *dtype_map_from_kernel(enum nft_data_types type)
+static const struct datatype *dtype_map_from_kernel(enum nft_data_types type)
 {
 	switch (type) {
 	case NFT_DATA_VERDICT:
@@ -567,10 +567,10 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 				    const struct nftnl_set *nls)
 {
 	const struct nftnl_udata *ud[NFTNL_UDATA_SET_MAX + 1] = {};
-	uint32_t flags, key, data, data_len, objtype = 0;
 	enum byteorder keybyteorder = BYTEORDER_INVALID;
 	enum byteorder databyteorder = BYTEORDER_INVALID;
-	const struct datatype *keytype, *datatype;
+	const struct datatype *keytype, *datatype = NULL;
+	uint32_t flags, key, objtype = 0;
 	bool automerge = false;
 	const char *udata;
 	struct set *set;
@@ -604,6 +604,8 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 
 	flags = nftnl_set_get_u32(nls, NFTNL_SET_FLAGS);
 	if (set_is_datamap(flags)) {
+		uint32_t data;
+
 		data = nftnl_set_get_u32(nls, NFTNL_SET_DATA_TYPE);
 		datatype = dtype_map_from_kernel(data);
 		if (datatype == NULL) {
@@ -612,8 +614,7 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 					 data);
 			return NULL;
 		}
-	} else
-		datatype = NULL;
+	}
 
 	if (set_is_objmap(flags)) {
 		objtype = nftnl_set_get_u32(nls, NFTNL_SET_OBJ_TYPE);
@@ -636,16 +637,13 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 
 	set->objtype = objtype;
 
+	set->data = NULL;
 	if (datatype)
-		set->datatype = datatype_get(set_datatype_alloc(datatype,
-								databyteorder));
-	else
-		set->datatype = NULL;
-
-	if (nftnl_set_is_set(nls, NFTNL_SET_DATA_LEN)) {
-		data_len = nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN);
-		set->datalen = data_len * BITS_PER_BYTE;
-	}
+		set->data = constant_expr_alloc(&netlink_location,
+						set_datatype_alloc(datatype, databyteorder),
+						databyteorder,
+						nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN) * BITS_PER_BYTE,
+						NULL);
 
 	if (nftnl_set_is_set(nls, NFTNL_SET_TIMEOUT))
 		set->timeout = nftnl_set_get_u64(nls, NFTNL_SET_TIMEOUT);
@@ -830,10 +828,10 @@ int netlink_delinearize_setelem(struct nftnl_set_elem *nlse,
 			goto out;
 
 		data = netlink_alloc_data(&netlink_location, &nld,
-					  set->datatype->type == TYPE_VERDICT ?
+					  set->data->dtype->type == TYPE_VERDICT ?
 					  NFT_REG_VERDICT : NFT_REG_1);
-		datatype_set(data, set->datatype);
-		data->byteorder = set->datatype->byteorder;
+		datatype_set(data, set->data->dtype);
+		data->byteorder = set->data->byteorder;
 		if (data->byteorder == BYTEORDER_HOST_ENDIAN)
 			mpz_switch_byteorder(data->value, data->len / BITS_PER_BYTE);
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index bff5e274c787..c531ee1d1dd8 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1690,9 +1690,8 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 						stmt_separator
 			{
 				$1->key = $3;
-				$1->datatype = $5->dtype;
+				$1->data = $5;
 
-				expr_free($5);
 				$1->flags |= NFT_SET_MAP;
 				$$ = $1;
 			}
diff --git a/src/parser_json.c b/src/parser_json.c
index a969bd4c3676..fbb90da60a83 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2791,11 +2791,15 @@ static struct cmd *json_parse_cmd_add_set(struct json_ctx *ctx, json_t *root,
 	}
 
 	if (!json_unpack(root, "{s:s}", "map", &dtype_ext)) {
+		const struct datatype *dtype;
+
 		set->objtype = string_to_nft_object(dtype_ext);
 		if (set->objtype) {
 			set->flags |= NFT_SET_OBJECT;
-		} else if (datatype_lookup_byname(dtype_ext)) {
-			set->datatype = datatype_lookup_byname(dtype_ext);
+		} else if ((dtype = datatype_lookup_byname(dtype_ext))) {
+			set->data = constant_expr_alloc(&netlink_location,
+							dtype, dtype->byteorder,
+							dtype->size, NULL);
 			set->flags |= NFT_SET_MAP;
 		} else {
 			json_error(ctx, "Invalid map type '%s'.", dtype_ext);
diff --git a/src/rule.c b/src/rule.c
index 5655e8c011e4..aee08ea12c8b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -330,8 +330,8 @@ struct set *set_clone(const struct set *set)
 	new_set->gc_int		= set->gc_int;
 	new_set->timeout	= set->timeout;
 	new_set->key		= expr_clone(set->key);
-	new_set->datatype	= datatype_get(set->datatype);
-	new_set->datalen	= set->datalen;
+	if (set->data)
+		new_set->data	= expr_clone(set->data);
 	new_set->objtype	= set->objtype;
 	new_set->policy		= set->policy;
 	new_set->automerge	= set->automerge;
@@ -354,7 +354,7 @@ void set_free(struct set *set)
 		expr_free(set->init);
 	handle_free(&set->handle);
 	expr_free(set->key);
-	set_datatype_destroy(set->datatype);
+	expr_free(set->data);
 	xfree(set);
 }
 
@@ -468,7 +468,7 @@ static void set_print_declaration(const struct set *set,
 	nft_print(octx, "%s%stype %s",
 		  opts->tab, opts->tab, set->key->dtype->name);
 	if (set_is_datamap(set->flags))
-		nft_print(octx, " : %s", set->datatype->name);
+		nft_print(octx, " : %s", set->data->dtype->name);
 	else if (set_is_objmap(set->flags))
 		nft_print(octx, " : %s", obj_type_name(set->objtype));
 
diff --git a/src/segtree.c b/src/segtree.c
index eff0653a8dfb..cfb892c3bb34 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -79,8 +79,12 @@ static void seg_tree_init(struct seg_tree *tree, const struct set *set,
 	tree->root	= RB_ROOT;
 	tree->keytype	= set->key->dtype;
 	tree->keylen	= set->key->len;
-	tree->datatype	= set->datatype;
-	tree->datalen	= set->datalen;
+	tree->datatype	= NULL;
+	tree->datalen	= 0;
+	if (set->data) {
+		tree->datatype	= set->data->dtype;
+		tree->datalen	= set->data->len;
+	}
 	tree->byteorder	= first->byteorder;
 	tree->debug_mask = debug_mask;
 }
-- 
2.21.0

[PATCH nftables 4/8] src: parser: add syntax to provide bitsize for non-spcific types

[Florian Westphal]

This allows creation of sets with string and integer types by
providing the datatype width using a comma, e.g.

"type string, 64" or "integer, 32".

This is mainly intended as a fallback for the upcoming "typeof"
keyword -- if we can't make sense of the kernel provided type
(or its missing entirely), we can then fallback to this format.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y | 18 ++++++++++++++++++
 src/rule.c         | 24 +++++++++++++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index c531ee1d1dd8..ee169fbac194 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1826,6 +1826,24 @@ data_type_atom_expr	:	type_identifier
 							 dtype->size, NULL);
 				xfree($1);
 			}
+			|	type_identifier	COMMA	NUM
+			{
+				const struct datatype *dtype = datatype_lookup_byname($1);
+				if (dtype == NULL) {
+					erec_queue(error(&@1, "unknown datatype %s", $1),
+						   state->msgs);
+					YYERROR;
+				}
+
+				if (dtype->size) {
+					erec_queue(error(&@1, "Datatype %s has a fixed type", $1),
+						   state->msgs);
+					YYERROR;
+				}
+				$$ = constant_expr_alloc(&@1, dtype, dtype->byteorder,
+							 $3, NULL);
+				xfree($1);
+			}
 			;
 
 data_type_expr		:	data_type_atom_expr
diff --git a/src/rule.c b/src/rule.c
index aee08ea12c8b..59369c9082a3 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -436,6 +436,16 @@ const char *set_policy2str(uint32_t policy)
 	}
 }
 
+static void set_print_key(const struct expr *expr, struct output_ctx *octx)
+{
+	const struct datatype *dtype = expr->dtype;
+
+	if (dtype->size)
+		nft_print(octx, " %s", dtype->name);
+	else
+		nft_print(octx, " %s,%d", dtype->name, expr->len);
+}
+
 static void set_print_declaration(const struct set *set,
 				  struct print_fmt_options *opts,
 				  struct output_ctx *octx)
@@ -465,12 +475,16 @@ static void set_print_declaration(const struct set *set,
 	if (nft_output_handle(octx))
 		nft_print(octx, " # handle %" PRIu64, set->handle.handle.id);
 	nft_print(octx, "%s", opts->nl);
-	nft_print(octx, "%s%stype %s",
-		  opts->tab, opts->tab, set->key->dtype->name);
-	if (set_is_datamap(set->flags))
-		nft_print(octx, " : %s", set->data->dtype->name);
-	else if (set_is_objmap(set->flags))
+	nft_print(octx, "%s%stype ",
+		  opts->tab, opts->tab);
+	set_print_key(set->key, octx);
+
+	if (set_is_datamap(set->flags)) {
+		nft_print(octx, " : ");
+		set_print_key(set->data, octx);
+	} else if (set_is_objmap(set->flags)) {
 		nft_print(octx, " : %s", obj_type_name(set->objtype));
+	}
 
 	nft_print(octx, "%s", opts->stmt_separator);
 
-- 
2.21.0

[PATCH nftables 5/8] src: add "typeof" keyword

[Florian Westphal]

This allows users to specify named sets by using the expression
directly, rather than having to lookup the data type to use, or
the needed size via 'nft describe".

Example:

table filter {
    set allowed_dports {
        type typeof(tcp dport);
    }
    map nametomark {
        type typeof(osf name) : typeof(meta mark);
    }
    map port2helper {
        type ipv4_addr . inet_service : typeof(ct helper);
    }
}

Currently, listing such a table will lose the typeof() expression:

nft will print the datatype instead, just as if "type inet_service"
would have been used.

For types with non-fixed widths, the new "type, width" format
added in previous patch is used.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y | 5 +++++
 src/scanner.l      | 1 +
 2 files changed, 6 insertions(+)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index ee169fbac194..876050ba6863 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -192,6 +192,7 @@ int nft_lex(void *, void *, void *);
 %token DEFINE			"define"
 %token REDEFINE			"redefine"
 %token UNDEFINE			"undefine"
+%token TYPEOF			"typeof"
 
 %token FIB			"fib"
 
@@ -1844,6 +1845,10 @@ data_type_atom_expr	:	type_identifier
 							 $3, NULL);
 				xfree($1);
 			}
+			|	TYPEOF	'('	primary_expr	')'
+			{
+				$$ = $3;
+			}
 			;
 
 data_type_expr		:	data_type_atom_expr
diff --git a/src/scanner.l b/src/scanner.l
index c1adcbddbd73..cd563aa0ca1f 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -243,6 +243,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "define"		{ return DEFINE; }
 "redefine"		{ return REDEFINE; }
 "undefine"		{ return UNDEFINE; }
+"typeof"		{ return TYPEOF; }
 
 "describe"		{ return DESCRIBE; }
 
-- 
2.21.0

[PATCH nftables 6/8] src: add "typeof" print support

[Florian Westphal]

Use the 'udata' facility to store the original string in the kernel,
then use the existing 'describe' facility to resolve it back to a name.

This isn't strictly required, in case resolution fails, the udata is
inconsistent (e.g., because set specifies a size that is not identical
to the expressions size that we get when decoding the expression), if
the udata is missing, etc. then the "type, width" format is used.

This allows us to list and restore such sets even if we can't
reconstruct the original expression.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/libnftables.c |   2 +-
 src/mnl.c         |  33 ++++++++++
 src/netlink.c     | 156 ++++++++++++++++++++++++++++++++++++++++++----
 src/rule.c        |  13 ++--
 4 files changed, 186 insertions(+), 18 deletions(-)

diff --git a/src/libnftables.c b/src/libnftables.c
index a1e2fd662a7a..e9ebc3f389fc 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -424,7 +424,7 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
 			.msgs	= msgs,
 		};
 		if (cmd_evaluate(&ectx, cmd) < 0 &&
-		    ++nft->state->nerrs == nft->parser_max_errors)
+		    ++nft->state->nerrs >= nft->parser_max_errors)
 			return -1;
 	}
 
diff --git a/src/mnl.c b/src/mnl.c
index 034f21709a19..5e5f3628cfe1 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -787,6 +787,35 @@ err:
 	return NULL;
 }
 
+static void set_key_expression(struct netlink_ctx *ctx,
+				struct expr *expr, uint32_t set_flags,
+				struct nftnl_udata_buf *udbuf,
+				unsigned int type)
+{
+	struct output_ctx octx = {};
+	char buf[64];
+
+	if (expr->flags & EXPR_F_CONSTANT ||
+	    set_flags & NFT_SET_ANONYMOUS)
+		return;
+
+	buf[0] = 0;
+	/* Set definition uses typeof to define datatype. */
+	octx.output_fp = fmemopen(buf, sizeof(buf), "w");
+	if (octx.output_fp) {
+		char *end;
+
+		expr_print(expr, &octx);
+		fclose(octx.output_fp);
+		end = strchr(buf, '&');
+		if (end)
+			* end = 0;
+
+		if (!nftnl_udata_put(udbuf, type, strlen(buf) + 1, buf))
+			memory_allocation_error();
+	}
+}
+
 /*
  * Set
  */
@@ -857,6 +886,10 @@ int mnl_nft_set_add(struct netlink_ctx *ctx, const struct cmd *cmd,
 				 set->automerge))
 		memory_allocation_error();
 
+	set_key_expression(ctx, set->key, set->flags, udbuf, NFTNL_UDATA_SET_TYPEOF_KEY);
+	if (set->data)
+		set_key_expression(ctx, set->data, set->flags, udbuf, NFTNL_UDATA_SET_TYPEOF_DATA);
+
 	nftnl_set_set_data(nls, NFTNL_SET_USERDATA, nftnl_udata_buf_data(udbuf),
 			   nftnl_udata_buf_len(udbuf));
 	nftnl_udata_buf_free(udbuf);
diff --git a/src/netlink.c b/src/netlink.c
index 773b98cec61a..99f7a3b0bca7 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -32,6 +32,7 @@
 #include <linux/netfilter.h>
 
 #include <nftables.h>
+#include <parser.h>
 #include <netlink.h>
 #include <mnl.h>
 #include <expression.h>
@@ -556,6 +557,11 @@ static int set_parse_udata_cb(const struct nftnl_udata *attr, void *data)
 		if (len != sizeof(uint32_t))
 			return -1;
 		break;
+	case NFTNL_UDATA_SET_TYPEOF_KEY:
+	case NFTNL_UDATA_SET_TYPEOF_DATA:
+		if (len < 3)
+			return -1;
+		break;
 	default:
 		return 0;
 	}
@@ -563,6 +569,98 @@ static int set_parse_udata_cb(const struct nftnl_udata *attr, void *data)
 	return 0;
 }
 
+static int parse_desc(struct nft_ctx *nft, const char *buf, struct list_head *cmds)
+{
+	static const struct input_descriptor indesc_udata = {
+		.type	= INDESC_BUFFER,
+		.name	= "<setudata>",
+	};
+	struct error_record *erec, *nexte;
+	LIST_HEAD(errors);
+	int ret;
+
+	parser_init(nft, nft->state, &errors, cmds);
+	nft->scanner = scanner_init(nft->state);
+	scanner_push_buffer(nft->scanner, &indesc_udata, buf);
+
+	ret = nft_parse(nft, nft->scanner, nft->state);
+
+	list_for_each_entry_safe(erec, nexte, &errors, list) {
+		list_del(&erec->list);
+		erec_destroy(erec);
+	}
+
+	if (nft->scanner) {
+		scanner_destroy(nft);
+		nft->scanner = NULL;
+	}
+
+	if (ret != 0 || nft->state->nerrs > 0)
+		return -1;
+
+	return 0;
+}
+
+static struct expr *set_make_key(const struct nftnl_udata *ud)
+{
+	struct cmd *cmd, *nextc;
+	struct nft_ctx *nft;
+	char cmdline[1024];
+	struct expr *expr;
+	const char *buf;
+	LIST_HEAD(cmds);
+	uint8_t len;
+	int i;
+
+	if (!ud)
+		return NULL;
+
+	len = nftnl_udata_len(ud);
+	if (!len)
+		return NULL;
+
+	buf = nftnl_udata_get(ud);
+	if (!buf)
+		return NULL;
+
+	for (i = 0; i < len; i++) {
+		if (buf[i] > 'z')
+			return NULL;
+		if (buf[i] == ' ')
+			continue;
+		if (buf[i] < 'a') {
+			if (buf[i] == '\0' && i == len - 1)
+				continue;
+
+			return NULL;
+		}
+	}
+
+	snprintf(cmdline, sizeof(cmdline), "describe %s\n", buf);
+
+	nft = __nft_ctx_new();
+	parse_desc(nft, cmdline, &cmds);
+
+	expr = NULL;
+	list_for_each_entry_safe(cmd, nextc, &cmds, list) {
+		if (cmd->op == CMD_DESCRIBE && !expr)
+			expr = expr_get(cmd->expr);
+		list_del(&cmd->list);
+		cmd_free(cmd);
+	}
+
+	__nft_ctx_free(nft);
+	return expr;
+}
+
+static bool set_udata_key_valid(const struct expr *e, const struct datatype *d, uint32_t len)
+{
+	if (!e)
+		return false;
+
+	return div_round_up(e->len, BITS_PER_BYTE) == len / BITS_PER_BYTE;
+}
+
 struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 				    const struct nftnl_set *nls)
 {
@@ -570,11 +668,17 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 	enum byteorder keybyteorder = BYTEORDER_INVALID;
 	enum byteorder databyteorder = BYTEORDER_INVALID;
 	const struct datatype *keytype, *datatype = NULL;
+	struct expr *typeof_expr_key, *typeof_expr_data;
 	uint32_t flags, key, objtype = 0;
+	const struct datatype *dtype;
 	bool automerge = false;
 	const char *udata;
 	struct set *set;
 	uint32_t ulen;
+	uint32_t klen;
+
+	typeof_expr_key = NULL;
+	typeof_expr_data = NULL;
 
 	if (nftnl_set_is_set(nls, NFTNL_SET_USERDATA)) {
 		udata = nftnl_set_get_data(nls, NFTNL_SET_USERDATA, &ulen);
@@ -592,6 +696,8 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 		GET_U32_UDATA(automerge, NFTNL_UDATA_SET_MERGE_ELEMENTS);
 
 #undef GET_U32_UDATA
+		typeof_expr_key = set_make_key(ud[NFTNL_UDATA_SET_TYPEOF_KEY]);
+		typeof_expr_data = set_make_key(ud[NFTNL_UDATA_SET_TYPEOF_DATA]);
 	}
 
 	key = nftnl_set_get_u32(nls, NFTNL_SET_KEY_TYPE);
@@ -612,12 +718,14 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 			netlink_io_error(ctx, NULL,
 					 "Unknown data type in set key %u",
 					 data);
+			datatype_free(keytype);
 			return NULL;
 		}
 	}
 
 	if (set_is_objmap(flags)) {
 		objtype = nftnl_set_get_u32(nls, NFTNL_SET_OBJ_TYPE);
+		assert(!datatype);
 		datatype = &string_type;
 	}
 
@@ -627,24 +735,46 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 	set->handle.set.name = xstrdup(nftnl_set_get_str(nls, NFTNL_SET_NAME));
 	set->automerge	   = automerge;
 
-	set->key     = constant_expr_alloc(&netlink_location,
-					   set_datatype_alloc(keytype, keybyteorder),
-					   keybyteorder,
-					   nftnl_set_get_u32(nls, NFTNL_SET_KEY_LEN) * BITS_PER_BYTE,
-					   NULL);
+	dtype = set_datatype_alloc(keytype, keybyteorder);
+	klen = nftnl_set_get_u32(nls, NFTNL_SET_KEY_LEN) * BITS_PER_BYTE;
+
+	if (set_udata_key_valid(typeof_expr_key, dtype, klen)) {
+		datatype_free(datatype_get(dtype));
+		set->key = typeof_expr_key;
+	} else {
+		expr_free(typeof_expr_key);
+		set->key = constant_expr_alloc(&netlink_location, dtype,
+					       keybyteorder, klen,
+					       NULL);
+	}
+
+	if (dtype != keytype)
+		datatype_free(keytype);
+
+	if (datatype) {
+		dtype = set_datatype_alloc(datatype, databyteorder);
+		klen = nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN) * BITS_PER_BYTE;
+
+		if (set_udata_key_valid(typeof_expr_data, dtype, klen)) {
+			datatype_free(datatype_get(dtype));
+			set->data = typeof_expr_data;
+		} else {
+			expr_free(typeof_expr_data);
+			set->data = constant_expr_alloc(&netlink_location,
+							dtype,
+							databyteorder, klen,
+							NULL);
+		}
+
+		if (dtype != datatype)
+			datatype_free(datatype);
+	}
+
 	set->flags   = nftnl_set_get_u32(nls, NFTNL_SET_FLAGS);
 	set->handle.handle.id = nftnl_set_get_u64(nls, NFTNL_SET_HANDLE);
 
 	set->objtype = objtype;
 
-	set->data = NULL;
-	if (datatype)
-		set->data = constant_expr_alloc(&netlink_location,
-						set_datatype_alloc(datatype, databyteorder),
-						databyteorder,
-						nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN) * BITS_PER_BYTE,
-						NULL);
-
 	if (nftnl_set_is_set(nls, NFTNL_SET_TIMEOUT))
 		set->timeout = nftnl_set_get_u64(nls, NFTNL_SET_TIMEOUT);
 	if (nftnl_set_is_set(nls, NFTNL_SET_GC_INTERVAL))
diff --git a/src/rule.c b/src/rule.c
index 59369c9082a3..db9401c6a035 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -440,10 +440,15 @@ static void set_print_key(const struct expr *expr, struct output_ctx *octx)
 {
 	const struct datatype *dtype = expr->dtype;
 
-	if (dtype->size)
-		nft_print(octx, " %s", dtype->name);
-	else
-		nft_print(octx, " %s,%d", dtype->name, expr->len);
+	if (dtype->size || dtype->type == TYPE_VERDICT)
+		nft_print(octx, "%s", dtype->name);
+	else if (expr->flags & EXPR_F_CONSTANT)
+		nft_print(octx, "%s,%d", dtype->name, expr->len);
+	else {
+		nft_print(octx, "typeof(");
+		expr_print(expr, octx);
+		nft_print(octx, ")");
+	}
 }
 
 static void set_print_declaration(const struct set *set,
-- 
2.21.0

[PATCH nftables 7/8] src: netlink: remove assertion

[Florian Westphal]

This assert can trigger as follows:

set s {
	type integer,8
	elemets = { 1 }
};
vlan id @s accept

reason is that 'vlan id' will store a 16 bit value into the dreg,
so set should use 'integer,16'.

The kernel won't detect this, as the lookup expression will only
verify that it can load one byte from the given register.

This removes the assertion, in case we hit this condition we can just
return without doing any further actions.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index fc2574b1dea9..11c2e2d87c54 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1796,9 +1796,20 @@ static void binop_adjust_one(const struct expr *binop, struct expr *value,
 {
 	struct expr *left = binop->left;
 
-	assert(value->len >= binop->right->len);
-
 	mpz_rshift_ui(value->value, shift);
+
+	/* This will happen when a set has a key that is
+	 * smaller than the amount of bytes loaded by the
+	 * payload/exthdr expression.
+	 *
+	 * This can't happen with normal nft frontend,
+	 * but it can happen with custom clients or with
+	 * nft sets defined via 'type integer,8' and then
+	 * asking "vlan id @myset".
+	 */
+	if (value->len < binop->right->len)
+		return;
+
 	switch (left->etype) {
 	case EXPR_PAYLOAD:
 	case EXPR_EXTHDR:
-- 
2.21.0

[PATCH nftables 8/8] tests: add typeof test cases

[Florian Westphal]

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../testcases/maps/dumps/typeof_maps_0.nft    | 16 ++++++++
 tests/shell/testcases/maps/typeof_maps_0      | 26 ++++++++++++
 .../testcases/sets/dumps/typeof_sets_0.nft    | 31 ++++++++++++++
 tests/shell/testcases/sets/typeof_sets_0      | 40 +++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 tests/shell/testcases/maps/dumps/typeof_maps_0.nft
 create mode 100755 tests/shell/testcases/maps/typeof_maps_0
 create mode 100644 tests/shell/testcases/sets/dumps/typeof_sets_0.nft
 create mode 100755 tests/shell/testcases/sets/typeof_sets_0

diff --git a/tests/shell/testcases/maps/dumps/typeof_maps_0.nft b/tests/shell/testcases/maps/dumps/typeof_maps_0.nft
new file mode 100644
index 000000000000..d691f406047f
--- /dev/null
+++ b/tests/shell/testcases/maps/dumps/typeof_maps_0.nft
@@ -0,0 +1,16 @@
+table inet t {
+	map m1 {
+		type typeof(osf name) : mark
+		elements = { "Linux" : 0x00000001 }
+	}
+
+	map m2 {
+		type string,128 : mark
+		elements = { "Linux" : 0x00000001 }
+	}
+
+	chain c {
+		ct mark set osf name map @m1
+		ct mark set osf name map @m2
+	}
+}
diff --git a/tests/shell/testcases/maps/typeof_maps_0 b/tests/shell/testcases/maps/typeof_maps_0
new file mode 100755
index 000000000000..59112c11e224
--- /dev/null
+++ b/tests/shell/testcases/maps/typeof_maps_0
@@ -0,0 +1,26 @@
+#!/bin/bash
+
+# support for strings/typeof in named maps.
+# m1 and m2 are identical, they just use different
+# ways for declaration.
+
+EXPECTED="table inet t {
+	map m1 {
+		type typeof(osf name) : typeof(ct mark)
+		elements = { \"Linux\" : 0x1 }
+	}
+
+	map m2 {
+		type string, 128 : mark
+		elements = { \"Linux\" : 0x1 }
+	}
+
+	chain c {
+		ct mark set osf name map @m1
+		ct mark set osf name map @m2
+	}
+}"
+
+set -e
+$NFT -f - <<< $EXPECTED
+
diff --git a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
new file mode 100644
index 000000000000..7f588fc89ab7
--- /dev/null
+++ b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
@@ -0,0 +1,31 @@
+table inet t {
+	set s1 {
+		type typeof(osf name)
+		elements = { "Linux" }
+	}
+
+	set s2 {
+		type string,128
+		elements = { "Linux" }
+	}
+
+	set s3 {
+		type typeof(vlan id)
+		elements = { 2, 3, 103 }
+	}
+
+	set s4 {
+		type integer,16
+		elements = { 2, 3, 103, 2003 }
+	}
+
+	chain c1 {
+		osf name @s1 accept
+		osf name @s2 accept
+	}
+
+	chain c2 {
+		vlan id @s3 accept
+		vlan id @s4 accept
+	}
+}
diff --git a/tests/shell/testcases/sets/typeof_sets_0 b/tests/shell/testcases/sets/typeof_sets_0
new file mode 100755
index 000000000000..6ffa10107727
--- /dev/null
+++ b/tests/shell/testcases/sets/typeof_sets_0
@@ -0,0 +1,40 @@
+#!/bin/bash
+
+# support for strings/typeof in named sets.
+# s1 and s2 are identical, they just use different
+# ways for declaration.
+
+EXPECTED="table inet t {
+	set s1 {
+		type typeof(osf name)
+		elements = { \"Linux\" }
+	}
+
+	set s2 {
+		type string, 128
+		elements = { \"Linux\" }
+	}
+
+	set s3 {
+		type typeof(vlan id)
+		elements = { 2, 3, 103 }
+	}
+
+	set s4 {
+		type integer,16
+		elements = { 2, 3, 103, 2003 }
+	}
+	chain c1 {
+		osf name @s1 accept
+		osf name @s2 accept
+	}
+
+	chain c2 {
+		ether type vlan vlan id @s3 accept
+		ether type vlan vlan id @s4 accept
+	}
+}"
+
+set -e
+$NFT -f - <<< $EXPECTED
+
-- 
2.21.0

Re: [PATCH nftables 0/8] add typeof keyword

[Pablo Neira Ayuso]

Hi Florian,

On Fri, Aug 16, 2019 at 04:42:33PM +0200, Florian Westphal wrote:
> This patch series adds the typeof keyword.
> 
> The only dependency is a small change to libnftnl to add two new
> UDATA_SET_TYPEOF enum values.

Thanks for working on this.

> named set can be configured as follows:
> 
> set os {
>    type typeof(osf name)
>    elements = { "Linux", "Windows" }
> }
>
> or
> nft add set ip filter allowed "{ type typeof(ip daddr) . typeof(tcp dport); }"

I know I sent a RFC using typeof(), I wonder if you could just use the
selector instead, it's a bit of a lot of type typeof() . typeof()
probably.

So this is left as this:

        type osf name

in concatenations, like this:

        nft add set ip filter allowed "{ type ip daddr . tcp dport; }"

Probably I would ask my sysadmin friends what they think. I spent too
much time on coding, so all these typeof() look natural to me, but it
might be a bit too much syntactic sugar for someone that is more in
network operations, not sure.

P.S: patch 1/8 and 2/8 are related to this patchset? After quick
glance, not obvious to me or if they are again related to multiple
nft_ctx_new() calls.

Re: [PATCH nftables 0/8] add typeof keyword

[Pablo Neira Ayuso]

On Sat, Aug 17, 2019 at 12:23:51PM +0200, Pablo Neira Ayuso wrote:
[...]
> On Fri, Aug 16, 2019 at 04:42:33PM +0200, Florian Westphal wrote:
[..]
> P.S: patch 1/8 and 2/8 are related to this patchset? After quick
> glance, not obvious to me or if they are again related to multiple
> nft_ctx_new() calls.

I found it, it is in patch 6/8, it making a call to set_make_key to
get the datatype to translate the selector to datatype using the
parser API.

Re: [PATCH nftables 0/8] add typeof keyword

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sat, Aug 17, 2019 at 12:23:51PM +0200, Pablo Neira Ayuso wrote:
> [...]
> > On Fri, Aug 16, 2019 at 04:42:33PM +0200, Florian Westphal wrote:
> [..]
> > P.S: patch 1/8 and 2/8 are related to this patchset? After quick
> > glance, not obvious to me or if they are again related to multiple
> > nft_ctx_new() calls.
> 
> I found it, it is in patch 6/8, it making a call to set_make_key to
> get the datatype to translate the selector to datatype using the
> parser API.

Yes, I could push patch #1 independently though.

Re: [PATCH nftables 0/8] add typeof keyword

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I know I sent a RFC using typeof(), I wonder if you could just use the
> selector instead, it's a bit of a lot of type typeof() . typeof()
> probably.
> 
> So this is left as this:
> 
>         type osf name
> 
> in concatenations, like this:
> 
>         nft add set ip filter allowed "{ type ip daddr . tcp dport; }"
> 
> Probably I would ask my sysadmin friends what they think.

Yes, please do, it would be good to get a non-developer perspective.

I'm very used to things like sizeof(), so typeof() felt natural to me.

Might be very un-intuitive for non-developers though, so it would be
good to get outside perspective.

Thanks!

Re: [PATCH nftables 0/8] add typeof keyword

[Arturo Borrero Gonzalez]

On 8/17/19 10:55 PM, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> I know I sent a RFC using typeof(), I wonder if you could just use the
>> selector instead, it's a bit of a lot of type typeof() . typeof()
>> probably.
>>
>> So this is left as this:
>>
>>         type osf name
>>
>> in concatenations, like this:
>>
>>         nft add set ip filter allowed "{ type ip daddr . tcp dport; }"
>>
>> Probably I would ask my sysadmin friends what they think.
> 
> Yes, please do, it would be good to get a non-developer perspective.
> 
> I'm very used to things like sizeof(), so typeof() felt natural to me.
> 
> Might be very un-intuitive for non-developers though, so it would be
> good to get outside perspective.
> 

From my point of view, this is a rather advanced operation. As long as it is
properly documented, I don't see any problem with `typeof()`.

Also, just `typeof` would work of course. Up to you.

Thanks for working on this!

Re: [PATCH nftables 0/8] add typeof keyword

[Pablo Neira Ayuso]

Hi Florian,

On Sun, Aug 18, 2019 at 04:33:15PM +0200, Arturo Borrero Gonzalez wrote:
> On 8/17/19 10:55 PM, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >> I know I sent a RFC using typeof(), I wonder if you could just use the
> >> selector instead, it's a bit of a lot of type typeof() . typeof()
> >> probably.
> >>
> >> So this is left as this:
> >>
> >>         type osf name
> >>
> >> in concatenations, like this:
> >>
> >>         nft add set ip filter allowed "{ type ip daddr . tcp dport; }"
> >>
> >> Probably I would ask my sysadmin friends what they think.
> > 
> > Yes, please do, it would be good to get a non-developer perspective.
> > 
> > I'm very used to things like sizeof(), so typeof() felt natural to me.
> > 
> > Might be very un-intuitive for non-developers though, so it would be
> > good to get outside perspective.
> 
> From my point of view, this is a rather advanced operation. As long as it is
> properly documented, I don't see any problem with `typeof()`.
> 
> Also, just `typeof` would work of course. Up to you.

My suggestion is:

        typeof ip saddr . tcp dport . meta mark

but, not to allow the use of the existing datatypes with typeof.

So either the user picks:

        type ipv4_addr . inet_service . mark

or this:

        typeof ip saddr . tcp dport . meta mark

Then, for the notation:

        integer,bits

I would suggest this one can only be used from 'type'.

Goal is basically:

* Avoiding repetitive typeof() that looks a bit like C programming.
* Mixing datatype with inferred datatypes looks a bit sloppy to me
  from syntax consistency point of view.

This approach is flexible enough to cover all use cases this patchset
is dealing with I think.

If you like this approach, I would suggest you just update the
patchset to support this and then push this out, I'll catch up later
on to revisit this before the next release, in case I can propose you
some refinement for your review.

Let me know, thanks.