From mboxrd@z Thu Jan 1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
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: Tue, 29 Mar 2022 10:37:14 +0100
Message-ID: <3CA4B0BB-1D54-41D5-A069-61AFA9262647@ipfire.org>
In-Reply-To: <349962CA-9202-427C-9D01-E1C5F7AE5764@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============4710843923513502903=="
List-Id: <development.lists.ipfire.org>
--===============4710843923513502903==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Hello,
This is indeed an absolute nightmare what I built here.
It has some advantages over the alternatives of which one is to have the DHCP=
server execute a couple of commands. The problems there are as follows:
* unbound does not keep a state, so we will have to make sure that we keep th=
e state of all records somewhere and can reload it when the system roboots.
* I have installations with thousands of devices in a single subnet. The DHCP=
server would just be busy constantly executing any commands which would caus=
e a lot of load. The current Python bridge can use up to 40% of one CPU core =
at busy times on the same system. I would argue that this approach does not s=
cale well.
* RFC2136 would be great, but we lack too many things to run this properly.
That being said, this is not a great way to solve this problem. I would like =
to replace some components entirely but probably not soon. The approach that =
we have works and - apart from the bug - doesn=E2=80=99t suck too hard. Shoul=
d we do it this way again? No, but we had to try to learn that lesson.
-Michael
> On 28 Mar 2022, at 20:32, Jon Murphy <jcmurphy26(a)gmail.com> wrote:
>=20
> Sorry for interrupting this train of thought. Wouldn=E2=80=99t it be easie=
r to use the tools available from dhcpd?
>=20
> dhcpd seems to have a way execute commands. And I would guess this would b=
e cleaner than "watching" a file.
>=20
> There are three execute commands:
> on commit
> on release
> on expiry
>=20
> and these could be used to launch the needed unbound bridge command.
>=20
> Here is where I found this:
> https://jpmens.net/2011/07/06/execute-a-script-when-isc-dhcp-hands-out-a-ne=
w-lease/
>=20
> And here is additional info:
> https://wiki.samba.org/index.php/Configure_DHCP_to_update_DNS_records
>=20
> (Search for "on commit" on this page)
>=20
>=20
> Jon
>=20
>=20
>> On Mar 28, 2022, at 12:00 PM, Michael Tremer <michael.tremer(a)ipfire.org>=
wrote:
>>=20
>> Hello Anthony,
>>=20
>> Thank you very much for submitting this patch and welcome to the list.
>>=20
>> I understand that it is easier to track any changes in the directory, but =
I don=E2=80=99t quite understand why the logic had to be changed that the cha=
nges will be applied at the end only.
>>=20
>> Is this just to avoid that multiple updates happen one after the other or =
what are you trying to achieve?
>>=20
>> -Michael
>>=20
>>> 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
>>>=20
>>=20
>=20
--===============4710843923513502903==--