netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org, kuba@kernel.org,
	pabeni@redhat.com
Subject: [PATCH net-next 08/17] netfilter: conntrack: remove __nf_ct_unconfirmed_destroy
Date: Fri, 13 May 2022 23:43:20 +0200	[thread overview]
Message-ID: <20220513214329.1136459-9-pablo@netfilter.org> (raw)
In-Reply-To: <20220513214329.1136459-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Its not needed anymore:

A. If entry is totally new, then the rcu-protected resource
must already have been removed from global visibility before call
to nf_ct_iterate_destroy.

B. If entry was allocated before, but is not yet in the hash table
   (uncofirmed case), genid gets incremented and synchronize_rcu() call
   makes sure access has completed.

C. Next attempt to peek at extension area will fail for unconfirmed
  conntracks, because ext->genid != genid.

D. Conntracks in the hash are iterated as before.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_core.c   | 46 ++++++++---------------------
 net/netfilter/nf_conntrack_helper.c |  5 ----
 net/netfilter/nfnetlink_cttimeout.c |  1 -
 3 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 76310940cbd7..7b4b3f5db959 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2457,34 +2457,6 @@ static int iter_net_only(struct nf_conn *i, void *data)
 	return d->iter(i, d->data);
 }
 
-static void
-__nf_ct_unconfirmed_destroy(struct net *net)
-{
-	int cpu;
-
-	for_each_possible_cpu(cpu) {
-		struct nf_conntrack_tuple_hash *h;
-		struct hlist_nulls_node *n;
-		struct ct_pcpu *pcpu;
-
-		pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu);
-
-		spin_lock_bh(&pcpu->lock);
-		hlist_nulls_for_each_entry(h, n, &pcpu->unconfirmed, hnnode) {
-			struct nf_conn *ct;
-
-			ct = nf_ct_tuplehash_to_ctrack(h);
-
-			/* we cannot call iter() on unconfirmed list, the
-			 * owning cpu can reallocate ct->ext at any time.
-			 */
-			set_bit(IPS_DYING_BIT, &ct->status);
-		}
-		spin_unlock_bh(&pcpu->lock);
-		cond_resched();
-	}
-}
-
 void nf_ct_iterate_cleanup_net(struct net *net,
 			       int (*iter)(struct nf_conn *i, void *data),
 			       void *data, u32 portid, int report)
@@ -2527,26 +2499,34 @@ nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)
 
 		if (atomic_read(&cnet->count) == 0)
 			continue;
-		__nf_ct_unconfirmed_destroy(net);
 		nf_queue_nf_hook_drop(net);
 	}
 	up_read(&net_rwsem);
 
 	/* Need to wait for netns cleanup worker to finish, if its
 	 * running -- it might have deleted a net namespace from
-	 * the global list, so our __nf_ct_unconfirmed_destroy() might
-	 * not have affected all namespaces.
+	 * the global list, so hook drop above might not have
+	 * affected all namespaces.
 	 */
 	net_ns_barrier();
 
-	/* a conntrack could have been unlinked from unconfirmed list
-	 * before we grabbed pcpu lock in __nf_ct_unconfirmed_destroy().
+	/* a skb w. unconfirmed conntrack could have been reinjected just
+	 * before we called nf_queue_nf_hook_drop().
+	 *
 	 * This makes sure its inserted into conntrack table.
 	 */
 	synchronize_net();
 
 	nf_ct_ext_bump_genid();
 	nf_ct_iterate_cleanup(iter, data, 0, 0);
+
+	/* Another cpu might be in a rcu read section with
+	 * rcu protected pointer cleared in iter callback
+	 * or hidden via nf_ct_ext_bump_genid() above.
+	 *
+	 * Wait until those are done.
+	 */
+	synchronize_rcu();
 }
 EXPORT_SYMBOL_GPL(nf_ct_iterate_destroy);
 
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 8dec42ec603e..c12a87ebc3ee 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -468,11 +468,6 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 
 	nf_ct_expect_iterate_destroy(expect_iter_me, NULL);
 	nf_ct_iterate_destroy(unhelp, me);
-
-	/* Maybe someone has gotten the helper already when unhelp above.
-	 * So need to wait it.
-	 */
-	synchronize_rcu();
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_helper_unregister);
 
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 9bc4ebe65faa..f069c24c6146 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -674,7 +674,6 @@ static void __exit cttimeout_exit(void)
 	RCU_INIT_POINTER(nf_ct_timeout_hook, NULL);
 
 	nf_ct_iterate_destroy(untimeout, NULL);
-	synchronize_rcu();
 }
 
 module_init(cttimeout_init);
-- 
2.30.2


  parent reply	other threads:[~2022-05-13 21:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-13 21:43 [PATCH net-next 00/17] Netfilter updates for net-next Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 01/17] netfilter: ecache: use dedicated list for event redelivery Pablo Neira Ayuso
2022-05-16  9:20   ` patchwork-bot+netdevbpf
2022-05-13 21:43 ` [PATCH net-next 02/17] netfilter: conntrack: include ecache dying list in dumps Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 03/17] netfilter: conntrack: remove the percpu dying list Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 04/17] netfilter: cttimeout: decouple unlink and free on netns destruction Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 05/17] netfilter: remove nf_ct_unconfirmed_destroy helper Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 06/17] netfilter: extensions: introduce extension genid count Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 07/17] netfilter: cttimeout: decouple unlink and free on netns destruction Pablo Neira Ayuso
2022-05-13 21:43 ` Pablo Neira Ayuso [this message]
2022-05-13 21:43 ` [PATCH net-next 09/17] netfilter: conntrack: remove unconfirmed list Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 10/17] netfilter: conntrack: avoid unconditional local_bh_disable Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 11/17] netfilter: conntrack: add nf_ct_iter_data object for nf_ct_iterate_cleanup*() Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 12/17] netfilter: nfnetlink: allow to detect if ctnetlink listeners exist Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 13/17] netfilter: conntrack: un-inline nf_ct_ecache_ext_add Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 14/17] netfilter: conntrack: add nf_conntrack_events autodetect mode Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 15/17] netfilter: prefer extension check to pointer check Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 16/17] netfilter: flowtable: nft_flow_route use more data for reverse route Pablo Neira Ayuso
2022-05-13 21:43 ` [PATCH net-next 17/17] netfilter: conntrack: skip verification of zero UDP checksum Pablo Neira Ayuso
  -- strict thread matches above, loose matches on Subject: below --
2022-05-10 12:21 [PATCH net-next 00/17] Netfilter updates for net-next Pablo Neira Ayuso
2022-05-10 12:21 ` [PATCH net-next 08/17] netfilter: conntrack: remove __nf_ct_unconfirmed_destroy Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220513214329.1136459-9-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).