Re: [PATCH pve-network v3 05/17] sdn: dhcp: update reservations when ip mappings change

[Stefan Hanreich <s.hanreich@proxmox.com>]

Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>

On 10/20/26 10:03 AM, Hannes Laimer wrote:
> 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. MACs and addresses are compared in lower
> case, since the cache keeps each as its writer spelled it. A cache
> entry without any address no longer keeps a stale line.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/PVE/API2/Network/SDN/Ips.pm     |   7 +
>  src/PVE/Network/SDN/Dhcp.pm         |  48 +++---
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 110 ++++++++----
>  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, 352 insertions(+), 75 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 5c9496c0..57ce1f73 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,29 +52,65 @@ 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) = @_;
>  
>      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 $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 $macdb = PVE::Network::SDN::Ipams::read_macdb();
> +        # the cache spells a MAC and an address as their writer did, while the
> +        # file holds one spelling of both. An entry without an address says
> +        # nothing about its MAC, so a line of it is stale
> +        my %macs;
> +        for my $key (sort keys $macdb->{macs}->%*) {
> +            my $entry = $macdb->{macs}->{$key};
> +            my %addresses =
> +                map { $_ => lc($entry->{$_}) } grep { defined($entry->{$_}) } qw(ip4 ip6);
> +            next if !%addresses;
> +            $macs{ lc($key) } = { %{ $macs{ lc($key) } // {} }, %addresses };
> +        }

we could simplify this here as well with integrating case-folding into the read_macdb fn.
The more I think about, the more I'd prefer a small object that provides an API similar
to HashMaps that handle case-folding when querying / inserting / ... I think that'd save
us lots of duplication.

> +
> +        my $entry = $macs{ lc($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);
> +        }
>  
> -    my $appendFn = sub {
> +        # 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);
>  
> -            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 (lc($parsed_mac) eq lc($mac)) {
> +                if (defined($reservation) && !$kept && lc($line) eq $reservation) {
> +                    $kept = 1;
> +                    print $out "$line\n";
> +                } else {
> +                    $changed = 1;
> +                }
>                  next;
>              }
>  
> @@ -81,50 +118,43 @@ sub add_ip_mapping {
>              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
>              if (
> -                !defined($macdb->{macs}->{$parsed_mac})
> +                !defined($macs{ lc($parsed_mac) })
>                  || ($parsed_ip4
> -                    && $macdb->{macs}->{$parsed_mac}->{'ip4'}
> -                    && $macdb->{macs}->{$parsed_mac}->{'ip4'} ne $parsed_ip4)
> +                    && $macs{ lc($parsed_mac) }->{'ip4'}
> +                    && $macs{ lc($parsed_mac) }->{'ip4'} ne $parsed_ip4)
>                  || ($parsed_ip6
> -                    && $macdb->{macs}->{$parsed_mac}->{'ip6'}
> -                    && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
> +                    && $macs{ lc($parsed_mac) }->{'ip6'}
> +                    && $macs{ lc($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;
> +                $changed = 1;
> +                my $current = $macs{ lc($parsed_mac) };
> +                next if !$current;
> +                my $replacement = lc($parsed_mac);

a dedicated class could simplify this here a lot

>                  $replacement .= ",$current->{ip4}" if $current->{ip4};
>                  $replacement .= ",[$current->{ip6}]" if $current->{ip6};
> -                print $out "$replacement\n" if $replacement ne $parsed_mac;
> +                print $out "$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;
> @@ -133,16 +163,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 36225a92..d324538e 100755
> --- a/src/test/run_test_vnets_blackbox.pl
> +++ b/src/test/run_test_vnets_blackbox.pl
> @@ -46,6 +46,7 @@ sub clear_test_state {
>          vnets_config => {},
>          macdb => {},
>          ipamdb => {},
> +        dnsmasq_calls => [],
>          ipam_config => {
>              'ids' => {
>                  'pve' => {
> @@ -220,7 +221,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 { },
>  );
>  
> @@ -239,6 +242,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; },
> @@ -288,6 +296,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 }); };
> @@ -330,6 +343,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();
> @@ -977,18 +1000,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();