pf tables locking

[Kajetan Staszkiewicz]

On Monday, 13 August 2018 17:59:15 CEST Kristof Provost wrote:

> pf keeps rules around until there are no more states left referencing the
> rule. Look at pf_commit_rules(): The old rules are unlinked rather than
> removed. They’re kept on the V_pf_unlinked rules list. Every so often pf
> runs through all states (in pf_purge_thread()) to mark their associated
> rules as still referenced. Only rules which are not referenced by any state
> are removed.
> 
> This means that while there’s still a state which was created by the rule
> (and can thus put packets towards its table), the rule will exist. Once the
> state goes away it’ll still take one full iteration through all states
> before the rule can be freed. Hence my statement that it’s highly unlikely
> (pretty much impossible) for us to run into a situation where the rule no
> longer exists.

OK, now it makes sense.

> >> I don’t want to re-take the rules lock for this, so my current
> >> thinking is that the best approach would be to already get rid of the
> >> potential memory leak by just always allocating the pfrke_counters when
> >> the table is created (i.e. when the rule is first set). That might waste
> >> a little memory if we didn’t need it, but it should simplify things a
> >> bit.
> >> 
> >> We can resolve the counting issue by using the counter_u64_*() functions
> >> for them. We should be able to get away with not locking this.

How about this?

https://github.com/innogames/freebsd/commit/
d44a0d9487285fac8ed1d7372cc99cca83f616e6

> Do you have a bit more information about your use case? What are you trying
> to accomplish with this change?

I have a loadbalancer which uses pf and route-to targets. After a server is 
added to a pool, I want this server to immediately take over much traffic. 
With round-robin the server receives new clients rather slowly. If kernel 
could measure amount of states per table entry, I could send new clients to 
this new server until it serves as many clients as other servers.

> > There are some more issues I found around pf_map_addr. Some of them I
> > mentioned in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=229092.
> > Some
> > more came out while working on this least-states loadbalancing. I will
> > group them into something meaningful and make another PR for them.
> 
> Yeah, that bug is still on my todo list somewhere, but things are extremely
> hectic at the moment, and I can’t make any promises about when I’ll have
> time for it.

I thought that was rather on my todo :)

If you can agree on patch sent in this message (I would still make a PR and 
submit the patch there, just for documentation), I will re-work my other 
patches and show you what I came up with. I had working code for counting 
states per table entry, I only lack the modified round-robin selection itself.

-- 
| pozdrawiam / greetings | powered by Debian, FreeBSD and CentOS |
|  Kajetan Staszkiewicz  | jabber,email: vegeta()tuxpowered net  |
|        Vegeta          | www: http://vegeta.tuxpowered.net     |
`------------------------^---------------------------------------'
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freebsd.org/pipermail/freebsd-pf/attachments/20180814/85cb525e/attachment.sig>