[PATCH nft 0/3] support ct/meta key lookups at runtime

[PATCH nft 0/3] support ct/meta key lookups at runtime

[Florian Westphal]

I have a patch series to add support of hash (skb hash) and sym hash
(symmetric skb hash) to nft_meta.

This series would allow extending meta without adding new hash/symhash
keywords in the scanner.

What do you think?

[PATCH nft 1/3] utils: provide snprintf helper macro

[Florian Westphal]

lifted from libnftnl, except that we will abort on snprintf errors.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/utils.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/utils.h b/include/utils.h
index d88676476efb..bb58ba424165 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -83,6 +83,16 @@
 	(void) (&_max1 == &_max2);		\
 	_max1 > _max2 ? _max1 : _max2; })
 
+#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset)	\
+	if (ret < 0)					\
+		abort();				\
+	offset += ret;					\
+	assert(ret < len);				\
+	if (ret > len)					\
+		ret = len;				\
+	size += ret;					\
+	len -= ret;
+
 #define MSEC_PER_SEC	1000L
 
 /**
-- 
2.7.3

[PATCH nft 2/3] ct: allow resolving ct keys at run time

[Florian Westphal]

... and remove those keywords we no longer need.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/ct.h       |  2 ++
 src/ct.c           | 35 +++++++++++++++++++++++++++++++++++
 src/parser_bison.y | 36 +++++++++++++++++++++++++++---------
 src/scanner.l      |  6 ------
 tests/py/any/ct.t  |  5 +++++
 5 files changed, 69 insertions(+), 15 deletions(-)

diff --git a/include/ct.h b/include/ct.h
index 945fcc4d829d..0aeeed60bfaa 100644
--- a/include/ct.h
+++ b/include/ct.h
@@ -29,4 +29,6 @@ extern void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr);
 
 extern struct error_record *ct_dir_parse(const struct location *loc,
 					 const char *str, int8_t *dir);
+extern struct error_record *ct_key_parse(const struct location *loc, const char *str,
+					 unsigned int *key);
 #endif /* NFTABLES_CT_H */
diff --git a/src/ct.c b/src/ct.c
index a68293896ed6..d50c92616afa 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -306,6 +306,41 @@ struct error_record *ct_dir_parse(const struct location *loc, const char *str,
 	return error(loc, "Could not parse direction %s", str);
 }
 
+struct error_record *ct_key_parse(const struct location *loc, const char *str,
+				  unsigned int *key)
+{
+	int ret, len, offset = 0;
+	const char *sep = "";
+	unsigned int i;
+	char buf[1024];
+	size_t size;
+
+	for (i = 0; i < array_size(ct_templates); i++) {
+		if (!ct_templates[i].token || strcmp(ct_templates[i].token, str))
+			continue;
+
+		*key = i;
+		return NULL;
+	}
+
+	len = (int)sizeof(buf);
+	size = sizeof(buf);
+
+	for (i = 0; i < array_size(ct_templates); i++) {
+		if (!ct_templates[i].token)
+			continue;
+
+		if (offset)
+			sep = ", ";
+
+		ret = snprintf(buf+offset, len, "%s%s", sep, ct_templates[i].token);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+		assert(offset < (int)sizeof(buf));
+	}
+
+	return error(loc, "Could not parse %s, known ct keys are: %s", str, buf);
+}
+
 struct expr *ct_expr_alloc(const struct location *loc, enum nft_ct_keys key,
 			   int8_t direction)
 {
diff --git a/src/parser_bison.y b/src/parser_bison.y
index baf0a539efa0..03cf590272e8 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2491,6 +2491,19 @@ ct_expr			: 	CT	ct_key
 			{
 				$$ = ct_expr_alloc(&@$, $2, -1);
 			}
+			| 	CT	STRING
+			{
+				struct error_record *erec;
+				unsigned int key;
+
+				erec = ct_key_parse(&@$, $2, &key);
+				if (erec != NULL) {
+					erec_queue(erec, state->msgs);
+					YYERROR;
+				}
+
+				$$ = ct_expr_alloc(&@$, key, -1);
+			}
 			|	CT	STRING	ct_key_dir
 			{
 				struct error_record *erec;
@@ -2506,15 +2519,7 @@ ct_expr			: 	CT	ct_key
 			}
 			;
 
-ct_key			:	STATE		{ $$ = NFT_CT_STATE; }
-			|	DIRECTION	{ $$ = NFT_CT_DIRECTION; }
-			|	STATUS		{ $$ = NFT_CT_STATUS; }
-			|	MARK		{ $$ = NFT_CT_MARK; }
-			|	EXPIRATION	{ $$ = NFT_CT_EXPIRATION; }
-			|	HELPER		{ $$ = NFT_CT_HELPER; }
-			|	LABEL		{ $$ = NFT_CT_LABELS; }
-			|	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
-			|	PROTOCOL	{ $$ = NFT_CT_PROTOCOL; }
+ct_key			:	MARK		{ $$ = NFT_CT_MARK; }
 			|	ct_key_counters
 			;
 ct_key_dir		:	SADDR		{ $$ = NFT_CT_SRC; }
@@ -2534,6 +2539,19 @@ ct_stmt			:	CT	ct_key		SET	expr
 			{
 				$$ = ct_stmt_alloc(&@$, $2, $4);
 			}
+			|	CT	STRING		SET	expr
+			{
+				struct error_record *erec;
+				unsigned int key;
+
+				erec = ct_key_parse(&@$, $2, &key);
+				if (erec != NULL) {
+					erec_queue(erec, state->msgs);
+					YYERROR;
+				}
+
+				$$ = ct_stmt_alloc(&@$, key, $4);
+			}
 			;
 
 payload_stmt		:	payload_expr		SET	expr
diff --git a/src/scanner.l b/src/scanner.l
index 8b5a383bd095..f11f06506f6c 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -457,15 +457,9 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "cgroup"		{ return CGROUP; }
 
 "ct"			{ return CT; }
-"direction"		{ return DIRECTION; }
-"state"			{ return STATE; }
-"status"		{ return STATUS; }
-"expiration"		{ return EXPIRATION; }
-"helper"		{ return HELPER; }
 "l3proto"		{ return L3PROTOCOL; }
 "proto-src"		{ return PROTO_SRC; }
 "proto-dst"		{ return PROTO_DST; }
-"label"			{ return LABEL; }
 
 "numgen"		{ return NUMGEN; }
 "inc"			{ return INC; }
diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t
index 7fd4f2cbdc9a..cc4f8e19c619 100644
--- a/tests/py/any/ct.t
+++ b/tests/py/any/ct.t
@@ -96,3 +96,8 @@ ct mark original;fail
 ct label 127;ok
 ct label set 127;ok
 ct label 128;fail
+
+ct invalid;fail
+ct invalid original;fail
+ct set invalid original 42;fail
+ct set invalid 42;fail
-- 
2.7.3

[PATCH nft 3/3] meta: allow resolving meta keys at run time

[Florian Westphal]

use the meta template to translate the textual token to the enum value.
This allows to remove two keywords from the scanner and also means we do
not need to introduce new keywords when more meta keys get added.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/meta.h     |  4 ++++
 src/meta.c         | 36 ++++++++++++++++++++++++++++++++++++
 src/parser_bison.y | 31 ++++++++++++++++++++++++++-----
 src/scanner.l      |  2 --
 4 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/include/meta.h b/include/meta.h
index f25b147a624f..c63787a7d1f0 100644
--- a/include/meta.h
+++ b/include/meta.h
@@ -30,4 +30,8 @@ struct stmt *meta_stmt_meta_iiftype(const struct location *loc, uint16_t type);
 
 const struct datatype ifindex_type;
 
+struct error_record *meta_type_get(const struct location *loc,
+				   const char *name,
+				   unsigned int *value);
+
 #endif /* NFTABLES_META_H */
diff --git a/src/meta.c b/src/meta.c
index 34f9ee8e36e2..972a84cf6978 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -638,3 +638,39 @@ struct stmt *meta_stmt_meta_iiftype(const struct location *loc, uint16_t type)
 	dep = relational_expr_alloc(loc, OP_EQ, left, right);
 	return expr_stmt_alloc(&dep->location, dep);
 }
+
+struct error_record *meta_type_get(const struct location *loc,
+                                   const char *str,
+                                   unsigned int *value)
+{
+	int ret, len, offset = 0;
+	const char *sep = "";
+	unsigned int i;
+	char buf[1024];
+	size_t size;
+
+	for (i = 0; i < array_size(meta_templates); i++) {
+		if (!meta_templates[i].token || strcmp(meta_templates[i].token, str))
+			continue;
+
+		*value = i;
+		return NULL;
+	}
+
+	len = (int)sizeof(buf);
+	size = sizeof(buf);
+
+	for (i = 0; i < array_size(meta_templates); i++) {
+		if (!meta_templates[i].token)
+			continue;
+
+		if (offset)
+			sep = ", ";
+
+		ret = snprintf(buf+offset, len, "%s%s", sep, meta_templates[i].token);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+		assert(offset < (int)sizeof(buf));
+	}
+
+	return error(loc, "Could not parse %s, known meta keys are: %s", str, buf);
+}
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 03cf590272e8..23982c132f8b 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -319,8 +319,6 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token MH			"mh"
 
 %token META			"meta"
-%token NFPROTO			"nfproto"
-%token L4PROTO			"l4proto"
 %token MARK			"mark"
 %token IIF			"iif"
 %token IIFNAME			"iifname"
@@ -2426,15 +2424,25 @@ meta_expr		:	META	meta_key
 			{
 				$$ = meta_expr_alloc(&@$, $1);
 			}
-			;
+			|	META	STRING
+			{
+				struct error_record *erec;
+				unsigned int key;
+
+				erec = meta_type_get(&@$, $2, &key);
+				if (erec != NULL) {
+					erec_queue(erec, state->msgs);
+					YYERROR;
+				}
+
+				$$ = meta_expr_alloc(&@$, key);
+			}
 
 meta_key		:	meta_key_qualified
 			|	meta_key_unqualified
 			;
 
 meta_key_qualified	:	LENGTH		{ $$ = NFT_META_LEN; }
-			|	NFPROTO		{ $$ = NFT_META_NFPROTO; }
-			|	L4PROTO		{ $$ = NFT_META_L4PROTO; }
 			|	PROTOCOL	{ $$ = NFT_META_PROTOCOL; }
 			|	PRIORITY	{ $$ = NFT_META_PRIORITY; }
 			|	RANDOM		{ $$ = NFT_META_PRANDOM; }
@@ -2468,6 +2476,19 @@ meta_stmt		:	META	meta_key	SET	expr
 			{
 				$$ = meta_stmt_alloc(&@$, $1, $3);
 			}
+			|	META	STRING	SET	expr
+			{
+				struct error_record *erec;
+				unsigned int key;
+
+				erec = meta_type_get(&@$, $2, &key);
+				if (erec != NULL) {
+					erec_queue(erec, state->msgs);
+					YYERROR;
+				}
+
+				$$ = meta_stmt_alloc(&@$, key, $4);
+			}
 			;
 
 numgen_type		:	INC		{ $$ = NFT_NG_INCREMENTAL; }
diff --git a/src/scanner.l b/src/scanner.l
index f11f06506f6c..4dd7d0c9820a 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -435,8 +435,6 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "mh"			{ return MH; }
 
 "meta"			{ return META; }
-"nfproto"		{ return NFPROTO; }
-"l4proto"		{ return L4PROTO; }
 "mark"			{ return MARK; }
 "iif"			{ return IIF; }
 "iifname"		{ return IIFNAME; }
-- 
2.7.3

Re: [PATCH nft 0/3] support ct/meta key lookups at runtime

[Pablo Neira Ayuso]

On Thu, Oct 27, 2016 at 12:36:05AM +0200, Florian Westphal wrote:
> I have a patch series to add support of hash (skb hash) and sym hash
> (symmetric skb hash) to nft_meta.
> 
> This series would allow extending meta without adding new hash/symhash
> keywords in the scanner.
> 
> What do you think?

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Only one suggestion, instead of:

 return error(loc, "Could not parse %s, known ct keys are: %s", str, buf);

Looking at current error reporting:

 # nft add rule x y ct
 <cmdline>:1:18-18: Error: syntax error, unexpected newline
 add rule x y ct
                ^
Probably something like looks better:

 "syntax error, unexpected %s, known keys are %s"

Thanks.

Re: [PATCH nft 0/3] support ct/meta key lookups at runtime

[Pablo Neira Ayuso]

On Thu, Oct 27, 2016 at 06:48:10PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Oct 27, 2016 at 12:36:05AM +0200, Florian Westphal wrote:
> > I have a patch series to add support of hash (skb hash) and sym hash
> > (symmetric skb hash) to nft_meta.
> > 
> > This series would allow extending meta without adding new hash/symhash
> > keywords in the scanner.
> > 
> > What do you think?
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Only one suggestion, instead of:
> 
>  return error(loc, "Could not parse %s, known ct keys are: %s", str, buf);
> 
> Looking at current error reporting:
> 
>  # nft add rule x y ct
>  <cmdline>:1:18-18: Error: syntax error, unexpected newline
>  add rule x y ct
>                 ^
> Probably something like looks better:
> 
>  "syntax error, unexpected %s, known keys are %s"

Actually, if we follow this approach, we probably have to revisit all
other existing error messages...

Keep this as it is, we can revisit this later.

Re: [PATCH nft 0/3] support ct/meta key lookups at runtime

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Oct 27, 2016 at 06:48:10PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Oct 27, 2016 at 12:36:05AM +0200, Florian Westphal wrote:
> > > I have a patch series to add support of hash (skb hash) and sym hash
> > > (symmetric skb hash) to nft_meta.
> > > 
> > > This series would allow extending meta without adding new hash/symhash
> > > keywords in the scanner.
> > > 
> > > What do you think?
> > 
> > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > 
> > Only one suggestion, instead of:
> > 
> >  return error(loc, "Could not parse %s, known ct keys are: %s", str, buf);
> > 
> > Looking at current error reporting:
> > 
> >  # nft add rule x y ct
> >  <cmdline>:1:18-18: Error: syntax error, unexpected newline
> >  add rule x y ct
> >                 ^
> > Probably something like looks better:
> > 
> >  "syntax error, unexpected %s, known keys are %s"
> 
> Actually, if we follow this approach, we probably have to revisit all
> other existing error messages...
> 
> Keep this as it is, we can revisit this later.

Are you sure?  I like the suggestion.

Re: [PATCH nft 0/3] support ct/meta key lookups at runtime

[Pablo Neira Ayuso]

On Thu, Oct 27, 2016 at 06:58:43PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Oct 27, 2016 at 06:48:10PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Oct 27, 2016 at 12:36:05AM +0200, Florian Westphal wrote:
> > > > I have a patch series to add support of hash (skb hash) and sym hash
> > > > (symmetric skb hash) to nft_meta.
> > > > 
> > > > This series would allow extending meta without adding new hash/symhash
> > > > keywords in the scanner.
> > > > 
> > > > What do you think?
> > > 
> > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > 
> > > Only one suggestion, instead of:
> > > 
> > >  return error(loc, "Could not parse %s, known ct keys are: %s", str, buf);
> > > 
> > > Looking at current error reporting:
> > > 
> > >  # nft add rule x y ct
> > >  <cmdline>:1:18-18: Error: syntax error, unexpected newline
> > >  add rule x y ct
> > >                 ^
> > > Probably something like looks better:
> > > 
> > >  "syntax error, unexpected %s, known keys are %s"
> > 
> > Actually, if we follow this approach, we probably have to revisit all
> > other existing error messages...
> > 
> > Keep this as it is, we can revisit this later.
> 
> Are you sure?  I like the suggestion.

Go ahead with it then, we can just make a follow up patch to revisit
other existing spots. Probably we can add a new function so we don't
need to hardcode this everywhere in the code?

We will get one Outreachy student soon btw, we can hand over this task
to her.