> > Turns out this is a rather simple issue that got slightly > complicated by the code diverging quite a bit since the > inception. Essentially the clr->ifname comes from the > interface specification in the "pfctl -i foo0 -Fs" for > if-bound states (floating states use fake interface "any"). > > Previously states have been hanging off of kif nodes but it's > long gone and we can simply iterate over the state table tree > (or even a state list like it's done in the DIOCGETSTATES in > pf_ioctl). > > Calling pf_kif_get here wouldn't be prudent because spawning > new objects while disposing of the other ones seems somewhat > counterproductive. > > diff --git sys/net/if_pfsync.c sys/net/if_pfsync.c > index 7d633db..fcaf5f5 100644 > --- sys/net/if_pfsync.c > +++ sys/net/if_pfsync.c > @@ -752,46 +752,28 @@ done: > > int > pfsync_in_clr(caddr_t buf, int len, int count, int flags) > { > struct pfsync_clr *clr; > - int i; > - > struct pf_state *st, *nexts; > - struct pf_state_key *sk, *nextsk; > - struct pf_state_item *si; > + struct pfi_kif *kif = NULL; > u_int32_t creatorid; > + int i; > > for (i = 0; i < count; i++) { > clr = (struct pfsync_clr *)buf + len * i; > creatorid = clr->creatorid; > + if (strlen(clr->ifname) && > + (kif = pfi_kif_find(clr->ifname)) == NULL) > + continue; > > - if (clr->ifname[0] == '\0') { > - for (st = RB_MIN(pf_state_tree_id, &tree_id); > - st; st = nexts) { > - nexts = RB_NEXT(pf_state_tree_id, &tree_id, st); > - if (st->creatorid == creatorid) { > - SET(st->state_flags, PFSTATE_NOSYNC); > - pf_unlink_state(st); > - } > - } > - } else { > - if (pfi_kif_get(clr->ifname) == NULL) > - continue; > - > - /* XXX correct? */ > - for (sk = RB_MIN(pf_state_tree, &pf_statetbl); > - sk; sk = nextsk) { > - nextsk = RB_NEXT(pf_state_tree, > - &pf_statetbl, sk); > - TAILQ_FOREACH(si, &sk->states, entry) { > - if (si->s->creatorid == creatorid) { > - SET(si->s->state_flags, > - PFSTATE_NOSYNC); > - pf_unlink_state(si->s); > - } > - } > + for (st = RB_MIN(pf_state_tree_id, &tree_id); st; st = nexts) { > + nexts = RB_NEXT(pf_state_tree_id, &tree_id, st); > + if (st->creatorid == creatorid && > + ((kif && st->kif == kif) || !kif)) { > + SET(st->state_flags, PFSTATE_NOSYNC); > + pf_unlink_state(st); > } > } > } > > return (0); > diff --git sys/net/pf_if.c sys/net/pf_if.c > index caaf9f9..bf77184 100644 > --- sys/net/pf_if.c > +++ sys/net/pf_if.c > @@ -97,18 +97,25 @@ pfi_initialize(void) > if ((pfi_all = pfi_kif_get(IFG_ALL)) == NULL) > panic("pfi_kif_get for pfi_all failed"); > } > > struct pfi_kif * > -pfi_kif_get(const char *kif_name) > +pfi_kif_find(const char *kif_name) > { > - struct pfi_kif *kif; > struct pfi_kif_cmp s; > > bzero(&s, sizeof(s)); > strlcpy(s.pfik_name, kif_name, sizeof(s.pfik_name)); > - if ((kif = RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s)) != NULL) > + return (RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s)); > +} > + > +struct pfi_kif * > +pfi_kif_get(const char *kif_name)
may be it's kind of bike shading... How about make kifs to stick to convention we see for other objects such as rulesets/anchors: pfi_kif_find() pfi_kif_find_or_create() and kill pfi_kif_get() completely, just to avoid confusion/surprise. anything else makes a sense to me. regards sasha