On Tue, Oct 13, 2015 at 20:36 +1000, David Gwynne wrote: > > > On 12 Oct 2015, at 12:00 AM, Alexandr Nedvedicky > > <alexandr.nedvedi...@oracle.com> wrote: > > > > Hello, > > > > patch below introduces struct refcnt to pfi_kif structure. Patch also > > changes > > pfi_kif_get() function to always return a reference to pfi_kif instance. > > > > Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown away > > in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention > > set by refcnt_init(9). Patch also removes kif reference types (enum > > pfi_kif_refs). > > > > Patch also updates if_pfsync.c file. I'm bit puzzled with test as follows > > in pfsync_in_clr(): > > > > 770 for (i = 0; i < count; i++) { > > 771 clr = (struct pfsync_clr *)buf + len * i; > > 772 creatorid = clr->creatorid; > > 773 > > 774 if (clr->ifname[0] == '\0') { > > ... > > 783 } else { > > 784 if (pfi_kif_get(clr->ifname) == NULL) > > 789 continue; > > 790 > > > > I could not make any sense of line 784. Are we just making sure particular > > kif > > object exists for clr->ifname? Note pfi_kif_get() creates kif for us if it > > does not exist. If we want to query for kif existence only patch offers > > pfi_kif_check() function. I need some advice here if we should keep, what's > > currently in patch or switch to pfi_kif_check(). > > my name is probably against that code. > > i didn't realise that pfi_kif_get creates a kif on demand. however, > it also uses malloc(, , M_NOWAIT) to try and allocate it, which can > fail and cause pfi_kif_get to return NULL. >
Revision 1.102 used to get the kif and check the state against it: if (si->s->creatorid == creatorid && si->s->kif == kif) { Then in the rev 1.103 you have removed the check. The question is why did you do that? (-: