search

openhab / openhabian

openHABian - empowering the smart home, for Raspberry Pi and Debian systems
https://community.openhab.org/t/13379
ISC License
818 stars 251 forks source link

Avoid running two DHCP clients (#1690) #1691

Closed Nadahar closed 2 years ago

Nadahar commented 2 years ago

This hopefully fixes #1690. It does so by reverting fc1943afa22208a40e0c372d6271989f9af24adb and 99a8be1770bc95d4925ad44cd516531ba25ce851 so that avahi-autoipd and isc-dhcp-client aren't installed. Removing avahi-autoipd should also make adding noipv4ll to /etc/dhcpcd.conf unnecessary.

If somebody actually needs AutoIP and thus installs avahi-autoipd, they wouldn't want the functionality to be blocked by the noipv4ll statement. I have no idea whether avahi-autoipd will work without isc-dhcp-client, but it's not defined as a dependency, only as "recommended" - so that's basically anybody's guess. At least there's a chance that it would work if they do:

apt-get install --no-install-recommends avahi-autoipd

Ideally this should be tested/verified and documented if it does work, for the very few that might need it.

Please note that I have no way to test/verify this except knowing that uninstalling avahi-autoipd and isc-dhcp-client makes my installation behave correctly and only assign one IP address to wlan0. Think of this more as a suggestion than as a PR that is ready to merge.

mstormi commented 2 years ago

Thanks for digging deep. Seems you found and revealed some very old (2018) bad implementation choice that I wasn't even aware existed.

Reading up on your analysis, I agree that we should not need to have avahi-autoipd there in default openHABian, anyone who wants to can install it themself. So your fix is probably the right one. Then again I don't know what other side effects your removal PR might bring and I have no means to test either in a universal sense other than to merge and see what happens to people on new installations. Guess I have to sleep it over.

Meanwhile, could you please fix signoff on your commit to fix the DCO test ? thx

Nadahar commented 2 years ago

Regarding the DCO stuff, I read your "contribution rules" and they clash with a strict rule of mine: Always be anonymous in public online, not because my identity is some kind of secret but because I don't want search engines to index everything I do on my name and make it very easy to get an overview over my activities. But, even if I were to break that "rule of mine" and use my real name, using my real e-mail address is completely out of the question. I've had to change e-mail address before because I got more than 1000 spam e-mails a day in the past (I used it in public back then), and I'm not about to repeat all the hassle changing my e-mail address involves again.

I've had to abandon PRs for the same reasons before, and it's also the reason I haven't bothered trying to make PRs for openHAB itself. These changes are tiny though, so it wouldn't cost you many seconds to do the same changes in a commit of your own.

If I could find the images built by GitHub Actions for this PR, I could at the very least flash one to my SD card and to a "fresh install" to verify that things "seems to work ok". Are they possible for me to get my hands on somewhere?

edit: I managed to find the image built by this PR under "actions", so I'm preparing to try to flash it now.

Nadahar commented 2 years ago

I've done a new installation using the image built by Github Actions on this PR. I can't find any problems as this is now. The change that makes the difference is to stop writing a WiFi configuration to /etc/network/interfaces.

It seems to me that neither avahi-autoipd nor the noipv4ll setting in /etc/dhcpcd.conf is directly related to the "dual DHCP client issue". I'm pretty sure that this branch would also fix #1456, since it avoids having two DHCP clients running creating all kinds of havoc. Therefore I've built this image without the noipv4ll setting in /etc/dhcpcd.conf. Auto-IP now also works like I think it should, if I disable my DHCP server it will assign a 169.254.x.x address, but with the DHCP running it will assign one leased from the DHCP server. The only change I can find when setting noipv4ll in /etc/dhcpcd.conf is that without the DHCP server, no IP will be assigned at all. It's hard to judge what is the "most desirable" behavior, but I guess I'd say that having it assign an auto-IP when no DHCP is available is marginally better - though probably completely irrelevant for most users.

From what I can deduct from my own testing and some online searching, dhcpcd doesn't use avahi-autoipd, but has its own implementation instead. Thus, only the noipv4ll setting matters for dhcpcd. dhclient on the other hand seems to need avahi-autoipd to assign auto-IPs, but as long as the choice has been made upstream to use dhcpcd instead of dhclient, the only sane thing to do is to stay far away from anything dhclient. So, I'd say that installing avahi-autoipd should be completely unnecessary, but is most likely harmless. I left it in during the build for "isolation purposes" when testing, to narrow the changes down as much as possible. Since it doesn't seem to be related to the dual IP issue, one could argue that it has nothing to do with this PR - at the same time I'd say that not installing packages that aren't needed or helpful should generally be the preferred behavior.

The good thing after finally getting rid of dhclient is that now network configuration is much, much simpler. All is done in /etc/dhcpcd.conf. It works with DHCP out of the box, and I can't see why openHABian would need to touch the file at all, except for during the "emergency hotspot configuration". That means that people can configure it manually if they so wish, and there should be no "interference" from openHABian in that configuration, making things suddenly break.

I've now tested this with a static IP configured in /etc/dhcpcd.conf and with a "static fallback" IP, as shown in the comments in the same file. Both solutions work as expected. This means that it should be safe to remove the "advise" for people not to configure openHABian with a static IP. It should just be pointed out that any such changes must be made in /etc/dhcpcd.conf and that /etc/network/interfaces must be left alone.

Since this PR can never be merged, I haven't rebased the commits (or even bothered to GPG sign them). That makes it easier to see the changes if you chose to use any part of this.

mstormi commented 2 years ago

I'd suggest to re-remove avahi-autoipd as you say it's unneeded and we can save on valuable RAM that way. If you put that change back in (i.e. avahi-autoipd out) I can merge it without that you would need to sign.

Nadahar commented 2 years ago

@mstormi I agree, it's done.