[nftables tool PATCH 0/5] Easier base chain declaration

[nftables tool PATCH 0/5] Easier base chain declaration

[Tomasz Bursztyka]

Hi,

So as proposed, patch 2 and 3 make hook more readable and add priority keyword.
patch 1 is a quick fix on base chain output (we should check the chain's flags, not it's hookunum).
and patch 4 is just to enforce the base chain type checking at input
patch 5 is uptading the tests set according to these syntax changes.

Tomasz Bursztyka (5):
  src: Fix base chain print out
  src: Wrap netfilter hooks around human readable strings
  syntax: Add priority keyword on base chain description
  src: Ensure given base chain type is a valid one
  tests: Update bate chain creation according to latest syntax changes

 include/rule.h      |  23 +++++++++++
 src/netlink.c       | 109 +++++++++++++++++++++++++++++++++++++++++++++++++---
 src/parser.y        |  37 ++++++++++++++----
 src/rule.c          |  51 +++++++++++++++++++-----
 src/scanner.l       |   6 ---
 tests/dictionary    |   2 +-
 tests/expr-ct       |   2 +-
 tests/expr-meta     |   2 +-
 tests/family-bridge |   2 +-
 tests/family-ipv4   |   2 +-
 tests/family-ipv6   |   2 +-
 tests/obj-chain     |   2 +-
 tests/payload-ll    |   2 +-
 tests/set           |   2 +-
 tests/stmt-log      |   2 +-
 tests/verdict-maps  |   2 +-
 16 files changed, 209 insertions(+), 39 deletions(-)

-- 
1.8.3.2

[nftables tool PATCH 1/5] src: Fix base chain print out

[Tomasz Bursztyka]

Relying on chain's hooknum to know whether the chain is a base one or
not is bogus: having 0 as hooknum is a valid number. Thus setting the
right flag and handling it is the way to go, as parser does already.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 src/netlink.c | 4 +++-
 src/rule.c    | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 962561f..7f99416 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -535,7 +535,9 @@ static int list_chain_cb(struct nft_chain *nlc, void *arg)
 		chain->priority      =
 			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_PRIO);
 		chain->type          =
-			xstrdup(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TYPE));
+			xstrdup(nft_chain_attr_get_str(nlc,
+				NFT_CHAIN_ATTR_TYPE));
+		chain->flags        |= CHAIN_F_BASECHAIN;
 	}
 	list_add_tail(&chain->list, &ctx->list);
 
diff --git a/src/rule.c b/src/rule.c
index fb0387c..73054ba 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -249,7 +249,7 @@ static void chain_print(const struct chain *chain)
 	struct rule *rule;
 
 	printf("\tchain %s {\n", chain->handle.chain);
-	if (chain->hooknum) {
+	if (chain->flags & CHAIN_F_BASECHAIN) {
 		printf("\t\t type %s hook %s %u;\n", chain->type,
 		       hooknum2str(chain->hooknum), chain->priority);
 	}
-- 
1.8.3.2

[nftables tool PATCH 2/5] src: Wrap netfilter hooks around human readable strings

[Tomasz Bursztyka]

This allows to use unique, human readable, hook names for the command
line and let the user being unaware of the complex netfilter's hook
names and there difference depending on the netfilter family.

So:
add chain foo bar { type route hook NF_INET_LOCAL_IN 0; }

becomes:
add chain foo bar { type route hook input 0; }

It also fixes then the difference in hook values between families.
I.e.: ARP family has different values for input, forward and output
compared to IPv4, IPv6 or BRIDGE.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/rule.h |  22 ++++++++++++
 src/netlink.c  | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/parser.y   |  21 +++++++++---
 src/rule.c     |  28 ++++++++++-----
 src/scanner.l  |   6 ----
 5 files changed, 159 insertions(+), 23 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 4f68431..97bace5 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -82,6 +82,28 @@ extern void table_free(struct table *table);
 extern void table_add_hash(struct table *table);
 extern struct table *table_lookup(const struct handle *h);
 
+/*
+ * enum hook_numbers - family agnostic hook identifiers
+ *
+ * @HOOK_PREROUTING:	prerouting hook (NF_INET_LOCAL_PRE_ROUTING in ipv4)
+ * @HOOK_INPUT:		input hook (NF_INET_LOCAL_IN in ipv4)
+ * @HOOK_FORWARD:	forward hook (NF_INET_LOCAL_FORWARD in ipv4)
+ * @HOOK_OUTPUT:	output hook (NF_INET_LOCAL_OUT in ipv4)
+ * @HOOK_POSTROUTING:	postrouting hook (NF_INET_LOCAL_POST_ROUTING in ipv4)
+ * @HOOK_NUMHOOKS:	maximum number of hooks
+ */
+enum hook_number {
+	HOOK_PREROUTING		= 0,
+	HOOK_INPUT		= 1,
+	HOOK_FORWARD		= 2,
+	HOOK_POSTROUTING	= 3,
+	HOOK_OUTPUT		= 4,
+	HOOK_NUMHOOKS		= 5,
+};
+
+extern unsigned int str2hooknum(const char *hook_name);
+extern const char *hooknum2str(unsigned int hooknum);
+
 /**
  * enum chain_flags - chain flags
  *
diff --git a/src/netlink.c b/src/netlink.c
index 7f99416..1eb6e52 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -20,6 +20,10 @@
 #include <libnftables/set.h>
 #include <linux/netfilter/nf_tables.h>
 
+#include <netinet/in.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_arp.h>
+
 #include <nftables.h>
 #include <netlink.h>
 #include <mnl.h>
@@ -449,16 +453,68 @@ void netlink_dump_chain(struct nft_chain *nlc)
 #endif
 }
 
+static uint32_t hooknum2nfhook(uint32_t family, enum hook_number hook_num)
+{
+	switch (family)
+	{
+	case NFPROTO_IPV4:
+	case NFPROTO_BRIDGE:
+	case NFPROTO_IPV6:
+		/* All these 3 families share actually
+		 * the same values for each hook */
+		switch (hook_num) {
+		case HOOK_PREROUTING:
+			return NF_INET_PRE_ROUTING;
+		case HOOK_INPUT:
+			return NF_INET_LOCAL_IN;
+		case HOOK_FORWARD:
+			return NF_INET_FORWARD;
+		case HOOK_OUTPUT:
+			return NF_INET_LOCAL_OUT;
+		case HOOK_POSTROUTING:
+			return NF_INET_POST_ROUTING;
+		default:
+			break;
+		}
+		break;
+	case NFPROTO_ARP:
+		switch (hook_num) {
+		case HOOK_INPUT:
+			return NF_ARP_IN;
+		case HOOK_FORWARD:
+			return NF_ARP_FORWARD;
+		case HOOK_OUTPUT:
+			return NF_ARP_OUT;
+		default:
+			break;
+		}
+	default:
+		break;
+	}
+
+	return NF_INET_NUMHOOKS;
+}
+
 int netlink_add_chain(struct netlink_ctx *ctx, const struct handle *h,
 		      const struct location *loc, const struct chain *chain)
 {
 	struct nft_chain *nlc;
-	int err;
+	int err = -1;
 
 	nlc = alloc_nft_chain(h);
 	if (chain != NULL && chain->flags & CHAIN_F_BASECHAIN) {
+		uint32_t hooknum = hooknum2nfhook(h->family, chain->hooknum);
+
+		if (hooknum == NF_INET_NUMHOOKS) {
+			netlink_io_error(ctx, loc, "No such hook \"%s\""
+					 " for current family",
+					 hooknum2str(chain->hooknum));
+			errno = EINVAL;
+			goto error;
+		}
+
 		nft_chain_attr_set_u32(nlc, NFT_CHAIN_ATTR_HOOKNUM,
-				       chain->hooknum);
+				       hooknum);
 		nft_chain_attr_set_u32(nlc, NFT_CHAIN_ATTR_PRIO,
 				       chain->priority);
 		nft_chain_attr_set_str(nlc, NFT_CHAIN_ATTR_TYPE,
@@ -466,6 +522,7 @@ int netlink_add_chain(struct netlink_ctx *ctx, const struct handle *h,
 	}
 	netlink_dump_chain(nlc);
 	err = mnl_nft_chain_add(nf_sock, nlc, NLM_F_EXCL);
+error:
 	nft_chain_free(nlc);
 
 	if (err < 0)
@@ -509,6 +566,46 @@ int netlink_delete_chain(struct netlink_ctx *ctx, const struct handle *h,
 	return err;
 }
 
+static uint32_t nfhook2hooknum(uint32_t family, uint32_t nf_hook)
+{
+	switch (family) {
+	case NFPROTO_IPV4:
+	case NFPROTO_BRIDGE:
+	case NFPROTO_IPV6:
+		switch (nf_hook) {
+		case NF_INET_PRE_ROUTING:
+			return HOOK_PREROUTING;
+		case NF_INET_LOCAL_IN:
+			return HOOK_INPUT;
+		case NF_INET_FORWARD:
+			return HOOK_FORWARD;
+		case NF_INET_LOCAL_OUT:
+			return HOOK_OUTPUT;
+		case NF_INET_POST_ROUTING:
+			return HOOK_POSTROUTING;
+		default:
+			break;
+		}
+		break;
+	case NFPROTO_ARP:
+		switch (nf_hook) {
+		case NF_ARP_IN:
+			return HOOK_INPUT;
+		case NF_ARP_FORWARD:
+			return HOOK_FORWARD;
+		case NF_ARP_OUT:
+			return HOOK_OUTPUT;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return -1;
+}
+
 static int list_chain_cb(struct nft_chain *nlc, void *arg)
 {
 	struct netlink_ctx *ctx = arg;
@@ -530,8 +627,8 @@ static int list_chain_cb(struct nft_chain *nlc, void *arg)
 	if (nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_HOOKNUM) &&
 	    nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_PRIO) &&
 	    nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_TYPE)) {
-		chain->hooknum       =
-			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_HOOKNUM);
+		chain->hooknum       = nfhook2hooknum(h->family,
+			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_HOOKNUM));
 		chain->priority      =
 			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_PRIO);
 		chain->type          =
diff --git a/src/parser.y b/src/parser.y
index f0eb8e3..4df7d44 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -155,7 +155,6 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token DEFINE			"define"
 
 %token HOOK			"hook"
-%token <val> HOOKNUM		"hooknum"
 %token TABLE			"table"
 %token TABLES			"tables"
 %token CHAIN			"chain"
@@ -766,19 +765,31 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 			}
 			;
 
-hook_spec		:	TYPE		STRING		HOOK		HOOKNUM		NUM
+hook_spec		:	TYPE		STRING		HOOK		STRING		NUM
 			{
 				$<chain>0->type		= $2;
-				$<chain>0->hooknum	= $4;
+				$<chain>0->hooknum	= str2hooknum($4);
 				$<chain>0->priority	= $5;
 				$<chain>0->flags	|= CHAIN_F_BASECHAIN;
+
+				if ($<chain>0->hooknum == HOOK_NUMHOOKS) {
+					erec_queue(error(&@4, "unknown hook %s", $4),
+						   state->msgs);
+					YYERROR;	
+				}
 			}
-			|	TYPE		STRING		HOOK		HOOKNUM		DASH	NUM
+			|	TYPE		STRING		HOOK		STRING		DASH	NUM
 			{
 				$<chain>0->type		= $2;
-				$<chain>0->hooknum	= $4;
+				$<chain>0->hooknum	= str2hooknum($4);
 				$<chain>0->priority	= -$6;
 				$<chain>0->flags	|= CHAIN_F_BASECHAIN;
+
+				if ($<chain>0->hooknum == HOOK_NUMHOOKS) {
+					erec_queue(error(&@4, "unknown hook %s", $4),
+						   state->msgs);
+					YYERROR;	
+				}
 			}
 			;
 
diff --git a/src/rule.c b/src/rule.c
index 73054ba..23b64a7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -228,17 +228,29 @@ struct chain *chain_lookup(const struct table *table, const struct handle *h)
 	return NULL;
 }
 
-static const char *hooknum2str_array[NF_INET_NUMHOOKS] = {
-	[NF_INET_PRE_ROUTING]	= "NF_INET_PRE_ROUTING",
-	[NF_INET_LOCAL_IN]	= "NF_INET_LOCAL_IN",
-	[NF_INET_FORWARD]	= "NF_INET_FORWARD",
-	[NF_INET_LOCAL_OUT]	= "NF_INET_LOCAL_OUT",
-	[NF_INET_POST_ROUTING]	= "NF_INET_POST_ROUTING",
+static const char *hooknum2str_array[HOOK_NUMHOOKS] = {
+	[HOOK_PREROUTING]	= "prerouting",
+	[HOOK_INPUT]		= "input",
+	[HOOK_FORWARD]		= "forward",
+	[HOOK_OUTPUT]		= "output",
+	[HOOK_POSTROUTING]	= "postrouting",
 };
 
-static const char *hooknum2str(unsigned int hooknum)
+unsigned int str2hooknum(const char *hook_name)
 {
-	if (hooknum >= NF_INET_NUMHOOKS)
+	int i;
+
+	for (i = 0; i < HOOK_NUMHOOKS; i++) {
+		if (!strcmp(hook_name, hooknum2str_array[i]))
+			return i;
+	}
+
+	return HOOK_NUMHOOKS;
+}
+
+const char *hooknum2str(unsigned int hooknum)
+{
+	if (hooknum >= HOOK_NUMHOOKS)
 		return "UNKNOWN";
 
 	return hooknum2str_array[hooknum];
diff --git a/src/scanner.l b/src/scanner.l
index 59e0aac..cee6aa6 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -212,12 +212,6 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "=>"			{ return ARROW; }
 "vmap"			{ return VMAP; }
 
-"NF_INET_PRE_ROUTING"	{ yylval->val = NF_INET_PRE_ROUTING;	return HOOKNUM; }
-"NF_INET_LOCAL_IN"	{ yylval->val = NF_INET_LOCAL_IN;	return HOOKNUM; }
-"NF_INET_FORWARD"	{ yylval->val = NF_INET_FORWARD;	return HOOKNUM; }
-"NF_INET_LOCAL_OUT"	{ yylval->val = NF_INET_LOCAL_OUT;	return HOOKNUM; }
-"NF_INET_POST_ROUTING"	{ yylval->val = NF_INET_POST_ROUTING;	return HOOKNUM; }
-
 "include"		{ return INCLUDE; }
 "define"		{ return DEFINE; }
 
-- 
1.8.3.2

[nftables tool PATCH 3/5] syntax: Add priority keyword on base chain description

[Tomasz Bursztyka]

Instead of:
add chain foo bar { type route hook input 0; }

it should be now:
add chain foo bar { type route hook input priority 0; }

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 src/parser.y | 8 ++++----
 src/rule.c   | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/parser.y b/src/parser.y
index 4df7d44..9a91490 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -765,11 +765,11 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 			}
 			;
 
-hook_spec		:	TYPE		STRING		HOOK		STRING		NUM
+hook_spec		:	TYPE	STRING	HOOK	STRING	PRIORITY	NUM
 			{
 				$<chain>0->type		= $2;
 				$<chain>0->hooknum	= str2hooknum($4);
-				$<chain>0->priority	= $5;
+				$<chain>0->priority	= $6;
 				$<chain>0->flags	|= CHAIN_F_BASECHAIN;
 
 				if ($<chain>0->hooknum == HOOK_NUMHOOKS) {
@@ -778,11 +778,11 @@ hook_spec		:	TYPE		STRING		HOOK		STRING		NUM
 					YYERROR;	
 				}
 			}
-			|	TYPE		STRING		HOOK		STRING		DASH	NUM
+			|	TYPE	STRING	HOOK	STRING	PRIORITY	DASH	NUM
 			{
 				$<chain>0->type		= $2;
 				$<chain>0->hooknum	= str2hooknum($4);
-				$<chain>0->priority	= -$6;
+				$<chain>0->priority	= -$7;
 				$<chain>0->flags	|= CHAIN_F_BASECHAIN;
 
 				if ($<chain>0->hooknum == HOOK_NUMHOOKS) {
diff --git a/src/rule.c b/src/rule.c
index 23b64a7..28a52b0 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -262,7 +262,7 @@ static void chain_print(const struct chain *chain)
 
 	printf("\tchain %s {\n", chain->handle.chain);
 	if (chain->flags & CHAIN_F_BASECHAIN) {
-		printf("\t\t type %s hook %s %u;\n", chain->type,
+		printf("\t\t type %s hook %s priority %u;\n", chain->type,
 		       hooknum2str(chain->hooknum), chain->priority);
 	}
 	list_for_each_entry(rule, &chain->rules, list) {
-- 
1.8.3.2

[nftables tool PATCH 4/5] src: Ensure given base chain type is a valid one

[Tomasz Bursztyka]

So it verifies already from given command line that type is "filter",
"nat" or "route".

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/rule.h |  1 +
 src/parser.y   | 12 ++++++++++++
 src/rule.c     | 19 +++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/include/rule.h b/include/rule.h
index 97bace5..161cee9 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -142,6 +142,7 @@ extern void chain_free(struct chain *chain);
 extern void chain_add_hash(struct chain *chain, struct table *table);
 extern struct chain *chain_lookup(const struct table *table,
 				  const struct handle *h);
+extern bool chain_type_verify(const char *type);
 
 /**
  * struct rule - nftables rule
diff --git a/src/parser.y b/src/parser.y
index 9a91490..49740a5 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -772,6 +772,12 @@ hook_spec		:	TYPE	STRING	HOOK	STRING	PRIORITY	NUM
 				$<chain>0->priority	= $6;
 				$<chain>0->flags	|= CHAIN_F_BASECHAIN;
 
+				if (!chain_type_verify($<chain>0->type)) {
+					erec_queue(error(&@2, "unknown type %s", $2),
+						   state->msgs);
+					YYERROR;
+				}
+
 				if ($<chain>0->hooknum == HOOK_NUMHOOKS) {
 					erec_queue(error(&@4, "unknown hook %s", $4),
 						   state->msgs);
@@ -785,6 +791,12 @@ hook_spec		:	TYPE	STRING	HOOK	STRING	PRIORITY	NUM
 				$<chain>0->priority	= -$7;
 				$<chain>0->flags	|= CHAIN_F_BASECHAIN;
 
+				if (!chain_type_verify($<chain>0->type)) {
+					erec_queue(error(&@2, "unknown type %s", $2),
+						   state->msgs);
+					YYERROR;
+				}
+
 				if ($<chain>0->hooknum == HOOK_NUMHOOKS) {
 					erec_queue(error(&@4, "unknown hook %s", $4),
 						   state->msgs);
diff --git a/src/rule.c b/src/rule.c
index 28a52b0..6ad2388 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -228,6 +228,25 @@ struct chain *chain_lookup(const struct table *table, const struct handle *h)
 	return NULL;
 }
 
+static const char *chain_type_str_array[] = {
+	"filter",
+	"nat",
+	"route",
+	NULL,
+};
+
+bool chain_type_verify(const char *type)
+{
+	int i;
+
+	for (i = 0; chain_type_str_array[i]; i++) {
+		if (!strcmp(type, chain_type_str_array[i]))
+			return true;
+	}
+
+	return false;
+}
+
 static const char *hooknum2str_array[HOOK_NUMHOOKS] = {
 	[HOOK_PREROUTING]	= "prerouting",
 	[HOOK_INPUT]		= "input",
-- 
1.8.3.2

[nftables tool PATCH 5/5] tests: Update bate chain creation according to latest syntax changes

[Tomasz Bursztyka]

Adding type, plain hook's name and priority keyword.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 tests/dictionary    | 2 +-
 tests/expr-ct       | 2 +-
 tests/expr-meta     | 2 +-
 tests/family-bridge | 2 +-
 tests/family-ipv4   | 2 +-
 tests/family-ipv6   | 2 +-
 tests/obj-chain     | 2 +-
 tests/payload-ll    | 2 +-
 tests/set           | 2 +-
 tests/stmt-log      | 2 +-
 tests/verdict-maps  | 2 +-
 11 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/dictionary b/tests/dictionary
index aad9ebe..4193529 100644
--- a/tests/dictionary
+++ b/tests/dictionary
@@ -1,7 +1,7 @@
 #! nft -f
 #
 add table ip filter
-add chain ip filter output { hook NF_INET_LOCAL_OUT 0 ; }
+add chain ip filter output { type filter hook output priority 0 ; }
 
 add chain ip filter chain1
 add rule ip filter chain1 counter
diff --git a/tests/expr-ct b/tests/expr-ct
index 39f1777..1dfc7ac 100644
--- a/tests/expr-ct
+++ b/tests/expr-ct
@@ -1,7 +1,7 @@
 #! nft -f
 
 add table ip filter
-add chain ip filter output { hook NF_INET_LOCAL_OUT 0 ; }
+add chain ip filter output { type filter hook output priority 0 ; }
 
 # ct: state
 add rule ip filter output ct state new,established counter
diff --git a/tests/expr-meta b/tests/expr-meta
index da16ae4..360caa7 100644
--- a/tests/expr-meta
+++ b/tests/expr-meta
@@ -1,7 +1,7 @@
 #! nft -f
 
 add table ip filter
-add chain ip filter output { hook NF_INET_LOCAL_OUT 0 ; }
+add chain ip filter output { type filter hook output priority 0 ; }
 
 # meta: skb len
 add rule ip filter output meta length 1000 counter
diff --git a/tests/family-bridge b/tests/family-bridge
index 98b7885..c87c832 100644
--- a/tests/family-bridge
+++ b/tests/family-bridge
@@ -1,7 +1,7 @@
 #! nft -f
 
 add table bridge filter
-add chain bridge filter output { hook NF_INET_LOCAL_OUT 0 ; }
+add chain bridge filter output { type filter hook output priority 0 ; }
 
 # LL protocol
 add rule bridge filter output eth type 0x0800 counter
diff --git a/tests/family-ipv4 b/tests/family-ipv4
index e744f02..0700e16 100644
--- a/tests/family-ipv4
+++ b/tests/family-ipv4
@@ -5,7 +5,7 @@ delete chain ip filter output
 delete table filter
 
 add table ip filter
-add chain ip filter output { hook NF_INET_LOCAL_IN 0; }
+add chain ip filter output { type filter hook input priority 0; }
 
 # IP address
 add rule ip filter output ip daddr 192.168.0.1 counter
diff --git a/tests/family-ipv6 b/tests/family-ipv6
index e76b2ce..cfc740c 100644
--- a/tests/family-ipv6
+++ b/tests/family-ipv6
@@ -1,7 +1,7 @@
 #! nft -f
 
 add table ip6 filter
-add chain ip6 filter output { hook NF_INET_LOCAL_OUT 0 ; }
+add chain ip6 filter output { type filter hook output priority 0 ; }
 
 # IP address
 add rule ip6 filter output ip6 daddr 2001:6f8:974::1 counter
diff --git a/tests/obj-chain b/tests/obj-chain
index f5094bb..2bce026 100644
--- a/tests/obj-chain
+++ b/tests/obj-chain
@@ -7,7 +7,7 @@ add chain filter testchain
 delete chain filter testchain
 
 # chains: add and delete base chain
-add chain filter input { hook NF_INET_LOCAL_OUT 0 ; }
+add chain filter input { type filter hook input priority 0 ; }
 delete chain filter input
 
 # chains: can not delete chain while referenced
diff --git a/tests/payload-ll b/tests/payload-ll
index feaf587..7f5660b 100644
--- a/tests/payload-ll
+++ b/tests/payload-ll
@@ -1,7 +1,7 @@
 #! nft -f
 
 add table ip filter
-add chain ip filter input NF_INET_LOCAL_IN 0
+add chain ip filter input { type filter hook input priority 0; }
 
 # mac source
 add rule ip filter input @ll,48,48 00:15:e9:f0:10:f8 counter
diff --git a/tests/set b/tests/set
index e2d8e49..3c040b0 100644
--- a/tests/set
+++ b/tests/set
@@ -1,7 +1,7 @@
 #! nft -f
 
 add table filter
-add chain filter output { hook NF_INET_LOCAL_OUT 0 ; }
+add chain filter output { type filter hook output priority 0 ; }
 
 # set: IP addresses
 add rule filter output ip daddr { \
diff --git a/tests/stmt-log b/tests/stmt-log
index 2153772..2ae7aae 100644
--- a/tests/stmt-log
+++ b/tests/stmt-log
@@ -1,6 +1,6 @@
 #! nft -f
 
 add table ip filter
-add chain ip filter output NF_INET_LOCAL_OUT 0
+add chain ip filter output { type filter hook output priority 0; }
 
 add rule ip filter output log saddr "prefix" group 0 counter
diff --git a/tests/verdict-maps b/tests/verdict-maps
index 25b60d1..72ef98f 100644
--- a/tests/verdict-maps
+++ b/tests/verdict-maps
@@ -2,7 +2,7 @@
 #
 
 add table ip filter
-add chain ip filter input { hook NF_INET_LOCAL_IN 0; }
+add chain ip filter input { type filter hook input priority 0; }
 
 add chain ip filter chain1
 add filter chain1 counter
-- 
1.8.3.2

Re: [nftables tool PATCH 1/5] src: Fix base chain print out

[Pablo Neira Ayuso]

On Wed, Aug 28, 2013 at 11:33:07AM +0300, Tomasz Bursztyka wrote:
> Relying on chain's hooknum to know whether the chain is a base one or
> not is bogus: having 0 as hooknum is a valid number. Thus setting the
> right flag and handling it is the way to go, as parser does already.

Applied, thanks.

Re: [nftables tool PATCH 2/5] src: Wrap netfilter hooks around human readable strings

[Pablo Neira Ayuso]

On Wed, Aug 28, 2013 at 11:33:08AM +0300, Tomasz Bursztyka wrote:
> This allows to use unique, human readable, hook names for the command
> line and let the user being unaware of the complex netfilter's hook
> names and there difference depending on the netfilter family.
> 
> So:
> add chain foo bar { type route hook NF_INET_LOCAL_IN 0; }
> 
> becomes:
> add chain foo bar { type route hook input 0; }
> 
> It also fixes then the difference in hook values between families.
> I.e.: ARP family has different values for input, forward and output
> compared to IPv4, IPv6 or BRIDGE.

I get this error here if I use arp and prerouting:

nft add chain arp test test \{ type filter hook prerouting 0\; \}
<cmdline>:1:1-58: Error: Could not use hook "prerouting" with this
family
add chain arp test test { type filter hook prerouting 0; }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This can be done better by checking this in the evaluation step, in
chain_evaluate (you can reach the family via ctx).

Moreover, you can store the hook as string in the parser. Then, in the
evaluation step you validate that it is correct and convert it to
numeric value. That will require two fields in the chain, one for the
hookstr and one for hooknum.

With this approach, I think we can avoid having the intermediate enum
hook_numbers.

> Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
> ---
>  include/rule.h |  22 ++++++++++++
>  src/netlink.c  | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/parser.y   |  21 +++++++++---
>  src/rule.c     |  28 ++++++++++-----
>  src/scanner.l  |   6 ----
>  5 files changed, 159 insertions(+), 23 deletions(-)
> 
> diff --git a/include/rule.h b/include/rule.h
> index 4f68431..97bace5 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -82,6 +82,28 @@ extern void table_free(struct table *table);
>  extern void table_add_hash(struct table *table);
>  extern struct table *table_lookup(const struct handle *h);
>  
> +/*
> + * enum hook_numbers - family agnostic hook identifiers
> + *
> + * @HOOK_PREROUTING:	prerouting hook (NF_INET_LOCAL_PRE_ROUTING in ipv4)
> + * @HOOK_INPUT:		input hook (NF_INET_LOCAL_IN in ipv4)
> + * @HOOK_FORWARD:	forward hook (NF_INET_LOCAL_FORWARD in ipv4)
> + * @HOOK_OUTPUT:	output hook (NF_INET_LOCAL_OUT in ipv4)
> + * @HOOK_POSTROUTING:	postrouting hook (NF_INET_LOCAL_POST_ROUTING in ipv4)
> + * @HOOK_NUMHOOKS:	maximum number of hooks
> + */
> +enum hook_number {
> +	HOOK_PREROUTING		= 0,
> +	HOOK_INPUT		= 1,
> +	HOOK_FORWARD		= 2,
> +	HOOK_POSTROUTING	= 3,
> +	HOOK_OUTPUT		= 4,
> +	HOOK_NUMHOOKS		= 5,
> +};

Re: [nftables tool PATCH 2/5] src: Wrap netfilter hooks around human readable strings

[Tomasz Bursztyka]

Hi Pablo,

> This can be done better by checking this in the evaluation step, in
> chain_evaluate (you can reach the family via ctx).

Ok, a step before indeed.
At first I was planning to do that at parsing stage but couldn't reach 
the chain's family.

> Moreover, you can store the hook as string in the parser. Then, in the
> evaluation step you validate that it is correct and convert it to
> numeric value. That will require two fields in the chain, one for the
> hookstr and one for hooknum.

Makes sense, though the struct chain will grow a bit then.
I will still validate the string at parsing stage to keep the error 
reporting on that particular place.

Tomasz