[PATCH pve-network v4 08/20] sdn: dhcp: update reservations when ip mappings change

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

Creating, updating or deleting an IP mapping through the API changed
the IPAM and the MAC cache, but never the zone's dnsmasq reservation
file. dnsmasq kept serving the old reservation, or none for a new
mapping, until the guest's next start. Deleting a guest NIC's
addresses left its reservation in the file as well.

A guest start, an API edit and the deletion of a NIC's addresses now
all rewrite the MAC's reservation from the cache, on the node the
change is made on. The other nodes catch up when the guest starts
there. The zone and its backend come from the applied config, and a
zone confined to other nodes is skipped. A failed update only warns,
since the IPAM records are already written.

The rewrite reads the cache only once it holds the file's lock. A
read from before could undo a rewrite that ran in between. The lock
is on a separate file, since a lock on the reservation file would not
survive the rename that replaces it. The MAC's line is written in full
from the cache, so a guest gaining an IPv6 address next to its IPv4
one gets it reserved too. The file's lines are written and compared
in lower case, since lines from before carry their writer's spelling.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---

Notes:
    v4:
     * use the cache as read, with its lower-cased keys

 src/PVE/API2/Network/SDN/Ips.pm     |   7 +
 src/PVE/Network/SDN/Dhcp.pm         |  48 +++---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 101 ++++++-----
 src/PVE/Network/SDN/Dhcp/Plugin.pm  |   7 +-
 src/PVE/Network/SDN/Vnets.pm        |   5 +-
 src/test/run_test_vnets_blackbox.pl | 250 +++++++++++++++++++++++++++-
 6 files changed, 340 insertions(+), 78 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 5ff05e75..d7b682dd 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -46,6 +46,8 @@ __PACKAGE__->register_method({
         eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
         die "$@\n" if $@;
 
+        PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
         return undef;
     },
 });
@@ -82,6 +84,8 @@ __PACKAGE__->register_method({
 
         PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
 
+        PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
         return undef;
     },
 });
@@ -132,6 +136,9 @@ __PACKAGE__->register_method({
         }
 
         die "$error\n" if $error;
+
+        PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
         return undef;
     },
 });
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 65e40d40..ab5d6b5b 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -6,7 +6,6 @@ use warnings;
 use PVE::Cluster;
 
 use PVE::Network::SDN;
-use PVE::Network::SDN::Ipams;
 use PVE::Network::SDN::Subnets;
 use PVE::Network::SDN::Dhcp::Plugin;
 use PVE::Network::SDN::Dhcp::Dnsmasq;
@@ -22,37 +21,32 @@ sub plugin_types {
     return PVE::Network::SDN::Dhcp::Plugin->lookup_types();
 }
 
-sub add_mapping {
-    my ($vnetid, $mac, $ip4, $ip6) = @_;
+# re-apply a MAC's mapping on this node from the current records, best effort
+# since the record write stays authoritative. A caller that knows the guest
+# passes its vmid, so only that guest's config is read
+sub update_mapping {
+    my ($vnetid, $mac, $vmid) = @_;
 
-    my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
-    return if !$vnet;
+    # a record without a MAC has no guest NIC behind it, so no backend serves it
+    return if !$mac;
 
-    my $zoneid = $vnet->{zone};
-    my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+    eval {
+        # the backend to push to is the applied one, since a zone edited but
+        # not applied has nothing running yet
+        my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1);
+        return if !$vnet;
 
-    return if !$zone->{ipam} || !$zone->{dhcp};
+        my $zoneid = $vnet->{zone};
+        my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, 1);
+        return if !$zone || !$zone->{ipam} || !$zone->{dhcp};
 
-    my $dhcptype = $zone->{dhcp};
+        # a zone confined to other nodes has no backend running here
+        return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
 
-    my $macdb = PVE::Network::SDN::Ipams::read_macdb();
-    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcptype);
-    $dhcp_plugin->add_ip_mapping($zoneid, $macdb, $mac, $ip4, $ip6);
-}
-
-sub remove_mapping {
-    my ($vnetid, $mac) = @_;
-
-    my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
-    return if !$vnet;
-
-    my $zoneid = $vnet->{zone};
-    my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
-
-    return if !$zone->{ipam} || !$zone->{dhcp};
-
-    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
-    $dhcp_plugin->del_ip_mapping($zoneid, $mac);
+        PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
+            ->update_ip_mapping($zoneid, $mac, $vmid);
+    };
+    warn "could not update dhcp mapping for $mac: $@" if $@;
 }
 
 sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index a1e4c70f..2d18706f 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -12,6 +12,7 @@ use File::Copy;
 use Net::DBus;
 
 use PVE::RESTEnvironment qw(log_warn);
+use PVE::Network::SDN::Ipams;
 
 my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
 my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -51,83 +52,97 @@ sub update_lease {
     $manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
 }
 
-sub add_ip_mapping {
-    my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+sub update_ip_mapping {
+    my ($class, $dhcpid, $mac) = @_;
     $mac = lc($mac);
 
     my $ethers_file = ethers_file($dhcpid);
     my $ethers_tmp_file = "$ethers_file.tmp";
 
-    my $reload = undef;
+    my ($ip4, $ip6);
+    my $reservation = undef;
+    my $changed = undef;
 
-    my $appendFn = sub {
+    my $rewriteFn = sub {
+        # read under the ethers lock. A snapshot from before it would sweep a
+        # line another push wrote meanwhile, and an older push would put its
+        # addresses back
+
+        my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
+
+        my $entry = $macs->{$mac};
+        ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
+        if ($ip4 || $ip6) {
+            $reservation = $mac;
+            $reservation .= ",$ip4" if $ip4;
+            $reservation .= ",[$ip6]" if $ip6;
+            # an external IPAM spells addresses its own way, while the file holds one spelling
+            $reservation = lc($reservation);
+        }
+
+        # the first reservation of a zone finds no file yet
+        file_set_contents($ethers_file, '') if !-e $ethers_file;
         open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
         open(my $out, '>', $ethers_tmp_file)
             or die "Could not open file '$ethers_tmp_file' $!\n";
 
-        my $match = undef;
+        my $kept = undef;
 
-        my $line_no = 0;
         while (my $line = <$in>) {
-            $line_no++;
             chomp($line);
+            next if $line !~ m/\S/;
             my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
+            # a line from before the file was written in lower case keeps its writer's spelling
+            $parsed_mac = lc($parsed_mac);
 
-            if (!defined($parsed_mac)) {
-                warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
+            # the MAC's own reservation is rewritten. An unchanged one stays in place
+            if ($parsed_mac eq $mac) {
+                if (defined($reservation) && !$kept && lc($line) eq $reservation) {
+                    $kept = 1;
+                    print $out "$line\n";
+                } else {
+                    $changed = 1;
+                }
                 next;
             }
-            # the cache is keyed in lower case, a line from before keeps its writer's spelling
-            $parsed_mac = lc($parsed_mac);
 
             my ($parsed_ip4, $parsed_ip6);
             if ($parsed_ip2) {
                 $parsed_ip4 = $parsed_ip1;
                 $parsed_ip6 = $parsed_ip2;
-            } elsif (Net::IP::ip_is_ipv4($parsed_ip1)) {
+            } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
                 $parsed_ip4 = $parsed_ip1;
-            } else {
+            } elsif (defined($parsed_ip1)) {
                 $parsed_ip6 = $parsed_ip1;
             }
             $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
+            # a line from before addresses were lower-cased may spell one in upper case
+            $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
 
             # 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
+            my $current = $macs->{$parsed_mac};
             if (
-                !defined($macdb->{macs}->{$parsed_mac})
-                || ($parsed_ip4
-                    && $macdb->{macs}->{$parsed_mac}->{'ip4'}
-                    && $macdb->{macs}->{$parsed_mac}->{'ip4'} ne $parsed_ip4)
-                || ($parsed_ip6
-                    && $macdb->{macs}->{$parsed_mac}->{'ip6'}
-                    && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
+                !$current
+                || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
+                || ($parsed_ip6 && $current->{ip6} && lc($current->{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;
+                $changed = 1;
+                next if !$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;
+                print $out lc($replacement) . "\n";
                 next;
             }
 
-            if ($parsed_mac eq $mac) {
-                $match = 1 if $ip4 && $parsed_ip4 && $ip4;
-                $match = 1 if $ip6 && $parsed_ip6 && $ip6;
-            }
-
             print $out "$line\n";
         }
 
-        if (!$match) {
-            my $reservation = $mac;
-            $reservation .= ",$ip4" if $ip4;
-            $reservation .= ",[$ip6]" if $ip6;
+        if (defined($reservation) && !$kept) {
             print $out "$reservation\n";
-            $reload = 1;
+            $changed = 1;
         }
 
         close $in;
@@ -136,16 +151,22 @@ sub add_ip_mapping {
         chmod 0644, $ethers_file;
     };
 
-    PVE::Tools::lock_file($ethers_file, 10, $appendFn);
+    # the rewrite replaces the reservation file, so a lock on the file itself
+    # would not outlive the rename
+    PVE::Tools::lock_file("$ethers_file.lock", 10, $rewriteFn);
 
     if ($@) {
-        warn "Unable to add $mac to the dnsmasq configuration: $@\n";
+        warn "Unable to update $mac in the dnsmasq configuration: $@\n";
         return;
     }
 
-    my $service_name = "dnsmasq\@$dhcpid";
-    systemctl_service('reload', $service_name) if $reload;
-    update_lease($dhcpid, $ip4, $mac);
+    systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
+
+    # the reservation is written, so a lease refresh that fails only warns
+    if ($ip4) {
+        eval { update_lease($dhcpid, $ip4, $mac) };
+        log_warn("could not update the dnsmasq lease of $mac: $@") if $@;
+    }
 }
 
 sub configure_subnet {
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index b5d32fa1..2fa8683c 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,8 +22,11 @@ sub private {
     return $defaultData;
 }
 
-sub add_ip_mapping {
-    my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+# the MAC's records changed, so make the backend serve the current ones. With no
+# record left its reservation goes. The vmid names the guest when the caller
+# knows it, which an API edit does not
+sub update_ip_mapping {
+    my ($class, $dhcpid, $mac, $vmid) = @_;
     die 'implement in sub class';
 }
 
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index c327a4be..84b08037 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -211,6 +211,8 @@ sub del_ips_from_mac {
     PVE::Network::SDN::Vnets::del_ip($vnetid, $ip4, $hostname, $mac) if $ip4;
     PVE::Network::SDN::Vnets::del_ip($vnetid, $ip6, $hostname, $mac) if $ip6;
 
+    PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) if $ip4 || $ip6;
+
     return ($ip4, $ip6);
 }
 
@@ -225,11 +227,12 @@ sub add_dhcp_mapping {
     return if !$zone->{ipam} || !$zone->{dhcp};
 
     my ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+    # one push at the end covers the allocated and the existing records alike
     add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 4) if !$ip4;
     add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 6) if !$ip6;
 
     ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
-    PVE::Network::SDN::Dhcp::add_mapping($vnetid, $mac, $ip4, $ip6) if $ip4 || $ip6;
+    PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac, $vmid) if $ip4 || $ip6;
 }
 
 1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index adad5e9a..33a5b44e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -47,6 +47,7 @@ sub clear_test_state {
         vnets_config => {},
         macdb => {},
         ipamdb => {},
+        dnsmasq_calls => [],
         ipam_config => {
             'ids' => {
                 'pve' => {
@@ -221,7 +222,9 @@ $mocked_sdn_dhcp_dnsmasq->mock(
     assert_dnsmasq_installed => sub { return 1; },
     before_configure => sub { },
     ethers_file => sub { return "/tmp/ethers"; },
-    systemctl_service => sub { },
+    systemctl_service => sub {
+        push $test_state->{dnsmasq_calls}->@*, $_[0];
+    },
     update_lease => sub { },
 );
 
@@ -240,6 +243,11 @@ $mocked_rpc_env_obj->mock(
     check_any => sub { return 1; },
 );
 
+my $mocked_pve_inotify = Test::MockModule->new('PVE::INotify');
+$mocked_pve_inotify->mock(
+    nodename => sub { return 'localnode'; },
+);
+
 my $mocked_pve_cluster_obj = Test::MockModule->new('PVE::Cluster');
 $mocked_pve_cluster_obj->mock(
     check_cfs_quorum => sub { return 1; },
@@ -289,6 +297,11 @@ sub create_zone {
     return $zone;
 }
 
+sub update_zone {
+    my ($zoneid, $params) = @_;
+    PVE::API2::Network::SDN::Zones->update({ zone => $zoneid, %$params });
+}
+
 sub get_vnet {
     my ($id) = @_;
     return eval { PVE::API2::Network::SDN::Vnets->read({ vnet => $id }); };
@@ -331,6 +344,16 @@ sub create_ip {
     return PVE::API2::Network::SDN::Ips->ipcreate($param);
 }
 
+sub update_ip {
+    my ($param) = @_;
+    return PVE::API2::Network::SDN::Ips->ipupdate($param);
+}
+
+sub delete_ip {
+    my ($param) = @_;
+    return PVE::API2::Network::SDN::Ips->ipdelete($param);
+}
+
 sub run_test {
     my $test = shift;
     clear_test_state();
@@ -1109,18 +1132,229 @@ sub test_dnsmasq_stale_reservation_replaced {
     };
     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,
-    );
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
 
-    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"],
+    is(
+        PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+        "$stale,10.0.1.11\n$added,10.0.1.13\n",
         "$test_name: the stale line is replaced, the unknown one dropped",
     );
 }
 
 run_test(\&test_dnsmasq_stale_reservation_replaced);
 
+# -------------- dnsmasq mapping pushes
+
+sub test_dnsmasq_mapping_push {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+    my $mac = "da:65:8f:18:9b:6f";
+
+    create_zone({
+        type => "simple",
+        dhcp => "dnsmasq",
+        ipam => "pve",
+        zone => $zoneid,
+    });
+    create_vnet({
+        type => "vnet",
+        zone => $zoneid,
+        vnet => $vnetid,
+    });
+    create_subnet({
+        type => "subnet",
+        vnet => $vnetid,
+        subnet => "10.0.0.0/24",
+        gateway => "10.0.0.1",
+        'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    });
+
+    # the first push of a zone finds no ethers file yet
+    unlink($TMP_ETHERS_FILE);
+
+    my $ethers = sub { PVE::Tools::file_get_contents($TMP_ETHERS_FILE) };
+    my $reloads = sub {
+        my $count = scalar(grep { $_ eq 'reload' } $test_state->{dnsmasq_calls}->@*);
+        $test_state->{dnsmasq_calls} = [];
+        return $count;
+    };
+
+    eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+    if ($@) {
+        fail("$test_name: nic_start: $@");
+        return;
+    }
+    is($ethers->(), "$mac,10.0.0.100\n", "$test_name: guest start reserves the allocation");
+    is($reloads->(), 1, "$test_name: guest start reloads once");
+
+    # the records exist already, so the restart pushes them without a change
+    nic_start($vnetid, $mac, "999", "testhostname");
+    is($ethers->(), "$mac,10.0.0.100\n", "$test_name: restart keeps the reservation");
+    is($reloads->(), 0, "$test_name: restart without a change does not reload");
+
+    update_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.150",
+    });
+    is($ethers->(), "$mac,10.0.0.150\n", "$test_name: mapping edit replaces the reservation");
+    is($reloads->(), 1, "$test_name: mapping edit reloads once");
+
+    # a push writes what the cache holds, so a repeated one rewrites nothing
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, $mac);
+    is($ethers->(), "$mac,10.0.0.150\n", "$test_name: a repeated push keeps the reservation");
+    is($reloads->(), 0, "$test_name: a repeated push changes nothing");
+
+    # the cache and the file may spell a MAC differently, but it is one MAC
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, uc($mac));
+    is(
+        $ethers->(),
+        "$mac,10.0.0.150\n",
+        "$test_name: a push spelled differently keeps the line as it is",
+    );
+    is($reloads->(), 0, "$test_name: and reloads nothing");
+
+    delete_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.150",
+    });
+    is($ethers->(), "", "$test_name: mapping delete drops the reservation");
+    is($reloads->(), 1, "$test_name: mapping delete reloads once");
+
+    # a record without a MAC has no NIC to serve, so nothing is pushed for it
+    my @warnings;
+    {
+        local $SIG{__WARN__} = sub { push @warnings, @_; };
+        create_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+        delete_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+    }
+    is($reloads->(), 0, "$test_name: a record without a MAC is not pushed");
+    eq_or_diff(\@warnings, [], "$test_name: and warns nothing");
+
+    # a zone confined to other nodes runs no dnsmasq here, so an edit is not
+    # pushed on this node
+    update_zone($zoneid, { nodes => 'other' });
+    create_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.150",
+    });
+    is($ethers->(), "", "$test_name: a zone confined to other nodes gets no push here");
+    is($reloads->(), 0, "$test_name: nothing to reload for it");
+}
+
+run_test(\&test_dnsmasq_mapping_push);
+
+sub test_dnsmasq_dual_stack_and_sweep {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+    my $mac = "da:65:8f:18:9b:6f";
+
+    create_zone({
+        type => "simple",
+        dhcp => "dnsmasq",
+        ipam => "pve",
+        zone => $zoneid,
+    });
+    create_vnet({
+        type => "vnet",
+        zone => $zoneid,
+        vnet => $vnetid,
+    });
+    create_subnet({
+        type => "subnet",
+        vnet => $vnetid,
+        subnet => "10.0.0.0/24",
+        gateway => "10.0.0.1",
+        'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    });
+    create_subnet({
+        type => "subnet",
+        vnet => $vnetid,
+        subnet => "8888::/64",
+        gateway => "8888::1",
+        'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    });
+
+    # a reservation of a MAC the cache does not know is swept by the next
+    # rewrite, and both addresses of the guest go on one line
+    PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "aa:bb:cc:dd:ee:ff,10.0.0.77\n");
+    $test_state->{dnsmasq_calls} = [];
+
+    eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+    if ($@) {
+        fail("$test_name: nic_start: $@");
+        return;
+    }
+    is(
+        PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+        "$mac,10.0.0.100,[8888::100]\n",
+        "$test_name: dual-stack reservation on one line, the unknown MAC swept",
+    );
+    is(
+        scalar(grep { $_ eq 'reload' } $test_state->{dnsmasq_calls}->@*),
+        1,
+        "$test_name: one reload for the rewrite",
+    );
+
+    # an external IPAM may spell an address in upper case, but the file holds
+    # one spelling
+    $test_state->{macdb}->{macs}->{$mac}->{ip6} = '8888::AB';
+    $test_state->{dnsmasq_calls} = [];
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, $mac);
+    is(
+        PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+        "$mac,10.0.0.100,[8888::ab]\n",
+        "$test_name: the reservation is written in one spelling",
+    );
+    is(
+        scalar(grep { $_ eq 'reload' } $test_state->{dnsmasq_calls}->@*),
+        1,
+        "$test_name: one reload for the changed address",
+    );
+
+    # an entry without an address, as a miss once left behind, says nothing
+    # about its MAC, so a line of it is swept
+    my $stale = 'da:65:8f:18:9b:70';
+    $test_state->{macdb}->{macs}->{$stale} = { ip4 => undef, ip6 => undef };
+    PVE::Tools::file_set_contents(
+        $TMP_ETHERS_FILE,
+        "$mac,10.0.0.100,[8888::ab]\n$stale,10.0.0.101\n",
+    );
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, $mac);
+    is(
+        PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+        "$mac,10.0.0.100,[8888::ab]\n",
+        "$test_name: a line of a MAC cached without an address is swept",
+    );
+
+    # a line with an upper-case IPv6 address from before they were lower-cased is kept
+    my $other = 'da:65:8f:18:9b:71';
+    $test_state->{macdb}->{macs}->{$other} = { ip4 => '10.0.0.102', ip6 => '8888::AB' };
+    PVE::Tools::file_set_contents(
+        $TMP_ETHERS_FILE,
+        "$mac,10.0.0.100,[8888::ab]\n$other,10.0.0.102,[8888::AB]\n",
+    );
+    $test_state->{dnsmasq_calls} = [];
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, $mac);
+    is(
+        PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+        "$mac,10.0.0.100,[8888::ab]\n$other,10.0.0.102,[8888::AB]\n",
+        "$test_name: a line of another MAC spelled by the old code is kept",
+    );
+    is(
+        scalar(grep { $_ eq 'reload' } $test_state->{dnsmasq_calls}->@*),
+        0,
+        "$test_name: without a reload",
+    );
+}
+
+run_test(\&test_dnsmasq_dual_stack_and_sweep);
+
 done_testing();
-- 
2.47.3