superseded: [PATCH pve-network v2] sdn: push mapping changes from the ipam API to the dhcp backend

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

superseded-by:
https://lore.proxmox.com/pve-devel/20260909104144.1110031-1-h.laimer@proxmox.com/T/#m344301ba0d2a23f23715a1ce7a5c0aa3cbc6da16

On 2026-09-05 11:21, Hannes Laimer wrote:
> Mapping edits through the API only wrote the IPAM record and the
> cache, the dhcp backend was never told, so dnsmasq kept serving
> stale reservations until a later guest start in the zone happened
> to sweep them. The same held for a guest whose NIC moved to another
> bridge, the old records were released and new ones allocated without
> the backend hearing of either.
> 
> Push the MAC's current answer after every record change, removing
> the old reservation first, since a re-add alone leaves a stale
> address behind when only one of the MAC's two addresses was
> deleted. The push is best effort, the record write stays
> authoritative. Mapping removal was dispatched to the plugins but
> implemented nowhere and never called, add the dnsmasq
> implementation for it.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> 
> v2:
>  - one push helper in the dhcp dispatcher instead of the push repeated
>    in the ips API handlers
>  - the guest allocation and release paths push too, so a bridge change
>    on a running guest reaches the backend
>  - a guest start pushes once, only for what it allocated
>  - the update goes through a plugin hook, remove then add stays the
>    default for dnsmasq
> 
>  src/PVE/API2/Network/SDN/Ips.pm     |  7 +++++++
>  src/PVE/Network/SDN/Dhcp.pm         | 21 +++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 32 ++++++++++++++++++++++++++++-
>  src/PVE/Network/SDN/Dhcp/Plugin.pm  | 13 ++++++++++++
>  src/PVE/Network/SDN/Vnets.pm        | 17 ++++++++++-----
>  5 files changed, 84 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
> index 5ff05e7..d7b682d 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 65e40d4..a9459b4 100644
> --- a/src/PVE/Network/SDN/Dhcp.pm
> +++ b/src/PVE/Network/SDN/Dhcp.pm
> @@ -55,6 +55,27 @@ sub remove_mapping {
>      $dhcp_plugin->del_ip_mapping($zoneid, $mac);
>  }
>  
> +# re-apply a MAC's mapping on this node from the current records, best
> +# effort, the record write stays authoritative
> +sub update_mapping {
> +    my ($vnetid, $mac) = @_;
> +
> +    eval {
> +        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 ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
> +        my $macdb = PVE::Network::SDN::Ipams::read_macdb();
> +        my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
> +        $dhcp_plugin->update_ip_mapping($zoneid, $macdb, $mac, $ip4, $ip6);
> +    };
> +    warn "could not update dhcp mapping for $mac: $@" if $@;
> +}
> +
>  sub regenerate_config {
>      my ($reload) = @_;
>  
> diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> index 477b700..4677330 100644
> --- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> +++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> @@ -6,7 +6,7 @@ use warnings;
>  use base qw(PVE::Network::SDN::Dhcp::Plugin);
>  
>  use Net::IP qw(:PROC);
> -use PVE::Tools qw(file_set_contents run_command lock_file);
> +use PVE::Tools qw(file_get_contents file_set_contents run_command lock_file);
>  
>  use File::Copy;
>  use Net::DBus;
> @@ -136,6 +136,36 @@ sub add_ip_mapping {
>      update_lease($dhcpid, $ip4, $mac);
>  }
>  
> +sub del_ip_mapping {
> +    my ($class, $dhcpid, $mac) = @_;
> +
> +    my $ethers_file = ethers_file($dhcpid);
> +
> +    my $reload = undef;
> +
> +    my $removeFn = sub {
> +        my @lines = split(/\n/, file_get_contents($ethers_file));
> +        my @remaining = grep {
> +            my ($parsed_mac) = split(/,/, $_);
> +            !defined($parsed_mac) || $parsed_mac ne $mac;
> +        } @lines;
> +
> +        return if scalar(@remaining) == scalar(@lines);
> +
> +        file_set_contents($ethers_file, join("\n", @remaining) . "\n", 0644);
> +        $reload = 1;
> +    };
> +
> +    PVE::Tools::lock_file($ethers_file, 10, $removeFn);
> +
> +    if ($@) {
> +        warn "Unable to remove $mac from the dnsmasq configuration: $@\n";
> +        return;
> +    }
> +
> +    systemctl_service('reload', "dnsmasq\@$dhcpid") if $reload;
> +}
> +
>  sub configure_subnet {
>      my ($class, $config, $dhcpid, $vnetid, $subnet_config) = @_;
>  
> diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
> index b5d32fa..659c938 100644
> --- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
> +++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
> @@ -27,6 +27,19 @@ sub add_ip_mapping {
>      die 'implement in sub class';
>  }
>  
> +sub del_ip_mapping {
> +    my ($class, $dhcpid, $mac) = @_;
> +    die 'implement in sub class';
> +}
> +
> +# the MAC's records changed, make the backend serve the current ones
> +sub update_ip_mapping {
> +    my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
> +
> +    $class->del_ip_mapping($dhcpid, $mac);
> +    $class->add_ip_mapping($dhcpid, $macdb, $mac, $ip4, $ip6) if $ip4 || $ip6;
> +}
> +
>  sub configure_range {
>      my ($class, $config, $dhcpid, $vnetid, $subnet_config, $range_config) = @_;
>      die 'implement in sub class';
> diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
> index c327a4b..daf6654 100644
> --- a/src/PVE/Network/SDN/Vnets.pm
> +++ b/src/PVE/Network/SDN/Vnets.pm
> @@ -154,6 +154,8 @@ sub add_next_free_cidr {
>              die "can't find any free ip in zone $zoneid for IPv$ipversion";
>          }
>      }
> +
> +    PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) if %$ips;
>  }
>  
>  sub add_ip {
> @@ -211,6 +213,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);
>  }
>  
> @@ -224,12 +228,15 @@ sub add_dhcp_mapping {
>  
>      return if !$zone->{ipam} || !$zone->{dhcp};
>  
> +    # an allocation pushes the mapping itself, a guest whose records exist
> +    # already has to be pushed here
>      my ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
> -    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;
> +    if (!$ip4 || !$ip6) {
> +        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;
> +    } else {
> +        PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac);
> +    }
>  }
>  
>  1;