Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Aug 2018 01:32:17 +0200
From:      Kajetan Staszkiewicz <vegeta@tuxpowered.net>
To:        Kristof Provost <kp@freebsd.org>
Cc:        freebsd-pf@freebsd.org
Subject:   Re: pf tables locking
Message-ID:  <1546233.jncNNXsBuh@energia>
In-Reply-To: <A308CDBA-61DD-4684-B76B-E25BCCC621C6@FreeBSD.org>
References:  <8680316.SccKl5VnxN@energia> <2313127.kTuY2QdDqf@energia> <A308CDBA-61DD-4684-B76B-E25BCCC621C6@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart10585032.jW1J6F8Yqn
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"

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=E2=80=99re kept on the V_pf_unlinked rules list. Every so o=
ften 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 sta=
te
> are removed.
>=20
> This means that while there=E2=80=99s still a state which was created by =
the rule
> (and can thus put packets towards its table), the rule will exist. Once t=
he
> state goes away it=E2=80=99ll still take one full iteration through all s=
tates
> before the rule can be freed. Hence my statement that it=E2=80=99s 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=E2=80=99t 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 was=
te
> >> a little memory if we didn=E2=80=99t need it, but it should simplify t=
hings a
> >> bit.
> >>=20
> >> We can resolve the counting issue by using the counter_u64_*() functio=
ns
> >> 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 tryi=
ng
> to accomplish with this change?

I have a loadbalancer which uses pf and route-to targets. After a server is=
=20
added to a pool, I want this server to immediately take over much traffic.=
=20
With round-robin the server receives new clients rather slowly. If kernel=20
could measure amount of states per table entry, I could send new clients to=
=20
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=3D229092.
> > Some
> > more came out while working on this least-states loadbalancing. I will
> > group them into something meaningful and make another PR for them.
>=20
> Yeah, that bug is still on my todo list somewhere, but things are extreme=
ly
> hectic at the moment, and I can=E2=80=99t make any promises about when I=
=E2=80=99ll 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=
=20
submit the patch there, just for documentation), I will re-work my other=20
patches and show you what I came up with. I had working code for counting=20
states per table entry, I only lack the modified round-robin selection itse=
lf.

=2D-=20
| pozdrawiam / greetings | powered by Debian, FreeBSD and CentOS |
|  Kajetan Staszkiewicz  | jabber,email: vegeta()tuxpowered net  |
|        Vegeta          | www: http://vegeta.tuxpowered.net     |
`------------------------^---------------------------------------'
--nextPart10585032.jW1J6F8Yqn
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part.
Content-Transfer-Encoding: 7Bit

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQSOEQZObv2B8mf0JbnjtFCvbXs6FAUCW3IVAQAKCRDjtFCvbXs6
FEJtAJ40MRDrNLR4WN9gc9CX4B4on1dmjwCgudhTlMok6Oubi4U8/LPKDmzNFEg=
=Y4em
-----END PGP SIGNATURE-----

--nextPart10585032.jW1J6F8Yqn--




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1546233.jncNNXsBuh>