From owner-freebsd-pf@freebsd.org Mon Aug 13 23:32:26 2018 Return-Path: Delivered-To: freebsd-pf@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DAD081055E07 for ; Mon, 13 Aug 2018 23:32:25 +0000 (UTC) (envelope-from vegeta@tuxpowered.net) Received: from mail-ed1-x541.google.com (mail-ed1-x541.google.com [IPv6:2a00:1450:4864:20::541]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 6E0F571EF1 for ; Mon, 13 Aug 2018 23:32:25 +0000 (UTC) (envelope-from vegeta@tuxpowered.net) Received: by mail-ed1-x541.google.com with SMTP id r4-v6so9164842edp.9 for ; Mon, 13 Aug 2018 16:32:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tuxpowered-net.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:organization:user-agent :in-reply-to:references:mime-version; bh=w+3bHCX5z0x8UDn9p8Pcsl45s9bLbDZsPFxKoF9pd9c=; b=sD9DY61kYhFcf6JU5jjCPTl1wjf1Qknw16qfhE13uDqJ6F031T5bNnH8SSWuPmwoMR Y64oAREiEeT/6ENxVOWTkgYPxpT3427jjefK/pfCPNXktb9SXYET1sCzolAXFidavTOr FCGQGDwHfuQZjgIWGImpa+tYbIRr6QIcbf/+R4xWcpiSWNsZesScVxHPkDK0ht4A1mjy lrvJlU/FDSmwMGrdgMumulw9ZU+lZOoiFTwngUAMru1HMz80D30Jar0SHAk73nd83uj9 RBi405rE87yVsuLKytPq4oAC2NX5FwMh0mgfm7GyzgoMg7BGjRdCqMhnMh5jN8LzwYRy MrXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version; bh=w+3bHCX5z0x8UDn9p8Pcsl45s9bLbDZsPFxKoF9pd9c=; b=Pqn5QxLMN6K3kqqt5Vk6twQvludy7X5VNZikH8JCWMN4u7OEu0tvntSSXGJPTd4Yqq Lu77JkTEleiV3CoWS8PaAiFYc60SSf+AYHlbhJFUptL9AwnXtPOu/5ICU7J+GSMLdg3F 4MkRK78Miyt3TJCjOSSqT3c3QXRVN4Tv29M7Nn7JprI6hVQDbineIjQKDNb5Y2AcX8sh SbSDJ344oHMMXq27oC7MSqtntASfOeReWsarbWhHIncXGPtVa6jZpB2LVLVkNhlrlPa/ cXpO008AOqJsnJITDmul90nuS9TqqXqmHQ3DhhDFfpqT7+pFri3q7NbJMDYDfS4HoOav gojA== X-Gm-Message-State: AOUpUlEiGZF86MyfN4APWA+Pxz33vGayv0qnUT3e2zq/UQff/yEIs1HX KMbLFDYRJbOq7vu6ujOn0bp+ZSV3llI= X-Google-Smtp-Source: AA+uWPyRxYpxi+AEZ2ucPnWqc86YTvrB6JsKy5GY5ahLr4NopjHDu5/PM/rgwUQ5DpBeSZNPgY3HIg== X-Received: by 2002:a50:8c06:: with SMTP id p6-v6mr24310644edp.282.1534203144192; Mon, 13 Aug 2018 16:32:24 -0700 (PDT) Received: from energia.localnet ([2a02:8108:50bf:d514::5]) by smtp.gmail.com with ESMTPSA id c21-v6sm14117607eda.21.2018.08.13.16.32.22 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Aug 2018 16:32:22 -0700 (PDT) From: Kajetan Staszkiewicz To: Kristof Provost Cc: freebsd-pf@freebsd.org Subject: Re: pf tables locking Date: Tue, 14 Aug 2018 01:32:17 +0200 Message-ID: <1546233.jncNNXsBuh@energia> Organization: tuxpowered.net User-Agent: KMail/5.2.3 (Linux/4.16.0-16.2-liquorix-amd64; KDE/5.28.0; x86_64; ; ) In-Reply-To: References: <8680316.SccKl5VnxN@energia> <2313127.kTuY2QdDqf@energia> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart10585032.jW1J6F8Yqn"; micalg="pgp-sha1"; protocol="application/pgp-signature" X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Aug 2018 23:32:26 -0000 --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--