From patchwork Sun Mar 24 15:49:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Nick Howitt X-Patchwork-Id: 7676 Return-Path: Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) client-signature ECDSA (secp384r1)) (Client CN "mail01.haj.ipfire.org", Issuer "R3" (verified OK)) by web04.haj.ipfire.org (Postfix) with ESMTPS id 4V2gVr0fRsz3wkd for ; Sun, 24 Mar 2024 15:49:56 +0000 (UTC) Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [172.28.1.201]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) client-signature ECDSA (secp384r1)) (Client CN "mail02.haj.ipfire.org", Issuer "R3" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 4V2gVp3zwWzmr; Sun, 24 Mar 2024 15:49:54 +0000 (UTC) Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4V2gVp2Qllz2xpL; Sun, 24 Mar 2024 15:49:54 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) client-signature ECDSA (secp384r1)) (Client CN "mail01.haj.ipfire.org", Issuer "R3" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4V2gVk6xMMz2xpL for ; Sun, 24 Mar 2024 15:49:50 +0000 (UTC) Received: from mailserver.howitts.co.uk (mailserver.howitts.co.uk [62.30.63.90]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mail01.ipfire.org (Postfix) with ESMTPS id 4V2gVk1d1Bz15g for ; Sun, 24 Mar 2024 15:49:50 +0000 (UTC) Authentication-Results: mail01.ipfire.org; dkim=pass header.d=howitts.co.uk header.s=202403 header.b=EDr0j3rH; spf=pass (mail01.ipfire.org: domain of nick@howitts.co.uk designates 62.30.63.90 as permitted sender) smtp.mailfrom=nick@howitts.co.uk; dmarc=pass (policy=quarantine) header.from=howitts.co.uk ARC-Seal: i=1; s=202003rsa; d=lists.ipfire.org; t=1711295390; a=rsa-sha256; cv=none; b=qxrqMA4+VlZ6+DRvD4X+sL54IBgFQIMNop6rWySnKbgnTvKXmwypMD3MPGGMXUM8G1TAiT t5ZAKLBNVw+E8+2cPWpk6uOO0XkQkaQ8bYzz5J61sXKWpVVMWA0DRbdo6RZygsldgmjbaX W1VF2olLK1HAg6A3IxZGPg7hAQq0evBGrYYywhygMgeqEMcFD+UCtfKHbcjHMnc3gcSUYd AaA8ukqtYSCcpYpGn0w0zsnA+5OnzUeeNCF0H23CZBeltGUl1ADVauZAexPPNrkgS1XEky TdyTMWusRwp7cajwqaJ7BSACeeVWdI/EpskKbqWcZ9aV6IevXDQ7A0Vs3ZETiA== ARC-Authentication-Results: i=1; mail01.ipfire.org; dkim=pass header.d=howitts.co.uk header.s=202403 header.b=EDr0j3rH; spf=pass (mail01.ipfire.org: domain of nick@howitts.co.uk designates 62.30.63.90 as permitted sender) smtp.mailfrom=nick@howitts.co.uk; dmarc=pass (policy=quarantine) header.from=howitts.co.uk ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=lists.ipfire.org; s=202003rsa; t=1711295390; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding:dkim-signature; bh=+iJOho5t6bdbb1N4CFDVVWdXtGaH56VMT42T9ukspg8=; b=NkCymQmlE/A66J8jtN2Lb7uLZjzrRrLK/CvEsKXrKIrL95Yc0VtdCBpcKyudlvXXzBzT0G +ONom4haZbQPRNqiDGJzEgUjqciRDgQNdawBDrS/cXDDysEY08E0EwBIrW1JcFXQiTk9by A6oG0hmPMILRjQUiSd0980TeiMULjltsx9xkTzRf28/E2G0gTioY1hfjhFRF1zgQmCW7ro /VSOyfT+16Ym1bVYp9feDDOJrX1Gpi9bPIkPz0yqq0+73ycclWS3z8UDI0+GRKRlybeB0p fE27M/b21lZsE6E4j1lqI1N4dRujMgtahlczLeILFlSx5TKIYfakCoxI+5J3cQ== Received: from localhost (localhost [127.0.0.1]) by mailserver.howitts.co.uk (Postfix) with ESMTP id BA67BC20EA3F for ; Sun, 24 Mar 2024 15:49:49 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=howitts.co.uk; h= content-transfer-encoding:content-type:content-type:subject :subject:from:from:content-language:user-agent:mime-version:date :date:message-id:received:received:received; s=202403; t= 1711295387; x=1713109788; bh=Rao22kXEY1QPLwKbvxC/9rIOemlXIypOUUc qSQnixds=; b=EDr0j3rHHyesmElZxdXXOea5B5kmzaq9Jqfu2DXbVX675NM8ZvN 2TiGSZiek4JSCZy7fyrGFot0hke6fHKDBJTR9EuSPhICC6VcGbYHfJxGIaxl2YhL FPw8xz6Ti45Muvfcn4ywSNF7XqWMvwNB55dwxwugaIYi+WTuet4U7FBcUbLP5eVs XE4SxhgP+Af0lEWCEyCxStDU+V/DiixFJefw//+8+F0I2XBHK2/V1cmd9VjyxLBa urKACKtmUEOvQHoNi7bvaKSTGIbqkuhJ4isgHCR5fl/ySn3k7aFDFKKxB6h2/vBb AtyYN8O1kCYb+UmW+kSiKcRBZUvaixvkKFjFD85EnI/lG4bsicDf8FKcAUBOcLjU Ku6eKgh0l93LBK/RU1DrAMR/cwW7Aiw9kd/LpApbsecwsAFpvaKwhGkmzv8TtbBx aAukYPT4ACirWo+7ITCtiqCQmNACcF2SEm1tWD1voBs2hHLpsxPkfIJQqlit8wy4 DvZHo1ls+RsPakwC+qq07QVAUTG8XZRmNb8Z+pYVUY5Q+dncW4uVJVkSijckYg+K VRrwb3idd93gVUFBu0aSHehp/LfvzFXCg5AWwoZbLz/rsaXF52L9a9oWkZ9Ca5u3 dHG4e4EeOBKCaOYGWfpCc9+UQQ4dl6/sxSeWpPWMjbRvFUbCZ7DM5mQQ= X-Virus-Scanned: amavisd-new at howitts.co.uk Received: from mailserver.howitts.co.uk ([127.0.0.1]) by localhost (server.howitts.co.uk [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qMJetVQ3z96C for ; Sun, 24 Mar 2024 15:49:47 +0000 (GMT) Received: from localhost (localhost [127.0.0.1]) by mailserver.howitts.co.uk (Postfix) with ESMTP id 009A2C20EAB4 for ; Sun, 24 Mar 2024 15:49:47 +0000 (GMT) Received: from [172.17.4.212] (unknown [172.17.4.212]) by mailserver.howitts.co.uk (Postfix) with ESMTPSA id E3B91C20EA3F for ; Sun, 24 Mar 2024 15:49:46 +0000 (GMT) Message-ID: Date: Sun, 24 Mar 2024 15:49:46 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-GB To: development@lists.ipfire.org From: Nick Howitt Subject: [PATCH] Stop unbound-dhcp-leases-bridge from continually restarting unbound X-Rspamd-Queue-Id: 4V2gVk1d1Bz15g X-Spamd-Result: default: False [-4.72 / 11.00]; BAYES_HAM(-2.99)[99.96%]; NEURAL_HAM(-0.74)[-0.742]; DMARC_POLICY_ALLOW(-0.50)[howitts.co.uk,quarantine]; R_SPF_ALLOW(-0.20)[+mx]; R_DKIM_ALLOW(-0.20)[howitts.co.uk:s=202403]; MIME_GOOD(-0.10)[text/plain]; IP_REPUTATION_SPAM(0.01)[asn: 5089(0.00), country: GB(0.00), ip: 62.30.63.90(0.00)]; MX_GOOD(-0.01)[]; XM_UA_NO_VERSION(0.01)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; RCPT_COUNT_ONE(0.00)[1]; ARC_NA(0.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; RCVD_TLS_LAST(0.00)[]; RCVD_COUNT_THREE(0.00)[4]; FROM_HAS_DN(0.00)[]; DKIM_TRACE(0.00)[howitts.co.uk:+]; PREVIOUSLY_DELIVERED(0.00)[development@lists.ipfire.org]; TO_DN_NONE(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; DKIM_REPUTATION(0.00)[0]; ASN(0.00)[asn:5089, ipnet:62.30.0.0/16, country:GB]; ARC_SIGNED(0.00)[lists.ipfire.org:s=202003rsa:i=1] X-Rspamd-Server: mail01.haj.ipfire.org X-Rspamd-Action: no action Message-ID-Hash: 4RK4PR7M57QLHJXHYFWQ32QUGPPW5PRQ X-Message-ID-Hash: 4RK4PR7M57QLHJXHYFWQ32QUGPPW5PRQ X-MailFrom: nick@howitts.co.uk X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.8 Precedence: list List-Id: IPFire development talk Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi all, **** This is a repost of an email incorrectly submitted. **** Please bear with me as I am new to IPFire and not at all used to this style of development, so any guidance would be appreciated. There is a bug, https://bugzilla.ipfire.org/show_bug.cgi?id=13254, where unbound-dhcp-leases-bridge restarts unbound a lot when it is totally unnecessary. It appears to be because when a lease is renewed, the script gets triggered, created a new /etc/unbound/dhcp-leases.conf then restarts Unbound to read the file. Generally this seems to be unnecessary as, with a renewed lease, the old and new /etc/unbound/dhcp-leases.conf files are the same. With 5 leases in that file (one for a machine not active) I am getting 3-4 restarts an hour of Unbound when I have a min/max lease time of 60/120min. Looking at the code, it is fairly easy to fix. The current code creates a temp file then copies it into place then restarts unbound. All I am doing is doing a file comparison before the copy and skipping the restart if the files are the same. There were a couple of gotchas because setting the file attributes and copying the file were done inside the "with" block for generating the temporary file. This meant a file comparison always returned False as the temp file was still open and so never the same is the old file. I moved those two statements outside the "with". This forced me to change the fchmod to chmod. It could be argued that the file copy should not be done if the files are not different, but it would take an extra "if" and you'd have to remember to delete the temp file. If required, I can make that change. Also, one small thing I noticed once is that the old and new dhcp-leases.conf files could occasionally contain the same leases but in a different order. I have been unable to reproduce, but to sidestep it, instead of stepping through the leases variable directly, I sort it and step through that. It should make the resulting file completely deterministic and make the file comparison more effective. My patch is: From 73873d4944944a2f02317a73074a6894726f36f7 Mon Sep 17 00:00:00 2001 From: Nick Howitt Date: Sun, 24 Mar 2024 15:17:19 +0000 Subject: [PATCH] Stop unbound-dhcp-leases-bridge from restarting unbound every  time it write dhcp-leases.conf as it is very often unchanged and does not  require a restart. ---  config/unbound/unbound-dhcp-leases-bridge | 28 +++++++++++++++--------  1 file changed, 19 insertions(+), 9 deletions(-)      def _control(self, *args):          command = ["unbound-control"] diff --git a/config/unbound/unbound-dhcp-leases-bridge b/config/unbound/unbound-dhcp-leases-bridge index e9f022aff..d22772066 100644 --- a/config/unbound/unbound-dhcp-leases-bridge +++ b/config/unbound/unbound-dhcp-leases-bridge @@ -22,6 +22,7 @@  import argparse  import datetime  import daemon +import filecmp  import functools  import ipaddress  import logging @@ -516,26 +517,35 @@ class UnboundConfigWriter(object):      def update_dhcp_leases(self, leases):          # Write out all leases -        self.write_dhcp_leases(leases) +        if self.write_dhcp_leases(leases): -        log.debug("Reloading Unbound...") +            log.debug("Reloading Unbound...") -        # Reload the configuration without dropping the cache -        self._control("reload_keep_cache") +            # Reload the configuration without dropping the cache +            self._control("reload_keep_cache") + +        else: + +            log.debug("Not reloading Unbound. Leases not changed.")      def write_dhcp_leases(self, leases):          log.debug("Writing DHCP leases...")          with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: -            for l in leases: +            for l in sorted(leases):                  for rr in l.rrset:                      f.write("local-data: \"%s\"\n" % " ".join(rr)) -            # Make file readable for everyone -            os.fchmod(f.fileno(), stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IROTH) +        filecmp.clear_cache() +        RequireUnboundReload = not filecmp.cmp(f.name, self.path, shallow=False) + +        # Make file readable for everyone +        os.chmod(f.name, stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IROTH) + +        # Move the file to its destination +        os.rename(f.name, self.path) -            # Move the file to its destination -            os.rename(f.name, self.path) +        return RequireUnboundReload