Hello,
</snip>
>
> claudio had some questions about numbers used by the diff. some of the
> maths here is set up so that every pf state purge run is guaranteed to
> do a MINUMUM amount of work. by default pf is configured with a purge
> interfval of 10 seconds, bu the pf state purge processing runs every
> second and tries to process 1/interval (ie 1/10th) of the states. if we
> have 4 states, then 4/10 is 0, so no states would be purged. by having a
> minimum of 64 states processed every second, we guarantee this small
> number of states gets processed.
>
> i would like to commit this diff now (at the start of h2k22) so i can
> keep an eye on it. ive been running it for a while without issues.
>
> the obvious next step will be to remove the NET_LOCKs.
I think this can go in as-is. I'm fine with change.
found just small nit, but it's more a matter of personal state.
leaving it up to David to go for it or not. Details are below.
OK sashan
>
> Index: pf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1141
> diff -u -p -r1.1141 pf.c
> --- pf.c 10 Oct 2022 16:43:12 -0000 1.1141
> +++ pf.c 6 Nov 2022 13:08:15 -0000
</snip>
> @@ -1559,28 +1619,38 @@ pf_purge_expired_states(u_int32_t maxche
>
> now = getuptime();
>
> - do {
> + for (scanned = 0; scanned < limit; scanned++) {
> uint8_t stimeout = cur->timeout;
> + unsigned int limited = 0;
>
> if ((stimeout == PFTM_UNLINKED) ||
> (pf_state_expires(cur, stimeout) <= now)) {
> st = pf_state_ref(cur);
> SLIST_INSERT_HEAD(&gcl, st, gc_list);
> +
> + if (++collected >= collect)
> + limited = 1;
> }
I think we can get away without introducing a 'limited' local
variable.
>
> /* don't iterate past the end of our view of the list */
> if (cur == tail) {
> + scanned = limit;
> cur = NULL;
> break;
> }
>
> cur = TAILQ_NEXT(cur, entry_list);
> - } while (maxcheck--);
> +
> + /* don't spend too much time here. */
> + if (ISSET(READ_ONCE(curcpu()->ci_schedstate.spc_schedflags),
> + SPCF_SHOULDYIELD) || limited)
> + break;
> + }
>
and of course the condition above needs to be changed to:
SPCF_SHOULDYIELD) || (collected >= collect))
As I've said it's just nit.
