From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id A93DB1FF0AB for ; Wed, 21 Oct 2026 09:21:14 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A7DF4214CC; Wed, 21 Oct 2026 09:21:12 +0200 (CEST) Message-ID: Date: Wed, 21 Oct 2026 09:21:04 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-network v3 05/17] sdn: dhcp: update reservations when ip mappings change To: Stefan Hanreich , pve-devel@lists.proxmox.com References: <20261020080005.470757-1-h.laimer@proxmox.com> <20261020080005.470757-6-h.laimer@proxmox.com> From: Hannes Laimer Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792567264447 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.387 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 72ACUZTSE2FX4LPT36USO2JNA42PSTA4 X-Message-ID-Hash: 72ACUZTSE2FX4LPT36USO2JNA42PSTA4 X-MailFrom: h.laimer@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 2026-10-20 15:42, Stefan Hanreich wrote: > Tested-by: Stefan Hanreich > > 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 >> --- >> 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(); > > > > >