From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Heading <ajrh@ajrh.net>
To: development@lists.ipfire.org
Subject: Re: [PATCH 1/3] unbound-dhcp-leases-bridge : fix bug 12694 - DHCP
 hosts not reliably propagated to DNS
Date: Mon, 28 Mar 2022 15:55:34 -0400
Message-ID: <7f606217-40a8-43ee-97f0-e4f83b6fd652@www.fastmail.com>
In-Reply-To: <6CBBDF3C-A821-4ED3-B056-11EEAF13ACA0@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1528773157081152156=="
List-Id: <development.lists.ipfire.org>

--===============1528773157081152156==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Michael,

That was exactly it, just sidestepping an intuitive race with dhcpd writing. =
 Somewhat hypothetical why that would happen, e.g. clustered updates for mult=
iple NICs or whatever similar non-atomicity,  but it was easy to express that=
 way,  put the update logic in one place (including the initial one at startu=
p), and made the event timestamps sequences a little easier to follow when it=
 wasn't clear why the updates were getting lost at all.  I thought the boolea=
n variables helped document the admittedly simple logic too.   But none of th=
at is critical.

A

On Mon, Mar 28, 2022, at 1:00 PM, Michael Tremer wrote:
> Hello Anthony,
>
> Thank you very much for submitting this patch and welcome to the list.
>
> I understand that it is easier to track any changes in the directory,=20
> but I don=E2=80=99t quite understand why the logic had to be changed that t=
he=20
> changes will be applied at the end only.
>
> Is this just to avoid that multiple updates happen one after the other=20
> or what are you trying to achieve?
>
> -Michael
>
>> On 22 Mar 2022, at 03:47, Anthony Heading <ajrh(a)ajrh.net> wrote:
>>=20
>> Switch from inotify watching individual files to monitoring the
>> containing directories, as because dhcpd renames its leases file into a
>> backup, monitoring the single inode does not work well.  Additionally,
>> python appears to have a bug with replacing expired inotify watches on
>> single files.
>> ---
>> unbound-dhcp-leases-bridge | 47 +++++++++++++++++++++++++-------------
>> 1 file changed, 31 insertions(+), 16 deletions(-)
>>=20
>> diff --git unbound-dhcp-leases-bridge unbound-dhcp-leases-bridge
>> index a2df5f1..6e22066 100644
>> --- unbound-dhcp-leases-bridge
>> +++ unbound-dhcp-leases-bridge
>> @@ -72,6 +72,15 @@ class UnboundDHCPLeasesBridge(object):
>> 		self.fix_leases_file =3D fix_leases_file
>> 		self.hosts_file =3D hosts_file
>>=20
>> +		# base mask for a completed file change
>> +		mask =3D inotify.constants.IN_CLOSE_WRITE | inotify.constants.IN_MOVED_=
TO
>> +		# IN_MODIFY since dhcpd appends lease updates to an open file
>> +		self.watches =3D {
>> +		    self.leases_file: mask | inotify.constants.IN_MODIFY,
>> +		    self.fix_leases_file: mask,
>> +		    self.hosts_file: mask
>> +		}
>> +
>> 		self.unbound =3D UnboundConfigWriter(unbound_leases_file)
>> 		self.running =3D False
>>=20
>> @@ -80,36 +89,42 @@ class UnboundDHCPLeasesBridge(object):
>> 		self.running =3D True
>>=20
>> 		# Initial setup
>> -		self.hosts =3D self.read_static_hosts()
>> -		self.update_dhcp_leases()
>> +		update_hosts =3D True
>> +		update_leases =3D True
>> +
>> +		i =3D inotify.adapters.Inotify()
>>=20
>> -		i =3D inotify.adapters.Inotify([
>> -			self.leases_file,
>> -			self.fix_leases_file,
>> -			self.hosts_file,
>> -		])
>> +		for f in self.watches:
>> +			i.add_watch(os.path.dirname(f), self.watches[f])
>>=20
>> 		for event in i.event_gen():
>> 			# End if we are requested to terminate
>> 			if not self.running:
>> 				break
>>=20
>> +			# Make pending updates once inotify queue is empty
>> 			if event is None:
>> +				if update_hosts:
>> +					self.hosts =3D self.read_static_hosts()
>> +					update_hosts =3D False
>> +				if update_leases:
>> +					self.update_dhcp_leases()
>> +					update_leases =3D False
>> 				continue
>>=20
>> 			header, type_names, watch_path, filename =3D event
>>=20
>> -			# Update leases after leases file has been modified
>> -			if "IN_MODIFY" in type_names:
>> -				# Reload hosts
>> -				if watch_path =3D=3D self.hosts_file:
>> -					self.hosts =3D self.read_static_hosts()
>> +			file =3D os.path.join(watch_path, filename)
>> +
>> +			if not file in self.watches:
>> +				continue
>> +
>> +			log.debug("Inotify %s: %s", file, " ".join(type_names))
>>=20
>> -				self.update_dhcp_leases()
>> +			update_leases =3D True
>>=20
>> -			# If the file is deleted, we re-add the watcher
>> -			if "IN_IGNORED" in type_names:
>> -				i.add_watch(watch_path)
>> +			if file =3D=3D self.hosts_file:
>> +				update_hosts =3D True
>>=20
>> 		log.info("Unbound DHCP Leases Bridge terminated")
>>=20
>> --=20
>> 2.34.1
>>

--===============1528773157081152156==--