Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Patrick McHardy wrote:
> Thanks, the previous approach doesn't seem to work properly without
> unpleasant event cache hacks. This patch takes a simpler approach
> and keeps the unconfirmed list iteration, but makes sure to make
> forward progress.
>
>
>
>
>
> [NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
>
> Fix {nf,ip}_ct_iterate_cleanup unconfirmed list handling:
>
Works great: survived three reboots without lockup or warning messages.
And it's a nice simple patch, too...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote:
> Patrick McHardy wrote:
>
>>This patch is against current stable-2.6.20, it applies
>>cleanly to 2.6.20 as well.
>
>
> Everything works OK, but I get:
>
> BUG: warning: (!list_empty()) at
> net/netfilter/nf_conntrack_core.c:1068/nf_ct_cleanup()
>
> <> nf_ct_cleanup+0x66/0x122 [nf_conntrack]
> <> kill_l4proto+0x0 [nf_conntrack]
> <> nf_conntrack_l4proto_unregister+0x7d/0x82 [nf_conntrack]
> <> nf_conntrack_l3proto_ipv4_fini+0x3c/0x46 [nf_conntrack_ipv4]
> <> sys_delete_module+0x18a/0x1b1
Thanks, the previous approach doesn't seem to work properly without
unpleasant event cache hacks. This patch takes a simpler approach
and keeps the unconfirmed list iteration, but makes sure to make
forward progress.
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
Fix {nf,ip}_ct_iterate_cleanup unconfirmed list handling:
- unconfirmed entries can not be killed manually, they are removed on
confirmation or final destruction of the conntrack entry, which means
we might iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
What the code really wants to do is make sure the references of the hash
table to the selected conntrack entries are released, so they will be
destroyed once all references from skbs and the event cache are dropped.
Since unconfirmed entries haven't even entered the hash yet, simply mark
them as dying and skip confirmation based on that.
Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>
---
commit 841de8621862a5406d2a236dd142a5e5db167d25
tree 347d137f0d6466f0b4da83256bfbeaa4150f105a
parent 8d1117a9f5d302d8d460fbe7ef322b382e45c9ce
author Patrick McHardy <[EMAIL PROTECTED]> Mon, 26 Feb 2007 18:48:05 +0100
committer Patrick McHardy <[EMAIL PROTECTED]> Wed, 28 Feb 2007 19:02:00 +0100
include/linux/netfilter_ipv4/ip_conntrack_core.h |2 +-
include/net/netfilter/nf_conntrack_core.h|2 +-
net/ipv4/netfilter/ip_conntrack_core.c |2 +-
net/netfilter/nf_conntrack_core.c|2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/netfilter_ipv4/ip_conntrack_core.h
b/include/linux/netfilter_ipv4/ip_conntrack_core.h
index 907d4f5..e3a6df0 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack_core.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack_core.h
@@ -45,7 +45,7 @@ static inline int ip_conntrack_confirm(s
int ret = NF_ACCEPT;
if (ct) {
- if (!is_confirmed(ct))
+ if (!is_confirmed(ct) && !is_dying(ct))
ret = __ip_conntrack_confirm(pskb);
ip_ct_deliver_cached_events(ct);
}
diff --git a/include/net/netfilter/nf_conntrack_core.h
b/include/net/netfilter/nf_conntrack_core.h
index 7fdc72c..85634e1 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -64,7 +64,7 @@ static inline int nf_conntrack_confirm(s
int ret = NF_ACCEPT;
if (ct) {
- if (!nf_ct_is_confirmed(ct))
+ if (!nf_ct_is_confirmed(ct) && !nf_ct_is_dying(ct))
ret = __nf_conntrack_confirm(pskb);
nf_ct_deliver_cached_events(ct);
}
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c
b/net/ipv4/netfilter/ip_conntrack_core.c
index 8556a4f..f8b3009 100644
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -1242,7 +1242,7 @@ get_next_corpse(int (*iter)(struct ip_co
list_for_each_entry(h, , list) {
ct = tuplehash_to_ctrack(h);
if (iter(ct, data))
- goto found;
+ set_bit(IPS_DYING_BIT, >status);
}
write_unlock_bh(_conntrack_lock);
return NULL;
diff --git a/net/netfilter/nf_conntrack_core.c
b/net/netfilter/nf_conntrack_core.c
index 9b02ec4..cb29ba7 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1052,7 +1052,7 @@ get_next_corpse(int (*iter)(struct nf_co
list_for_each_entry(h, , list) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (iter(ct, data))
- goto found;
+ set_bit(IPS_DYING_BIT, >status);
}
write_unlock_bh(_conntrack_lock);
return NULL;
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote:
Patrick McHardy wrote:
This patch is against current stable-2.6.20, it applies
cleanly to 2.6.20 as well.
Everything works OK, but I get:
BUG: warning: (!list_empty(unconfirmed)) at
net/netfilter/nf_conntrack_core.c:1068/nf_ct_cleanup()
nf_ct_cleanup+0x66/0x122 [nf_conntrack]
kill_l4proto+0x0 [nf_conntrack]
nf_conntrack_l4proto_unregister+0x7d/0x82 [nf_conntrack]
nf_conntrack_l3proto_ipv4_fini+0x3c/0x46 [nf_conntrack_ipv4]
sys_delete_module+0x18a/0x1b1
Thanks, the previous approach doesn't seem to work properly without
unpleasant event cache hacks. This patch takes a simpler approach
and keeps the unconfirmed list iteration, but makes sure to make
forward progress.
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
Fix {nf,ip}_ct_iterate_cleanup unconfirmed list handling:
- unconfirmed entries can not be killed manually, they are removed on
confirmation or final destruction of the conntrack entry, which means
we might iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
What the code really wants to do is make sure the references of the hash
table to the selected conntrack entries are released, so they will be
destroyed once all references from skbs and the event cache are dropped.
Since unconfirmed entries haven't even entered the hash yet, simply mark
them as dying and skip confirmation based on that.
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
---
commit 841de8621862a5406d2a236dd142a5e5db167d25
tree 347d137f0d6466f0b4da83256bfbeaa4150f105a
parent 8d1117a9f5d302d8d460fbe7ef322b382e45c9ce
author Patrick McHardy [EMAIL PROTECTED] Mon, 26 Feb 2007 18:48:05 +0100
committer Patrick McHardy [EMAIL PROTECTED] Wed, 28 Feb 2007 19:02:00 +0100
include/linux/netfilter_ipv4/ip_conntrack_core.h |2 +-
include/net/netfilter/nf_conntrack_core.h|2 +-
net/ipv4/netfilter/ip_conntrack_core.c |2 +-
net/netfilter/nf_conntrack_core.c|2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/netfilter_ipv4/ip_conntrack_core.h
b/include/linux/netfilter_ipv4/ip_conntrack_core.h
index 907d4f5..e3a6df0 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack_core.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack_core.h
@@ -45,7 +45,7 @@ static inline int ip_conntrack_confirm(s
int ret = NF_ACCEPT;
if (ct) {
- if (!is_confirmed(ct))
+ if (!is_confirmed(ct) !is_dying(ct))
ret = __ip_conntrack_confirm(pskb);
ip_ct_deliver_cached_events(ct);
}
diff --git a/include/net/netfilter/nf_conntrack_core.h
b/include/net/netfilter/nf_conntrack_core.h
index 7fdc72c..85634e1 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -64,7 +64,7 @@ static inline int nf_conntrack_confirm(s
int ret = NF_ACCEPT;
if (ct) {
- if (!nf_ct_is_confirmed(ct))
+ if (!nf_ct_is_confirmed(ct) !nf_ct_is_dying(ct))
ret = __nf_conntrack_confirm(pskb);
nf_ct_deliver_cached_events(ct);
}
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c
b/net/ipv4/netfilter/ip_conntrack_core.c
index 8556a4f..f8b3009 100644
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -1242,7 +1242,7 @@ get_next_corpse(int (*iter)(struct ip_co
list_for_each_entry(h, unconfirmed, list) {
ct = tuplehash_to_ctrack(h);
if (iter(ct, data))
- goto found;
+ set_bit(IPS_DYING_BIT, ct-status);
}
write_unlock_bh(ip_conntrack_lock);
return NULL;
diff --git a/net/netfilter/nf_conntrack_core.c
b/net/netfilter/nf_conntrack_core.c
index 9b02ec4..cb29ba7 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1052,7 +1052,7 @@ get_next_corpse(int (*iter)(struct nf_co
list_for_each_entry(h, unconfirmed, list) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (iter(ct, data))
- goto found;
+ set_bit(IPS_DYING_BIT, ct-status);
}
write_unlock_bh(nf_conntrack_lock);
return NULL;
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Patrick McHardy wrote:
Thanks, the previous approach doesn't seem to work properly without
unpleasant event cache hacks. This patch takes a simpler approach
and keeps the unconfirmed list iteration, but makes sure to make
forward progress.
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
Fix {nf,ip}_ct_iterate_cleanup unconfirmed list handling:
Works great: survived three reboots without lockup or warning messages.
And it's a nice simple patch, too...
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Patrick McHardy wrote: > This patch is against current stable-2.6.20, it applies > cleanly to 2.6.20 as well. > Everything works OK, but I get: BUG: warning: (!list_empty()) at net/netfilter/nf_conntrack_core.c:1068/nf_ct_cleanup() <> nf_ct_cleanup+0x66/0x122 [nf_conntrack] <> kill_l4proto+0x0 [nf_conntrack] <> nf_conntrack_l4proto_unregister+0x7d/0x82 [nf_conntrack] <> nf_conntrack_l3proto_ipv4_fini+0x3c/0x46 [nf_conntrack_ipv4] <> sys_delete_module+0x18a/0x1b1 ... netconsole doesn't seem to work as a module for me, so hand copied from screen. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote:
> Changes to nf_nat_core.c, ip_nat_core.c and nf_conntrack_proto.c
> do not apply, and nf_conntrack_core.c changes are already there.
>
> This is vanilla 2.6.20; looks like there have been a bunch
> of changes since then. I tried adding all of the RCU patches
> but even they won't apply.
It seems to conflict with the net/ whitespace cleanup we had since
then. This patch is against current stable-2.6.20, it applies
cleanly to 2.6.20 as well.
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
{nf,ip}_ct_iterate_cleanup iterate over the unconfirmed list for cleaning
up conntrack entries, which is wrong for multiple reasons:
- unconfirmed entries can not be killed manually, which means we might
iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
Split ip_ct_iterate_cleanup in ip_ct_iterate, which iterates over both
confirmed and unconfirmed entries, but doesn't attempt to kill them,
and ip_ct_cleanup, which makes sure no unconfirmed entries exist by
calling synchronize_net() prior to walking the conntrack hash.
Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>
---
commit 7934f9001e23be06a1a5cbce1b68e2a64cf31a6e
tree fe74c1500d086d67071d323e84ce95d3e2faf650
parent 8d1117a9f5d302d8d460fbe7ef322b382e45c9ce
author Patrick McHardy <[EMAIL PROTECTED]> Mon, 26 Feb 2007 18:48:05 +0100
committer Patrick McHardy <[EMAIL PROTECTED]> Mon, 26 Feb 2007 18:48:05 +0100
include/linux/netfilter_ipv4/ip_conntrack.h |5 ++--
include/net/netfilter/nf_conntrack.h |5 +++-
include/net/netfilter/nf_nat_rule.h |3 ++
net/ipv4/netfilter/ip_conntrack_core.c | 30 ++--
net/ipv4/netfilter/ip_conntrack_standalone.c |5 ++--
net/ipv4/netfilter/ip_nat_core.c |5 ++--
net/ipv4/netfilter/ipt_MASQUERADE.c |4 ++-
net/ipv4/netfilter/nf_nat_core.c |7 ++
net/netfilter/nf_conntrack_core.c| 33 --
net/netfilter/nf_conntrack_proto.c |4 ++-
10 files changed, 69 insertions(+), 32 deletions(-)
diff --git a/include/linux/netfilter_ipv4/ip_conntrack.h
b/include/linux/netfilter_ipv4/ip_conntrack.h
index 33581c1..8eb45e7 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -250,8 +250,9 @@ ip_ct_gather_frags(struct sk_buff *skb,
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-ip_ct_iterate_cleanup(int (*iter)(struct ip_conntrack *i, void *data),
- void *data);
+ip_ct_cleanup(int (*iter)(struct ip_conntrack *i, void *data), void *data);
+extern void
+ip_ct_iterate(void (*iter)(struct ip_conntrack *i, void *data), void *data);
extern struct ip_conntrack_helper *
__ip_conntrack_helper_find_byname(const char *);
diff --git a/include/net/netfilter/nf_conntrack.h
b/include/net/netfilter/nf_conntrack.h
index bd01b46..520ca0e 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -231,7 +231,10 @@ extern int nf_ct_no_defrag;
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-nf_ct_iterate_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+nf_ct_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+extern void
+nf_ct_iterate(void (*iter)(struct nf_conn *i, void *data), void *data);
+
extern void nf_conntrack_free(struct nf_conn *ct);
extern struct nf_conn *
nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
diff --git a/include/net/netfilter/nf_nat_rule.h
b/include/net/netfilter/nf_nat_rule.h
index f191c67..2f16114 100644
--- a/include/net/netfilter/nf_nat_rule.h
+++ b/include/net/netfilter/nf_nat_rule.h
@@ -11,7 +11,8 @@ #define ip_conntrack_get nf_ct_get
#define ip_conntrack nf_conn
#define ip_nat_setup_info nf_nat_setup_info
#define ip_nat_multi_range_compat nf_nat_multi_range_compat
-#define ip_ct_iterate_cleanup nf_ct_iterate_cleanup
+#define ip_ct_cleanup nf_ct_cleanup
+#define ip_ct_iterate nf_ct_iterate
#defineIP_NF_ASSERTNF_CT_ASSERT
extern int nf_nat_rule_init(void) __init;
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c
b/net/ipv4/netfilter/ip_conntrack_core.c
index 8556a4f..d2d36b3 100644
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -1239,11 +1239,6 @@ get_next_corpse(int (*iter)(struct ip_co
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Patrick McHardy wrote:
> Chuck Ebbert wrote:
>
> Can you try this patch please?
>
>
> [NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
>
> include/linux/netfilter_ipv4/ip_conntrack.h |5 ++--
> include/net/netfilter/nf_conntrack.h |5 +++-
> include/net/netfilter/nf_nat_rule.h |3 ++
> net/ipv4/netfilter/ip_conntrack_core.c | 30 ++--
> net/ipv4/netfilter/ip_conntrack_standalone.c |5 ++--
> net/ipv4/netfilter/ip_nat_core.c |5 ++--
> net/ipv4/netfilter/ipt_MASQUERADE.c |4 ++-
> net/ipv4/netfilter/nf_nat_core.c |7 ++
> net/netfilter/nf_conntrack_core.c| 33
> --
> net/netfilter/nf_conntrack_proto.c |4 ++-
> 10 files changed, 69 insertions(+), 32 deletions(-)
>
Changes to nf_nat_core.c, ip_nat_core.c and nf_conntrack_proto.c
do not apply, and nf_conntrack_core.c changes are already there.
This is vanilla 2.6.20; looks like there have been a bunch
of changes since then. I tried adding all of the RCU patches
but even they won't apply.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Patrick McHardy wrote:
Chuck Ebbert wrote:
Can you try this patch please?
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
include/linux/netfilter_ipv4/ip_conntrack.h |5 ++--
include/net/netfilter/nf_conntrack.h |5 +++-
include/net/netfilter/nf_nat_rule.h |3 ++
net/ipv4/netfilter/ip_conntrack_core.c | 30 ++--
net/ipv4/netfilter/ip_conntrack_standalone.c |5 ++--
net/ipv4/netfilter/ip_nat_core.c |5 ++--
net/ipv4/netfilter/ipt_MASQUERADE.c |4 ++-
net/ipv4/netfilter/nf_nat_core.c |7 ++
net/netfilter/nf_conntrack_core.c| 33
--
net/netfilter/nf_conntrack_proto.c |4 ++-
10 files changed, 69 insertions(+), 32 deletions(-)
Changes to nf_nat_core.c, ip_nat_core.c and nf_conntrack_proto.c
do not apply, and nf_conntrack_core.c changes are already there.
This is vanilla 2.6.20; looks like there have been a bunch
of changes since then. I tried adding all of the RCU patches
but even they won't apply.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote:
Changes to nf_nat_core.c, ip_nat_core.c and nf_conntrack_proto.c
do not apply, and nf_conntrack_core.c changes are already there.
This is vanilla 2.6.20; looks like there have been a bunch
of changes since then. I tried adding all of the RCU patches
but even they won't apply.
It seems to conflict with the net/ whitespace cleanup we had since
then. This patch is against current stable-2.6.20, it applies
cleanly to 2.6.20 as well.
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
{nf,ip}_ct_iterate_cleanup iterate over the unconfirmed list for cleaning
up conntrack entries, which is wrong for multiple reasons:
- unconfirmed entries can not be killed manually, which means we might
iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
Split ip_ct_iterate_cleanup in ip_ct_iterate, which iterates over both
confirmed and unconfirmed entries, but doesn't attempt to kill them,
and ip_ct_cleanup, which makes sure no unconfirmed entries exist by
calling synchronize_net() prior to walking the conntrack hash.
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
---
commit 7934f9001e23be06a1a5cbce1b68e2a64cf31a6e
tree fe74c1500d086d67071d323e84ce95d3e2faf650
parent 8d1117a9f5d302d8d460fbe7ef322b382e45c9ce
author Patrick McHardy [EMAIL PROTECTED] Mon, 26 Feb 2007 18:48:05 +0100
committer Patrick McHardy [EMAIL PROTECTED] Mon, 26 Feb 2007 18:48:05 +0100
include/linux/netfilter_ipv4/ip_conntrack.h |5 ++--
include/net/netfilter/nf_conntrack.h |5 +++-
include/net/netfilter/nf_nat_rule.h |3 ++
net/ipv4/netfilter/ip_conntrack_core.c | 30 ++--
net/ipv4/netfilter/ip_conntrack_standalone.c |5 ++--
net/ipv4/netfilter/ip_nat_core.c |5 ++--
net/ipv4/netfilter/ipt_MASQUERADE.c |4 ++-
net/ipv4/netfilter/nf_nat_core.c |7 ++
net/netfilter/nf_conntrack_core.c| 33 --
net/netfilter/nf_conntrack_proto.c |4 ++-
10 files changed, 69 insertions(+), 32 deletions(-)
diff --git a/include/linux/netfilter_ipv4/ip_conntrack.h
b/include/linux/netfilter_ipv4/ip_conntrack.h
index 33581c1..8eb45e7 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -250,8 +250,9 @@ ip_ct_gather_frags(struct sk_buff *skb,
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-ip_ct_iterate_cleanup(int (*iter)(struct ip_conntrack *i, void *data),
- void *data);
+ip_ct_cleanup(int (*iter)(struct ip_conntrack *i, void *data), void *data);
+extern void
+ip_ct_iterate(void (*iter)(struct ip_conntrack *i, void *data), void *data);
extern struct ip_conntrack_helper *
__ip_conntrack_helper_find_byname(const char *);
diff --git a/include/net/netfilter/nf_conntrack.h
b/include/net/netfilter/nf_conntrack.h
index bd01b46..520ca0e 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -231,7 +231,10 @@ extern int nf_ct_no_defrag;
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-nf_ct_iterate_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+nf_ct_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+extern void
+nf_ct_iterate(void (*iter)(struct nf_conn *i, void *data), void *data);
+
extern void nf_conntrack_free(struct nf_conn *ct);
extern struct nf_conn *
nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
diff --git a/include/net/netfilter/nf_nat_rule.h
b/include/net/netfilter/nf_nat_rule.h
index f191c67..2f16114 100644
--- a/include/net/netfilter/nf_nat_rule.h
+++ b/include/net/netfilter/nf_nat_rule.h
@@ -11,7 +11,8 @@ #define ip_conntrack_get nf_ct_get
#define ip_conntrack nf_conn
#define ip_nat_setup_info nf_nat_setup_info
#define ip_nat_multi_range_compat nf_nat_multi_range_compat
-#define ip_ct_iterate_cleanup nf_ct_iterate_cleanup
+#define ip_ct_cleanup nf_ct_cleanup
+#define ip_ct_iterate nf_ct_iterate
#defineIP_NF_ASSERTNF_CT_ASSERT
extern int nf_nat_rule_init(void) __init;
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c
b/net/ipv4/netfilter/ip_conntrack_core.c
index 8556a4f..d2d36b3 100644
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -1239,11 +1239,6 @@ get_next_corpse(int (*iter)(struct ip_co
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Patrick McHardy wrote: This patch is against current stable-2.6.20, it applies cleanly to 2.6.20 as well. Everything works OK, but I get: BUG: warning: (!list_empty(unconfirmed)) at net/netfilter/nf_conntrack_core.c:1068/nf_ct_cleanup() nf_ct_cleanup+0x66/0x122 [nf_conntrack] kill_l4proto+0x0 [nf_conntrack] nf_conntrack_l4proto_unregister+0x7d/0x82 [nf_conntrack] nf_conntrack_l3proto_ipv4_fini+0x3c/0x46 [nf_conntrack_ipv4] sys_delete_module+0x18a/0x1b1 ... netconsole doesn't seem to work as a module for me, so hand copied from screen. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
On Sun, 2007-02-25 at 18:31 +0100, Patrick McHardy wrote:
> [NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
>
> {nf,ip}_ct_iterate_cleanup iterate over the unconfirmed list for cleaning
> up conntrack entries, which is wrong for multiple reasons:
>
> - unconfirmed entries can not be killed manually, which means we might
> iterate forever without making forward progress.
>
> This can happen in combination with the conntrack event cache, which
> holds a reference to the conntrack entry, which is only released when
> the packet makes it all the way through the stack or a different
> packet is handled.
>
> - taking references to an unconfirmed entry and using it outside the
> locked section doesn't work, the list entries are not refcounted and
> another CPU might already be waiting to destroy the entry
>
> Split ip_ct_iterate_cleanup in ip_ct_iterate, which iterates over both
> confirmed and unconfirmed entries, but doesn't attempt to kill them,
> and ip_ct_cleanup, which makes sure no unconfirmed entries exist by
> calling synchronize_net() prior to walking the conntrack hash.
What about this case:
1. Conntrack entry is created and placed on the unconfirmed list
2. The event cache bumps the refcount of the conntrack entry
3. module removal of ip_conntrack unregisters all hooks
4. packet is dropped by an iptables rule
5. packet is freed but we still have a refcount on the conntrack entry
Now there's no way to get that refcount to decrease as that only happens
when the event cache receives another packet or the current packet makes
it through the stack as you wrote above. And neither of this will happen
since we unregistered the hooks providing the packets and dropped the
packet.
I ran into this case a while ago during stresstesting and rewrote the
event cache to not increase the refcount, but this has the drawback that
events caused by dropped packets won't be reported. This may not be a
good thing...
Old patch can be found here:
http://performance.netfilter.org/patches/nf_conntrack_ecache-fix
--
/Martin
signature.asc
Description: This is a digitally signed message part
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Martin Josefsson wrote: > What about this case: > > 1. Conntrack entry is created and placed on the unconfirmed list > 2. The event cache bumps the refcount of the conntrack entry > 3. module removal of ip_conntrack unregisters all hooks > 4. packet is dropped by an iptables rule > 5. packet is freed but we still have a refcount on the conntrack entry > > Now there's no way to get that refcount to decrease as that only happens > when the event cache receives another packet or the current packet makes > it through the stack as you wrote above. And neither of this will happen > since we unregistered the hooks providing the packets and dropped the > packet. The event cache is flushed on conntrack module unload after the hooks have been unregistered, which should release all references. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote:
> It is the standard Fedora script "iptables" and it recursively
> unloads all referring modules, then the base ones: ip_tables and
> ip_conntrack. I can't tell which module it stopped on because
> that computer is at home today.
>
> And yes, it really locks up until it's caught by the softlockup
> watchdog. Has only happened on a uniprocessor i686 system so far,
> but happens every time on shutdown.
Can you try this patch please?
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
{nf,ip}_ct_iterate_cleanup iterate over the unconfirmed list for cleaning
up conntrack entries, which is wrong for multiple reasons:
- unconfirmed entries can not be killed manually, which means we might
iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
Split ip_ct_iterate_cleanup in ip_ct_iterate, which iterates over both
confirmed and unconfirmed entries, but doesn't attempt to kill them,
and ip_ct_cleanup, which makes sure no unconfirmed entries exist by
calling synchronize_net() prior to walking the conntrack hash.
Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>
---
commit 08ace206d2703e377efebe1852539177458593ff
tree 1d27d4759a77db062d48c4420459c810e7fec14a
parent 9654640d0af8f2de40ff3807d3695109d3463f54
author Patrick McHardy <[EMAIL PROTECTED]> Sun, 25 Feb 2007 17:57:21 +0100
committer Patrick McHardy <[EMAIL PROTECTED]> Sun, 25 Feb 2007 18:28:22 +0100
include/linux/netfilter_ipv4/ip_conntrack.h |5 ++--
include/net/netfilter/nf_conntrack.h |5 +++-
include/net/netfilter/nf_nat_rule.h |3 ++
net/ipv4/netfilter/ip_conntrack_core.c | 30 ++--
net/ipv4/netfilter/ip_conntrack_standalone.c |5 ++--
net/ipv4/netfilter/ip_nat_core.c |5 ++--
net/ipv4/netfilter/ipt_MASQUERADE.c |4 ++-
net/ipv4/netfilter/nf_nat_core.c |7 ++
net/netfilter/nf_conntrack_core.c| 33 --
net/netfilter/nf_conntrack_proto.c |4 ++-
10 files changed, 69 insertions(+), 32 deletions(-)
diff --git a/include/linux/netfilter_ipv4/ip_conntrack.h
b/include/linux/netfilter_ipv4/ip_conntrack.h
index da9274e..90b12b2 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -250,8 +250,9 @@ ip_ct_gather_frags(struct sk_buff *skb,
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-ip_ct_iterate_cleanup(int (*iter)(struct ip_conntrack *i, void *data),
- void *data);
+ip_ct_cleanup(int (*iter)(struct ip_conntrack *i, void *data), void *data);
+extern void
+ip_ct_iterate(void (*iter)(struct ip_conntrack *i, void *data), void *data);
extern struct ip_conntrack_helper *
__ip_conntrack_helper_find_byname(const char *);
diff --git a/include/net/netfilter/nf_conntrack.h
b/include/net/netfilter/nf_conntrack.h
index 0e690e3..a9271df 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -233,7 +233,10 @@ extern int nf_ct_no_defrag;
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-nf_ct_iterate_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+nf_ct_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+extern void
+nf_ct_iterate(void (*iter)(struct nf_conn *i, void *data), void *data);
+
extern void nf_conntrack_free(struct nf_conn *ct);
extern struct nf_conn *
nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
diff --git a/include/net/netfilter/nf_nat_rule.h
b/include/net/netfilter/nf_nat_rule.h
index f191c67..2f16114 100644
--- a/include/net/netfilter/nf_nat_rule.h
+++ b/include/net/netfilter/nf_nat_rule.h
@@ -11,7 +11,8 @@ #define ip_conntrack_get nf_ct_get
#define ip_conntrack nf_conn
#define ip_nat_setup_info nf_nat_setup_info
#define ip_nat_multi_range_compat nf_nat_multi_range_compat
-#define ip_ct_iterate_cleanup nf_ct_iterate_cleanup
+#define ip_ct_cleanup nf_ct_cleanup
+#define ip_ct_iterate nf_ct_iterate
#defineIP_NF_ASSERTNF_CT_ASSERT
extern int nf_nat_rule_init(void) __init;
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c
b/net/ipv4/netfilter/ip_conntrack_core.c
index 07ba1dd..eaf6b07 100644
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -1251,11 +1251,6 @@ get_next_corpse(int (*iter)(struct ip_co
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote:
It is the standard Fedora script iptables and it recursively
unloads all referring modules, then the base ones: ip_tables and
ip_conntrack. I can't tell which module it stopped on because
that computer is at home today.
And yes, it really locks up until it's caught by the softlockup
watchdog. Has only happened on a uniprocessor i686 system so far,
but happens every time on shutdown.
Can you try this patch please?
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
{nf,ip}_ct_iterate_cleanup iterate over the unconfirmed list for cleaning
up conntrack entries, which is wrong for multiple reasons:
- unconfirmed entries can not be killed manually, which means we might
iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
Split ip_ct_iterate_cleanup in ip_ct_iterate, which iterates over both
confirmed and unconfirmed entries, but doesn't attempt to kill them,
and ip_ct_cleanup, which makes sure no unconfirmed entries exist by
calling synchronize_net() prior to walking the conntrack hash.
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
---
commit 08ace206d2703e377efebe1852539177458593ff
tree 1d27d4759a77db062d48c4420459c810e7fec14a
parent 9654640d0af8f2de40ff3807d3695109d3463f54
author Patrick McHardy [EMAIL PROTECTED] Sun, 25 Feb 2007 17:57:21 +0100
committer Patrick McHardy [EMAIL PROTECTED] Sun, 25 Feb 2007 18:28:22 +0100
include/linux/netfilter_ipv4/ip_conntrack.h |5 ++--
include/net/netfilter/nf_conntrack.h |5 +++-
include/net/netfilter/nf_nat_rule.h |3 ++
net/ipv4/netfilter/ip_conntrack_core.c | 30 ++--
net/ipv4/netfilter/ip_conntrack_standalone.c |5 ++--
net/ipv4/netfilter/ip_nat_core.c |5 ++--
net/ipv4/netfilter/ipt_MASQUERADE.c |4 ++-
net/ipv4/netfilter/nf_nat_core.c |7 ++
net/netfilter/nf_conntrack_core.c| 33 --
net/netfilter/nf_conntrack_proto.c |4 ++-
10 files changed, 69 insertions(+), 32 deletions(-)
diff --git a/include/linux/netfilter_ipv4/ip_conntrack.h
b/include/linux/netfilter_ipv4/ip_conntrack.h
index da9274e..90b12b2 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h
@@ -250,8 +250,9 @@ ip_ct_gather_frags(struct sk_buff *skb,
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-ip_ct_iterate_cleanup(int (*iter)(struct ip_conntrack *i, void *data),
- void *data);
+ip_ct_cleanup(int (*iter)(struct ip_conntrack *i, void *data), void *data);
+extern void
+ip_ct_iterate(void (*iter)(struct ip_conntrack *i, void *data), void *data);
extern struct ip_conntrack_helper *
__ip_conntrack_helper_find_byname(const char *);
diff --git a/include/net/netfilter/nf_conntrack.h
b/include/net/netfilter/nf_conntrack.h
index 0e690e3..a9271df 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -233,7 +233,10 @@ extern int nf_ct_no_defrag;
/* Iterate over all conntracks: if iter returns true, it's deleted. */
extern void
-nf_ct_iterate_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+nf_ct_cleanup(int (*iter)(struct nf_conn *i, void *data), void *data);
+extern void
+nf_ct_iterate(void (*iter)(struct nf_conn *i, void *data), void *data);
+
extern void nf_conntrack_free(struct nf_conn *ct);
extern struct nf_conn *
nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
diff --git a/include/net/netfilter/nf_nat_rule.h
b/include/net/netfilter/nf_nat_rule.h
index f191c67..2f16114 100644
--- a/include/net/netfilter/nf_nat_rule.h
+++ b/include/net/netfilter/nf_nat_rule.h
@@ -11,7 +11,8 @@ #define ip_conntrack_get nf_ct_get
#define ip_conntrack nf_conn
#define ip_nat_setup_info nf_nat_setup_info
#define ip_nat_multi_range_compat nf_nat_multi_range_compat
-#define ip_ct_iterate_cleanup nf_ct_iterate_cleanup
+#define ip_ct_cleanup nf_ct_cleanup
+#define ip_ct_iterate nf_ct_iterate
#defineIP_NF_ASSERTNF_CT_ASSERT
extern int nf_nat_rule_init(void) __init;
diff --git a/net/ipv4/netfilter/ip_conntrack_core.c
b/net/ipv4/netfilter/ip_conntrack_core.c
index 07ba1dd..eaf6b07 100644
--- a/net/ipv4/netfilter/ip_conntrack_core.c
+++ b/net/ipv4/netfilter/ip_conntrack_core.c
@@ -1251,11 +1251,6 @@ get_next_corpse(int (*iter)(struct ip_co
goto
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Martin Josefsson wrote: What about this case: 1. Conntrack entry is created and placed on the unconfirmed list 2. The event cache bumps the refcount of the conntrack entry 3. module removal of ip_conntrack unregisters all hooks 4. packet is dropped by an iptables rule 5. packet is freed but we still have a refcount on the conntrack entry Now there's no way to get that refcount to decrease as that only happens when the event cache receives another packet or the current packet makes it through the stack as you wrote above. And neither of this will happen since we unregistered the hooks providing the packets and dropped the packet. The event cache is flushed on conntrack module unload after the hooks have been unregistered, which should release all references. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
On Sun, 2007-02-25 at 18:31 +0100, Patrick McHardy wrote:
[NETFILTER]: conntrack: fix {nf,ip}_ct_iterate_cleanup endless loops
{nf,ip}_ct_iterate_cleanup iterate over the unconfirmed list for cleaning
up conntrack entries, which is wrong for multiple reasons:
- unconfirmed entries can not be killed manually, which means we might
iterate forever without making forward progress.
This can happen in combination with the conntrack event cache, which
holds a reference to the conntrack entry, which is only released when
the packet makes it all the way through the stack or a different
packet is handled.
- taking references to an unconfirmed entry and using it outside the
locked section doesn't work, the list entries are not refcounted and
another CPU might already be waiting to destroy the entry
Split ip_ct_iterate_cleanup in ip_ct_iterate, which iterates over both
confirmed and unconfirmed entries, but doesn't attempt to kill them,
and ip_ct_cleanup, which makes sure no unconfirmed entries exist by
calling synchronize_net() prior to walking the conntrack hash.
What about this case:
1. Conntrack entry is created and placed on the unconfirmed list
2. The event cache bumps the refcount of the conntrack entry
3. module removal of ip_conntrack unregisters all hooks
4. packet is dropped by an iptables rule
5. packet is freed but we still have a refcount on the conntrack entry
Now there's no way to get that refcount to decrease as that only happens
when the event cache receives another packet or the current packet makes
it through the stack as you wrote above. And neither of this will happen
since we unregistered the hooks providing the packets and dropped the
packet.
I ran into this case a while ago during stresstesting and rewrote the
event cache to not increase the refcount, but this has the drawback that
events caused by dropped packets won't be reported. This may not be a
good thing...
Old patch can be found here:
http://performance.netfilter.org/patches/nf_conntrack_ecache-fix
--
/Martin
signature.asc
Description: This is a digitally signed message part
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote: > Chuck Ebbert wrote: > >>I was testing a 2.6.20 kernel and got a soft >>lockup on shutdown: >> >>_raw_write_lock+0x5a >>nf_ct_iterate_cleanup+0x3e >>kill_l3proto+0x0 >>nf_conntrack_l3proto_unregister+0x85 >>nf_conntrack_l3proto_ipv4_fini+0x1e >>sys_delete_module+0x18a >>remove_vma+0x45 >>do_munmap+0x196 >>syscall_call+0x7 > > > Happened three times in a row, trying to remove > netfilter modules. Does it happen when you unload only nf_conntrack_ipv4, or do you need to unload other modules previously? Does it really lock up? Please also send your config. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote: Chuck Ebbert wrote: I was testing a 2.6.20 kernel and got a soft lockup on shutdown: _raw_write_lock+0x5a nf_ct_iterate_cleanup+0x3e kill_l3proto+0x0 nf_conntrack_l3proto_unregister+0x85 nf_conntrack_l3proto_ipv4_fini+0x1e sys_delete_module+0x18a remove_vma+0x45 do_munmap+0x196 syscall_call+0x7 Happened three times in a row, trying to remove netfilter modules. Does it happen when you unload only nf_conntrack_ipv4, or do you need to unload other modules previously? Does it really lock up? Please also send your config. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote: > I was testing a 2.6.20 kernel and got a soft > lockup on shutdown: > > _raw_write_lock+0x5a > nf_ct_iterate_cleanup+0x3e > kill_l3proto+0x0 > nf_conntrack_l3proto_unregister+0x85 > nf_conntrack_l3proto_ipv4_fini+0x1e > sys_delete_module+0x18a > remove_vma+0x45 > do_munmap+0x196 > syscall_call+0x7 Happened three times in a row, trying to remove netfilter modules. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Soft lockup on shutdown in nf_ct_iterate_cleanup()
Chuck Ebbert wrote: I was testing a 2.6.20 kernel and got a soft lockup on shutdown: _raw_write_lock+0x5a nf_ct_iterate_cleanup+0x3e kill_l3proto+0x0 nf_conntrack_l3proto_unregister+0x85 nf_conntrack_l3proto_ipv4_fini+0x1e sys_delete_module+0x18a remove_vma+0x45 do_munmap+0x196 syscall_call+0x7 Happened three times in a row, trying to remove netfilter modules. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
