dhcp: patch up static leases with missing prefix et al #4642
Comments
|
Isn't that a duplicate of #3657? Don't get me wrong, I would appreciate a solution, but am a bit surprised about the sudden change of mind. |
|
Yes, looks like it. I switched jobs in January to work on OPNsense full time so this is more of a reachable scope now. |
|
PR at #4723 |
Move all the duplication out of Unbound/Dnsmasq code and just iterate over the results there.
At first the idea was to pass it, but especially since this only passes one entry as noted by @maurice-w it is probably better to ditch it. :)
|
d0822b0 + e73db9c seems to fix it for DHCPv6 + Unbound. 👍 Didn't test dnsmasq, didn't test DHCPv4. Following up on my comment at d0822b0: Would be nice to have the full addresses in status_dhcpv6_leases.php, too. Not a functional issue, just a cosmetic one. While at it, can we avoid restarting radvd every time the dhcpd6 config changes? It's a long standing issue which causes the entire network to go down briefly when you e.g. add a static mapping. I guess we'd have to find out whether this entanglement is actually required or purely historic. |
|
Hmm, the code was pretty bad, it should have checked for ipaddrv6: db29e02#diff-892d19c546842230914680258f3f81046a0d541c8f0196bd82a1dbe628c3c82bL461 Anyway, db29e02 should be an improvement on the leases page. That's half of your requests done :) |
|
Did a quick test and it's looking good, thanks! |
|
We could pull the radvd reconfiguration from the main restart sequence and start it one layer up manually after dhcp reconfiguration. The resulting commit would be enough to discuss which calls need to stay and which need to be removed, ok? |
|
@fichtner Sounds good to me! |
fichtner
changed the title
Router advertisement pages were already doing it but the DHCPv4/6 pages not. That is problematic for the IPv6 case because there router advertisements would be reconfigured as well. This may not be the final form, but we need to see if this solves the main concern about intermittent connectivity bumps on GUI operations. PR: #4642
|
@maurice-w can you have a look at 012e0f7 ? Slightly different approach, see commit message |
|
Definitely an improvement. Changing DHCP settings doesn't restart radvd anymore. 👍 The only side effect I've noticed so far: If Use the DNS settings of the DHCPv6 server is enabled in the RA settings, changing the DHCPv6 DNS servers now requires a manual radvd restart to update RDNSS in radvd.conf. No surprise. |
|
There is a likely issue with the v6 leases page now expecting an IP address and a hostname so static leases may not show up. The question is what should be mandatory for static leases and what can we fill out from the lease page add button. IPv6 also not merging system leases and static leases. What is their respective lookup key.. the IP address or DUID or a combination? Reference: https://forum.opnsense.org/index.php?topic=22742.0 |
|
Hm, the bug report on the forum is a bit hard to follow...
Technically, a static mapping only requires a DUID and an IP address. Some DHCPv6 servers (e.g. Microsoft) won't allow reservations without a hostname though, so making this mandatory might be acceptable. For converting dynamic leases to static mappings, we should grab DUID + IP address + hostname from the leases page. Isn't that the case? Never really used that feature.
Sorry, I don't understand. Could you elaborate? Unless this is just a "note to self". :-) |
Router advertisement pages were already doing it but the DHCPv4/6 pages not. That is problematic for the IPv6 case because there router advertisements would be reconfigured as well. This may not be the final form, but we need to see if this solves the main concern about intermittent connectivity bumps on GUI operations. PR: #4642 (cherry picked from commit 012e0f7)
|
All code for this issue is now in 21.1.6 and we are almost there. One small regression reported with Unbound waiting for confirmation: |
yes, IPv6 leases page is a little weird still... otherwise I think we are set for 21.7. |
|
@maurice-w I'm following your suggestion to make only the IP mandatory for static lease consumers. For DHCP registration in DNS obviously we need to double-check for the hostname otherwise there is nothing to register. DHCPv6 leases page now merges static entries on DUID match and the DHCPv4 leases page will also use dhcpd_staticmap() which brings this particular saga to a happy end. @marjohn56 @maurice-w Thanks for all the help! Cheers, |
o Move the IPv6 recompress to dhcpd_staticmap() o Add DHCPv4 leases page as consumer of dhcpd_staticmap() o Emit the MAC address in IPv4 case in dhcpd_staticmap() o Let dhcpd_staticmap() emit valid entries with an IP address o Check for required hostname in Dnsmasq and Unbound integration
Router advertisement pages were already doing it but the DHCPv4/6 pages not. That is problematic for the IPv6 case because there router advertisements would be reconfigured as well. This may not be the final form, but we need to see if this solves the main concern about intermittent connectivity bumps on GUI operations. PR: opnsense/core#4642
Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
Describe the bug
When tracking DHCPv6 manually you can set suffix addresses and DHCPv6/RADVD completes them, but Unbound and Dnsmasq do not.
While here it would make sense to create an iterator to get rid of all the duplication in the code for IPv4 and IPv6 staticmap handling.
To Reproduce
See forum link.
Expected behavior
Patch up addresses in the manual tracking case but do not solve the changing prefix issue.
Screenshots
N/A
Relevant log files
N/A
Additional context
https://forum.opnsense.org/index.php?topic=21108.0
#2544
Environment
Most.
The text was updated successfully, but these errors were encountered: