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

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

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

yes, keyed by lc mac with a list per mac since one can appear on several
guests

will do in v4

> 
>> +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:
> 

[1]
> https://lore.proxmox.com/all/20260709140625.275618-1-s.hanreich@proxmox.com/
> 
> Might be the better option here as well?

i did consider that, but at the time it seemed like "a lot" for
something where a different precedent exists already, but yes, `pvesh`
did seem *meh* here. also wasn't aware of [1]

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

yes, this should work, and be the better approach here. the only thing
it introduces is a cluster endpoint whose edit fails when another node
isn't reachable, which we don't have yet.. but tbf, it's niche, and
arguably also correct here

will include in v4!

> 
>>  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";
> 
> 
> 
> 
>