pf tables locking

[Kajetan Staszkiewicz]

On Tuesday, 14 August 2018 16:15:48 CEST Kristof Provost wrote:
> On 14 Aug 2018, at 0:32, Kajetan Staszkiewicz wrote:
> > On Monday, 13 August 2018 17:59:15 CEST Kristof Provost wrote:
> > How about this?
> > 
> > https://github.com/innogames/freebsd/commit/
> > d44a0d9487285fac8ed1d7372cc99cca83f616e6
> 
> That looks good to me.
> There’s a few minor issues, things like inconsistent indentation and
> overly long lines, but that’s about the only criticism I have.

I fixed some issues with unallocated counters and submitted bug 230619.

> I see. I’m not quite sure yet if that’s a feature we want to import
> or not,
> but at least your ‘support’ patches should probably go in. The above
> one certainly.

There are some more things which require changes before I can do least-
connections balancing.

If you have a moment, please have a look at https://github.com/innogames/
freebsd/commits/iglb/11.2/GetOnWithIt_2 , maybe some of those things can get 
imported anyway, like full support for counters of states.

> >> 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 :)
> 
> I’m not going to stop you. I love it when other people do the work ;)

Since I have you here, let me explain the issues I see with pf_map_addr(). For 
round-robin target a list of interface,table pairs can be specified. This list 
is iterated and within each table addresses are iterated too. There is no 
locking around it "because performance is assumed more important than round-
robin precision" according to comment in code.

Yet I believe there are way more serious issues possible with the current 
approach. Interface is in fact picked up outside of pf_map_addr(). Another 
thread could have already moved the rpool->counter to another table for which 
the interface is not valid anymore.

I came up with this: https://github.com/innogames/freebsd/commit/
61ffb96a4dc948a0b06204ff39210c0578f77f08 although without locking this is 
still not really a solution. It only moves interface selection to inside of 
pf_map_addr()

Another one is https://github.com/innogames/freebsd/commit/
8fe6cd2d820052d2166afbaa311f34318a41db48 which stores table used for 
loadbalancing in state and src_node. Then the table can be used for state 
counting.

The 2 patches above are also included in the first link I gave above.

-- 
| 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/8ed92727/attachment.sig>