[patch] Source entries removing is awfully slow.

[Kajetan Staszkiewicz]

I managed to run your patch on production machine and it produced panic in a 
few seconds after network traffic was directed through it.

The difference I spotted is that you insert the State entry to state_list near 
the end of pf_create_state:

@@ -3950,8 +3970,18 @@ pf_create_state(struct pf_rule *r, struc
                pool_put(&pf_state_pl, s); 
 #endif
                return (PF_DROP);
+#ifdef __FreeBSD__
+       } else {
+               if (sn != NULL)
+                       TAILQ_INSERT_HEAD(&sn->state_list, s, srcnode_link);
+               if (nsn != NULL)
+                       TAILQ_INSERT_HEAD(&nsn->state_list, s, srcnode_link);
+               *sm = s;
+       }
+#else
        } else
                *sm = s;
+#endif
 

while my original aproach was to do it much earlier. The working version is:

@@ -3895,12 +3915,18 @@ pf_create_state(struct pf_rule *r, struc
        if (sn != NULL) {
                s->src_node = sn;
                s->src_node->states++;
+#ifdef __FreeBSD__
+               TAILQ_INSERT_HEAD(&sn->state_list, s, srcnode_link);
+#endif
        }
        if (nsn != NULL) {
                /* XXX We only modify one side for now. */
                PF_ACPY(&nsn->raddr, &nk->addr[1], pd->af);
                s->nat_src_node = nsn;
                s->nat_src_node->states++;
+#ifdef __FreeBSD__
+               TAILQ_INSERT_HEAD(&nsn->state_list, s, srcnode_link);
+#endif
        }
        if (pd->proto == IPPROTO_TCP) {
                if ((pd->flags & PFDESC_TCP_NORM) && pf_normalize_tcp_init(m,

backtrace:
#7  0xffffffff810d7d2a in pf_src_tree_remove_state (s=0xfffffe007bfd55f0)
    at /usr/src/sys/modules/pf/../../contrib/pf/net/pf.c:1564
#8  0xffffffff810e01c2 in pf_test_rule (rm=0xffffff862f6ec848,
    sm=0xffffff862f6ec840, direction=2, kif=0xfffffe000ab67400,
    m=0xfffffe0070be9500, off=20, h=0xfffffe0070be9580,
    pd=0xffffff862f6ec780, am=0xffffff862f6ec850, rsm=0xffffff862f6ec838,
    ifq=0x0, inp=0x0)
    at /usr/src/sys/modules/pf/../../contrib/pf/net/pf.c:3965
#9  0xffffffff810e1d26 in pf_test (dir=2, ifp=Variable "ifp" is not available.
)   
    at /usr/src/sys/modules/pf/../../contrib/pf/net/pf.c:6964
#10 0xffffffff810e8cc1 in pf_check_out (arg=Variable "arg" is not available.
)   
    at /usr/src/sys/modules/pf/../../contrib/pf/net/pf_ioctl.c:4187
#11 0xffffffff8078816e in pfil_run_hooks (ph=Variable "ph" is not available.
) at /usr/src/sys/net/pfil.c:82
#12 0xffffffff807a20f5 in ip_fastforward (m=0xfffffe0070be9500)
    at /usr/src/sys/netinet/ip_fastfwd.c:445

contrib/pf/net/pf.c:3965
3961         if (pf_state_insert(BOUND_IFACE(r, kif), skw, sks, s)) {
3962                 if (pd->proto == IPPROTO_TCP)
3963                         pf_normalize_tcp_cleanup(s);
3964                 REASON_SET(&reason, PFRES_STATEINS);
3965                 pf_src_tree_remove_state(s);
3966                 STATE_DEC_COUNTERS(s);
3967 #ifdef __FreeBSD__
3968                 pool_put(&V_pf_state_pl, s);
3969 #else
3970                 pool_put(&pf_state_pl, s);
3971 #endif
3972                 return (PF_DROP);

contrib/pf/net/pf.c:1564
1561                 }
1562 #ifdef __FreeBSD__
1563                 if (!TAILQ_EMPTY(&s->nat_src_node->state_list))
1564                         TAILQ_REMOVE(&s->nat_src_node->state_list, s, 
srcnode_link);
1565 #endif
1566         }
1567         s->src_node = s->nat_src_node = NULL;


Following s -> nat_src_node -> state_list:
state_list = {tqh_first = 0xfffffe01fe360130, tqh_last = 0x0}

s and nat_src_node seem ok for me, not any memory garbage, e.g. they have 
proper creation and expiration times.

Do we hit a src_node that has the state_list uninitialised, and performing an 
early insert automatically fixes it?

-- 
| pozdrawiam / greetings | powered by Debian, CentOS and FreeBSD |
|  Kajetan Staszkiewicz  | jabber,email: vegeta()tuxpowered net  |
|        Vegeta          | www: http://vegeta.tuxpowered.net     |
`------------------------^---------------------------------------'