* [PATCH nf-next,v1 1/6] netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
@ 2025-05-14 21:42 ` Pablo Neira Ayuso
2025-05-15 9:56 ` Phil Sutter
2025-05-14 21:42 ` [PATCH nf-next,v1 2/6] netfilter: nf_tables: honor validation state in preparation phase Pablo Neira Ayuso
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-14 21:42 UTC (permalink / raw)
To: netfilter-devel
Do not return EAGAIN to replay the transaction if table validation
reports EINTR. Abort the transaction and report EINTR error instead.
Fixes: 169384fbe851 ("netfilter: nf_tables: allow loop termination for pending fatal signal")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b28f6730e26d..d5de843ee773 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9885,6 +9885,7 @@ static int nf_tables_validate(struct net *net)
{
struct nftables_pernet *nft_net = nft_pernet(net);
struct nft_table *table;
+ int err;
list_for_each_entry(table, &nft_net->tables, list) {
switch (table->validate_state) {
@@ -9894,15 +9895,24 @@ static int nf_tables_validate(struct net *net)
nft_validate_state_update(table, NFT_VALIDATE_DO);
fallthrough;
case NFT_VALIDATE_DO:
- if (nft_table_validate(net, table) < 0)
- return -EAGAIN;
+ err = nft_table_validate(net, table);
+ if (err < 0) {
+ if (err == EINTR)
+ goto err_eintr;
+ return -EAGAIN;
+ }
nft_validate_state_update(table, NFT_VALIDATE_SKIP);
break;
}
}
return 0;
+err_eintr:
+ list_for_each_entry(table, &nft_net->tables, list)
+ nft_validate_state_update(table, NFT_VALIDATE_SKIP);
+
+ return -EINTR;
}
/* a drop policy has to be deferred until all rules have been activated,
@@ -10710,7 +10720,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
}
/* 0. Validate ruleset, otherwise roll back for error reporting. */
- if (nf_tables_validate(net) < 0) {
+ err = nf_tables_validate(net);
+ if (err < 0) {
+ if (err == -EINTR)
+ return -EINTR;
+
nft_net->validate_state = NFT_VALIDATE_DO;
return -EAGAIN;
}
@@ -11054,9 +11068,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
};
int err = 0;
- if (action == NFNL_ABORT_VALIDATE &&
- nf_tables_validate(net) < 0)
- err = -EAGAIN;
+ if (action == NFNL_ABORT_VALIDATE) {
+ err = nf_tables_validate(net);
+ if (err < 0 && err != -EINTR)
+ err = -EAGAIN;
+ }
list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
list) {
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH nf-next,v1 1/6] netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path
2025-05-14 21:42 ` [PATCH nf-next,v1 1/6] netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path Pablo Neira Ayuso
@ 2025-05-15 9:56 ` Phil Sutter
0 siblings, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2025-05-15 9:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, May 14, 2025 at 11:42:11PM +0200, Pablo Neira Ayuso wrote:
> Do not return EAGAIN to replay the transaction if table validation
> reports EINTR. Abort the transaction and report EINTR error instead.
>
> Fixes: 169384fbe851 ("netfilter: nf_tables: allow loop termination for pending fatal signal")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> net/netfilter/nf_tables_api.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index b28f6730e26d..d5de843ee773 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -9885,6 +9885,7 @@ static int nf_tables_validate(struct net *net)
> {
> struct nftables_pernet *nft_net = nft_pernet(net);
> struct nft_table *table;
> + int err;
>
> list_for_each_entry(table, &nft_net->tables, list) {
> switch (table->validate_state) {
> @@ -9894,15 +9895,24 @@ static int nf_tables_validate(struct net *net)
> nft_validate_state_update(table, NFT_VALIDATE_DO);
> fallthrough;
> case NFT_VALIDATE_DO:
> - if (nft_table_validate(net, table) < 0)
> - return -EAGAIN;
> + err = nft_table_validate(net, table);
> + if (err < 0) {
> + if (err == EINTR)
This should be -EINTR here, I guess.
Cheers, Phil
> + goto err_eintr;
>
> + return -EAGAIN;
> + }
> nft_validate_state_update(table, NFT_VALIDATE_SKIP);
> break;
> }
> }
>
> return 0;
> +err_eintr:
> + list_for_each_entry(table, &nft_net->tables, list)
> + nft_validate_state_update(table, NFT_VALIDATE_SKIP);
> +
> + return -EINTR;
> }
>
> /* a drop policy has to be deferred until all rules have been activated,
> @@ -10710,7 +10720,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
> }
>
> /* 0. Validate ruleset, otherwise roll back for error reporting. */
> - if (nf_tables_validate(net) < 0) {
> + err = nf_tables_validate(net);
> + if (err < 0) {
> + if (err == -EINTR)
> + return -EINTR;
> +
> nft_net->validate_state = NFT_VALIDATE_DO;
> return -EAGAIN;
> }
> @@ -11054,9 +11068,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
> };
> int err = 0;
>
> - if (action == NFNL_ABORT_VALIDATE &&
> - nf_tables_validate(net) < 0)
> - err = -EAGAIN;
> + if (action == NFNL_ABORT_VALIDATE) {
> + err = nf_tables_validate(net);
> + if (err < 0 && err != -EINTR)
> + err = -EAGAIN;
> + }
>
> list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
> list) {
> --
> 2.30.2
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH nf-next,v1 2/6] netfilter: nf_tables: honor validation state in preparation phase
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
2025-05-14 21:42 ` [PATCH nf-next,v1 1/6] netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path Pablo Neira Ayuso
@ 2025-05-14 21:42 ` Pablo Neira Ayuso
2025-05-14 21:42 ` [PATCH nf-next,v1 3/6] netfilter: nf_tables: add infrastructure for chain validation on updates Pablo Neira Ayuso
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-14 21:42 UTC (permalink / raw)
To: netfilter-devel
There are three states for table validation:
- SKIP, this is the initial state, skip validation from the preparation
phase and the commit/abort path.
- NEED, this state is entered from the preparation phase, to validate
the table from the commit/abort path. In case that validation fails,
this enters the DO state and the transaction is replayed.
- DO, this state validates updates incremental from the preparation
step for error reporting.
Currently the NEED state is set on if:
- A new rule contains expressions that require validation, this
includes jump/goto chain.
- A new set element with jump/goto chain.
However, there are two issues:
- Validation is performed incrementally with new rules and elements
regardless the states. This patch updates the logic to perform the
validation only in the DO state.
- Reset validate state in case the transaction is finally aborted with
with NFNL_ABORT_NONE. Otherwise, the next batch can observe the DO
state and it performs the validation incrementally.
Fixes: a654de8fdc18 ("netfilter: nf_tables: fix chain dependency validation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 50 ++++++++++++++++++++++++++---------
1 file changed, 38 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d5de843ee773..46465a8c255f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3997,16 +3997,8 @@ static void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *r
nf_tables_rule_destroy(ctx, rule);
}
-/** nft_chain_validate - loop detection and hook validation
- *
- * @ctx: context containing call depth and base chain
- * @chain: chain to validate
- *
- * Walk through the rules of the given chain and chase all jumps/gotos
- * and set lookups until either the jump limit is hit or all reachable
- * chains have been validated.
- */
-int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
+static int __nft_chain_validate(const struct nft_ctx *ctx,
+ const struct nft_chain *chain)
{
struct nft_expr *expr, *last;
struct nft_rule *rule;
@@ -4037,6 +4029,30 @@ int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
return 0;
}
+
+/** nft_chain_validate - loop detection and hook validation
+ *
+ * @ctx: context containing call depth and base chain
+ * @chain: chain to validate
+ *
+ * Walk through the rules of the given chain and chase all jumps/gotos and set
+ * lookups until either the jump limit is hit or all reachable chains have been
+ * validated.
+ *
+ * This function is called from preparation phase: initial state is SKIP which
+ * means that validation can be skipped entirely. However, in case of a new rule
+ * or set element needs validation, then the NEED state is entered and the
+ * validation is performed from the commit/abort phase. In case this fails, the
+ * transaction is aborted and it is replayed in the DO validation state, then
+ * incremental validation is performed for error reporting.
+ */
+int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
+{
+ if (ctx->table->validate_state != NFT_VALIDATE_DO)
+ return 0;
+
+ return __nft_chain_validate(ctx, chain);
+}
EXPORT_SYMBOL_GPL(nft_chain_validate);
static int nft_table_validate(struct net *net, const struct nft_table *table)
@@ -4044,6 +4060,7 @@ static int nft_table_validate(struct net *net, const struct nft_table *table)
struct nft_chain *chain;
struct nft_ctx ctx = {
.net = net,
+ .table = (struct nft_table *)table,
.family = table->family,
};
int err;
@@ -4053,7 +4070,7 @@ static int nft_table_validate(struct net *net, const struct nft_table *table)
continue;
ctx.chain = chain;
- err = nft_chain_validate(&ctx, chain);
+ err = __nft_chain_validate(&ctx, chain);
if (err < 0)
return err;
@@ -11063,15 +11080,24 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
struct nft_trans *trans, *next;
LIST_HEAD(set_update_list);
struct nft_trans_elem *te;
+ struct nft_table *table;
struct nft_ctx ctx = {
.net = net,
};
int err = 0;
- if (action == NFNL_ABORT_VALIDATE) {
+ switch (action) {
+ case NFNL_ABORT_NONE:
+ list_for_each_entry(table, &nft_net->tables, list)
+ nft_validate_state_update(table, NFT_VALIDATE_SKIP);
+ break;
+ case NFNL_ABORT_AUTOLOAD:
+ break;
+ case NFNL_ABORT_VALIDATE:
err = nf_tables_validate(net);
if (err < 0 && err != -EINTR)
err = -EAGAIN;
+ break;
}
list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH nf-next,v1 3/6] netfilter: nf_tables: add infrastructure for chain validation on updates
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
2025-05-14 21:42 ` [PATCH nf-next,v1 1/6] netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path Pablo Neira Ayuso
2025-05-14 21:42 ` [PATCH nf-next,v1 2/6] netfilter: nf_tables: honor validation state in preparation phase Pablo Neira Ayuso
@ 2025-05-14 21:42 ` Pablo Neira Ayuso
2025-05-14 21:42 ` [PATCH nf-next,v1 4/6] netfilter: nf_tables: add new binding infrastructure Pablo Neira Ayuso
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-14 21:42 UTC (permalink / raw)
To: netfilter-devel
Add infrastructure to validate rulesets at chain granularity to improve
the situation for incremental rule and set element updates.
Instead of fully validating the table on updates, annotate chains in the
table that needs to be validated again after the updates in this batch.
Add a validation list per netns that contains chains that are pending to
be validated. A chain is added to the validation list under the
following circunstances:
- A new rule is added, then add the chain that contains this rule.
This allows to validate if the rule expressions are supported from
this chain.
- A new rule performs a jump/goto another chain. The destination
chain is added to the validation list.
- A new set element is added, then add the jump/goto chain via
element (verdict maps).
Add the chain that need validation to the validation list, then from the
commit/abort path, remove from the validation list. The validation list
becomes empty after the commit/abort phase.
This is a preparation patch, note that full table validation is still
in place, the next patch adds more infrastructure to enable chain
validation.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_tables.h | 5 +++-
net/netfilter/nf_tables_api.c | 45 ++++++++++++++++++++++++++++---
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 803d5f1601f9..d391990d1a96 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1119,11 +1119,13 @@ struct nft_chain {
struct nft_rule_blob __rcu *blob_gen_1;
struct list_head rules;
struct list_head list;
+ struct list_head validate_list;
struct rhlist_head rhlhead;
struct nft_table *table;
u64 handle;
u32 use;
- u8 flags:5,
+ u8 flags:4,
+ validate:1,
bound:1,
genmask:2;
char *name;
@@ -1910,6 +1912,7 @@ struct nftables_pernet {
struct list_head binding_list;
struct list_head module_list;
struct list_head notify_list;
+ struct list_head validate_list;
struct mutex commit_mutex;
u64 table_handle;
u64 tstamp;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 46465a8c255f..d35cad55c99b 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -123,6 +123,25 @@ static void nft_validate_state_update(struct nft_table *table, u8 new_validate_s
table->validate_state = new_validate_state;
}
+
+static void nft_validate_chain_pending(struct net *net, struct nft_chain *chain)
+{
+ struct nftables_pernet *nft_net = nft_pernet(net);
+
+ if (chain->validate)
+ return;
+
+ chain->validate = 1;
+ list_add_tail(&chain->validate_list, &nft_net->validate_list);
+}
+
+static void nft_validate_chain_need(struct nft_ctx *ctx,
+ struct nft_chain *chain)
+{
+ nft_validate_chain_pending(ctx->net, chain);
+ nft_validate_state_update(ctx->table, NFT_VALIDATE_NEED);
+}
+
static void nf_tables_trans_destroy_work(struct work_struct *w);
static void nft_trans_gc_work(struct work_struct *work);
@@ -274,6 +293,8 @@ static void nft_chain_trans_bind(const struct nft_ctx *ctx,
int nf_tables_bind_chain(const struct nft_ctx *ctx, struct nft_chain *chain)
{
+ nft_validate_chain_need((struct nft_ctx *)ctx, chain);
+
if (!nft_chain_binding(chain))
return 0;
@@ -4297,7 +4318,7 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
}
if (expr_info[i].ops->validate)
- nft_validate_state_update(table, NFT_VALIDATE_NEED);
+ nft_validate_chain_need(&ctx, ctx.chain);
expr_info[i].ops = NULL;
expr = nft_expr_next(expr);
@@ -7392,8 +7413,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
if (desc.type == NFT_DATA_VERDICT &&
(elem.data.val.verdict.code == NFT_GOTO ||
elem.data.val.verdict.code == NFT_JUMP))
- nft_validate_state_update(ctx->table,
- NFT_VALIDATE_NEED);
+ nft_validate_chain_need(ctx, elem.data.val.verdict.chain);
}
err = nft_set_ext_add_length(&tmpl, NFT_SET_EXT_DATA, desc.len);
@@ -9898,6 +9918,17 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
},
};
+static void nft_validate_chain_release(struct net *net)
+{
+ struct nftables_pernet *nft_net = nft_pernet(net);
+ struct nft_chain *chain, *next;
+
+ list_for_each_entry_safe(chain, next, &nft_net->validate_list, validate_list) {
+ list_del(&chain->validate_list);
+ chain->validate = 0;
+ }
+}
+
static int nf_tables_validate(struct net *net)
{
struct nftables_pernet *nft_net = nft_pernet(net);
@@ -10506,6 +10537,8 @@ static void nf_tables_module_autoload_cleanup(struct net *net)
struct nft_module_request *req, *next;
WARN_ON_ONCE(!list_empty(&nft_net->commit_list));
+ WARN_ON_ONCE(!list_empty(&nft_net->validate_list));
+
list_for_each_entry_safe(req, next, &nft_net->module_list, list) {
WARN_ON_ONCE(!req->done);
list_del(&req->list);
@@ -10705,6 +10738,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
int err;
if (list_empty(&nft_net->commit_list)) {
+ nft_validate_chain_release(net);
mutex_unlock(&nft_net->commit_mutex);
return 0;
}
@@ -10745,6 +10779,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
nft_net->validate_state = NFT_VALIDATE_DO;
return -EAGAIN;
}
+ nft_validate_chain_release(net);
err = nft_flow_rule_offload_commit(net);
if (err < 0)
@@ -11099,6 +11134,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
err = -EAGAIN;
break;
}
+ nft_validate_chain_release(net);
list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
list) {
@@ -11311,6 +11347,7 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb,
nft_gc_seq_end(nft_net, gc_seq);
WARN_ON_ONCE(!list_empty(&nft_net->commit_list));
+ WARN_ON_ONCE(!list_empty(&nft_net->validate_list));
/* module autoload needs to happen after GC sequence update because it
* temporarily releases and grabs mutex again.
@@ -11969,6 +12006,7 @@ static int __net_init nf_tables_init_net(struct net *net)
INIT_LIST_HEAD(&nft_net->binding_list);
INIT_LIST_HEAD(&nft_net->module_list);
INIT_LIST_HEAD(&nft_net->notify_list);
+ INIT_LIST_HEAD(&nft_net->validate_list);
mutex_init(&nft_net->commit_mutex);
nft_net->base_seq = 1;
nft_net->gc_seq = 0;
@@ -12013,6 +12051,7 @@ static void __net_exit nf_tables_exit_net(struct net *net)
WARN_ON_ONCE(!list_empty(&nft_net->module_list));
WARN_ON_ONCE(!list_empty(&nft_net->notify_list));
WARN_ON_ONCE(!list_empty(&nft_net->destroy_list));
+ WARN_ON_ONCE(!list_empty(&nft_net->validate_list));
}
static void nf_tables_exit_batch(struct list_head *net_exit_list)
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH nf-next,v1 4/6] netfilter: nf_tables: add new binding infrastructure
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
` (2 preceding siblings ...)
2025-05-14 21:42 ` [PATCH nf-next,v1 3/6] netfilter: nf_tables: add infrastructure for chain validation on updates Pablo Neira Ayuso
@ 2025-05-14 21:42 ` Pablo Neira Ayuso
2025-05-15 12:32 ` Phil Sutter
2025-05-14 21:42 ` [PATCH nf-next,v1 5/6] netfilter: nf_tables: use " Pablo Neira Ayuso
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-14 21:42 UTC (permalink / raw)
To: netfilter-devel
Add new binding infrastructure to build a graph that relates chains and
sets via jump/goto such as:
- chain to chain, ie. rule jump/goto chain.
- set to chain, ie. set element jump/goto chain.
- chain to set, ie. rule lookup to set.
The binding is composed of two tuples that describe [ from, to ], each
component of this tuple is defined as the object type and the pointer to
the object. This patch adds a hashtable for bindings per table to allow
for lookups of existing bindings in the preparation phase.
The bindings allows to backtrack to the basechain that refers to
this object for validation, and to go forward to check for loops
and to perform the rule validation.
This patch adds interfaces to deactivate/activate these bindings during
the preparation phase. Reference counter of zero tells us this binding
will be removed after this transaction.
This is still a preparation patch, a follow patch uses this infrastructure.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_tables.h | 45 ++++
net/netfilter/nf_tables_api.c | 352 ++++++++++++++++++++++++++++++
2 files changed, 397 insertions(+)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index d391990d1a96..807097746d24 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -585,6 +585,8 @@ struct nft_set_elem_expr {
struct nft_set {
struct list_head list;
struct list_head bindings;
+ struct list_head binding_list;
+ struct list_head backbinding_list;
refcount_t refs;
struct nft_table *table;
possible_net_t net;
@@ -1117,6 +1119,8 @@ struct nft_rule_blob {
struct nft_chain {
struct nft_rule_blob __rcu *blob_gen_0;
struct nft_rule_blob __rcu *blob_gen_1;
+ struct list_head binding_list;
+ struct list_head backbinding_list;
struct list_head rules;
struct list_head list;
struct list_head validate_list;
@@ -1293,6 +1297,7 @@ struct nft_table {
struct list_head sets;
struct list_head objects;
struct list_head flowtables;
+ struct rhashtable bindings_ht;
u64 hgenerator;
u64 handle;
u32 use;
@@ -1628,6 +1633,46 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
return test_bit(NFT_SET_ELEM_DEAD_BIT, word);
}
+enum nft_binding_type {
+ NFT_BIND_CHAIN = 0,
+ NFT_BIND_SET,
+};
+
+/* Bindings. */
+struct nft_binding_key {
+ union {
+ const struct nft_chain *chain;
+ const struct nft_set *set;
+ const void *ptr;
+ };
+ enum nft_binding_type type;
+};
+
+struct nft_binding {
+ struct rhash_head node;
+ struct list_head list;
+ struct list_head backlist;
+ struct nft_binding_key from;
+ struct nft_binding_key to;
+ uint32_t use;
+};
+
+struct nft_chain;
+struct nft_set;
+
+int nft_add_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
+void nft_activate_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
+void nft_deactivate_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
+void nft_del_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
+int nft_add_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
+void nft_deactivate_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
+void nft_activate_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
+void nft_del_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
+int nft_add_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
+void nft_deactivate_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
+void nft_activate_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
+void nft_del_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
+
/**
* struct nft_trans - nf_tables object update in transaction
*
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d35cad55c99b..463ee7b4ec19 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -954,6 +954,347 @@ void __nft_reg_track_cancel(struct nft_regs_track *track, u8 dreg)
}
EXPORT_SYMBOL_GPL(__nft_reg_track_cancel);
+struct nft_binding_cmp_key {
+ const struct nft_binding_key *from;
+ const struct nft_binding_key *to;
+};
+
+static u32 nft_binding_hash(const void *data, u32 len, u32 seed)
+{
+ const struct nft_binding_cmp_key *key = data;
+ unsigned long tuple[4];
+
+ tuple[0] = (unsigned long)key->from->ptr;
+ tuple[1] = (unsigned long)key->from->type;
+ tuple[2] = (unsigned long)key->to->ptr;
+ tuple[3] = (unsigned long)key->to->type;
+
+ return jhash(tuple, sizeof(tuple), seed);
+}
+
+static u32 nft_binding_hash_obj(const void *data, u32 len, u32 seed)
+{
+ const struct nft_binding *binding = data;
+ unsigned long tuple[4];
+
+ tuple[0] = (unsigned long)binding->from.ptr;
+ tuple[1] = (unsigned long)binding->from.type;
+ tuple[2] = (unsigned long)binding->to.ptr;
+ tuple[3] = (unsigned long)binding->to.type;
+
+ return jhash(tuple, sizeof(tuple), seed);
+}
+
+static int nft_binding_hash_cmp(struct rhashtable_compare_arg *arg,
+ const void *ptr)
+{
+ const struct nft_binding_cmp_key *key = arg->key;
+ const struct nft_binding *binding = ptr;
+
+ return key->from->ptr != binding->from.ptr ||
+ key->from->type != binding->from.type ||
+ key->to->ptr != binding->to.ptr ||
+ key->to->type != binding->to.type;
+}
+
+static const struct rhashtable_params nft_binding_ht_params = {
+ .head_offset = offsetof(struct nft_binding, node),
+ .hashfn = nft_binding_hash,
+ .obj_hashfn = nft_binding_hash_obj,
+ .obj_cmpfn = nft_binding_hash_cmp,
+ .automatic_shrinking = true,
+};
+
+static struct nft_binding *nft_binding_lookup(struct nft_table *table,
+ const struct nft_binding_key *from,
+ const struct nft_binding_key *to)
+{
+ struct nft_binding_cmp_key key = {
+ .from = from,
+ .to = to,
+ };
+
+ return rhashtable_lookup_fast(&table->bindings_ht, &key,
+ nft_binding_ht_params);
+}
+
+static void nft_deactivate_binding(struct nft_table *table,
+ const struct nft_binding_key *from,
+ const struct nft_binding_key *to)
+{
+ struct nft_binding *binding;
+
+ binding = nft_binding_lookup(table, from, to);
+ if (WARN_ON_ONCE(!binding))
+ return;
+ if (WARN_ON_ONCE(binding->use == 0))
+ return;
+
+ binding->use--;
+}
+
+void nft_deactivate_chain_binding(struct nft_chain *chain1,
+ struct nft_chain *chain2)
+{
+ struct nft_binding_key from = {
+ .ptr = chain1,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain2,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ nft_deactivate_binding(chain1->table, &from, &to);
+}
+
+void nft_deactivate_chain_set_binding(struct nft_chain *chain,
+ struct nft_set *set)
+{
+ struct nft_binding_key from = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+
+ nft_deactivate_binding(chain->table, &from, &to);
+}
+
+void nft_deactivate_set_chain_binding(struct nft_set *set,
+ struct nft_chain *chain)
+{
+ struct nft_binding_key from = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ nft_deactivate_binding(chain->table, &from, &to);
+}
+
+static void nft_activate_binding(struct nft_table *table,
+ const struct nft_binding_key *from,
+ const struct nft_binding_key *to)
+{
+ struct nft_binding *binding;
+
+ binding = nft_binding_lookup(table, from, to);
+ if (WARN_ON_ONCE(!binding))
+ return;
+
+ binding->use++;
+}
+
+void nft_activate_chain_binding(struct nft_chain *chain1,
+ struct nft_chain *chain2)
+{
+ struct nft_binding_key from = {
+ .ptr = chain1,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain2,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ nft_activate_binding(chain1->table, &from, &to);
+}
+
+void nft_activate_chain_set_binding(struct nft_chain *chain,
+ struct nft_set *set)
+{
+ struct nft_binding_key from = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+
+ nft_activate_binding(chain->table, &from, &to);
+}
+
+void nft_activate_set_chain_binding(struct nft_set *set,
+ struct nft_chain *chain)
+{
+ struct nft_binding_key from = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ nft_activate_binding(chain->table, &from, &to);
+}
+
+static void nft_del_binding(struct nft_table *table,
+ const struct nft_binding_key *from,
+ const struct nft_binding_key *to)
+{
+ struct nft_binding *binding;
+ int err;
+
+ binding = nft_binding_lookup(table, from, to);
+ /* With several references to object, deactivate deals to zero use,
+ * then first delete binding call remove it.
+ */
+ if (!binding)
+ return;
+
+ if (binding->use != 0)
+ return;
+
+ list_del(&binding->list);
+ list_del(&binding->backlist);
+
+ err = rhashtable_remove_fast(&table->bindings_ht,
+ &binding->node, nft_binding_ht_params);
+ if (WARN_ON_ONCE(err < 0))
+ return;
+
+ kfree(binding);
+}
+
+void nft_del_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2)
+{
+ struct nft_binding_key from = {
+ .ptr = chain1,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain2,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ nft_del_binding(chain1->table, &from, &to);
+}
+
+void nft_del_chain_set_binding(struct nft_chain *chain, struct nft_set *set)
+{
+ struct nft_binding_key from = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+
+ nft_del_binding(chain->table, &from, &to);
+}
+
+void nft_del_set_chain_binding(struct nft_set *set, struct nft_chain *chain)
+{
+ struct nft_binding_key from = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ nft_del_binding(chain->table, &from, &to);
+}
+
+static int __nft_add_binding(struct nft_table *table,
+ const struct nft_binding_key *from,
+ const struct nft_binding_key *to,
+ struct list_head *binding_list,
+ struct list_head *backbinding_list)
+{
+ struct nft_binding *binding;
+
+ binding = kzalloc(sizeof(struct nft_binding), GFP_KERNEL);
+ if (!binding)
+ return -ENOMEM;
+
+ binding->from = *from;
+ binding->to = *to;
+ binding->use++;
+
+ list_add_tail(&binding->list, binding_list);
+ list_add_tail(&binding->backlist, backbinding_list);
+
+ return rhashtable_insert_fast(&table->bindings_ht, &binding->node,
+ nft_binding_ht_params);
+}
+
+static int nft_add_binding(struct nft_table *table,
+ const struct nft_binding_key *from,
+ const struct nft_binding_key *to,
+ struct list_head *binding_list,
+ struct list_head *backbinding_list)
+{
+ struct nft_binding *binding;
+
+ binding = nft_binding_lookup(table, from, to);
+ if (!binding)
+ return __nft_add_binding(table, from, to,
+ binding_list, backbinding_list);
+
+ if (binding->use == UINT_MAX)
+ return -EOVERFLOW;
+
+ binding->use++;
+
+ return 0;
+}
+
+int nft_add_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2)
+{
+ struct nft_binding_key from = {
+ .ptr = chain1,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain2,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ return nft_add_binding(chain1->table, &from, &to,
+ &chain1->binding_list, &chain2->backbinding_list);
+}
+
+int nft_add_chain_set_binding(struct nft_chain *chain, struct nft_set *set)
+{
+ struct nft_binding_key from = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+ struct nft_binding_key to = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+
+ return nft_add_binding(chain->table, &from, &to,
+ &chain->binding_list, &set->backbinding_list);
+}
+
+int nft_add_set_chain_binding(struct nft_set *set, struct nft_chain *chain)
+{
+ struct nft_binding_key from = {
+ .ptr = set,
+ .type = NFT_BIND_SET,
+ };
+ struct nft_binding_key to = {
+ .ptr = chain,
+ .type = NFT_BIND_CHAIN,
+ };
+
+ return nft_add_binding(chain->table, &from, &to,
+ &set->binding_list, &chain->backbinding_list);
+}
+
/*
* Tables
*/
@@ -1611,6 +1952,10 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
if (err)
goto err_chain_ht;
+ err = rhashtable_init(&table->bindings_ht, &nft_binding_ht_params);
+ if (err)
+ goto err_binding_ht;
+
INIT_LIST_HEAD(&table->chains);
INIT_LIST_HEAD(&table->sets);
INIT_LIST_HEAD(&table->objects);
@@ -1629,6 +1974,8 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
list_add_tail_rcu(&table->list, &nft_net->tables);
return 0;
err_trans:
+ rhashtable_destroy(&table->bindings_ht);
+err_binding_ht:
rhltable_destroy(&table->chains_ht);
err_chain_ht:
kfree(table->udata);
@@ -1794,6 +2141,7 @@ static void nf_tables_table_destroy(struct nft_table *table)
if (WARN_ON(table->use > 0))
return;
+ rhashtable_destroy(&table->bindings_ht);
rhltable_destroy(&table->chains_ht);
kfree(table->name);
kfree(table->udata);
@@ -2691,6 +3039,8 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 policy,
ctx->chain = chain;
INIT_LIST_HEAD(&chain->rules);
+ INIT_LIST_HEAD(&chain->binding_list);
+ INIT_LIST_HEAD(&chain->backbinding_list);
chain->handle = nf_tables_alloc_handle(table);
chain->table = table;
@@ -5523,6 +5873,8 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
}
INIT_LIST_HEAD(&set->bindings);
+ INIT_LIST_HEAD(&set->binding_list);
+ INIT_LIST_HEAD(&set->backbinding_list);
INIT_LIST_HEAD(&set->catchall_list);
refcount_set(&set->refs, 1);
set->table = table;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH nf-next,v1 4/6] netfilter: nf_tables: add new binding infrastructure
2025-05-14 21:42 ` [PATCH nf-next,v1 4/6] netfilter: nf_tables: add new binding infrastructure Pablo Neira Ayuso
@ 2025-05-15 12:32 ` Phil Sutter
0 siblings, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2025-05-15 12:32 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, May 14, 2025 at 11:42:14PM +0200, Pablo Neira Ayuso wrote:
> Add new binding infrastructure to build a graph that relates chains and
> sets via jump/goto such as:
>
> - chain to chain, ie. rule jump/goto chain.
> - set to chain, ie. set element jump/goto chain.
> - chain to set, ie. rule lookup to set.
>
> The binding is composed of two tuples that describe [ from, to ], each
> component of this tuple is defined as the object type and the pointer to
> the object. This patch adds a hashtable for bindings per table to allow
> for lookups of existing bindings in the preparation phase.
>
> The bindings allows to backtrack to the basechain that refers to
> this object for validation, and to go forward to check for loops
> and to perform the rule validation.
>
> This patch adds interfaces to deactivate/activate these bindings during
> the preparation phase. Reference counter of zero tells us this binding
> will be removed after this transaction.
>
> This is still a preparation patch, a follow patch uses this infrastructure.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> include/net/netfilter/nf_tables.h | 45 ++++
> net/netfilter/nf_tables_api.c | 352 ++++++++++++++++++++++++++++++
> 2 files changed, 397 insertions(+)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index d391990d1a96..807097746d24 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -585,6 +585,8 @@ struct nft_set_elem_expr {
> struct nft_set {
> struct list_head list;
> struct list_head bindings;
> + struct list_head binding_list;
> + struct list_head backbinding_list;
> refcount_t refs;
> struct nft_table *table;
> possible_net_t net;
> @@ -1117,6 +1119,8 @@ struct nft_rule_blob {
> struct nft_chain {
> struct nft_rule_blob __rcu *blob_gen_0;
> struct nft_rule_blob __rcu *blob_gen_1;
> + struct list_head binding_list;
> + struct list_head backbinding_list;
> struct list_head rules;
> struct list_head list;
> struct list_head validate_list;
One could get away with just a single list by inserting the reverse ones
up front and appending the forward ones. While traversing, one would
either traverse forward or reverse and stop once the 'to' or 'from'
pointer doesn't match the current chain/set anymore. Maybe not worth to
save the 16B per set and chain, though.
> @@ -1293,6 +1297,7 @@ struct nft_table {
> struct list_head sets;
> struct list_head objects;
> struct list_head flowtables;
> + struct rhashtable bindings_ht;
> u64 hgenerator;
> u64 handle;
> u32 use;
> @@ -1628,6 +1633,46 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
> return test_bit(NFT_SET_ELEM_DEAD_BIT, word);
> }
>
> +enum nft_binding_type {
> + NFT_BIND_CHAIN = 0,
> + NFT_BIND_SET,
> +};
> +
> +/* Bindings. */
> +struct nft_binding_key {
> + union {
> + const struct nft_chain *chain;
> + const struct nft_set *set;
> + const void *ptr;
> + };
> + enum nft_binding_type type;
> +};
> +
> +struct nft_binding {
> + struct rhash_head node;
> + struct list_head list;
> + struct list_head backlist;
> + struct nft_binding_key from;
> + struct nft_binding_key to;
> + uint32_t use;
> +};
> +
> +struct nft_chain;
> +struct nft_set;
> +
> +int nft_add_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
> +void nft_activate_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
> +void nft_deactivate_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
> +void nft_del_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2);
> +int nft_add_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
> +void nft_deactivate_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
> +void nft_activate_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
> +void nft_del_chain_set_binding(struct nft_chain *chain, struct nft_set *set);
> +int nft_add_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
> +void nft_deactivate_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
> +void nft_activate_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
> +void nft_del_set_chain_binding(struct nft_set *set, struct nft_chain *chain);
> +
> /**
> * struct nft_trans - nf_tables object update in transaction
> *
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index d35cad55c99b..463ee7b4ec19 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -954,6 +954,347 @@ void __nft_reg_track_cancel(struct nft_regs_track *track, u8 dreg)
> }
> EXPORT_SYMBOL_GPL(__nft_reg_track_cancel);
>
> +struct nft_binding_cmp_key {
> + const struct nft_binding_key *from;
> + const struct nft_binding_key *to;
> +};
> +
> +static u32 nft_binding_hash(const void *data, u32 len, u32 seed)
> +{
> + const struct nft_binding_cmp_key *key = data;
> + unsigned long tuple[4];
> +
> + tuple[0] = (unsigned long)key->from->ptr;
> + tuple[1] = (unsigned long)key->from->type;
> + tuple[2] = (unsigned long)key->to->ptr;
> + tuple[3] = (unsigned long)key->to->type;
> +
> + return jhash(tuple, sizeof(tuple), seed);
> +}
> +
> +static u32 nft_binding_hash_obj(const void *data, u32 len, u32 seed)
> +{
> + const struct nft_binding *binding = data;
> + unsigned long tuple[4];
> +
> + tuple[0] = (unsigned long)binding->from.ptr;
> + tuple[1] = (unsigned long)binding->from.type;
> + tuple[2] = (unsigned long)binding->to.ptr;
> + tuple[3] = (unsigned long)binding->to.type;
> +
> + return jhash(tuple, sizeof(tuple), seed);
> +}
> +
> +static int nft_binding_hash_cmp(struct rhashtable_compare_arg *arg,
> + const void *ptr)
> +{
> + const struct nft_binding_cmp_key *key = arg->key;
> + const struct nft_binding *binding = ptr;
> +
> + return key->from->ptr != binding->from.ptr ||
> + key->from->type != binding->from.type ||
> + key->to->ptr != binding->to.ptr ||
> + key->to->type != binding->to.type;
> +}
> +
> +static const struct rhashtable_params nft_binding_ht_params = {
> + .head_offset = offsetof(struct nft_binding, node),
> + .hashfn = nft_binding_hash,
> + .obj_hashfn = nft_binding_hash_obj,
> + .obj_cmpfn = nft_binding_hash_cmp,
> + .automatic_shrinking = true,
> +};
> +
> +static struct nft_binding *nft_binding_lookup(struct nft_table *table,
> + const struct nft_binding_key *from,
> + const struct nft_binding_key *to)
> +{
> + struct nft_binding_cmp_key key = {
> + .from = from,
> + .to = to,
> + };
> +
> + return rhashtable_lookup_fast(&table->bindings_ht, &key,
> + nft_binding_ht_params);
> +}
> +
> +static void nft_deactivate_binding(struct nft_table *table,
> + const struct nft_binding_key *from,
> + const struct nft_binding_key *to)
Changing the signature to:
| static void nft_deactivate_binding(struct nft_table *table,
| void *from, enum nft_binding_type typef,
| void *to, enum nft_binding_type typet)
> +{
> + struct nft_binding *binding;
And creating the keys here:
| struct nft_binding_key from = { .ptr = from, .type = typef };
| struct nft_binding_key to = { .ptr = to, .type = typet };
> +
> + binding = nft_binding_lookup(table, from, to);
> + if (WARN_ON_ONCE(!binding))
> + return;
> + if (WARN_ON_ONCE(binding->use == 0))
> + return;
> +
> + binding->use--;
> +}
> +
> +void nft_deactivate_chain_binding(struct nft_chain *chain1,
> + struct nft_chain *chain2)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain1,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain2,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + nft_deactivate_binding(chain1->table, &from, &to);
> +}
Would reduce the above and following functions to one-line wrappers,
like:
| {
| nft_deactivate_binding(chain1->table,
| chain1, NFT_BIND_CHAIN,
| chain2, NFT_BIND_CHAIN);
| }
This could also be just a macro, too.
> +
> +void nft_deactivate_chain_set_binding(struct nft_chain *chain,
> + struct nft_set *set)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> +
> + nft_deactivate_binding(chain->table, &from, &to);
> +}
> +
> +void nft_deactivate_set_chain_binding(struct nft_set *set,
> + struct nft_chain *chain)
> +{
> + struct nft_binding_key from = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + nft_deactivate_binding(chain->table, &from, &to);
> +}
> +
> +static void nft_activate_binding(struct nft_table *table,
> + const struct nft_binding_key *from,
> + const struct nft_binding_key *to)
> +{
> + struct nft_binding *binding;
> +
> + binding = nft_binding_lookup(table, from, to);
> + if (WARN_ON_ONCE(!binding))
> + return;
> +
> + binding->use++;
> +}
> +
> +void nft_activate_chain_binding(struct nft_chain *chain1,
> + struct nft_chain *chain2)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain1,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain2,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + nft_activate_binding(chain1->table, &from, &to);
> +}
> +
> +void nft_activate_chain_set_binding(struct nft_chain *chain,
> + struct nft_set *set)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> +
> + nft_activate_binding(chain->table, &from, &to);
> +}
> +
> +void nft_activate_set_chain_binding(struct nft_set *set,
> + struct nft_chain *chain)
> +{
> + struct nft_binding_key from = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + nft_activate_binding(chain->table, &from, &to);
> +}
> +
> +static void nft_del_binding(struct nft_table *table,
> + const struct nft_binding_key *from,
> + const struct nft_binding_key *to)
> +{
> + struct nft_binding *binding;
> + int err;
> +
> + binding = nft_binding_lookup(table, from, to);
> + /* With several references to object, deactivate deals to zero use,
> + * then first delete binding call remove it.
> + */
> + if (!binding)
> + return;
> +
> + if (binding->use != 0)
> + return;
The asymmetry between add and del in that add increments the use-counter
while del does not decrement is a bit confusing. I believe it is an
optimization since add is always followed by enable otherwise?
I also wonder, does the code expectedly call del for enabled bindings or
is this a bug and there should be a warning above?
Cheers, Phil
> +
> + list_del(&binding->list);
> + list_del(&binding->backlist);
> +
> + err = rhashtable_remove_fast(&table->bindings_ht,
> + &binding->node, nft_binding_ht_params);
> + if (WARN_ON_ONCE(err < 0))
> + return;
> +
> + kfree(binding);
> +}
> +
> +void nft_del_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain1,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain2,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + nft_del_binding(chain1->table, &from, &to);
> +}
> +
> +void nft_del_chain_set_binding(struct nft_chain *chain, struct nft_set *set)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> +
> + nft_del_binding(chain->table, &from, &to);
> +}
> +
> +void nft_del_set_chain_binding(struct nft_set *set, struct nft_chain *chain)
> +{
> + struct nft_binding_key from = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + nft_del_binding(chain->table, &from, &to);
> +}
> +
> +static int __nft_add_binding(struct nft_table *table,
> + const struct nft_binding_key *from,
> + const struct nft_binding_key *to,
> + struct list_head *binding_list,
> + struct list_head *backbinding_list)
> +{
> + struct nft_binding *binding;
> +
> + binding = kzalloc(sizeof(struct nft_binding), GFP_KERNEL);
> + if (!binding)
> + return -ENOMEM;
> +
> + binding->from = *from;
> + binding->to = *to;
> + binding->use++;
> +
> + list_add_tail(&binding->list, binding_list);
> + list_add_tail(&binding->backlist, backbinding_list);
> +
> + return rhashtable_insert_fast(&table->bindings_ht, &binding->node,
> + nft_binding_ht_params);
> +}
> +
> +static int nft_add_binding(struct nft_table *table,
> + const struct nft_binding_key *from,
> + const struct nft_binding_key *to,
> + struct list_head *binding_list,
> + struct list_head *backbinding_list)
> +{
> + struct nft_binding *binding;
> +
> + binding = nft_binding_lookup(table, from, to);
> + if (!binding)
> + return __nft_add_binding(table, from, to,
> + binding_list, backbinding_list);
> +
> + if (binding->use == UINT_MAX)
> + return -EOVERFLOW;
> +
> + binding->use++;
> +
> + return 0;
> +}
> +
> +int nft_add_chain_binding(struct nft_chain *chain1, struct nft_chain *chain2)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain1,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain2,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + return nft_add_binding(chain1->table, &from, &to,
> + &chain1->binding_list, &chain2->backbinding_list);
> +}
> +
> +int nft_add_chain_set_binding(struct nft_chain *chain, struct nft_set *set)
> +{
> + struct nft_binding_key from = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> + struct nft_binding_key to = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> +
> + return nft_add_binding(chain->table, &from, &to,
> + &chain->binding_list, &set->backbinding_list);
> +}
> +
> +int nft_add_set_chain_binding(struct nft_set *set, struct nft_chain *chain)
> +{
> + struct nft_binding_key from = {
> + .ptr = set,
> + .type = NFT_BIND_SET,
> + };
> + struct nft_binding_key to = {
> + .ptr = chain,
> + .type = NFT_BIND_CHAIN,
> + };
> +
> + return nft_add_binding(chain->table, &from, &to,
> + &set->binding_list, &chain->backbinding_list);
> +}
> +
> /*
> * Tables
> */
> @@ -1611,6 +1952,10 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
> if (err)
> goto err_chain_ht;
>
> + err = rhashtable_init(&table->bindings_ht, &nft_binding_ht_params);
> + if (err)
> + goto err_binding_ht;
> +
> INIT_LIST_HEAD(&table->chains);
> INIT_LIST_HEAD(&table->sets);
> INIT_LIST_HEAD(&table->objects);
> @@ -1629,6 +1974,8 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
> list_add_tail_rcu(&table->list, &nft_net->tables);
> return 0;
> err_trans:
> + rhashtable_destroy(&table->bindings_ht);
> +err_binding_ht:
> rhltable_destroy(&table->chains_ht);
> err_chain_ht:
> kfree(table->udata);
> @@ -1794,6 +2141,7 @@ static void nf_tables_table_destroy(struct nft_table *table)
> if (WARN_ON(table->use > 0))
> return;
>
> + rhashtable_destroy(&table->bindings_ht);
> rhltable_destroy(&table->chains_ht);
> kfree(table->name);
> kfree(table->udata);
> @@ -2691,6 +3039,8 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 policy,
> ctx->chain = chain;
>
> INIT_LIST_HEAD(&chain->rules);
> + INIT_LIST_HEAD(&chain->binding_list);
> + INIT_LIST_HEAD(&chain->backbinding_list);
> chain->handle = nf_tables_alloc_handle(table);
> chain->table = table;
>
> @@ -5523,6 +5873,8 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
> }
>
> INIT_LIST_HEAD(&set->bindings);
> + INIT_LIST_HEAD(&set->binding_list);
> + INIT_LIST_HEAD(&set->backbinding_list);
> INIT_LIST_HEAD(&set->catchall_list);
> refcount_set(&set->refs, 1);
> set->table = table;
> --
> 2.30.2
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH nf-next,v1 5/6] netfilter: nf_tables: use new binding infrastructure
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
` (3 preceding siblings ...)
2025-05-14 21:42 ` [PATCH nf-next,v1 4/6] netfilter: nf_tables: add new binding infrastructure Pablo Neira Ayuso
@ 2025-05-14 21:42 ` Pablo Neira Ayuso
2025-05-15 12:34 ` Phil Sutter
2025-05-14 21:42 ` [PATCH nf-next,v1 6/6] netfilter: nf_tables: add support for validating incremental ruleset updates Pablo Neira Ayuso
2025-05-15 6:03 ` [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Florian Westphal
6 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-14 21:42 UTC (permalink / raw)
To: netfilter-devel
This patch uses the new binding infrastructure to link chains and sets:
- Chain-to-chain and chain-to-set bindings already exists, so it is
relatively trivial to add the code to add, deactivate, activate and
delete the bindings.
- Set-to-chain is a new binding required by verdict maps, this requires
to release this new type of binding from the commit/abort path.
Special cases are the deletion of sets with elements and netns/netlink
release path, that require the removal of all set-to-chain bindings.
Another special case is anonymous sets, that also require to delete
all set-to-chain bindings when unbound.
The binding graph is not yet used to validate rulesets, this is enabled
in the follow up patch.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_tables.h | 2 +-
net/netfilter/nf_tables_api.c | 149 ++++++++++++++++++++++++++++--
net/netfilter/nft_immediate.c | 25 ++++-
3 files changed, 164 insertions(+), 12 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 807097746d24..c33820fc1abd 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1929,7 +1929,7 @@ struct nft_trans_gc *nft_trans_gc_catchall_async(struct nft_trans_gc *gc,
struct nft_trans_gc *nft_trans_gc_catchall_sync(struct nft_trans_gc *gc);
void nft_setelem_data_deactivate(const struct net *net,
- const struct nft_set *set,
+ struct nft_set *set,
struct nft_elem_priv *elem_priv);
int __init nft_chain_filter_init(void);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 463ee7b4ec19..e92cccc834d9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6077,6 +6077,7 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
{
struct nft_set_binding *i;
struct nft_set_iter iter;
+ int err;
if (!list_empty(&set->bindings) && nft_set_is_anonymous(set))
return -EBUSY;
@@ -6109,6 +6110,12 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
if (!nft_use_inc(&set->use))
return -EMFILE;
+ err = nft_add_chain_set_binding(ctx->chain, set);
+ if (err < 0) {
+ nft_use_dec_restore(&set->use);
+ return err;
+ }
+
binding->chain = ctx->chain;
list_add_tail_rcu(&binding->list, &set->bindings);
nft_set_trans_bind(ctx, set);
@@ -6117,14 +6124,27 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
}
EXPORT_SYMBOL_GPL(nf_tables_bind_set);
+static void nft_del_set_chain_binding_all(struct nft_set *set)
+{
+ struct nft_binding *binding, *next;
+ struct nft_chain *chain;
+
+ list_for_each_entry_safe(binding, next, &set->binding_list, list) {
+ chain = (struct nft_chain *)binding->to.chain;
+ nft_del_set_chain_binding(set, chain);
+ }
+}
+
static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *binding, bool event)
{
list_del_rcu(&binding->list);
+ nft_del_chain_set_binding(ctx->chain, set);
if (list_empty(&set->bindings) && nft_set_is_anonymous(set)) {
list_del_rcu(&set->list);
set->dead = 1;
+ nft_del_set_chain_binding_all(set);
if (event)
nf_tables_set_notify(ctx, set, NFT_MSG_DELSET,
GFP_KERNEL);
@@ -6132,7 +6152,7 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
}
static void nft_setelem_data_activate(const struct net *net,
- const struct nft_set *set,
+ struct nft_set *set,
struct nft_elem_priv *elem_priv);
static int nft_mapelem_activate(const struct nft_ctx *ctx,
@@ -6193,6 +6213,7 @@ void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set)
nft_clear(ctx->net, set);
}
+ nft_activate_chain_set_binding(ctx->chain, set);
nft_use_inc_restore(&set->use);
}
EXPORT_SYMBOL_GPL(nf_tables_activate_set);
@@ -6211,6 +6232,8 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
else
list_del_rcu(&binding->list);
+ nft_deactivate_chain_set_binding(ctx->chain, set);
+ nft_del_chain_set_binding(ctx->chain, set);
nft_use_dec(&set->use);
break;
case NFT_TRANS_PREPARE:
@@ -6220,6 +6243,7 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
nft_deactivate_next(ctx->net, set);
}
+ nft_deactivate_chain_set_binding(ctx->chain, set);
nft_use_dec(&set->use);
return;
case NFT_TRANS_ABORT:
@@ -6228,6 +6252,7 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
nft_map_deactivate(ctx, set);
+ nft_deactivate_chain_set_binding(ctx->chain, set);
nft_use_dec(&set->use);
fallthrough;
default:
@@ -7124,6 +7149,7 @@ static void __nft_set_elem_destroy(const struct nft_ctx *ctx,
nft_data_release(nft_set_ext_key(ext), NFT_DATA_VALUE);
if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
nft_data_release(nft_set_ext_data(ext), set->dtype);
+ // XXX gc
if (destroy_expr && nft_set_ext_exists(ext, NFT_SET_EXT_EXPRESSIONS))
nft_set_elem_expr_destroy(ctx, nft_set_ext_expr(ext));
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
@@ -7454,6 +7480,27 @@ static void nft_setelem_remove(const struct net *net,
set->ops->remove(net, set, elem_priv);
}
+static void nft_setelem_data_binding_remove(struct nft_set *set,
+ struct nft_elem_priv *elem_priv)
+{
+ struct nft_set_ext *ext;
+
+ ext = nft_set_elem_ext(set, elem_priv);
+ if (set->dtype == NFT_DATA_VERDICT &&
+ nft_set_ext_exists(ext, NFT_SET_EXT_DATA)) {
+ struct nft_data *data = nft_set_ext_data(ext);
+ struct nft_chain *chain;
+
+ switch (data->verdict.code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ chain = data->verdict.chain;
+ nft_del_set_chain_binding(set, chain);
+ break;
+ }
+ }
+}
+
static void nft_trans_elems_remove(const struct nft_ctx *ctx,
const struct nft_trans_elem *te)
{
@@ -7471,6 +7518,8 @@ static void nft_trans_elems_remove(const struct nft_ctx *ctx,
atomic_dec(&te->set->nelems);
te->set->ndeact--;
}
+
+ nft_setelem_data_binding_remove(te->set, te->elems[i].priv);
}
}
@@ -7768,9 +7817,17 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
nft_validate_chain_need(ctx, elem.data.val.verdict.chain);
}
+ if (desc.type == NFT_DATA_VERDICT &&
+ (elem.data.val.verdict.code == NFT_GOTO ||
+ elem.data.val.verdict.code == NFT_JUMP)) {
+ err = nft_add_set_chain_binding(set, elem.data.val.verdict.chain);
+ if (err < 0)
+ goto err_parse_data;
+ }
+
err = nft_set_ext_add_length(&tmpl, NFT_SET_EXT_DATA, desc.len);
if (err < 0)
- goto err_parse_data;
+ goto err_binding;
}
/* The full maximum length of userdata can exceed the maximum
@@ -7784,7 +7841,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
err = nft_set_ext_add_length(&tmpl, NFT_SET_EXT_USERDATA,
ulen);
if (err < 0)
- goto err_parse_data;
+ goto err_binding;
}
}
@@ -7793,7 +7850,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
timeout, expiration, GFP_KERNEL_ACCOUNT);
if (IS_ERR(elem.priv)) {
err = PTR_ERR(elem.priv);
- goto err_parse_data;
+ goto err_binding;
}
ext = nft_set_elem_ext(set, elem.priv);
@@ -7903,6 +7960,15 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
kfree(trans);
err_elem_free:
nf_tables_set_elem_destroy(ctx, set, elem.priv);
+err_binding:
+ if (nla[NFTA_SET_ELEM_DATA] != NULL) {
+ if (desc.type == NFT_DATA_VERDICT &&
+ (elem.data.val.verdict.code == NFT_GOTO ||
+ elem.data.val.verdict.code == NFT_JUMP)) {
+ nft_deactivate_set_chain_binding(set, elem.data.val.verdict.chain);
+ nft_del_set_chain_binding(set, elem.data.val.verdict.chain);
+ }
+ }
err_parse_data:
if (nla[NFTA_SET_ELEM_DATA] != NULL)
nft_data_release(&elem.data.val, desc.type);
@@ -8007,30 +8073,90 @@ static int nft_setelem_active_next(const struct net *net,
return nft_set_elem_active(ext, genmask);
}
+static void nft_setelem_data_hold(const struct net *net,
+ const struct nft_set *set,
+ const struct nft_set_ext *ext)
+{
+ if (set->dtype == NFT_DATA_VERDICT) {
+ struct nft_data *data = nft_set_ext_data(ext);
+ struct nft_chain *chain;
+
+ switch (data->verdict.code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ chain = data->verdict.chain;
+ nft_activate_set_chain_binding((struct nft_set *)set, chain);
+ nft_use_inc_restore(&chain->use);
+ break;
+ }
+ }
+}
+
static void nft_setelem_data_activate(const struct net *net,
- const struct nft_set *set,
+ struct nft_set *set,
struct nft_elem_priv *elem_priv)
{
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem_priv);
if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
- nft_data_hold(nft_set_ext_data(ext), set->dtype);
+ nft_setelem_data_hold(net, set, ext);
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
nft_use_inc_restore(&(*nft_set_ext_obj(ext))->use);
}
+static void nft_setelem_data_release(struct nft_set *set,
+ const struct nft_set_ext *ext)
+{
+ struct nft_data *data = nft_set_ext_data(ext);
+
+ if (set->dtype == NFT_DATA_VERDICT) {
+ struct nft_chain *chain;
+
+ switch (data->verdict.code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ chain = data->verdict.chain;
+ nft_deactivate_set_chain_binding(set, chain);
+ nft_use_dec(&chain->use);
+ break;
+ }
+ }
+}
+
void nft_setelem_data_deactivate(const struct net *net,
- const struct nft_set *set,
+ struct nft_set *set,
struct nft_elem_priv *elem_priv)
{
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem_priv);
if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
- nft_data_release(nft_set_ext_data(ext), set->dtype);
+ nft_setelem_data_release(set, ext);
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
nft_use_dec(&(*nft_set_ext_obj(ext))->use);
}
+static void nft_setelem_data_abort(struct nft_set *set,
+ struct nft_elem_priv *elem_priv)
+{
+ struct nft_set_ext *ext;
+
+ ext = nft_set_elem_ext(set, elem_priv);
+ if (set->dtype == NFT_DATA_VERDICT &&
+ nft_set_ext_exists(ext, NFT_SET_EXT_DATA)) {
+ struct nft_data *data = nft_set_ext_data(ext);
+ struct nft_chain *chain;
+
+ switch (data->verdict.code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ chain = data->verdict.chain;
+ nft_deactivate_set_chain_binding(set, chain);
+ nft_del_set_chain_binding(set, chain);
+ break;
+ }
+ }
+}
+
/* similar to nft_trans_elems_remove, but called from abort path to undo newsetelem.
* No notifications and no ndeact changes.
*
@@ -8057,6 +8183,7 @@ static bool nft_trans_elems_new_abort(const struct nft_ctx *ctx,
if (!nft_setelem_is_catchall(te->set, te->elems[i].priv))
atomic_dec(&te->set->nelems);
+ nft_setelem_data_abort(te->set, te->elems[i].priv);
removed = true;
}
@@ -11290,6 +11417,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
case NFT_MSG_DESTROYSET:
nft_trans_set(trans)->dead = 1;
list_del_rcu(&nft_trans_set(trans)->list);
+ nft_del_set_chain_binding_all(nft_trans_set(trans));
nf_tables_set_notify(&ctx, nft_trans_set(trans),
trans->msg_type, GFP_KERNEL);
break;
@@ -12258,9 +12386,10 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
list_for_each_entry_safe(set, ns, &table->sets, list) {
list_del(&set->list);
nft_use_dec(&table->use);
- if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
+ if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT)) {
nft_map_deactivate(&ctx, set);
-
+ nft_del_set_chain_binding_all(set);
+ }
nft_set_destroy(&ctx, set);
}
list_for_each_entry_safe(obj, ne, &table->objects, list) {
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index 02ee5fb69871..0b886725ed37 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -76,9 +76,16 @@ static int nft_immediate_init(const struct nft_ctx *ctx,
switch (priv->data.verdict.code) {
case NFT_JUMP:
case NFT_GOTO:
- err = nf_tables_bind_chain(ctx, chain);
+ err = nft_add_chain_binding(ctx->chain, chain);
if (err < 0)
goto err1;
+
+ err = nf_tables_bind_chain(ctx, chain);
+ if (err < 0) {
+ nft_deactivate_chain_binding(ctx->chain, chain);
+ nft_del_chain_binding(ctx->chain, chain);
+ goto err1;
+ }
break;
default:
break;
@@ -106,6 +113,8 @@ static void nft_immediate_activate(const struct nft_ctx *ctx,
case NFT_JUMP:
case NFT_GOTO:
chain = data->verdict.chain;
+ nft_activate_chain_binding(ctx->chain, chain);
+
if (!nft_chain_binding(chain))
break;
@@ -152,6 +161,20 @@ static void nft_immediate_deactivate(const struct nft_ctx *ctx,
case NFT_JUMP:
case NFT_GOTO:
chain = data->verdict.chain;
+ switch (phase) {
+ case NFT_TRANS_PREPARE_ERROR:
+ case NFT_TRANS_PREPARE:
+ nft_deactivate_chain_binding(ctx->chain, chain);
+ break;
+ case NFT_TRANS_ABORT:
+ case NFT_TRANS_RELEASE:
+ nft_deactivate_chain_binding(ctx->chain, chain);
+ fallthrough;
+ case NFT_TRANS_COMMIT:
+ nft_del_chain_binding(ctx->chain, chain);
+ break;
+ }
+
if (!nft_chain_binding(chain))
break;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH nf-next,v1 5/6] netfilter: nf_tables: use new binding infrastructure
2025-05-14 21:42 ` [PATCH nf-next,v1 5/6] netfilter: nf_tables: use " Pablo Neira Ayuso
@ 2025-05-15 12:34 ` Phil Sutter
0 siblings, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2025-05-15 12:34 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, May 14, 2025 at 11:42:15PM +0200, Pablo Neira Ayuso wrote:
> This patch uses the new binding infrastructure to link chains and sets:
>
> - Chain-to-chain and chain-to-set bindings already exists, so it is
> relatively trivial to add the code to add, deactivate, activate and
> delete the bindings.
>
> - Set-to-chain is a new binding required by verdict maps, this requires
> to release this new type of binding from the commit/abort path.
> Special cases are the deletion of sets with elements and netns/netlink
> release path, that require the removal of all set-to-chain bindings.
> Another special case is anonymous sets, that also require to delete
> all set-to-chain bindings when unbound.
>
> The binding graph is not yet used to validate rulesets, this is enabled
> in the follow up patch.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> include/net/netfilter/nf_tables.h | 2 +-
> net/netfilter/nf_tables_api.c | 149 ++++++++++++++++++++++++++++--
> net/netfilter/nft_immediate.c | 25 ++++-
> 3 files changed, 164 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index 807097746d24..c33820fc1abd 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -1929,7 +1929,7 @@ struct nft_trans_gc *nft_trans_gc_catchall_async(struct nft_trans_gc *gc,
> struct nft_trans_gc *nft_trans_gc_catchall_sync(struct nft_trans_gc *gc);
>
> void nft_setelem_data_deactivate(const struct net *net,
> - const struct nft_set *set,
> + struct nft_set *set,
> struct nft_elem_priv *elem_priv);
>
> int __init nft_chain_filter_init(void);
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 463ee7b4ec19..e92cccc834d9 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -6077,6 +6077,7 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
> {
> struct nft_set_binding *i;
> struct nft_set_iter iter;
> + int err;
>
> if (!list_empty(&set->bindings) && nft_set_is_anonymous(set))
> return -EBUSY;
> @@ -6109,6 +6110,12 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
> if (!nft_use_inc(&set->use))
> return -EMFILE;
>
> + err = nft_add_chain_set_binding(ctx->chain, set);
> + if (err < 0) {
> + nft_use_dec_restore(&set->use);
> + return err;
> + }
> +
> binding->chain = ctx->chain;
> list_add_tail_rcu(&binding->list, &set->bindings);
> nft_set_trans_bind(ctx, set);
> @@ -6117,14 +6124,27 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
> }
> EXPORT_SYMBOL_GPL(nf_tables_bind_set);
>
> +static void nft_del_set_chain_binding_all(struct nft_set *set)
> +{
> + struct nft_binding *binding, *next;
> + struct nft_chain *chain;
> +
> + list_for_each_entry_safe(binding, next, &set->binding_list, list) {
> + chain = (struct nft_chain *)binding->to.chain;
> + nft_del_set_chain_binding(set, chain);
> + }
> +}
> +
> static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
> struct nft_set_binding *binding, bool event)
> {
> list_del_rcu(&binding->list);
> + nft_del_chain_set_binding(ctx->chain, set);
>
> if (list_empty(&set->bindings) && nft_set_is_anonymous(set)) {
> list_del_rcu(&set->list);
> set->dead = 1;
> + nft_del_set_chain_binding_all(set);
> if (event)
> nf_tables_set_notify(ctx, set, NFT_MSG_DELSET,
> GFP_KERNEL);
> @@ -6132,7 +6152,7 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
> }
>
> static void nft_setelem_data_activate(const struct net *net,
> - const struct nft_set *set,
> + struct nft_set *set,
> struct nft_elem_priv *elem_priv);
>
> static int nft_mapelem_activate(const struct nft_ctx *ctx,
> @@ -6193,6 +6213,7 @@ void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set)
> nft_clear(ctx->net, set);
> }
>
> + nft_activate_chain_set_binding(ctx->chain, set);
> nft_use_inc_restore(&set->use);
> }
> EXPORT_SYMBOL_GPL(nf_tables_activate_set);
> @@ -6211,6 +6232,8 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
> else
> list_del_rcu(&binding->list);
>
> + nft_deactivate_chain_set_binding(ctx->chain, set);
> + nft_del_chain_set_binding(ctx->chain, set);
> nft_use_dec(&set->use);
> break;
> case NFT_TRANS_PREPARE:
> @@ -6220,6 +6243,7 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
>
> nft_deactivate_next(ctx->net, set);
> }
> + nft_deactivate_chain_set_binding(ctx->chain, set);
> nft_use_dec(&set->use);
> return;
> case NFT_TRANS_ABORT:
> @@ -6228,6 +6252,7 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
> set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
> nft_map_deactivate(ctx, set);
>
> + nft_deactivate_chain_set_binding(ctx->chain, set);
> nft_use_dec(&set->use);
> fallthrough;
> default:
> @@ -7124,6 +7149,7 @@ static void __nft_set_elem_destroy(const struct nft_ctx *ctx,
> nft_data_release(nft_set_ext_key(ext), NFT_DATA_VALUE);
> if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
> nft_data_release(nft_set_ext_data(ext), set->dtype);
> + // XXX gc
Leftover?
Cheers, Phil
> if (destroy_expr && nft_set_ext_exists(ext, NFT_SET_EXT_EXPRESSIONS))
> nft_set_elem_expr_destroy(ctx, nft_set_ext_expr(ext));
> if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
> @@ -7454,6 +7480,27 @@ static void nft_setelem_remove(const struct net *net,
> set->ops->remove(net, set, elem_priv);
> }
>
> +static void nft_setelem_data_binding_remove(struct nft_set *set,
> + struct nft_elem_priv *elem_priv)
> +{
> + struct nft_set_ext *ext;
> +
> + ext = nft_set_elem_ext(set, elem_priv);
> + if (set->dtype == NFT_DATA_VERDICT &&
> + nft_set_ext_exists(ext, NFT_SET_EXT_DATA)) {
> + struct nft_data *data = nft_set_ext_data(ext);
> + struct nft_chain *chain;
> +
> + switch (data->verdict.code) {
> + case NFT_JUMP:
> + case NFT_GOTO:
> + chain = data->verdict.chain;
> + nft_del_set_chain_binding(set, chain);
> + break;
> + }
> + }
> +}
> +
> static void nft_trans_elems_remove(const struct nft_ctx *ctx,
> const struct nft_trans_elem *te)
> {
> @@ -7471,6 +7518,8 @@ static void nft_trans_elems_remove(const struct nft_ctx *ctx,
> atomic_dec(&te->set->nelems);
> te->set->ndeact--;
> }
> +
> + nft_setelem_data_binding_remove(te->set, te->elems[i].priv);
> }
> }
>
> @@ -7768,9 +7817,17 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> nft_validate_chain_need(ctx, elem.data.val.verdict.chain);
> }
>
> + if (desc.type == NFT_DATA_VERDICT &&
> + (elem.data.val.verdict.code == NFT_GOTO ||
> + elem.data.val.verdict.code == NFT_JUMP)) {
> + err = nft_add_set_chain_binding(set, elem.data.val.verdict.chain);
> + if (err < 0)
> + goto err_parse_data;
> + }
> +
> err = nft_set_ext_add_length(&tmpl, NFT_SET_EXT_DATA, desc.len);
> if (err < 0)
> - goto err_parse_data;
> + goto err_binding;
> }
>
> /* The full maximum length of userdata can exceed the maximum
> @@ -7784,7 +7841,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> err = nft_set_ext_add_length(&tmpl, NFT_SET_EXT_USERDATA,
> ulen);
> if (err < 0)
> - goto err_parse_data;
> + goto err_binding;
> }
> }
>
> @@ -7793,7 +7850,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> timeout, expiration, GFP_KERNEL_ACCOUNT);
> if (IS_ERR(elem.priv)) {
> err = PTR_ERR(elem.priv);
> - goto err_parse_data;
> + goto err_binding;
> }
>
> ext = nft_set_elem_ext(set, elem.priv);
> @@ -7903,6 +7960,15 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
> kfree(trans);
> err_elem_free:
> nf_tables_set_elem_destroy(ctx, set, elem.priv);
> +err_binding:
> + if (nla[NFTA_SET_ELEM_DATA] != NULL) {
> + if (desc.type == NFT_DATA_VERDICT &&
> + (elem.data.val.verdict.code == NFT_GOTO ||
> + elem.data.val.verdict.code == NFT_JUMP)) {
> + nft_deactivate_set_chain_binding(set, elem.data.val.verdict.chain);
> + nft_del_set_chain_binding(set, elem.data.val.verdict.chain);
> + }
> + }
> err_parse_data:
> if (nla[NFTA_SET_ELEM_DATA] != NULL)
> nft_data_release(&elem.data.val, desc.type);
> @@ -8007,30 +8073,90 @@ static int nft_setelem_active_next(const struct net *net,
> return nft_set_elem_active(ext, genmask);
> }
>
> +static void nft_setelem_data_hold(const struct net *net,
> + const struct nft_set *set,
> + const struct nft_set_ext *ext)
> +{
> + if (set->dtype == NFT_DATA_VERDICT) {
> + struct nft_data *data = nft_set_ext_data(ext);
> + struct nft_chain *chain;
> +
> + switch (data->verdict.code) {
> + case NFT_JUMP:
> + case NFT_GOTO:
> + chain = data->verdict.chain;
> + nft_activate_set_chain_binding((struct nft_set *)set, chain);
> + nft_use_inc_restore(&chain->use);
> + break;
> + }
> + }
> +}
> +
> static void nft_setelem_data_activate(const struct net *net,
> - const struct nft_set *set,
> + struct nft_set *set,
> struct nft_elem_priv *elem_priv)
> {
> const struct nft_set_ext *ext = nft_set_elem_ext(set, elem_priv);
>
> if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
> - nft_data_hold(nft_set_ext_data(ext), set->dtype);
> + nft_setelem_data_hold(net, set, ext);
> if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
> nft_use_inc_restore(&(*nft_set_ext_obj(ext))->use);
> }
>
> +static void nft_setelem_data_release(struct nft_set *set,
> + const struct nft_set_ext *ext)
> +{
> + struct nft_data *data = nft_set_ext_data(ext);
> +
> + if (set->dtype == NFT_DATA_VERDICT) {
> + struct nft_chain *chain;
> +
> + switch (data->verdict.code) {
> + case NFT_JUMP:
> + case NFT_GOTO:
> + chain = data->verdict.chain;
> + nft_deactivate_set_chain_binding(set, chain);
> + nft_use_dec(&chain->use);
> + break;
> + }
> + }
> +}
> +
> void nft_setelem_data_deactivate(const struct net *net,
> - const struct nft_set *set,
> + struct nft_set *set,
> struct nft_elem_priv *elem_priv)
> {
> const struct nft_set_ext *ext = nft_set_elem_ext(set, elem_priv);
>
> if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
> - nft_data_release(nft_set_ext_data(ext), set->dtype);
> + nft_setelem_data_release(set, ext);
> if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
> nft_use_dec(&(*nft_set_ext_obj(ext))->use);
> }
>
> +static void nft_setelem_data_abort(struct nft_set *set,
> + struct nft_elem_priv *elem_priv)
> +{
> + struct nft_set_ext *ext;
> +
> + ext = nft_set_elem_ext(set, elem_priv);
> + if (set->dtype == NFT_DATA_VERDICT &&
> + nft_set_ext_exists(ext, NFT_SET_EXT_DATA)) {
> + struct nft_data *data = nft_set_ext_data(ext);
> + struct nft_chain *chain;
> +
> + switch (data->verdict.code) {
> + case NFT_JUMP:
> + case NFT_GOTO:
> + chain = data->verdict.chain;
> + nft_deactivate_set_chain_binding(set, chain);
> + nft_del_set_chain_binding(set, chain);
> + break;
> + }
> + }
> +}
> +
> /* similar to nft_trans_elems_remove, but called from abort path to undo newsetelem.
> * No notifications and no ndeact changes.
> *
> @@ -8057,6 +8183,7 @@ static bool nft_trans_elems_new_abort(const struct nft_ctx *ctx,
> if (!nft_setelem_is_catchall(te->set, te->elems[i].priv))
> atomic_dec(&te->set->nelems);
>
> + nft_setelem_data_abort(te->set, te->elems[i].priv);
> removed = true;
> }
>
> @@ -11290,6 +11417,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
> case NFT_MSG_DESTROYSET:
> nft_trans_set(trans)->dead = 1;
> list_del_rcu(&nft_trans_set(trans)->list);
> + nft_del_set_chain_binding_all(nft_trans_set(trans));
> nf_tables_set_notify(&ctx, nft_trans_set(trans),
> trans->msg_type, GFP_KERNEL);
> break;
> @@ -12258,9 +12386,10 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
> list_for_each_entry_safe(set, ns, &table->sets, list) {
> list_del(&set->list);
> nft_use_dec(&table->use);
> - if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
> + if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT)) {
> nft_map_deactivate(&ctx, set);
> -
> + nft_del_set_chain_binding_all(set);
> + }
> nft_set_destroy(&ctx, set);
> }
> list_for_each_entry_safe(obj, ne, &table->objects, list) {
> diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
> index 02ee5fb69871..0b886725ed37 100644
> --- a/net/netfilter/nft_immediate.c
> +++ b/net/netfilter/nft_immediate.c
> @@ -76,9 +76,16 @@ static int nft_immediate_init(const struct nft_ctx *ctx,
> switch (priv->data.verdict.code) {
> case NFT_JUMP:
> case NFT_GOTO:
> - err = nf_tables_bind_chain(ctx, chain);
> + err = nft_add_chain_binding(ctx->chain, chain);
> if (err < 0)
> goto err1;
> +
> + err = nf_tables_bind_chain(ctx, chain);
> + if (err < 0) {
> + nft_deactivate_chain_binding(ctx->chain, chain);
> + nft_del_chain_binding(ctx->chain, chain);
> + goto err1;
> + }
> break;
> default:
> break;
> @@ -106,6 +113,8 @@ static void nft_immediate_activate(const struct nft_ctx *ctx,
> case NFT_JUMP:
> case NFT_GOTO:
> chain = data->verdict.chain;
> + nft_activate_chain_binding(ctx->chain, chain);
> +
> if (!nft_chain_binding(chain))
> break;
>
> @@ -152,6 +161,20 @@ static void nft_immediate_deactivate(const struct nft_ctx *ctx,
> case NFT_JUMP:
> case NFT_GOTO:
> chain = data->verdict.chain;
> + switch (phase) {
> + case NFT_TRANS_PREPARE_ERROR:
> + case NFT_TRANS_PREPARE:
> + nft_deactivate_chain_binding(ctx->chain, chain);
> + break;
> + case NFT_TRANS_ABORT:
> + case NFT_TRANS_RELEASE:
> + nft_deactivate_chain_binding(ctx->chain, chain);
> + fallthrough;
> + case NFT_TRANS_COMMIT:
> + nft_del_chain_binding(ctx->chain, chain);
> + break;
> + }
> +
> if (!nft_chain_binding(chain))
> break;
>
> --
> 2.30.2
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH nf-next,v1 6/6] netfilter: nf_tables: add support for validating incremental ruleset updates
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
` (4 preceding siblings ...)
2025-05-14 21:42 ` [PATCH nf-next,v1 5/6] netfilter: nf_tables: use " Pablo Neira Ayuso
@ 2025-05-14 21:42 ` Pablo Neira Ayuso
2025-05-15 12:05 ` Phil Sutter
2025-05-15 6:03 ` [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Florian Westphal
6 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-14 21:42 UTC (permalink / raw)
To: netfilter-devel
Use the new binding graph to validate incremental ruleset updates.
Perform full validation if the table is new, which is the case of the
initial ruleset reload. Subsequent incremental ruleset updates use the
validation list which contains chains with new rules or chains that can
be now reached via jump/goto from another rule/set element.
When validating a chain from commit/abort phase, backtrack to collect
the basechains that can reach this chain, then perform the validation of
rules in this chain. If no basechains can be reached, then skip
validation for this chain. However, if basechains are off the jump stack
limit, then resort to full ruleset validation. This is to prevent
inconsistent validation between the preparation and commit/abort phase
validations.
As for loop checking, stick to the existing approach which uses the jump
stack limit to detect cycles.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 188 +++++++++++++++++++++++++++++++++-
1 file changed, 183 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index e92cccc834d9..0f183abbc94f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10397,6 +10397,160 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
},
};
+struct basechain_array {
+ const struct nft_chain **basechain;
+ uint32_t num_basechains;
+ uint32_t max_basechains;
+ int max_level;
+};
+
+static int basechain_array_add(struct basechain_array *array,
+ const struct nft_chain *chain, int level)
+{
+ const struct nft_chain **new_basechain;
+ uint32_t new_max_basechains;
+
+ if (array->num_basechains == array->max_basechains) {
+ new_max_basechains = array->max_basechains + 16;
+ new_basechain = krealloc_array(array->basechain, new_max_basechains, sizeof(struct nft_chain *), GFP_KERNEL);
+ if (!new_basechain)
+ return -ENOMEM;
+
+ array->basechain = new_basechain;
+ array->max_basechains = new_max_basechains;
+ }
+ array->basechain[array->num_basechains++] = chain;
+
+ if (level > array->max_level)
+ array->max_level = level;
+
+ return 0;
+}
+
+static int nft_chain_validate_backtrack(struct basechain_array *array,
+ const struct list_head *backbinding_list,
+ int *level)
+{
+ struct nft_binding *binding;
+ int err;
+
+ /* Basechain is unreachable, fall back to slow path validation. */
+ if (*level >= NFT_JUMP_STACK_SIZE)
+ return -ENOENT;
+
+ list_for_each_entry(binding, backbinding_list, backlist) {
+ if (binding->from.type == NFT_BIND_CHAIN &&
+ binding->from.chain->flags & NFT_CHAIN_BASE &&
+ binding->use > 0) {
+ if (basechain_array_add(array, binding->from.chain, *level) < 0)
+ return -ENOMEM;
+
+ continue;
+ }
+
+ switch (binding->from.type) {
+ case NFT_BIND_CHAIN:
+ if (binding->use == 0)
+ break;
+
+ (*level)++;
+ err = nft_chain_validate_backtrack(array,
+ &binding->from.chain->backbinding_list,
+ level);
+ if (err < 0)
+ return err;
+
+ (*level)--;
+ break;
+ case NFT_BIND_SET:
+ if (binding->use == 0)
+ break;
+
+ /* no level update for sets. */
+ err = nft_chain_validate_backtrack(array,
+ &binding->from.set->backbinding_list,
+ level);
+ if (err < 0)
+ return err;
+
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int nft_chain_validate_incremental(struct net *net,
+ const struct nft_chain *chain)
+{
+ struct basechain_array array = {};
+ uint32_t i, level = 1;
+ int err;
+
+ array.max_basechains = 16;
+ array.basechain = kcalloc(16, sizeof(struct nft_chain *), GFP_KERNEL);
+ if (!array.basechain)
+ return -ENOMEM;
+
+ if (nft_is_base_chain(chain)) {
+ err = basechain_array_add(&array, chain, 0);
+ if (err < 0) {
+ kfree(array.basechain);
+ return -ENOMEM;
+ }
+ } else {
+ err = nft_chain_validate_backtrack(&array,
+ &chain->backbinding_list,
+ &level);
+ if (err < 0) {
+ kfree(array.basechain);
+ return err;
+ }
+ }
+
+ for (i = 0; i < array.num_basechains; i++) {
+ struct nft_ctx ctx = {
+ .net = net,
+ .family = chain->table->family,
+ .table = chain->table,
+ .chain = (struct nft_chain *)array.basechain[i],
+ .level = array.max_level,
+ };
+
+ if (WARN_ON_ONCE(!nft_is_base_chain(array.basechain[i])))
+ continue;
+
+ err = nft_chain_validate(&ctx, chain);
+ if (err < 0)
+ break;
+ }
+
+ kfree(array.basechain);
+
+ return err;
+}
+
+static int nft_validate_incremental(struct net *net, struct nft_table *table)
+{
+ struct nftables_pernet *nft_net = nft_pernet(net);
+ struct nft_chain *chain, *next;
+ int err;
+
+ err = 0;
+ list_for_each_entry_safe(chain, next, &nft_net->validate_list, validate_list) {
+ if (chain->table != table)
+ continue;
+
+ if (err >= 0)
+ err = nft_chain_validate_incremental(net, chain);
+
+ list_del(&chain->validate_list);
+ chain->validate = 0;
+ }
+
+ return err;
+}
+
static void nft_validate_chain_release(struct net *net)
{
struct nftables_pernet *nft_net = nft_pernet(net);
@@ -10422,12 +10576,36 @@ static int nf_tables_validate(struct net *net)
nft_validate_state_update(table, NFT_VALIDATE_DO);
fallthrough;
case NFT_VALIDATE_DO:
- err = nft_table_validate(net, table);
- if (err < 0) {
- if (err == EINTR)
- goto err_eintr;
+ /* If this table is new, then this is the initial
+ * ruleset restore, perform full table validation,
+ * otherwise, perform incremental validation.
+ */
+ if (!nft_is_active(net, table)) {
+ err = nft_table_validate(net, table);
+ if (err < 0) {
+ if (err == EINTR)
+ goto err_eintr;
- return -EAGAIN;
+ return -EAGAIN;
+ }
+ } else {
+ err = nft_validate_incremental(net, table);
+ if (err < 0) {
+ if (err != -ENOMEM && err != -ENOENT)
+ return -EAGAIN;
+
+ /* Either no memory or it cannot reach
+ * basechain, then fallback to full
+ * validation.
+ */
+ err = nft_table_validate(net, table);
+ if (err < 0) {
+ if (err == EINTR)
+ goto err_eintr;
+
+ return -EAGAIN;
+ }
+ }
}
nft_validate_state_update(table, NFT_VALIDATE_SKIP);
break;
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH nf-next,v1 6/6] netfilter: nf_tables: add support for validating incremental ruleset updates
2025-05-14 21:42 ` [PATCH nf-next,v1 6/6] netfilter: nf_tables: add support for validating incremental ruleset updates Pablo Neira Ayuso
@ 2025-05-15 12:05 ` Phil Sutter
0 siblings, 0 replies; 15+ messages in thread
From: Phil Sutter @ 2025-05-15 12:05 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, May 14, 2025 at 11:42:16PM +0200, Pablo Neira Ayuso wrote:
> Use the new binding graph to validate incremental ruleset updates.
>
> Perform full validation if the table is new, which is the case of the
> initial ruleset reload. Subsequent incremental ruleset updates use the
> validation list which contains chains with new rules or chains that can
> be now reached via jump/goto from another rule/set element.
>
> When validating a chain from commit/abort phase, backtrack to collect
> the basechains that can reach this chain, then perform the validation of
> rules in this chain. If no basechains can be reached, then skip
> validation for this chain. However, if basechains are off the jump stack
> limit, then resort to full ruleset validation. This is to prevent
> inconsistent validation between the preparation and commit/abort phase
> validations.
>
> As for loop checking, stick to the existing approach which uses the jump
> stack limit to detect cycles.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> net/netfilter/nf_tables_api.c | 188 +++++++++++++++++++++++++++++++++-
> 1 file changed, 183 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index e92cccc834d9..0f183abbc94f 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -10397,6 +10397,160 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
> },
> };
>
> +struct basechain_array {
> + const struct nft_chain **basechain;
> + uint32_t num_basechains;
> + uint32_t max_basechains;
> + int max_level;
> +};
> +
> +static int basechain_array_add(struct basechain_array *array,
> + const struct nft_chain *chain, int level)
> +{
> + const struct nft_chain **new_basechain;
> + uint32_t new_max_basechains;
> +
> + if (array->num_basechains == array->max_basechains) {
> + new_max_basechains = array->max_basechains + 16;
> + new_basechain = krealloc_array(array->basechain, new_max_basechains, sizeof(struct nft_chain *), GFP_KERNEL);
> + if (!new_basechain)
> + return -ENOMEM;
> +
> + array->basechain = new_basechain;
> + array->max_basechains = new_max_basechains;
> + }
> + array->basechain[array->num_basechains++] = chain;
> +
> + if (level > array->max_level)
> + array->max_level = level;
> +
> + return 0;
> +}
> +
> +static int nft_chain_validate_backtrack(struct basechain_array *array,
> + const struct list_head *backbinding_list,
> + int *level)
> +{
> + struct nft_binding *binding;
> + int err;
> +
> + /* Basechain is unreachable, fall back to slow path validation. */
> + if (*level >= NFT_JUMP_STACK_SIZE)
> + return -ENOENT;
> +
> + list_for_each_entry(binding, backbinding_list, backlist) {
> + if (binding->from.type == NFT_BIND_CHAIN &&
> + binding->from.chain->flags & NFT_CHAIN_BASE &&
> + binding->use > 0) {
> + if (basechain_array_add(array, binding->from.chain, *level) < 0)
> + return -ENOMEM;
> +
> + continue;
> + }
> +
> + switch (binding->from.type) {
> + case NFT_BIND_CHAIN:
> + if (binding->use == 0)
> + break;
> +
> + (*level)++;
> + err = nft_chain_validate_backtrack(array,
> + &binding->from.chain->backbinding_list,
> + level);
> + if (err < 0)
> + return err;
> +
> + (*level)--;
It looks like you're trying to record the max level value for error
path, but I don't see it used in callers. Is this a leftover from before
introduction of basechain_array::max_level? If so, one may just pass
'level + 1' by value to the recursive call, right?
> + break;
> + case NFT_BIND_SET:
> + if (binding->use == 0)
> + break;
> +
> + /* no level update for sets. */
> + err = nft_chain_validate_backtrack(array,
> + &binding->from.set->backbinding_list,
> + level);
> + if (err < 0)
> + return err;
> +
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int nft_chain_validate_incremental(struct net *net,
> + const struct nft_chain *chain)
> +{
> + struct basechain_array array = {};
> + uint32_t i, level = 1;
> + int err;
> +
> + array.max_basechains = 16;
> + array.basechain = kcalloc(16, sizeof(struct nft_chain *), GFP_KERNEL);
> + if (!array.basechain)
> + return -ENOMEM;
> +
> + if (nft_is_base_chain(chain)) {
> + err = basechain_array_add(&array, chain, 0);
> + if (err < 0) {
> + kfree(array.basechain);
> + return -ENOMEM;
> + }
> + } else {
> + err = nft_chain_validate_backtrack(&array,
> + &chain->backbinding_list,
> + &level);
> + if (err < 0) {
> + kfree(array.basechain);
> + return err;
> + }
> + }
> +
> + for (i = 0; i < array.num_basechains; i++) {
> + struct nft_ctx ctx = {
> + .net = net,
> + .family = chain->table->family,
> + .table = chain->table,
> + .chain = (struct nft_chain *)array.basechain[i],
> + .level = array.max_level,
> + };
> +
> + if (WARN_ON_ONCE(!nft_is_base_chain(array.basechain[i])))
> + continue;
> +
> + err = nft_chain_validate(&ctx, chain);
> + if (err < 0)
> + break;
> + }
> +
> + kfree(array.basechain);
> +
> + return err;
> +}
> +
> +static int nft_validate_incremental(struct net *net, struct nft_table *table)
> +{
> + struct nftables_pernet *nft_net = nft_pernet(net);
> + struct nft_chain *chain, *next;
> + int err;
> +
> + err = 0;
> + list_for_each_entry_safe(chain, next, &nft_net->validate_list, validate_list) {
> + if (chain->table != table)
> + continue;
> +
> + if (err >= 0)
> + err = nft_chain_validate_incremental(net, chain);
Is there a future use-case for err > 0? I don't see where that function
would return a positive value. If this is not to change, I guess just
checking err || !err would simplify things a bit and also avoid the
EINTR != !EINTR pitfall (see below).
> +
> + list_del(&chain->validate_list);
> + chain->validate = 0;
> + }
> +
> + return err;
> +}
> +
> static void nft_validate_chain_release(struct net *net)
> {
> struct nftables_pernet *nft_net = nft_pernet(net);
> @@ -10422,12 +10576,36 @@ static int nf_tables_validate(struct net *net)
> nft_validate_state_update(table, NFT_VALIDATE_DO);
> fallthrough;
> case NFT_VALIDATE_DO:
> - err = nft_table_validate(net, table);
> - if (err < 0) {
> - if (err == EINTR)
This should be -EINTR, right?
> - goto err_eintr;
> + /* If this table is new, then this is the initial
> + * ruleset restore, perform full table validation,
> + * otherwise, perform incremental validation.
> + */
> + if (!nft_is_active(net, table)) {
> + err = nft_table_validate(net, table);
> + if (err < 0) {
> + if (err == EINTR)
Same here?
> + goto err_eintr;
>
> - return -EAGAIN;
> + return -EAGAIN;
> + }
> + } else {
> + err = nft_validate_incremental(net, table);
> + if (err < 0) {
> + if (err != -ENOMEM && err != -ENOENT)
> + return -EAGAIN;
> +
> + /* Either no memory or it cannot reach
> + * basechain, then fallback to full
> + * validation.
> + */
> + err = nft_table_validate(net, table);
> + if (err < 0) {
> + if (err == EINTR)
And here as well?
Cheers, Phil
> + goto err_eintr;
> +
> + return -EAGAIN;
> + }
> + }
> }
> nft_validate_state_update(table, NFT_VALIDATE_SKIP);
> break;
> --
> 2.30.2
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation
2025-05-14 21:42 [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Pablo Neira Ayuso
` (5 preceding siblings ...)
2025-05-14 21:42 ` [PATCH nf-next,v1 6/6] netfilter: nf_tables: add support for validating incremental ruleset updates Pablo Neira Ayuso
@ 2025-05-15 6:03 ` Florian Westphal
2025-05-19 15:10 ` Pablo Neira Ayuso
6 siblings, 1 reply; 15+ messages in thread
From: Florian Westphal @ 2025-05-15 6:03 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Pablo Neira Ayuso (6):
> netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path
Do this via nf.git?
> netfilter: nf_tables: honor validation state in preparation phase
> netfilter: nf_tables: add infrastructure for chain validation on updates
> netfilter: nf_tables: add new binding infrastructure
> netfilter: nf_tables: use new binding infrastructure
> netfilter: nf_tables: add support for validating incremental ruleset updates
>
> include/net/netfilter/nf_tables.h | 52 +-
> net/netfilter/nf_tables_api.c | 800 ++++++++++++++++++++++++++++--
> net/netfilter/nft_immediate.c | 25 +-
> 3 files changed, 844 insertions(+), 33 deletions(-)
This is a lot of new code but no explanation as to why is given.
Does this fix bugs with the existing scheme?
Or is this an optimization? If so, how big is the speedup?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation
2025-05-15 6:03 ` [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation Florian Westphal
@ 2025-05-19 15:10 ` Pablo Neira Ayuso
2025-05-20 10:42 ` Florian Westphal
0 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-19 15:10 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
Hi Florian,
On Thu, May 15, 2025 at 08:03:13AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Pablo Neira Ayuso (6):
> > netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path
>
> Do this via nf.git?
nf-next.git should be fine, this late in the development cycle.
> > netfilter: nf_tables: honor validation state in preparation phase
> > netfilter: nf_tables: add infrastructure for chain validation on updates
> > netfilter: nf_tables: add new binding infrastructure
> > netfilter: nf_tables: use new binding infrastructure
> > netfilter: nf_tables: add support for validating incremental ruleset updates
> >
> > include/net/netfilter/nf_tables.h | 52 +-
> > net/netfilter/nf_tables_api.c | 800 ++++++++++++++++++++++++++++--
> > net/netfilter/nft_immediate.c | 25 +-
> > 3 files changed, 844 insertions(+), 33 deletions(-)
>
> This is a lot of new code but no explanation as to why is given.
These are the results of a test program I made, which is incrementally
adding elements to an already populated verdict map with 100k elements.
The ruleset validation shows in the perf callgraph, for each new
element that is added:
55.06% nft-buffer [kernel.kallsyms] [k] nft_chain_validate
7.68% nft-buffer [kernel.kallsyms] [k] nf_tables_commit
7.19% nft-buffer [kernel.kallsyms] [k] nft_table_validate
5.34% nft-buffer [kernel.kallsyms] [k] nft_rhash_walk
2.82% swapper [kernel.kallsyms] [k] mwait_idle_with_hints.constprop.0
1.26% nft-buffer [kernel.kallsyms] [k] __rhashtable_walk_find_next
1.09% nft-buffer [kernel.kallsyms] [k] nft_setelem_validate
0.94% nft-buffer [kernel.kallsyms] [k] rhashtable_walk_next
0.54% nft-buffer libnftables.so.1.1.0 [.] nft_parse
This is a test adding 100 new elements including jump to chain in a
existing 100k verdict map.
With this approach, there is no need to fully validate the chain
graph.
> Does this fix bugs with the existing scheme?
The two initial patches are targetted at fixing minor issues. The
remaining patches are new code, they are passing tests/shell with
CONFIG_KASAN and CONFIG_KMEMLEAK. I will post v2 but I would like to
run more fuzz test on the error path.
> Or is this an optimization? If so, how big is the speedup?
Optimization. After this series, validation does not show up near the
top 10; this is the first symbol in the perf call graph:
0.03% nft-buffer [nf_tables] [k] __nft_chain_validate
and it is far from the top 10.
I can include this information in v2 so they can sit in the mailing
list for a while, there is a few bugs in v1 that I have addressed.
Phil has spotted one in them.
Moreover, I can move the bindings hashtable to make it per-net to
control the maximum number of jump/goto per family. This is a lot
larger than Shaun's update, you have to tell me your preference on
this.
Old binding code can possibly go away, but that requires closer look
too.
Let me know.
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation
2025-05-19 15:10 ` Pablo Neira Ayuso
@ 2025-05-20 10:42 ` Florian Westphal
2025-05-20 11:29 ` Pablo Neira Ayuso
0 siblings, 1 reply; 15+ messages in thread
From: Florian Westphal @ 2025-05-20 10:42 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, May 15, 2025 at 08:03:13AM +0200, Florian Westphal wrote:
> > > netfilter: nf_tables: honor EINTR in ruleset validation from commit/abort path
> >
> > Do this via nf.git?
>
> nf-next.git should be fine, this late in the development cycle.
Right, agreed.
> > > netfilter: nf_tables: honor validation state in preparation phase
> > > netfilter: nf_tables: add infrastructure for chain validation on updates
> > > netfilter: nf_tables: add new binding infrastructure
> > > netfilter: nf_tables: use new binding infrastructure
> > > netfilter: nf_tables: add support for validating incremental ruleset updates
> > >
> > > include/net/netfilter/nf_tables.h | 52 +-
> > > net/netfilter/nf_tables_api.c | 800 ++++++++++++++++++++++++++++--
> > > net/netfilter/nft_immediate.c | 25 +-
> > > 3 files changed, 844 insertions(+), 33 deletions(-)
> >
> > This is a lot of new code but no explanation as to why is given.
>
> These are the results of a test program I made, which is incrementally
> adding elements to an already populated verdict map with 100k elements.
>
> The ruleset validation shows in the perf callgraph, for each new
> element that is added:
>
> 55.06% nft-buffer [kernel.kallsyms] [k] nft_chain_validate
> 7.68% nft-buffer [kernel.kallsyms] [k] nf_tables_commit
> 7.19% nft-buffer [kernel.kallsyms] [k] nft_table_validate
> 5.34% nft-buffer [kernel.kallsyms] [k] nft_rhash_walk
> 2.82% swapper [kernel.kallsyms] [k] mwait_idle_with_hints.constprop.0
> 1.26% nft-buffer [kernel.kallsyms] [k] __rhashtable_walk_find_next
> 1.09% nft-buffer [kernel.kallsyms] [k] nft_setelem_validate
> 0.94% nft-buffer [kernel.kallsyms] [k] rhashtable_walk_next
> 0.54% nft-buffer libnftables.so.1.1.0 [.] nft_parse
>
> This is a test adding 100 new elements including jump to chain in a
> existing 100k verdict map.
>
> With this approach, there is no need to fully validate the chain
> graph.
>
> > Does this fix bugs with the existing scheme?
>
> The two initial patches are targetted at fixing minor issues. The
> remaining patches are new code, they are passing tests/shell with
> CONFIG_KASAN and CONFIG_KMEMLEAK. I will post v2 but I would like to
> run more fuzz test on the error path.
>
> > Or is this an optimization? If so, how big is the speedup?
>
> Optimization. After this series, validation does not show up near the
> top 10; this is the first symbol in the perf call graph:
>
> 0.03% nft-buffer [nf_tables] [k] __nft_chain_validate
>
> and it is far from the top 10.
>
> I can include this information in v2 so they can sit in the mailing
> list for a while, there is a few bugs in v1 that I have addressed.
> Phil has spotted one in them.
>
> Moreover, I can move the bindings hashtable to make it per-net to
> control the maximum number of jump/goto per family. This is a lot
> larger than Shaun's update, you have to tell me your preference on
> this.
I think Shauns approach is better due to backporting reasons.
Once the new code is stable enough you can move the jump limitation
check there.
Makes sense?
> Old binding code can possibly go away, but that requires closer look
> too.
Yes, lets keep it for now until there is confidence that the split
approach catches all issues.
I suggest to do this by adding a WARN_ON_ONCE() in the old path once
you think the new code should have prevented cycles in 100% of cases,
then remove it after e.g. 6-12 months or so.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH nf-next,v1 0/6] revisiting nf_tables ruleset validation
2025-05-20 10:42 ` Florian Westphal
@ 2025-05-20 11:29 ` Pablo Neira Ayuso
0 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2025-05-20 11:29 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Tue, May 20, 2025 at 12:42:50PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, May 15, 2025 at 08:03:13AM +0200, Florian Westphal wrote:
[...]
> > > > netfilter: nf_tables: honor validation state in preparation phase
> > > > netfilter: nf_tables: add infrastructure for chain validation on updates
> > > > netfilter: nf_tables: add new binding infrastructure
> > > > netfilter: nf_tables: use new binding infrastructure
> > > > netfilter: nf_tables: add support for validating incremental ruleset updates
> > > >
> > > > include/net/netfilter/nf_tables.h | 52 +-
> > > > net/netfilter/nf_tables_api.c | 800 ++++++++++++++++++++++++++++--
> > > > net/netfilter/nft_immediate.c | 25 +-
> > > > 3 files changed, 844 insertions(+), 33 deletions(-)
> > >
> > > This is a lot of new code but no explanation as to why is given.
> >
> > These are the results of a test program I made, which is incrementally
> > adding elements to an already populated verdict map with 100k elements.
> >
> > The ruleset validation shows in the perf callgraph, for each new
> > element that is added:
> >
> > 55.06% nft-buffer [kernel.kallsyms] [k] nft_chain_validate
> > 7.68% nft-buffer [kernel.kallsyms] [k] nf_tables_commit
> > 7.19% nft-buffer [kernel.kallsyms] [k] nft_table_validate
> > 5.34% nft-buffer [kernel.kallsyms] [k] nft_rhash_walk
> > 2.82% swapper [kernel.kallsyms] [k] mwait_idle_with_hints.constprop.0
> > 1.26% nft-buffer [kernel.kallsyms] [k] __rhashtable_walk_find_next
> > 1.09% nft-buffer [kernel.kallsyms] [k] nft_setelem_validate
> > 0.94% nft-buffer [kernel.kallsyms] [k] rhashtable_walk_next
> > 0.54% nft-buffer libnftables.so.1.1.0 [.] nft_parse
> >
> > This is a test adding 100 new elements including jump to chain in a
> > existing 100k verdict map.
> >
> > With this approach, there is no need to fully validate the chain
> > graph.
> >
> > > Does this fix bugs with the existing scheme?
> >
> > The two initial patches are targetted at fixing minor issues. The
> > remaining patches are new code, they are passing tests/shell with
> > CONFIG_KASAN and CONFIG_KMEMLEAK. I will post v2 but I would like to
> > run more fuzz test on the error path.
> >
> > > Or is this an optimization? If so, how big is the speedup?
> >
> > Optimization. After this series, validation does not show up near the
> > top 10; this is the first symbol in the perf call graph:
> >
> > 0.03% nft-buffer [nf_tables] [k] __nft_chain_validate
> >
> > and it is far from the top 10.
> >
> > I can include this information in v2 so they can sit in the mailing
> > list for a while, there is a few bugs in v1 that I have addressed.
> > Phil has spotted one in them.
> >
> > Moreover, I can move the bindings hashtable to make it per-net to
> > control the maximum number of jump/goto per family. This is a lot
> > larger than Shaun's update, you have to tell me your preference on
> > this.
>
> I think Shauns approach is better due to backporting reasons.
Yes, it is small.
I could not find a way to extend the existing loop and validation
routines to make it incremental.
> Once the new code is stable enough you can move the jump limitation
> check there.
>
> Makes sense?
OK.
> > Old binding code can possibly go away, but that requires closer look
> > too.
>
> Yes, lets keep it for now until there is confidence that the split
> approach catches all issues.
>
> I suggest to do this by adding a WARN_ON_ONCE() in the old path once
> you think the new code should have prevented cycles in 100% of cases,
> then remove it after e.g. 6-12 months or so.
This is how it works in v2:
On new table, the classic full ruleset validation from basechain is
done.
On existing tables, from the chain that has been updated (either new
rule or new jump/goto to reach it), the incremental routine backtracks
to find the basechain. If chain callstack is too deep, then it falls
back to the classic full ruleset validation from basechains. If it
detects a loop when backtracking, it also falls back to classic full
ruleset validation (it could be a loop in chains that are not
connected to basechain, this is currently allowed). Then, from the
chain that has been updated, call the existing nft_validate_chain()
using the collected basechains that can reach such updated chain. The
nft_validate_chain() function already relies on too deep callstack to
detect loops.
So the backtracking is used to find and collect the basechains that
can reach this chain.
Loop and checking if expression is accepted from this chain (eg.
masquerade from filter chain type) is done in one single pass.
Incremental is a fast path, but in case backtracking cannot collect
basechains, then it fallbacks to classic ruleset validation.
I will look into the WARN_ON_ONCE() you suggest.
^ permalink raw reply [flat|nested] 15+ messages in thread