[PATCH pve-network 06/10] 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.

A guest start, an API edit and the deletion of a NIC's addresses now
all rewrite the MAC's reservation from the cache. The reservations
are a file on each node, and only the guest's node answers its DHCP
requests, so an API edit is proxied to the node of the guest whose
NIC carries the MAC on that vnet. The other nodes catch up when the
guest starts on one of them. A failed update only warns, since the
IPAM records are already written.

The rewrite refreshes the cluster state and reads the cache only
once it holds the file's lock, so a rewrite that ran in between is
not undone. 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.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/API2/Network/SDN/Ips.pm     |  28 +++++
 src/PVE/Network/SDN/Dhcp.pm         |  56 ++++++----
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 104 ++++++++++-------
 src/PVE/Network/SDN/Dhcp/Plugin.pm  |   6 +-
 src/PVE/Network/SDN/Vnets.pm        |   5 +-
 src/test/run_test_vnets_blackbox.pl | 168 ++++++++++++++++++++++++++--
 6 files changed, 294 insertions(+), 73 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 5ff05e75e30a..b1faee71c6ba 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -3,9 +3,11 @@ package PVE::API2::Network::SDN::Ips;
 use strict;
 use warnings;
 
+use PVE::Cluster;
 use PVE::Tools qw(extract_param);
 
 use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Zones;
 use PVE::Network::SDN::Dhcp;
 
 use PVE::JSONSchema qw(get_standard_option);
@@ -13,12 +15,29 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+# only the guest's node answers its DHCP requests, so the edit runs there.
+# With that node offline it runs here, and the node catches up at the
+# guest's next start
+my $proxy_to_guest_node = sub {
+    my ($rpcenv, $proxyto, $param) = @_;
+
+    my $vnet = PVE::Network::SDN::Vnets::get_vnet($param->{vnet}, 1);
+    my $zone = $vnet ? PVE::Network::SDN::Zones::get_zone($vnet->{zone}, 1) : undef;
+    return 'localhost' if !$zone || !$zone->{dhcp};
+
+    my $node = PVE::Network::SDN::Dhcp::guest_node_by_mac($param->{vnet}, $param->{mac});
+    my $members = PVE::Cluster::get_members();
+    return 'localhost' if !$node || ($members && !($members->{$node} // {})->{online});
+    return $node;
+};
+
 __PACKAGE__->register_method({
     name => 'ipdelete',
     path => '',
     method => 'DELETE',
     description => 'Delete IP Mappings in a VNet',
     protected => 1,
+    proxyto_callback => $proxy_to_guest_node,
     permissions => {
         check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
     },
@@ -46,6 +65,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;
     },
 });
@@ -56,6 +77,7 @@ __PACKAGE__->register_method({
     method => 'POST',
     description => 'Create IP Mapping in a VNet',
     protected => 1,
+    proxyto_callback => $proxy_to_guest_node,
     permissions => {
         check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
     },
@@ -82,6 +104,8 @@ __PACKAGE__->register_method({
 
         PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
 
+        PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
         return undef;
     },
 });
@@ -91,6 +115,7 @@ __PACKAGE__->register_method({
     method => 'PUT',
     description => 'Update IP Mapping in a VNet',
     protected => 1,
+    proxyto_callback => $proxy_to_guest_node,
     permissions => {
         check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
     },
@@ -132,6 +157,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 65e40d40465a..4650270df09f 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,50 @@ sub plugin_types {
     return PVE::Network::SDN::Dhcp::Plugin->lookup_types();
 }
 
-sub add_mapping {
-    my ($vnetid, $mac, $ip4, $ip6) = @_;
+sub update_mapping {
+    my ($vnetid, $mac) = @_;
 
-    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 vnet bridge on this node
+        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);
+        PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
+            ->update_ip_mapping($zoneid, $mac);
+    };
+    # the records are written, so a failed update only warns
+    warn "could not update dhcp mapping for $mac: $@" if $@;
 }
 
-sub remove_mapping {
+sub guest_node_by_mac {
     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 $vmlist = PVE::Cluster::get_vmlist();
+    my $nets = PVE::Cluster::get_guest_config_properties([map { "net$_" } 0 .. 31]);
+    for my $vmid (sort keys %$nets) {
+        my $guest = $vmlist->{ids}->{$vmid} // next;
+        for my $net (values %{ $nets->{$vmid} }) {
+            my ($bridge) = $net =~ m/(?:^|,)bridge=([^,]+)/ or next;
+            next if $bridge ne $vnetid;
+            # a VM names the MAC as the value of its model, a container as hwaddr
+            my ($nic_mac) = $net =~ m/(?:^|,)[a-z0-9_-]+=([0-9a-f]{2}(?::[0-9a-f]{2}){5})(?:,|$)/i
+                or next;
+            return $guest->{node} if lc($nic_mac) eq lc($mac);
+        }
+    }
 
-    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
-    $dhcp_plugin->del_ip_mapping($zoneid, $mac);
+    return undef;
 }
 
 sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index a1e4c70f9ba4..4767fb5162ad 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -11,7 +11,9 @@ use PVE::Tools qw(file_set_contents run_command lock_file);
 use File::Copy;
 use Net::DBus;
 
+use PVE::Cluster;
 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 +53,100 @@ 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 {
+        # the lines are decided from the cache, which this process may hold an
+        # old read of, and the local lock does not refresh it
+        PVE::Cluster::cfs_update();
+
+        # 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);
+        }
+
+        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 +155,21 @@ 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;
+
+    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 b5d32fa13c70..a3a5ae24403f 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,8 +22,10 @@ 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
+sub update_ip_mapping {
+    my ($class, $dhcpid, $mac) = @_;
     die 'implement in sub class';
 }
 
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index c327a4beba81..8453a2781ae9 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) if $ip4 || $ip6;
 }
 
 1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index fc301a54de5f..bde61f2d3401 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,10 +243,18 @@ $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; },
     cfs_lock_domain => $mocked_cfs_lock_domain,
+    cfs_update => sub { },
+    get_vmlist => sub { return $test_state->{vmlist} // { ids => {} }; },
+    get_guest_config_properties => sub { return $test_state->{guest_nets} // {}; },
 );
 
 # ------- TEST FUNCTIONS --------------
@@ -289,6 +300,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 }); };
@@ -359,6 +375,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();
@@ -1110,18 +1136,144 @@ 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_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+    # 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) };
+
+    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");
+
+    # the records exist already, so the restart pushes them without a change
+    $test_state->{dnsmasq_calls} = [];
+    nic_start($vnetid, $mac, "999", "testhostname");
+    is($ethers->(), "$mac,10.0.0.100\n", "$test_name: restart keeps the reservation");
+    is_deeply(
+        $test_state->{dnsmasq_calls},
+        [],
+        "$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");
+
+    delete_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.150",
+    });
+    is($ethers->(), "", "$test_name: mapping delete drops the reservation");
+
+    # 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" });
+    }
+    eq_or_diff(\@warnings, [], "$test_name: a record without a MAC warns nothing");
+
+    # 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
+    create_dhcp_subnet($vnetid, "8888::/64");
+    PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "aa:bb:cc:dd:ee:ff,10.0.0.77\n");
+    nic_start($vnetid, $mac, "999", "testhostname");
+    is(
+        $ethers->(),
+        "$mac,10.0.0.100,[8888::64]\n",
+        "$test_name: dual-stack reservation on one line, the unknown MAC swept",
+    );
+
+    # 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->(),
+        "$mac,10.0.0.100,[8888::64]\n",
+        "$test_name: a zone confined to other nodes gets no push here",
+    );
+}
+
+run_test(\&test_dnsmasq_mapping_push);
+
+sub test_mapping_edit_target {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+    my $mac = "da:65:8f:18:9b:6f";
+
+    create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+    # the guest behind a MAC is found by its NIC on the vnet, and the config
+    # writes the MAC in upper case
+    $test_state->{vmlist} = {
+        ids => {
+            100 => { type => 'qemu', node => 'other' },
+        },
+    };
+    $test_state->{guest_nets} = {
+        1 => { net0 => "virtio=$mac,bridge=$vnetid" },
+        100 => { net0 => "virtio=" . uc($mac) . ",bridge=$vnetid" },
+    };
+    is(
+        PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, $mac),
+        'other',
+        "$test_name: the MAC resolves to the node of its guest, a guest the list lacks is skipped",
+    );
+
+    # a mapping edit through the API runs on the guest's node
+    my $proxyto = PVE::API2::Network::SDN::Ips->map_method_by_name('ipupdate')->{proxyto_callback};
+    is(
+        $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => $mac }),
+        'other',
+        "$test_name: a mapping edit runs on the guest's node",
+    );
+    is(
+        $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => '00:11:22:33:44:66' }),
+        'localhost',
+        "$test_name: one for a MAC without a guest runs here",
+    );
+    delete $test_state->{vmlist};
+    delete $test_state->{guest_nets};
+}
+
+run_test(\&test_mapping_edit_target);
+
 done_testing();
-- 
2.47.3