* [PATCH] netfilter: Fix wastful cleanup check for unconfirmed conn in get_next_corpse
@ 2014-10-21 0:31 Feng Gao
[not found] ` <CA+6hz4oodWXXyWhcR=Zt4Esf1Riwatvy0xYMhFnQ8CkTaqV=Pg@mail.gmail.com>
0 siblings, 1 reply; 2+ messages in thread
From: Feng Gao @ 2014-10-21 0:31 UTC (permalink / raw)
To: Pablo Neira Ayuso, Patrick McHardy, kadlec, davem
Cc: Netfilter Developer Mailing List, coreteam, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 788 bytes --]
Hi all,
I am sorry to send the patch commit again because the last email is
not plain text and is rejected by some servers.
This is the patch to branch master of kernel.
The function get_next_corpse is only invoked by nf_ct_iterate_cleanup
in one while loop, and it will check the per cpu unconfirmed conntrack
list every time.
I think the whole list of unconfirmed conntracks could be accessed by
one call, so the others are not necessary.
So I move the checks outside the get_next_corpse, and create one new
function clean_up_unconfirmed_conntracks.
Let the nf_ct_iterate_cleanup invokes the
clean_up_unconfirmed_conntracks after the while loop.
These codes have already exist for a long time. Firstly I think maybe
there is some reason, but I fail to get it.
Best Regards
Feng
[-- Attachment #2: 0001-netfilter-Fix-wastful-cleanup-check-for-unconfirmed-.patch --]
[-- Type: application/octet-stream, Size: 2964 bytes --]
From 52de457b3cfd1e94a52df1c3dfcd9dbf3511fa0d Mon Sep 17 00:00:00 2001
From: fgao <gfree.wind@gmail.com>
Date: Mon, 20 Oct 2014 07:18:05 -0700
Subject: [PATCH 1/1] netfilter: Fix wastful cleanup check for unconfirmed conn in get_next_corpse
The function get_next_corpse is used to iterate the conntracks.
It will check the per cpu unconfirmed list of every cpu too.
Now it is only invoked by nf_ct_iterate_cleanup in one while loop.
Actually the unconfirmed list could be accessed completely by one call, then the others are wastful.
So move the unconfirmed list check outside the function get_next_corpse and create one new function
Let the nf_ct_iterate_cleanup invokes the new function clean_up_unconfirmed_conntracks once after the loops.
Signed-off-by: fgao <gfree.wind@gmail.com>
---
net/netfilter/nf_conntrack_core.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 5016a69..ace7c2c2 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1348,6 +1348,28 @@ static void nf_conntrack_attach(struct sk_buff *nskb, const struct sk_buff *skb)
nf_conntrack_get(nskb->nfct);
}
+static void clean_up_unconfirmed_conntracks(struct net *net,
+ int (*iter)(struct nf_conn *i, void *data),
+ void *data)
+{
+ struct nf_conntrack_tuple_hash *h;
+ struct nf_conn *ct;
+ struct hlist_nulls_node *n;
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ struct ct_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) {
+ ct = nf_ct_tuplehash_to_ctrack(h);
+ if (iter(ct, data))
+ set_bit(IPS_DYING_BIT, &ct->status);
+ }
+ spin_unlock_bh(&pcpu->lock);
+ }
+}
+
/* Bring out ya dead! */
static struct nf_conn *
get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
@@ -1356,7 +1378,6 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
struct nf_conntrack_tuple_hash *h;
struct nf_conn *ct;
struct hlist_nulls_node *n;
- int cpu;
spinlock_t *lockp;
for (; *bucket < net->ct.htable_size; (*bucket)++) {
@@ -1376,17 +1397,6 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
local_bh_enable();
}
- for_each_possible_cpu(cpu) {
- struct ct_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) {
- ct = nf_ct_tuplehash_to_ctrack(h);
- if (iter(ct, data))
- set_bit(IPS_DYING_BIT, &ct->status);
- }
- spin_unlock_bh(&pcpu->lock);
- }
return NULL;
found:
atomic_inc(&ct->ct_general.use);
@@ -1411,6 +1421,8 @@ void nf_ct_iterate_cleanup(struct net *net,
nf_ct_put(ct);
}
+
+ clean_up_unconfirmed_conntracks(net, iter, data);
}
EXPORT_SYMBOL_GPL(nf_ct_iterate_cleanup);
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread[parent not found: <CA+6hz4oodWXXyWhcR=Zt4Esf1Riwatvy0xYMhFnQ8CkTaqV=Pg@mail.gmail.com>]
* Re: [PATCH] netfilter: Fix wastful cleanup check for unconfirmed conn in get_next_corpse [not found] ` <CA+6hz4oodWXXyWhcR=Zt4Esf1Riwatvy0xYMhFnQ8CkTaqV=Pg@mail.gmail.com> @ 2014-10-21 13:48 ` Feng Gao 0 siblings, 0 replies; 2+ messages in thread From: Feng Gao @ 2014-10-21 13:48 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, kadlec, davem Cc: Netfilter Developer Mailing List, coreteam, netdev, linux-kernel Sorry. I get it is not an issue after read the codes again. The unconfirmed conn list check is only checked once in the current codes. Because it will be checked only when no matched conntracks found in function get_next_corpse. Then I think current codes may confuse the reader. I am an example. So could my changes be as the enhancement ? Best Regards Feng On Tue, Oct 21, 2014 at 10:47 AM, Feng Gao <gfree.wind@gmail.com> wrote: > Paste my changes directly instead of the attachment. > > Subject: [PATCH 1/1] netfilter: Fix wastful cleanup check for unconfirmed > conn in get_next_corpse > > The function get_next_corpse is used to iterate the conntracks. > It will check the per cpu unconfirmed list of every cpu too. > Now it is only invoked by nf_ct_iterate_cleanup in one while loop. > Actually the unconfirmed list could be accessed completely by one call, then > the others are wastful. > > So move the unconfirmed list check outside the function get_next_corpse and > create one new function > Let the nf_ct_iterate_cleanup invokes the new function > clean_up_unconfirmed_conntracks once after the loops. > > Signed-off-by: fgao <gfree.wind@gmail.com> > --- > net/netfilter/nf_conntrack_core.c | 36 ++++++++++++++++++++++++------------ > 1 file changed, 24 insertions(+), 12 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_core.c > b/net/netfilter/nf_conntrack_core.c > index 5016a69..ace7c2c2 100644 > --- a/net/netfilter/nf_conntrack_core.c > +++ b/net/netfilter/nf_conntrack_core.c > @@ -1348,6 +1348,28 @@ static void nf_conntrack_attach(struct sk_buff *nskb, > const struct sk_buff *skb) > nf_conntrack_get(nskb->nfct); > } > > +static void clean_up_unconfirmed_conntracks(struct net *net, > + int (*iter)(struct nf_conn *i, void *data), > + void *data) > +{ > + struct nf_conntrack_tuple_hash *h; > + struct nf_conn *ct; > + struct hlist_nulls_node *n; > + int cpu; > + > + for_each_possible_cpu(cpu) { > + struct ct_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) { > + ct = nf_ct_tuplehash_to_ctrack(h); > + if (iter(ct, data)) > + set_bit(IPS_DYING_BIT, &ct->status); > + } > + spin_unlock_bh(&pcpu->lock); > + } > +} > + > /* Bring out ya dead! */ > static struct nf_conn * > get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void > *data), > @@ -1356,7 +1378,6 @@ get_next_corpse(struct net *net, int (*iter)(struct > nf_conn *i, void *data), > struct nf_conntrack_tuple_hash *h; > struct nf_conn *ct; > struct hlist_nulls_node *n; > - int cpu; > spinlock_t *lockp; > > for (; *bucket < net->ct.htable_size; (*bucket)++) { > @@ -1376,17 +1397,6 @@ get_next_corpse(struct net *net, int (*iter)(struct > nf_conn *i, void *data), > local_bh_enable(); > } > > - for_each_possible_cpu(cpu) { > - struct ct_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) { > - ct = nf_ct_tuplehash_to_ctrack(h); > - if (iter(ct, data)) > - set_bit(IPS_DYING_BIT, &ct->status); > - } > - spin_unlock_bh(&pcpu->lock); > - } > return NULL; > found: > atomic_inc(&ct->ct_general.use); > @@ -1411,6 +1421,8 @@ void nf_ct_iterate_cleanup(struct net *net, > > nf_ct_put(ct); > } > + > + clean_up_unconfirmed_conntracks(net, iter, data); > } > EXPORT_SYMBOL_GPL(nf_ct_iterate_cleanup); > > -- > 1.9.1 > > > On Tue, Oct 21, 2014 at 8:31 AM, Feng Gao <gfree.wind@gmail.com> wrote: >> >> Hi all, >> >> I am sorry to send the patch commit again because the last email is >> not plain text and is rejected by some servers. >> >> This is the patch to branch master of kernel. >> >> The function get_next_corpse is only invoked by nf_ct_iterate_cleanup >> in one while loop, and it will check the per cpu unconfirmed conntrack >> list every time. >> >> I think the whole list of unconfirmed conntracks could be accessed by >> one call, so the others are not necessary. >> >> So I move the checks outside the get_next_corpse, and create one new >> function clean_up_unconfirmed_conntracks. >> Let the nf_ct_iterate_cleanup invokes the >> clean_up_unconfirmed_conntracks after the while loop. >> >> These codes have already exist for a long time. Firstly I think maybe >> there is some reason, but I fail to get it. >> >> >> Best Regards >> Feng > > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-10-21 13:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21 0:31 [PATCH] netfilter: Fix wastful cleanup check for unconfirmed conn in get_next_corpse Feng Gao
[not found] ` <CA+6hz4oodWXXyWhcR=Zt4Esf1Riwatvy0xYMhFnQ8CkTaqV=Pg@mail.gmail.com>
2014-10-21 13:48 ` Feng Gao
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).