From dda33be9e3fadf0b47e2afc8a0b381c3667622c3 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@...>
Date: Wed, 29 Aug 2018 14:41:32 +0200
Subject: [PATCH 1/5] netfilter: nf_tables: asynchronous release

Release the committed transaction log from a work queue, moving
expensive synchronize_rcu out of the locked section and providing
opportunity to batch this.

On my test machine this cuts runtime of nft-test.py in half.
Based on earlier patch from Pablo Neira Ayuso.

Signed-off-by: Florian Westphal <fw@...>
Signed-off-by: Pablo Neira Ayuso <pablo@...>
(cherry picked from commit 0935d558840099b3679c67bb7468dc78fcbad940)
---
include/net/netfilter/nf_tables.h | 2 ++
net/netfilter/nf_tables_api.c | 56 +++++++++++++++++++++++++++----
2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 93253ba1eeac..c60d281c9a58 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1316,12 +1316,14 @@ static inline void nft_set_elem_clear_busy(struct nft_set_ext *ext)
*
* @list: used internally
* @msg_type: message type
+ * @put_net: ctx->net needs to be put
* @ctx: transaction context
* @data: internal information related to the transaction
*/
struct nft_trans {
struct list_head list;
int msg_type;
+ bool put_net;
struct nft_ctx ctx;
char data[0];
};
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9cc8e92f4b00..02e577e8fb8a 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -29,6 +29,8 @@
static LIST_HEAD(nf_tables_expressions);
static LIST_HEAD(nf_tables_objects);
static LIST_HEAD(nf_tables_flowtables);
+static LIST_HEAD(nf_tables_destroy_list);
+static DEFINE_SPINLOCK(nf_tables_destroy_list_lock);
static u64 table_handle;

enum {
@@ -66,6 +68,8 @@ static void nft_validate_state_update(struct net *net, u8 new_validate_state)

net->nft.validate_state = new_validate_state;
}
+static void nf_tables_trans_destroy_work(struct work_struct *w);
+static DECLARE_WORK(trans_destroy_work, nf_tables_trans_destroy_work);

static void nft_ctx_init(struct nft_ctx *ctx,
struct net *net,
@@ -2503,7 +2507,6 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx,
{
struct nft_expr *expr, *next;

- lockdep_assert_held(&ctx->net->nft.commit_mutex);
/*
* Careful: some expressions might not be initialized in case this
* is called on error from nf_tables_newrule().
@@ -6300,19 +6303,28 @@ static void nft_commit_release(struct nft_trans *trans)
nf_tables_flowtable_destroy(nft_trans_flowtable(trans));
break;
}
+
+ if (trans->put_net)
+ put_net(trans->ctx.net);
+
kfree(trans);
}

-static void nf_tables_commit_release(struct net *net)
+static void nf_tables_trans_destroy_work(struct work_struct *w)
{
struct nft_trans *trans, *next;
+ LIST_HEAD(head);
+
+ spin_lock(&nf_tables_destroy_list_lock);
+ list_splice_init(&nf_tables_destroy_list, &head);
+ spin_unlock(&nf_tables_destroy_list_lock);

- if (list_empty(&net->nft.commit_list))
+ if (list_empty(&head))
return;

synchronize_rcu();

- list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+ list_for_each_entry_safe(trans, next, &head, list) {
list_del(&trans->list);
nft_commit_release(trans);
}
@@ -6443,6 +6455,37 @@ static void nft_chain_del(struct nft_chain *chain)
list_del_rcu(&chain->list);
}

+static void nf_tables_commit_release(struct net *net)
+{
+ struct nft_trans *trans;
+
+ /* all side effects have to be made visible.
+ * For example, if a chain named 'foo' has been deleted, a
+ * new transaction must not find it anymore.
+ *
+ * Memory reclaim happens asynchronously from work queue
+ * to prevent expensive synchronize_rcu() in commit phase.
+ */
+ if (list_empty(&net->nft.commit_list)) {
+ mutex_unlock(&net->nft.commit_mutex);
+ return;
+ }
+
+ trans = list_last_entry(&net->nft.commit_list,
+ struct nft_trans, list);
+ get_net(trans->ctx.net);
+ WARN_ON_ONCE(trans->put_net);
+
+ trans->put_net = true;
+ spin_lock(&nf_tables_destroy_list_lock);
+ list_splice_tail_init(&net->nft.commit_list, &nf_tables_destroy_list);
+ spin_unlock(&nf_tables_destroy_list_lock);
+
+ mutex_unlock(&net->nft.commit_mutex);
+
+ schedule_work(&trans_destroy_work);
+}
+
static int nf_tables_commit(struct net *net, struct sk_buff *skb)
{
struct nft_trans *trans, *next;
@@ -6604,9 +6647,8 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
}
}

- nf_tables_commit_release(net);
nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
- mutex_unlock(&net->nft.commit_mutex);
+ nf_tables_commit_release(net);

return 0;
}
@@ -7387,6 +7429,7 @@ static int __init nf_tables_module_init(void)
{
int err;

+ spin_lock_init(&nf_tables_destroy_list_lock);
err = register_pernet_subsys(&nf_tables_net_ops);
if (err < 0)
return err;
@@ -7426,6 +7469,7 @@ static void __exit nf_tables_module_exit(void)
unregister_netdevice_notifier(&nf_tables_flowtable_notifier);
nft_chain_filter_fini();
unregister_pernet_subsys(&nf_tables_net_ops);
+ cancel_work_sync(&trans_destroy_work);
rcu_barrier();
nf_tables_core_module_exit();
}
--
2.34.1

Hi!

Ok, let me take a look at the series. But... could you provide some
kind of explanation of what these are and why you want them in -cip?

These are not usual "hardware enablement" patches...

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Hi!

Ok, let me take a look at the series. But... could you provide some
kind of explanation of what these are and why you want them in -cip?

These are not usual "hardware enablement" patches...

0/6 email was not part of the thread, so I missed it in the first
look.

I see it fixes sysbot report.

Do you have special loads where you are hitting these netfilter
problems? Are similar fixes needed in 4.4 / 5.10?

Would it make sense to get it fixed in -stable kernels?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

On Mon, Jan 10, 2022 at 07:06:35PM +0100, Pavel Machek wrote:

Hi!

Ok, let me take a look at the series. But... could you provide some
kind of explanation of what these are and why you want them in -cip?

These are not usual "hardware enablement" patches...

0/6 email was not part of the thread, so I missed it in the first
look.

I see it fixes sysbot report.

Do you have special loads where you are hitting these netfilter
problems? Are similar fixes needed in 4.4 / 5.10?
Would it make sense to get it fixed in -stable kernels?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

We haven't been able to reproduce the issue but apparently it's being reproduced
periodically.

The offending patch doesn't appear in linux-4.4.y-cip
linux-5.10.y-cip has the fix already.

Both patches were introduced in 5.5.0-rc5

Thanks.
Amy

Hi!

Ok, let me take a look at the series. But... could you provide some
kind of explanation of what these are and why you want them in -cip?

These are not usual "hardware enablement" patches...

0/6 email was not part of the thread, so I missed it in the first
look.

I see it fixes sysbot report.

Do you have special loads where you are hitting these netfilter
problems? Are similar fixes needed in 4.4 / 5.10?
Would it make sense to get it fixed in -stable kernels?

We haven't been able to reproduce the issue but apparently it's being reproduced
periodically.

The offending patch doesn't appear in linux-4.4.y-cip
linux-5.10.y-cip has the fix already.

Both patches were introduced in 5.5.0-rc5

Thank you for answers.

Is there reason not to submit it to stable? You are describing it is
as a bugfix, and Greg takes those. [Advantages are a) community
review, b) less patches to maintain for us, c) bug fixed for everyone,
not just us].

Best regards,

Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

On Tue, Jan 11, 2022 at 10:02:18AM +0100, Pavel Machek wrote:

Hi!

Ok, let me take a look at the series. But... could you provide some
kind of explanation of what these are and why you want them in -cip?

These are not usual "hardware enablement" patches...

0/6 email was not part of the thread, so I missed it in the first
look.

I see it fixes sysbot report.

Do you have special loads where you are hitting these netfilter
problems? Are similar fixes needed in 4.4 / 5.10?
Would it make sense to get it fixed in -stable kernels?

We haven't been able to reproduce the issue but apparently it's being reproduced
periodically.

The offending patch doesn't appear in linux-4.4.y-cip
linux-5.10.y-cip has the fix already.

Both patches were introduced in 5.5.0-rc5

Thank you for answers.

Is there reason not to submit it to stable? You are describing it is
as a bugfix, and Greg takes those. [Advantages are a) community
review, b) less patches to maintain for us, c) bug fixed for everyone,
not just us].

Best regards,

Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

I will submit the patches to stable.

Thanks.
Amy

Hi!

Is there reason not to submit it to stable? You are describing it is
as a bugfix, and Greg takes those. [Advantages are a) community
review, b) less patches to maintain for us, c) bug fixed for everyone,
not just us].

I will submit the patches to stable.

Thank you. The patches look good to me. You may want to include sha1
hash on the first line of changelog as

[ Upstream commit c1833c3964d5bd8c163bd4e01736a38bc473cb8a ]

...as that's what's usually used in stable AFAICT. If they apply to
more than one -stable version, you should mention that. Feel free to
Cc me.

Thanks and best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

On 11.01.22 18:53, Pavel Machek wrote:

Hi!

Is there reason not to submit it to stable? You are describing it is
as a bugfix, and Greg takes those. [Advantages are a) community
review, b) less patches to maintain for us, c) bug fixed for everyone,
not just us].

I will submit the patches to stable.

Thank you. The patches look good to me. You may want to include sha1
hash on the first line of changelog as

[ Upstream commit c1833c3964d5bd8c163bd4e01736a38bc473cb8a ]

...as that's what's usually used in stable AFAICT. If they apply to
more than one -stable version, you should mention that. Feel free to
Cc me.

Thanks and best regards,
Pavel

A colleague just pointed out that we still have these internally while
4.19-lts is missing them. Did you follow up with LTS on the series back
then, Amy? Was there any push-back?

Jan

--
Siemens AG, Technology
Competence Center Embedded Linux