[PATCH pve-network v4 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it

[Hannes Laimer <h.laimer@proxmox.com>]

Adding a reservation rewrote the zone's dnsmasq reservation file and
dropped every line that no longer matched the MAC cache. Since dnsmasq
answers only MACs with a reservation, the guest behind such a line got
no DHCP answer until its next start. Such a line now gets the
addresses the cache holds for its MAC, and is dropped only when the
cache holds none.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 11 ++++++++++-
 src/test/run_test_vnets_blackbox.pl | 28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index dc70b9a99a2e..c736d02d8f23 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -91,7 +91,9 @@ sub add_ip_mapping {
             }
             $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
 
-            #delete changed
+            # a line that no longer matches the cache is replaced by the cache's
+            # current one, a dropped one would leave that guest unknown to
+            # dnsmasq until its next start
             if (
                 !defined($macdb->{macs}->{$parsed_mac})
                 || ($parsed_ip4
@@ -102,6 +104,13 @@ sub add_ip_mapping {
                     && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
             ) {
                 $reload = 1;
+                # the MAC being added gets its line below, like a missing one
+                my $current = $macdb->{macs}->{$parsed_mac};
+                next if $parsed_mac eq $mac || !$current;
+                my $replacement = $parsed_mac;
+                $replacement .= ",$current->{ip4}" if $current->{ip4};
+                $replacement .= ",[$current->{ip6}]" if $current->{ip6};
+                print $out "$replacement\n" if $replacement ne $parsed_mac;
                 next;
             }
 
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index be4c581e414e..90241268077c 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1107,4 +1107,32 @@ sub test_update_vnet_without_zone {
 }
 run_test(\&test_update_vnet_without_zone);
 
+sub test_dnsmasq_stale_reservation_replaced {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+
+    my $stale = "da:65:8f:18:9b:01";
+    my $gone = "da:65:8f:18:9b:02";
+    my $added = "da:65:8f:18:9b:03";
+    $test_state->{macdb} = {
+        macs => {
+            $stale => { ip4 => '10.0.1.11' },
+            $added => { ip4 => '10.0.1.13' },
+        },
+    };
+    PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
+
+    PVE::Network::SDN::Dhcp::Dnsmasq->add_ip_mapping(
+        'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
+    );
+
+    my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
+    eq_or_diff(
+        \@lines,
+        ["$stale,10.0.1.11", "$added,10.0.1.13"],
+        "$test_name: the stale line is replaced, the unknown one dropped",
+    );
+}
+
+run_test(\&test_dnsmasq_stale_reservation_replaced);
+
 done_testing();
-- 
2.47.3