Hello,
Patch fixes two small nits related to source node table in PF (a.k.a.
pf_src_tree_tracking).
The first issue comes to `global` argument of pf_insert_src_node(). It is
always 0 everywhere in source code. The `global` is supposed to indicate
whether particular state is bound to global/main rule or to rule created on
behalf of admin. However in reality PF always uses 0 for `global` everywhere.
The second issue is related to pf_remove_src_node() function, which refuses
the remove source node from table (pf_src_tree_tracking) if node to be removed
is not bound to rule (sn->rule.ptr == NULL). Such node would hang in tree
forever. I think we never hit this problem, since source node is always
bound to rule.
OK?
thanks and
regards
sasha
--------8<---------------8<---------------8<------------------8<--------
Index: pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.946
diff -u -p -r1.946 pf.c
--- pf.c 8 Oct 2015 11:36:51 -0000 1.946
+++ pf.c 10 Oct 2015 16:49:40 -0000
@@ -501,7 +501,7 @@ pf_src_connlimit(struct pf_state **state
int
pf_insert_src_node(struct pf_src_node **sn, struct pf_rule *rule,
enum pf_sn_types type, sa_family_t af, struct pf_addr *src,
- struct pf_addr *raddr, int global)
+ struct pf_addr *raddr)
{
struct pf_src_node k;
@@ -509,10 +509,7 @@ pf_insert_src_node(struct pf_src_node **
k.af = af;
k.type = type;
PF_ACPY(&k.addr, src, af);
- if (global)
- k.rule.ptr = NULL;
- else
- k.rule.ptr = rule;
+ k.rule.ptr = rule;
pf_status.scounters[SCNT_SRC_NODE_SEARCH]++;
*sn = RB_FIND(pf_src_tree, &tree_src_tracking, &k);
}
@@ -531,10 +528,7 @@ pf_insert_src_node(struct pf_src_node **
(*sn)->type = type;
(*sn)->af = af;
- if (global)
- (*sn)->rule.ptr = NULL;
- else
- (*sn)->rule.ptr = rule;
+ (*sn)->rule.ptr = rule;
PF_ACPY(&(*sn)->addr, src, af);
if (raddr)
PF_ACPY(&(*sn)->raddr, raddr, af);
@@ -570,16 +564,14 @@ pf_remove_src_node(struct pf_src_node *s
if (sn->states > 0 || sn->expire > time_uptime)
return;
- if (sn->rule.ptr != NULL) {
- sn->rule.ptr->src_nodes--;
- if (sn->rule.ptr->states_cur == 0 &&
- sn->rule.ptr->src_nodes == 0)
- pf_rm_rule(NULL, sn->rule.ptr);
- RB_REMOVE(pf_src_tree, &tree_src_tracking, sn);
- pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++;
- pf_status.src_nodes--;
- pool_put(&pf_src_tree_pl, sn);
- }
+ sn->rule.ptr->src_nodes--;
+ if (sn->rule.ptr->states_cur == 0 &&
+ sn->rule.ptr->src_nodes == 0)
+ pf_rm_rule(NULL, sn->rule.ptr);
+ RB_REMOVE(pf_src_tree, &tree_src_tracking, sn);
+ pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++;
+ pf_status.src_nodes--;
+ pool_put(&pf_src_tree_pl, sn);
}
struct pf_src_node *
@@ -3381,7 +3373,7 @@ pf_test_rule(struct pf_pdesc *pd, struct
if (r->rule_flag & PFRULE_SRCTRACK &&
pf_insert_src_node(&sns[PF_SN_NONE], r, PF_SN_NONE, pd->af,
- pd->src, NULL, 0) != 0) {
+ pd->src, NULL) != 0) {
REASON_SET(&reason, PFRES_SRCLIMIT);
goto cleanup;
}
Index: pf_lb.c
===================================================================
RCS file: /cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.49
diff -u -p -r1.49 pf_lb.c
--- pf_lb.c 3 Aug 2015 13:33:12 -0000 1.49
+++ pf_lb.c 10 Oct 2015 16:49:41 -0000
@@ -621,8 +621,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
pf_remove_src_node(sns[type]);
sns[type] = NULL;
}
- if (pf_insert_src_node(&sns[type], r, type, af, saddr, naddr,
- 0))
+ if (pf_insert_src_node(&sns[type], r, type, af, saddr, naddr))
return (1);
}
Index: pfvar.h
===================================================================
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.420
diff -u -p -r1.420 pfvar.h
--- pfvar.h 19 Aug 2015 21:22:41 -0000 1.420
+++ pfvar.h 10 Oct 2015 16:49:43 -0000
@@ -1681,7 +1681,7 @@ extern int pf_state_insert(struct
pfi
int pf_insert_src_node(struct pf_src_node **,
struct pf_rule *, enum pf_sn_types,
sa_family_t, struct pf_addr *,
- struct pf_addr *, int);
+ struct pf_addr *);
void pf_remove_src_node(struct pf_src_node *);
struct pf_src_node *pf_get_src_node(struct pf_state *,
enum pf_sn_types);
