Hi,
I was surprised to see 'State Creations' rule counter go up when
no real state creation happens. This is because we increment all
counters too early, but then don't decrement 'states_tot' which
is a total number of states created by the rule. Not entirely
sure why was it done but I see no reason for it to stay this way.
IMO total has to increase only when we succeed inserting the state
into the table.
This has an additional benefit of making error handling simpler:
we don't need to unroll our counter increments if we do them right
after pf_state_insert.
OK?
diff --git sys/net/pf.c sys/net/pf.c
index 4192f14..efa1388 100644
--- sys/net/pf.c
+++ sys/net/pf.c
@@ -273,20 +273,10 @@ struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = {
} \
SLIST_FOREACH(mrm, &s->match_rules, entry) \
mrm->r->states_cur++; \
} while (0)
-#define STATE_DEC_COUNTERS(s) \
- do { \
- struct pf_rule_item *mrm; \
- if (s->anchor.ptr != NULL) \
- s->anchor.ptr->states_cur--; \
- s->rule.ptr->states_cur--; \
- SLIST_FOREACH(mrm, &s->match_rules, entry) \
- mrm->r->states_cur--; \
- } while (0)
-
static __inline int pf_src_compare(struct pf_src_node *, struct pf_src_node *);
static __inline int pf_state_compare_key(struct pf_state_key *,
struct pf_state_key *);
static __inline int pf_state_compare_id(struct pf_state *,
struct pf_state *);
@@ -3464,11 +3454,10 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r,
struct pf_rule *a,
}
s->rule.ptr = r;
s->anchor.ptr = a;
s->natrule.ptr = nr;
memcpy(&s->match_rules, rules, sizeof(s->match_rules));
- STATE_INC_COUNTERS(s);
if (r->allow_opts)
s->state_flags |= PFSTATE_ALLOWOPTS;
if (r->rule_flag & PFRULE_STATESLOPPY)
s->state_flags |= PFSTATE_SLOPPY;
if (r->rule_flag & PFRULE_PFLOW)
@@ -3591,10 +3580,12 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r,
struct pf_rule *a,
REASON_SET(&reason, PFRES_STATEINS);
goto csfailed;
} else
*sm = s;
+ STATE_INC_COUNTERS(s);
+
if (tag > 0) {
pf_tag_ref(tag);
s->tag = tag;
}
if (pd->proto == IPPROTO_TCP && (th->th_flags & (TH_SYN|TH_ACK)) ==
@@ -3621,21 +3612,17 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r,
struct pf_rule *a,
csfailed:
if (s) {
pf_normalize_tcp_cleanup(s); /* safe even w/o init */
pf_src_tree_remove_state(s);
+ pool_put(&pf_state_pl, s);
}
for (i = 0; i < PF_SN_MAX; i++)
if (sns[i] != NULL)
pf_remove_src_node(sns[i]);
- if (s) {
- STATE_DEC_COUNTERS(s);
- pool_put(&pf_state_pl, s);
- }
-
return (PF_DROP);
}
int
pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,
