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

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

A reservation rewrite dropped every line that no longer matched the
MAC cache and wrote nothing in its place. The guest behind such a
line was unknown to dnsmasq until its next start, so its requests
were ignored. The line is now replaced by the cache's current one.
Only a MAC the cache holds no address for is dropped.

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 d3b78fe9a894..a1e4c70f9ba4 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. Dropping it left 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 e9c9bbb6be32..368ed5565554 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1079,4 +1079,32 @@ run_test(
     2,
 );
 
+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