> On 17 Dec 2015, at 13:30, Richard Procter <richard.n.proc...@gmail.com> wrote:
>
>
> 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.
right now pf doesnt run concurrently so extra locking is unnecessary. when it
does run concurrently then it will need a mutex, despite how infrequently the
gc runs.
>
> 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.
i believe the purge thread exists because once upon a time pool_free from
interrupt context wasnt the best. i might be misremembering that though.
currently that thread only looks at states. traversing the ruleset is possible
i guess. if the semantic of the once rules is that they only match once, and if
pf will run on multiple cpus, then coordinating between the cpus will require
atomicity or serialisation of some sort.
dlg
>
> Ok, enough from me on this.
>
> best,
> Richard.
>
