* [PATCH nf 1/4] netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention
@ 2023-09-08 1:30 Pablo Neira Ayuso
2023-09-08 1:30 ` [PATCH nf 2/4] netfilter: nft_set_pipapo: call nft_trans_gc_queue_sync() in catchall GC Pablo Neira Ayuso
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2023-09-08 1:30 UTC (permalink / raw)
To: netfilter-devel
rbtree GC does not modify the datastructure, instead it collects expired
elements and it enqueues a GC transaction. Use a read spinlock instead
to avoid data contention while GC worker is running.
Fixes: f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_set_rbtree.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index f250b5399344..70491ba98dec 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -622,8 +622,7 @@ static void nft_rbtree_gc(struct work_struct *work)
if (!gc)
goto done;
- write_lock_bh(&priv->lock);
- write_seqcount_begin(&priv->count);
+ read_lock_bh(&priv->lock);
for (node = rb_first(&priv->root); node != NULL; node = rb_next(node)) {
/* Ruleset has been updated, try later. */
@@ -673,8 +672,7 @@ static void nft_rbtree_gc(struct work_struct *work)
gc = nft_trans_gc_catchall(gc, gc_seq);
try_later:
- write_seqcount_end(&priv->count);
- write_unlock_bh(&priv->lock);
+ read_unlock_bh(&priv->lock);
if (gc)
nft_trans_gc_queue_async_done(gc);
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH nf 2/4] netfilter: nft_set_pipapo: call nft_trans_gc_queue_sync() in catchall GC 2023-09-08 1:30 [PATCH nf 1/4] netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention Pablo Neira Ayuso @ 2023-09-08 1:30 ` Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 3/4] netfilter: nft_set_pipapo: stop GC iteration if GC transaction allocation fails Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 4/4] netfilter: nft_set_hash: try later when GC hits EAGAIN on iteration Pablo Neira Ayuso 2 siblings, 0 replies; 4+ messages in thread From: Pablo Neira Ayuso @ 2023-09-08 1:30 UTC (permalink / raw) To: netfilter-devel pipapo needs to enqueue GC transactions for catchall elements through nft_trans_gc_queue_sync(). Add nft_trans_gc_catchall_sync() and nft_trans_gc_catchall_async() to handle GC transaction queueing accordingly. Fixes: 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane") Fixes: f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- include/net/netfilter/nf_tables.h | 5 +++-- net/netfilter/nf_tables_api.c | 22 +++++++++++++++++++--- net/netfilter/nft_set_hash.c | 2 +- net/netfilter/nft_set_pipapo.c | 2 +- net/netfilter/nft_set_rbtree.c | 2 +- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index dd40c75011d2..a4455f4995ab 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -1700,8 +1700,9 @@ void nft_trans_gc_queue_sync_done(struct nft_trans_gc *trans); void nft_trans_gc_elem_add(struct nft_trans_gc *gc, void *priv); -struct nft_trans_gc *nft_trans_gc_catchall(struct nft_trans_gc *gc, - unsigned int gc_seq); +struct nft_trans_gc *nft_trans_gc_catchall_async(struct nft_trans_gc *gc, + unsigned int gc_seq); +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, diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 895c6e4fba97..7b59311931fb 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -9613,8 +9613,9 @@ void nft_trans_gc_queue_sync_done(struct nft_trans_gc *trans) call_rcu(&trans->rcu, nft_trans_gc_trans_free); } -struct nft_trans_gc *nft_trans_gc_catchall(struct nft_trans_gc *gc, - unsigned int gc_seq) +static struct nft_trans_gc *nft_trans_gc_catchall(struct nft_trans_gc *gc, + unsigned int gc_seq, + bool sync) { struct nft_set_elem_catchall *catchall; const struct nft_set *set = gc->set; @@ -9630,7 +9631,11 @@ struct nft_trans_gc *nft_trans_gc_catchall(struct nft_trans_gc *gc, nft_set_elem_dead(ext); dead_elem: - gc = nft_trans_gc_queue_async(gc, gc_seq, GFP_ATOMIC); + if (sync) + gc = nft_trans_gc_queue_sync(gc, GFP_ATOMIC); + else + gc = nft_trans_gc_queue_async(gc, gc_seq, GFP_ATOMIC); + if (!gc) return NULL; @@ -9640,6 +9645,17 @@ struct nft_trans_gc *nft_trans_gc_catchall(struct nft_trans_gc *gc, return gc; } +struct nft_trans_gc *nft_trans_gc_catchall_async(struct nft_trans_gc *gc, + unsigned int gc_seq) +{ + return nft_trans_gc_catchall(gc, gc_seq, false); +} + +struct nft_trans_gc *nft_trans_gc_catchall_sync(struct nft_trans_gc *gc) +{ + return nft_trans_gc_catchall(gc, 0, true); +} + static void nf_tables_module_autoload_cleanup(struct net *net) { struct nftables_pernet *nft_net = nft_pernet(net); diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index 524763659f25..eca20dc60138 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -372,7 +372,7 @@ static void nft_rhash_gc(struct work_struct *work) nft_trans_gc_elem_add(gc, he); } - gc = nft_trans_gc_catchall(gc, gc_seq); + gc = nft_trans_gc_catchall_async(gc, gc_seq); try_later: /* catchall list iteration requires rcu read side lock. */ diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c index 6af9c9ed4b5c..10b89ac74476 100644 --- a/net/netfilter/nft_set_pipapo.c +++ b/net/netfilter/nft_set_pipapo.c @@ -1610,7 +1610,7 @@ static void pipapo_gc(const struct nft_set *_set, struct nft_pipapo_match *m) } } - gc = nft_trans_gc_catchall(gc, 0); + gc = nft_trans_gc_catchall_sync(gc); if (gc) { nft_trans_gc_queue_sync_done(gc); priv->last_gc = jiffies; diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c index 70491ba98dec..487572dcd614 100644 --- a/net/netfilter/nft_set_rbtree.c +++ b/net/netfilter/nft_set_rbtree.c @@ -669,7 +669,7 @@ static void nft_rbtree_gc(struct work_struct *work) nft_trans_gc_elem_add(gc, rbe); } - gc = nft_trans_gc_catchall(gc, gc_seq); + gc = nft_trans_gc_catchall_async(gc, gc_seq); try_later: read_unlock_bh(&priv->lock); -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH nf 3/4] netfilter: nft_set_pipapo: stop GC iteration if GC transaction allocation fails 2023-09-08 1:30 [PATCH nf 1/4] netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 2/4] netfilter: nft_set_pipapo: call nft_trans_gc_queue_sync() in catchall GC Pablo Neira Ayuso @ 2023-09-08 1:30 ` Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 4/4] netfilter: nft_set_hash: try later when GC hits EAGAIN on iteration Pablo Neira Ayuso 2 siblings, 0 replies; 4+ messages in thread From: Pablo Neira Ayuso @ 2023-09-08 1:30 UTC (permalink / raw) To: netfilter-devel nft_trans_gc_queue_sync() enqueues the GC transaction and it allocates a new one. If this allocation fails, then stop this GC sync run and retry later. Fixes: 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nft_set_pipapo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c index 10b89ac74476..c0dcc40de358 100644 --- a/net/netfilter/nft_set_pipapo.c +++ b/net/netfilter/nft_set_pipapo.c @@ -1596,7 +1596,7 @@ static void pipapo_gc(const struct nft_set *_set, struct nft_pipapo_match *m) gc = nft_trans_gc_queue_sync(gc, GFP_ATOMIC); if (!gc) - break; + return; nft_pipapo_gc_deactivate(net, set, e); pipapo_drop(m, rulemap); -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH nf 4/4] netfilter: nft_set_hash: try later when GC hits EAGAIN on iteration 2023-09-08 1:30 [PATCH nf 1/4] netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 2/4] netfilter: nft_set_pipapo: call nft_trans_gc_queue_sync() in catchall GC Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 3/4] netfilter: nft_set_pipapo: stop GC iteration if GC transaction allocation fails Pablo Neira Ayuso @ 2023-09-08 1:30 ` Pablo Neira Ayuso 2 siblings, 0 replies; 4+ messages in thread From: Pablo Neira Ayuso @ 2023-09-08 1:30 UTC (permalink / raw) To: netfilter-devel Skip GC run if iterator rewinds to the beginning with EAGAIN, otherwise GC might collect the same element more than once. Fixes: f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nft_set_hash.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index eca20dc60138..2013de934cef 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -338,12 +338,9 @@ static void nft_rhash_gc(struct work_struct *work) while ((he = rhashtable_walk_next(&hti))) { if (IS_ERR(he)) { - if (PTR_ERR(he) != -EAGAIN) { - nft_trans_gc_destroy(gc); - gc = NULL; - goto try_later; - } - continue; + nft_trans_gc_destroy(gc); + gc = NULL; + goto try_later; } /* Ruleset has been updated, try later. */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-09-08 1:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-08 1:30 [PATCH nf 1/4] netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 2/4] netfilter: nft_set_pipapo: call nft_trans_gc_queue_sync() in catchall GC Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 3/4] netfilter: nft_set_pipapo: stop GC iteration if GC transaction allocation fails Pablo Neira Ayuso 2023-09-08 1:30 ` [PATCH nf 4/4] netfilter: nft_set_hash: try later when GC hits EAGAIN on iteration Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).