Fix wrong order in event notification

The following sequence is displayed during events dumping of an ICMP connection:

[NEW]
[DESTROY]
[UPDATE]

This happens because the event IPCT_DESTROY is delivered in death_by_timeout, that is called from the icmp protocol helper (ct->timeout.function) once we see the reply.

To fix this, I propose to move this event to destroy_conntrack instead.

Index: netfilter-2.6.14/net/ipv4/netfilter/ip_conntrack_core.c
===================================================================
@@ -327,6 +327,7 @@
 	IP_NF_ASSERT(atomic_read(&nfct->use) == 0);
 	IP_NF_ASSERT(!timer_pending(&ct->timeout));
 
+	ip_conntrack_event(IPCT_DESTROY, ct);
 	set_bit(IPS_DYING_BIT, &ct->status);
 
 	/* To make sure we don't get any weird locking issues here:

@@ -366,7 +367,6 @@
 {
 	struct ip_conntrack *ct = (void *)ul_conntrack;
 
-	ip_conntrack_event(IPCT_DESTROY, ct);
 	write_lock_bh(&ip_conntrack_lock);
 	/* Inside lock so preempt is disabled on module removal path.
 	 * Otherwise we can get spurious warnings. */

thanks, applied.

- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

I want to "backport" this patch to 2.6.11.7 (before IPS_DYING was invented to help shrink the conntrack struct)

The obvious fixup is below, I just wanted to check that it was safe, and that sending the event earlier instead of during death by timeout wasn't also depending on side effects of IPS_DYING

--- net/ipv4/netfilter/ip_conntrack_core.c.orig 2005-09-09 16:57:44.000000000 +0100
+++ net/ipv4/netfilter/ip_conntrack_core.c 2005-09-09
17:01:45.000000000 +0100
@@ -257,6 +257,7 @@

        IP_NF_ASSERT(atomic_read(&nfct->use) == 0);
        IP_NF_ASSERT(!timer_pending(&ct->timeout));

+       ip_conntrack_event(IPCT_DESTROY, ct);
        set_bit(IPS_DESTROYED_BIT, &ct->status);

        /* To make sure we don't get any weird locking issues here:
@@ -304,7 +305,6 @@
 {
        struct ip_conntrack *ct = (void *)ul_conntrack;

-       ip_conntrack_event(IPCT_DESTROY, ct);
        WRITE_LOCK(&ip_conntrack_lock);
        /* Inside lock so preempt is disabled on module removal path.
         * Otherwise we can get spurious warnings. */

Sam

Hi Sam,

ctnetlink is already in the Linus tree, so it will be out in the next kernel release. Because of that, I don't think that it's worth applying that patch to the event API in pom-ng. We should focus on what is in mainline.

--
Pablo