[nft PATCH 3/4] Support 'add/insert rule index <IDX>'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Allow to specify an absolute rule position in add/insert commands like
with iptables. The translation to rule handle takes place in userspace,
so no kernel support for this is needed. Possible undesired effects are
pointed out in man page to make users aware that this way of specifying
a rule location might not be ideal.

Signed-off-by: Phil Sutter <phil@xxxxxx>
---
 doc/nft.xml        | 31 ++++++++++++++++++++++++-------
 include/rule.h     |  1 +
 src/evaluate.c     | 45 +++++++++++++++++++++++++++++++++++++++++++++
 src/parser_bison.y | 18 ++++++++++++++++--
 src/rule.c         |  2 ++
 src/scanner.l      |  1 +
 6 files changed, 89 insertions(+), 9 deletions(-)

diff --git a/doc/nft.xml b/doc/nft.xml
index b80c8c439e2c7..ab94bff447326 100644
--- a/doc/nft.xml
+++ b/doc/nft.xml
@@ -875,13 +875,19 @@ add table inet mytable
 				<arg choice="opt"><replaceable>family</replaceable></arg>
 				<replaceable>table</replaceable>
 				<replaceable>chain</replaceable>
-				<arg choice="opt">
-					<group choice="req">
-						<arg>handle</arg>
-						<arg>position</arg>
-					</group>
-					<replaceable>handle</replaceable>
-				</arg>
+				<group choice="opt">
+					<arg>
+						<group choice="req">
+							<arg>handle</arg>
+							<arg>position</arg>
+						</group>
+						<replaceable>handle</replaceable>
+					</arg>
+					<arg>
+						<literal>index</literal>
+						<replaceable>index</replaceable>
+					</arg>
+				</group>
 				<replaceable>statement</replaceable>...
 			</cmdsynopsis>
 			<cmdsynopsis>
@@ -909,6 +915,17 @@ add table inet mytable
 			Rules are constructed from two kinds of components according to a set
 			of grammatical rules: expressions and statements.
 		</para>
+		<para>
+			The <literal>add</literal> and <literal>insert</literal> commands support an optional
+			location specifier, which is either a <replaceable>handle</replaceable> of an existing
+			rule or an absolute <replaceable>index</replaceable> (starting at zero).  Internally,
+			rule locations are always identified by <replaceable>handle</replaceable> and the
+			translation from <replaceable>index</replaceable> happens in userspace. This has two
+			potential implications in case a concurrent ruleset change happens after the translation
+			was done: The effective rule index might change if a rule was inserted or deleted before
+			the referred one.  If the referred rule was deleted, the command is rejected by the
+			kernel just as if an invalid <replaceable>handle</replaceable> was given.
+		</para>
 
 		<variablelist>
 			<varlistentry>
diff --git a/include/rule.h b/include/rule.h
index b265690d3c964..a3764bda74ce2 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -69,6 +69,7 @@ struct handle {
 	const char		*flowtable;
 	struct handle_spec	handle;
 	struct position_spec	position;
+	struct position_spec	index;
 	uint32_t		set_id;
 };
 
diff --git a/src/evaluate.c b/src/evaluate.c
index 46c97606ea8af..cb27f7c269049 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2851,6 +2851,47 @@ static int flowtable_evaluate(struct eval_ctx *ctx, struct flowtable *ft)
 	return 0;
 }
 
+/* Convert rule's handle.index into handle.position. */
+static int rule_translate_index(struct eval_ctx *ctx, struct rule *rule)
+{
+	struct table *table;
+	struct chain *chain;
+	uint64_t index = 0;
+	struct rule *r;
+	int ret;
+
+	/* update cache with CMD_LIST so that rules are fetched, too */
+	ret = cache_update(ctx->nf_sock, ctx->cache, CMD_LIST,
+			ctx->msgs, ctx->debug_mask, ctx->octx);
+	if (ret < 0)
+		return ret;
+
+	table = table_lookup(&rule->handle, ctx->cache);
+	if (!table)
+		return cmd_error(ctx, &rule->handle.table.location,
+				"Could not process rule: %s",
+				strerror(ENOENT));
+
+	chain = chain_lookup(table, &rule->handle);
+	if (!chain)
+		return cmd_error(ctx, &rule->handle.chain.location,
+				"Could not process rule: %s",
+				strerror(ENOENT));
+
+	list_for_each_entry(r, &chain->rules, list) {
+		if (++index < rule->handle.index.id)
+			continue;
+		rule->handle.position.id = r->handle.handle.id;
+		rule->handle.position.location = rule->handle.index.location;
+		break;
+	}
+	if (!rule->handle.position.id)
+		return cmd_error(ctx, &rule->handle.index.location,
+				"Could not process rule: %s",
+				strerror(EINVAL));
+	return 0;
+}
+
 static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule)
 {
 	struct stmt *stmt, *tstmt = NULL;
@@ -2879,6 +2920,10 @@ static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule)
 		return -1;
 	}
 
+	if (rule->handle.index.id &&
+	    rule_translate_index(ctx, rule))
+		return -1;
+
 	return 0;
 }
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 832c86642ce72..ba365160222dd 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -485,6 +485,7 @@ int nft_lex(void *, void *, void *);
 %token SEED			"seed"
 
 %token POSITION			"position"
+%token INDEX			"index"
 %token COMMENT			"comment"
 
 %token XML			"xml"
@@ -512,8 +513,8 @@ int nft_lex(void *, void *, void *);
 %type <cmd>			base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd get_cmd list_cmd reset_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd import_cmd
 %destructor { cmd_free($$); }	base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd get_cmd list_cmd reset_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd import_cmd
 
-%type <handle>			table_spec tableid_spec chain_spec chainid_spec flowtable_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
-%destructor { handle_free(&$$); } table_spec tableid_spec chain_spec chainid_spec flowtable_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
+%type <handle>			table_spec tableid_spec chain_spec chainid_spec flowtable_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec index_spec
+%destructor { handle_free(&$$); } table_spec tableid_spec chain_spec chainid_spec flowtable_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec index_spec
 %type <handle>			set_spec setid_spec set_identifier flowtable_identifier obj_spec objid_spec obj_identifier
 %destructor { handle_free(&$$); } set_spec setid_spec set_identifier obj_spec objid_spec obj_identifier
 %type <val>			family_spec family_spec_explicit chain_policy prio_spec
@@ -1961,6 +1962,14 @@ position_spec		:	POSITION	NUM
 			}
 			;
 
+index_spec		:	INDEX		NUM
+			{
+				memset(&$$, 0, sizeof($$));
+				$$.index.location	= @$;
+				$$.index.id		= $2 + 1;
+			}
+			;
+
 rule_position		:	chain_spec
 			{
 				$$ = $1;
@@ -1978,6 +1987,11 @@ rule_position		:	chain_spec
 				handle_merge(&$1, &$2);
 				$$ = $1;
 			}
+			|	chain_spec	index_spec
+			{
+				handle_merge(&$1, &$2);
+				$$ = $1;
+			}
 			;
 
 ruleid_spec		:	chain_spec	handle_spec
diff --git a/src/rule.c b/src/rule.c
index f0c6048cc0479..d61b82d529e7b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -60,6 +60,8 @@ void handle_merge(struct handle *dst, const struct handle *src)
 		dst->handle = src->handle;
 	if (dst->position.id == 0)
 		dst->position = src->position;
+	if (dst->index.id == 0)
+		dst->index = src->index;
 }
 
 static int cache_init_tables(struct netlink_ctx *ctx, struct handle *h,
diff --git a/src/scanner.l b/src/scanner.l
index 70366d1930351..9cf4aa5e9780d 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -285,6 +285,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "monitor"		{ return MONITOR; }
 
 "position"		{ return POSITION; }
+"index"			{ return INDEX; }
 "comment"		{ return COMMENT; }
 
 "constant"		{ return CONSTANT; }
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux