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>