[PATCH pve-network v2 10/16] sdn: dhcp: apply mapping edits on the node serving the guest

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

Mapping edits change cluster-wide records, but the ebpf backend answers
from a per-node map. So an edit made through another node's API left the
map of the node running the guest stale until the next apply or guest
start there. Resolve the MAC to its guest and re-apply the mapping on
that node through a node-scoped call, the same record update the editing
node runs locally. No other node needs the record, each picks it up at
that guest's next start there. So the cost of an edit stays one call
however large the cluster is. dnsmasq is covered by the same reasoning,
a simple zone's instance is node-local and only the guest's node serves
it.

The push runs detached from the edit request, so an unreachable node
cannot hold the edit up. It just catches up on its next apply or the
guest's next start.

The poke is bounded by a timeout, a node that takes the connection but
never answers must not keep it around.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/API2/Network/SDN/Ips.pm          |  3 +
 src/PVE/API2/Network/SDN/Nodes/Status.pm | 37 ++++++++++
 src/PVE/Network/SDN/Dhcp.pm              | 77 ++++++++++++++++++++
 src/test/run_test_vnets_blackbox.pl      | 90 ++++++++++++++++++++++++
 4 files changed, 207 insertions(+)

diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index d7b682d..5b45de7 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -47,6 +47,7 @@ __PACKAGE__->register_method({
         die "$@\n" if $@;
 
         PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+        PVE::Network::SDN::Dhcp::notify_guest_node($vnet, $mac);
 
         return undef;
     },
@@ -85,6 +86,7 @@ __PACKAGE__->register_method({
         PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
 
         PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+        PVE::Network::SDN::Dhcp::notify_guest_node($vnet, $mac);
 
         return undef;
     },
@@ -138,6 +140,7 @@ __PACKAGE__->register_method({
         die "$error\n" if $error;
 
         PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+        PVE::Network::SDN::Dhcp::notify_guest_node($vnet, $mac);
 
         return undef;
     },
diff --git a/src/PVE/API2/Network/SDN/Nodes/Status.pm b/src/PVE/API2/Network/SDN/Nodes/Status.pm
index 7977e0c..fe750a8 100644
--- a/src/PVE/API2/Network/SDN/Nodes/Status.pm
+++ b/src/PVE/API2/Network/SDN/Nodes/Status.pm
@@ -9,6 +9,9 @@ use PVE::API2::Network::SDN::Nodes::Vnets;
 
 use PVE::JSONSchema qw(get_standard_option);
 
+use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Dhcp;
+
 use PVE::RESTHandler;
 use base qw(PVE::RESTHandler);
 
@@ -27,6 +30,40 @@ __PACKAGE__->register_method({
     path => 'vnets',
 });
 
+__PACKAGE__->register_method({
+    name => 'dhcp_mapping',
+    path => 'dhcp-mapping',
+    method => 'POST',
+    description =>
+        'Re-apply the DHCP mapping of a MAC address on this node from the current records.',
+    permissions => {
+        check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
+    },
+    protected => 1,
+    proxyto => 'node',
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            zone => get_standard_option('pve-sdn-zone-id'),
+            vnet => get_standard_option('pve-sdn-vnet-id'),
+            mac => get_standard_option('mac-addr'),
+        },
+    },
+    returns => { type => 'null' },
+    code => sub {
+        my ($param) = @_;
+
+        my $vnet = PVE::Network::SDN::Vnets::get_vnet($param->{vnet}, 1);
+        die "vnet '$param->{vnet}' does not exist in zone '$param->{zone}'\n"
+            if !$vnet || $vnet->{zone} ne $param->{zone};
+
+        PVE::Network::SDN::Dhcp::update_mapping($param->{vnet}, $param->{mac});
+
+        return undef;
+    },
+});
+
 __PACKAGE__->register_method({
     name => 'sdnindex',
     path => '',
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 3b2d798..e57468d 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -3,6 +3,8 @@ package PVE::Network::SDN::Dhcp;
 use strict;
 use warnings;
 
+use POSIX qw();
+
 use PVE::Cluster;
 
 use PVE::Network::SDN;
@@ -82,6 +84,81 @@ sub guest_nics {
     return $nics;
 }
 
+sub guest_node_by_mac {
+    my ($vnetid, $mac) = @_;
+
+    for my $nic (guest_nics()->@*) {
+        return $nic->{node}
+            if lc($nic->{mac} // '') eq lc($mac) && $nic->{bridge} eq $vnetid;
+    }
+
+    return undef;
+}
+
+# The records are cluster-wide, but a node answers from its own map.
+# So an edit has to reach the node running the guest behind the MAC.
+# Every other node picks the record up at that guest's next start
+# there. The push runs detached from the request, an unreachable node
+# must not hold the edit up.
+sub notify_guest_node {
+    my ($vnetid, $mac) = @_;
+
+    # best effort like the push itself, the record is written by now
+    my ($zoneid, $node) = eval {
+        my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1);
+        return if !$vnet;
+
+        my $zone = PVE::Network::SDN::Zones::get_zone($vnet->{zone}, 1);
+        return if !$zone || !$zone->{ipam} || !$zone->{dhcp};
+
+        return ($vnet->{zone}, guest_node_by_mac($vnetid, $mac));
+    };
+    if ($@) {
+        warn "could not find the node to push the dhcp mapping of $mac to: $@";
+        return;
+    }
+    return if !$node || $node eq PVE::INotify::nodename();
+
+    # double fork, the grandchild is reparented to init so nothing on
+    # the request path ever waits on it
+    my $pid = fork();
+    if (!defined($pid)) {
+        warn "could not fork for the dhcp mapping push to $node: $!\n";
+        return;
+    }
+    if ($pid) {
+        waitpid($pid, 0);
+        return;
+    }
+
+    POSIX::setsid();
+    my $child = fork();
+    POSIX::_exit(1) if !defined($child);
+    POSIX::_exit(0) if $child;
+
+    # nothing of the request stays with the push, and since the daemon's
+    # stderr is closed a failed push reports through the journal
+    open(STDIN, '<', '/dev/null') or POSIX::_exit(1);
+    open(STDOUT, '>', '/dev/null') or POSIX::_exit(1);
+    open(STDERR, '|-', 'logger', '-t', 'pve-sdn', '-p', 'daemon.warning')
+        or POSIX::_exit(1);
+    # a node that takes the connection but never answers must not keep the
+    # push around
+    exec(
+        'timeout',
+        '60',
+        'pvesh',
+        'create',
+        "/nodes/$node/sdn/dhcp-mapping",
+        '--zone',
+        $zoneid,
+        '--vnet',
+        $vnetid,
+        '--mac',
+        $mac,
+    ) or POSIX::_exit(1);
+}
+
 # the interface may come from a vnet of another backend, or of none, so
 # every other backend drops what it holds for it first
 sub tap_plug {
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 064a96c..2c11dad 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -49,6 +49,7 @@ sub clear_test_state {
         dnsmasq_calls => [],
         ebpf_calls => [],
         order => [],
+        notify_calls => [],
         warnings => [],
         ipam_config => {
             'ids' => {
@@ -239,6 +240,11 @@ $mocked_sdn_dhcp_dnsmasq->mock(
     update_lease => sub { },
 );
 
+my $mocked_sdn_dhcp = Test::MockModule->new('PVE::Network::SDN::Dhcp');
+$mocked_sdn_dhcp->mock(
+    notify_guest_node => sub { push $test_state->{notify_calls}->@*, [@_]; },
+);
+
 my $mocked_pve_rs_dhcp = Test::MockModule->new('PVE::RS::SDN::Dhcp');
 $mocked_pve_rs_dhcp->mock(
     next_generation => sub {
@@ -1230,6 +1236,90 @@ sub test_dhcp_backend_needed_per_node {
 
 run_test(\&test_dhcp_backend_needed_per_node);
 
+sub test_mapping_push_target {
+    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",
+    });
+
+    # the guest behind a MAC is found by its NIC on the vnet, the config
+    # writes the MAC in upper case
+    $test_state->{vmlist} = {
+        ids => {
+            100 => { type => 'qemu', node => 'other' },
+            101 => { type => 'lxc', node => 'third' },
+        },
+    };
+    $test_state->{guest_nets} = {
+        1 => { net0 => "virtio=$mac,bridge=$vnetid" },
+        100 => { net0 => "virtio=" . uc($mac) . ",bridge=$vnetid" },
+        101 => { net0 => "name=eth0,bridge=vmbr0,hwaddr=00:11:22:33:44:55" },
+    };
+    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",
+    );
+    is(
+        PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, '00:11:22:33:44:55'),
+        undef,
+        "$test_name: a MAC on another bridge does not resolve",
+    );
+    is(
+        PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, '00:11:22:33:44:66'),
+        undef,
+        "$test_name: an unknown MAC does not resolve",
+    );
+
+    # every mapping edit through the API pokes the guest's node once
+    $test_state->{notify_calls} = [];
+    create_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.50",
+    });
+    update_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.51",
+    });
+    delete_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.51",
+    });
+    eq_or_diff(
+        $test_state->{notify_calls},
+        [[$vnetid, $mac], [$vnetid, $mac], [$vnetid, $mac]],
+        "$test_name: create, edit and delete each poke the guest's node",
+    );
+    delete $test_state->{vmlist};
+    delete $test_state->{guest_nets};
+}
+
+run_test(\&test_mapping_push_target);
+
 sub test_ipam_cache_misses {
     my $test_name = (split(/::/, (caller(0))[3]))[-1];
     my $zoneid = "TESTZONE";
-- 
2.47.3