Hi Sasha, On Fri, 18 Dec 2015, Alexandr Nedvedicky wrote:
> > Right. I'll just note though that the patch as it stands allows > > multiple winners [...] Whether that's a realistic issue, I don't know. > > I have though been bitten by enough edge cases like this to be very > > wary of them. > > I think it's not realistic with current PF at OpenBSD. The pf_test() function > does not run concurrently, so there can be no such race. Fair enough :) I presumed that an upcoming concurrent pf_test() would rely on this patch. > FYI: PF@solaris uses mutex there to protect the insertion to gcl. Code goes as > follows at Solaris: > > if (!(r->rule_flags & PFRULE_EXPIRED)) { > mutex_enter(&gcl_mutex); > /* retest under exclusive protection */ > if (!(r->rule_flags & PFRULE_EXPIRED)) { > r->rule_flag |= PFRULE_EXPIRED; > SLIST_INSERT_HEAD(&pf_rule_gcl, r, gcle); > } > } I was wondering about that. For this patch presumably it's thought unnecessary to mutex the SLIST ops (or for that matter to preserve rule lifetime) for one-shot rules as the purge thread runs so infrequently. BTW, the purge queue exists to pass a result between threads; an alternative is to recompute it in the purge thread by searching for expired rules: no need for the queue, its mutexes, etc. Easier to preserve rule lifetime, too. May need another anchor stack though. I might have a go next year when the code is more settled. Ok, enough from me on this. best, Richard.