this diff reimplements interface bound states in a non-retarded way -
I'll save you the details, the executive summay is that this gets us a
much cleaner design and about 5..10% more performance (benching for
exact numbers pending).
I need somebody who relies on ifbound states because he's doing crazy
route-to games or the like (so that you end up having two identical
states in the table that only differ in the interface bound to) to give
this a test. the faster the better, there is more stuff pending that
depends on this diff

thanks.

Index: if_pfsync.c
================================================== =================
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.78
diff -u -p -r1.78 if_pfsync.c
--- if_pfsync.c 1 Jun 2007 18:44:22 -0000 1.78
+++ if_pfsync.c 1 Jun 2007 21:16:09 -0000
@@ -427,10 +427,12 @@ pfsync_input(struct mbuf *m, ...)
&kif->pfik_lan_ext); sk; sk = nextsk) {
nextsk = RB_NEXT(pf_state_tree_lan_ext,
&kif->pfik_lan_ext, sk);
- if (sk->state->creatorid == creatorid) {
- sk->state->sync_flags |=
- PFSTATE_FROMSYNC;
- pf_unlink_state(sk->state);
+ SIMPLEQ_FOREACH(st, &sk->states, next) {
+ if (st->creatorid == creatorid) {
+ st->sync_flags |=
+ PFSTATE_FROMSYNC;
+ pf_unlink_state(st);
+ }
}
}
}
Index: pf.c
================================================== =================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.540
diff -u -p -r1.540 pf.c
--- pf.c 1 Jun 2007 18:44:22 -0000 1.540
+++ pf.c 1 Jun 2007 21:16:10 -0000
@@ -153,6 +153,8 @@ struct pf_rule *pf_get_translation(stru
struct pf_addr *, u_int16_t,
struct pf_addr *, u_int16_t,
struct pf_addr *, u_int16_t *);
+void pf_attach_state(struct pf_state_key *,
+ struct pf_state *, int);
int pf_test_rule(struct pf_rule **, struct pf_state **,
int, struct pfi_kif *, struct mbuf *, int,
void *, struct pf_pdesc *, struct pf_rule **,
@@ -206,9 +208,11 @@ int pf_check_proto_cksum(struct mbuf
u_int8_t, sa_family_t);
int pf_addr_wrap_neq(struct pf_addr_wrap *,
struct pf_addr_wrap *);
-struct pf_state *pf_find_state_recurse(struct pfi_kif *,
+struct pf_state *pf_find_state(struct pfi_kif *,
struct pf_state_key_cmp *, u_int8_t);
int pf_src_connlimit(struct pf_state **);
+void pf_stateins_err(const char *, struct pf_state *,
+ struct pfi_kif *);
int pf_check_congestion(struct ifqueue *);

extern struct pool pfr_ktable_pl;
@@ -225,11 +229,9 @@ struct pf_pool_limit pf_pool_limits[PF_L
#define STATE_LOOKUP() \
do { \
if (direction == PF_IN) \
- *state = pf_find_state_recurse( \
- kif, &key, PF_EXT_GWY); \
+ *state = pf_find_state(kif, &key, PF_EXT_GWY); \
else \
- *state = pf_find_state_recurse( \
- kif, &key, PF_LAN_EXT); \
+ *state = pf_find_state(kif, &key, PF_LAN_EXT); \
if (*state == NULL || (*state)->timeout == PFTM_PURGE) \
return (PF_DROP); \
if (direction == PF_OUT && \
@@ -516,39 +518,39 @@ pf_find_state_byid(struct pf_state_cmp *
}

struct pf_state *
-pf_find_state_recurse(struct pfi_kif *kif, struct pf_state_key_cmp *key,
- u_int8_t tree)
+pf_find_state(struct pfi_kif *kif, struct pf_state_key_cmp *key, u_int8_t tree)
{
- struct pf_state_key *sk;
+ struct pf_state_key *sk;
+ struct pf_state *s;

pf_status.fcounters[FCNT_STATE_SEARCH]++;

switch (tree) {
case PF_LAN_EXT:
- if ((sk = RB_FIND(pf_state_tree_lan_ext, &kif->pfik_lan_ext,
- (struct pf_state_key *)key)) != NULL)
- return (sk->state);
- if ((sk = RB_FIND(pf_state_tree_lan_ext, &pfi_all->pfik_lan_ext,
- (struct pf_state_key *)key)) != NULL)
- return (sk->state);
- return (NULL);
+ sk = RB_FIND(pf_state_tree_lan_ext, &pfi_all->pfik_lan_ext,
+ (struct pf_state_key *)key);
+ break;
case PF_EXT_GWY:
- if ((sk = RB_FIND(pf_state_tree_ext_gwy, &kif->pfik_ext_gwy,
- (struct pf_state_key *)key)) != NULL)
- return (sk->state);
- if ((sk = RB_FIND(pf_state_tree_ext_gwy, &pfi_all->pfik_ext_gwy,
- (struct pf_state_key *)key)) != NULL)
- return (sk->state);
- return (NULL);
+ sk = RB_FIND(pf_state_tree_ext_gwy, &pfi_all->pfik_ext_gwy,
+ (struct pf_state_key *)key);
+ break;
default:
- panic("pf_find_state_recurse");
+ panic("pf_find_state");
}
+
+ if (sk != NULL)
+ SIMPLEQ_FOREACH(s, &sk->states, next)
+ if (s->u.s.kif == pfi_all || s->u.s.kif == kif)
+ return (s);
+
+ return (NULL);
}

struct pf_state *
pf_find_state_all(struct pf_state_key_cmp *key, u_int8_t tree, int *more)
{
- struct pf_state_key *sk, *sks = NULL;
+ struct pf_state_key *sk;
+ struct pf_state *s, *ret = NULL;
struct pfi_kif *kif;

pf_status.fcounters[FCNT_STATE_SEARCH]++;
@@ -560,10 +562,12 @@ pf_find_state_all(struct pf_state_key_cm
&kif->pfik_lan_ext, (struct pf_state_key *)key);
if (sk == NULL)
continue;
+ ret = SIMPLEQ_FIRST(&sk->states);
if (more == NULL)
- return (sk->state);
- sks = sk;
- (*more)++;
+ return (ret);
+ else
+ SIMPLEQ_FOREACH(s, &sk->states, next)
+ (*more)++;
}
break;
case PF_EXT_GWY:
@@ -572,19 +576,19 @@ pf_find_state_all(struct pf_state_key_cm
&kif->pfik_ext_gwy, (struct pf_state_key *)key);
if (sk == NULL)
continue;
+ ret = SIMPLEQ_FIRST(&sk->states);
if (more == NULL)
- return (sk->state);
- sks = sk;
- (*more)++;
+ return (ret);
+ else
+ SIMPLEQ_FOREACH(s, &sk->states, next)
+ (*more)++;
}
break;
default:
panic("pf_find_state_all");
}
- if (sks != NULL)
- return (sks->state);
- else
- return (NULL);
+
+ return (ret);
}

void
@@ -781,53 +785,64 @@ pf_insert_src_node(struct pf_src_node **
return (0);
}

+void
+pf_stateins_err(const char *tree, struct pf_state *s, struct pfi_kif *kif)
+{
+ struct pf_state_key *sk = s->state_key;
+
+ if (pf_status.debug >= PF_DEBUG_MISC) {
+ printf("pf: state insert failed: %s %s", tree, kif->pfik_name);
+ printf(" lan: ");
+ pf_print_host(&sk->lan.addr, sk->lan.port,
+ sk->af);
+ printf(" gwy: ");
+ pf_print_host(&sk->gwy.addr, sk->gwy.port,
+ sk->af);
+ printf(" ext: ");
+ pf_print_host(&sk->ext.addr, sk->ext.port,
+ sk->af);
+ if (s->sync_flags & PFSTATE_FROMSYNC)
+ printf(" (from sync)");
+ printf("\n");
+ }
+}
+
int
pf_insert_state(struct pfi_kif *kif, struct pf_state *s)
{
- struct pf_state_key *sk;
+ struct pf_state_key *sk;
+ struct pf_state_key *cur;
+ struct pf_state *sp;

KASSERT(s->state_key != NULL);
sk = s->state_key;
-
- /* Thou MUST NOT insert multiple duplicate keys */
s->u.s.kif = kif;
- if (RB_INSERT(pf_state_tree_lan_ext, &kif->pfik_lan_ext, sk)) {
- if (pf_status.debug >= PF_DEBUG_MISC) {
- printf("pf: state insert failed: tree_lan_ext");
- printf(" lan: ");
- pf_print_host(&sk->lan.addr, sk->lan.port,
- sk->af);
- printf(" gwy: ");
- pf_print_host(&sk->gwy.addr, sk->gwy.port,
- sk->af);
- printf(" ext: ");
- pf_print_host(&sk->ext.addr, sk->ext.port,
- sk->af);
- if (s->sync_flags & PFSTATE_FROMSYNC)
- printf(" (from sync)");
- printf("\n");
- }
- return (-1);
+
+ if ((cur = RB_INSERT(pf_state_tree_lan_ext, &kif->pfik_lan_ext, sk)) !=
+ NULL) {
+ /* key exists. check for same kif, if none, add to key */
+ SIMPLEQ_FOREACH(sp, &cur->states, next)
+ if (sp->u.s.kif == kif) { /* collision! */
+ pf_stateins_err("tree_lan_ext", s, kif);
+ return (-1);
+ }
+ pool_put(&pf_state_key_pl, s->state_key);
+ pf_attach_state(cur, s, kif == pfi_all ? 1 : 0);
}

- if (RB_INSERT(pf_state_tree_ext_gwy, &kif->pfik_ext_gwy, sk)) {
- if (pf_status.debug >= PF_DEBUG_MISC) {
- printf("pf: state insert failed: tree_ext_gwy");
- printf(" lan: ");
- pf_print_host(&sk->lan.addr, sk->lan.port,
- sk->af);
- printf(" gwy: ");
- pf_print_host(&sk->gwy.addr, sk->gwy.port,
- sk->af);
- printf(" ext: ");
- pf_print_host(&sk->ext.addr, sk->ext.port,
- sk->af);
- if (s->sync_flags & PFSTATE_FROMSYNC)
- printf(" (from sync)");
- printf("\n");
- }
- RB_REMOVE(pf_state_tree_lan_ext, &kif->pfik_lan_ext, sk);
- return (-1);
+ if ((cur = RB_INSERT(pf_state_tree_ext_gwy, &kif->pfik_ext_gwy, sk)) !=
+ NULL) {
+ /* key exists. check for same kif, if none, add to key */
+ SIMPLEQ_FOREACH(sp, &cur->states, next)
+ if (sp->u.s.kif == kif) { /* collision! */
+ pf_stateins_err("tree_ext_gwy", s, kif);
+ RB_REMOVE(pf_state_tree_lan_ext,
+ &kif->pfik_lan_ext, sk);
+ return (-1);
+ }
+
+ pool_put(&pf_state_key_pl, s->state_key);
+ pf_attach_state(cur, s, kif == pfi_all ? 1 : 0);
}

if (s->id == 0 && s->creatorid == 0) {
@@ -1033,7 +1048,8 @@ pf_free_state(struct pf_state *cur)
TAILQ_REMOVE(&state_list, cur, u.s.entry_list);
if (cur->tag)
pf_tag_unref(cur->tag);
- pool_put(&pf_state_key_pl, cur->state_key);
+ if (--cur->state_key->refcnt == 0)
+ pool_put(&pf_state_key_pl, cur->state_key);
pool_put(&pf_state_pl, cur);
pf_status.fcounters[FCNT_STATE_REMOVALS]++;
pf_status.states--;
@@ -2789,6 +2805,18 @@ pf_set_rt_ifp(struct pf_state *s, struct
}
}

+void
+pf_attach_state(struct pf_state_key *sk, struct pf_state *s, int tail)
+{
+ s->state_key = sk;
+ sk->refcnt++;
+
+ if (tail)
+ SIMPLEQ_INSERT_TAIL(&sk->states, s, next);
+ else
+ SIMPLEQ_INSERT_HEAD(&sk->states, s, next);
+}
+
struct pf_state_key *
pf_alloc_state_key(struct pf_state *s)
{
@@ -2797,8 +2825,8 @@ pf_alloc_state_key(struct pf_state *s)
if ((sk = pool_get(&pf_state_key_pl, PR_NOWAIT)) == NULL)
return (NULL);
bzero(sk, sizeof(*sk));
- sk->state = s;
- s->state_key = sk;
+ SIMPLEQ_INIT(&sk->states);
+ pf_attach_state(sk, s, 0);

return (sk);
}
Index: pfvar.h
================================================== =================
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.249
diff -u -p -r1.249 pfvar.h
--- pfvar.h 1 Jun 2007 18:44:23 -0000 1.249
+++ pfvar.h 1 Jun 2007 21:16:10 -0000
@@ -696,6 +696,8 @@ struct pf_state_key_cmp {
u_int8_t pad;
};

+SIMPLEQ_HEAD(pf_statelist, pf_state);
+
struct pf_state_key {
struct pf_state_host lan;
struct pf_state_host gwy;
@@ -707,7 +709,8 @@ struct pf_state_key {

RB_ENTRY(pf_state_key) entry_lan_ext;
RB_ENTRY(pf_state_key) entry_ext_gwy;
- struct pf_state *state;
+ struct pf_statelist states;
+ u_short refcnt; /* same size as if_index */
};


@@ -723,6 +726,7 @@ struct pf_state {
u_int32_t creatorid;
u_int32_t pad;

+ SIMPLEQ_ENTRY(pf_state) next;
RB_ENTRY(pf_state) entry_id;
struct pf_state_key *state_key;
u_int8_t log;