Hello, The updated patch addresses additional nit found by mpi: > > Here can't you also change: > > > > if ((*sn)->rule.ptr != NULL) > > (*sn)->rule.ptr->src_nodes++; > > > > into: > > > > (*sn)->rule.ptr->src_nodes++;
> > > > I don't know enough to say if it's correct or not, but I'd suggest > > sending another diff for that dealing with all the NULL checks :) > > What about pf_state_export() for example? > > > > I think you might be confusing state rule pointers and source node > rule pointers. I think Sasha has got all of the latter ones (albeit > with your correction), but on the other hand I would love to know > how st->rule.ptr can be NULL in the pf_state_export as pf_rm_rule is > not supposed to remove a rule with active states. thanks Mike, I was looking at if_pfsync.c and related stuff in PF. It looks like source node table is not updated at all by PF-sync. I'm still not sure about rules themselves, but it seems to me there is other mean than PF-sync to sync the rules between the firewall nodes. I could not spot nothing related to 'rule transfer' in if_pfsync.c/pf sources. thanks and regards sasha --------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 12 Oct 2015 15:52:47 -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); @@ -550,8 +544,7 @@ pf_insert_src_node(struct pf_src_node ** return (-1); } (*sn)->creation = time_uptime; - if ((*sn)->rule.ptr != NULL) - (*sn)->rule.ptr->src_nodes++; + (*sn)->rule.ptr->src_nodes++; pf_status.scounters[SCNT_SRC_NODE_INSERT]++; pf_status.src_nodes++; } else { @@ -570,16 +563,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 +3372,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 12 Oct 2015 15:52:48 -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 12 Oct 2015 15:52:48 -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);