Re: [PATCH pve-network v3 11/17] sdn: dhcp: push IP mapping edits to the guest's node

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

On 10/20/26 10:02 AM, Hannes Laimer wrote:
> IP mappings live in the cluster-wide IPAM and can be edited through
> the API on any node. The DHCP backends hold their copy on each node,
> dnsmasq in a reservation file and the ebpf responder in a kernel map.
> An edit only updated the backend of the node it was made on, so a
> guest on another node kept getting the old mapping.
> 
> The new endpoint POST /nodes/{node}/sdn/dhcp-mapping updates the
> mapping of one MAC on that node from the current records. An edit
> made on a node other than the guest's calls it on the guest's node.
> Only that node answers the guest's DHCP requests, so no other node is
> called. The others catch up when the guest starts on one of them.
> 
> The call runs in the background, so the request returns without
> waiting on the other node. A call that fails logs its error to the
> journal of the node the edit was made on. One that gets no answer is
> stopped after a minute. The record stays as written either way, and
> the guest's node picks it up at the latest when the guest next starts
> there.
> 
> 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              | 74 +++++++++++++++++++
>  src/test/run_test_vnets_blackbox.pl      | 90 ++++++++++++++++++++++++
>  4 files changed, 204 insertions(+)
> 
> diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
> index d7b682dd..5b45de7c 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 7977e0c8..fe750a85 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 cf805a03..1f43745f 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;
> @@ -86,6 +88,78 @@ 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;
> +}

seems like we're always (or at least most of the time) querying by MAC
so would assembling a hash keyed by MAC addresses in the first place be sensible?


> +sub notify_guest_node {
> +    my ($vnetid, $mac) = @_;
> +
> +    return if !$mac;
> +
> +    # best effort like the push itself, since 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);
> +}
> +

There've been efforts of moving away from pvesh - see my other patch
series introducing the api client instead of calling pvesh here:

https://lore.proxmox.com/all/20260709140625.275618-1-s.hanreich@proxmox.com/

Might be the better option here as well?

Also, there is proxyto_callback which we could use to proxy to the correct
node in the first place?

>  sub tap_plug {
>      my ($bridge, $iface, $mac) = @_;
>  
> diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
> index 783d0d47..9a4dbb9c 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' => {
> @@ -238,6 +239,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(
>      detach => sub {
> @@ -1280,6 +1286,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, and 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";