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

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

On 2026-10-20 15:42, Stefan Hanreich wrote:
> 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.
> 

like on 06/17 yes, not sure about the hash though, having the existing
functions do it should do the same i think

>> +
>> +        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();
> 
> 
> 
> 
>