Re: [PATCH pve-network v3 04/17] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it

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

Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>

On 10/20/26 10:00 AM, Hannes Laimer wrote:
> A reservation rewrite dropped every line that no longer matched the
> MAC cache and wrote nothing in its place. The guest behind such a
> line was unknown to dnsmasq until its next start, so its requests
> were ignored. The line is now replaced by the cache's current one.
> Only a MAC the cache holds no address for is dropped.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 11 ++++++++++-
>  src/test/run_test_vnets_blackbox.pl | 28 ++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> index 477b700f..5c9496c0 100644
> --- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> +++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> @@ -88,7 +88,9 @@ sub add_ip_mapping {
>              }
>              $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
>  
> -            #delete changed
> +            # 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
>              if (
>                  !defined($macdb->{macs}->{$parsed_mac})
>                  || ($parsed_ip4
> @@ -99,6 +101,13 @@ sub add_ip_mapping {
>                      && $macdb->{macs}->{$parsed_mac}->{'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;
> +                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;
>                  next;
>              }
>  
> diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
> index 9f4c424f..36225a92 100755
> --- a/src/test/run_test_vnets_blackbox.pl
> +++ b/src/test/run_test_vnets_blackbox.pl
> @@ -963,4 +963,32 @@ run_test(
>      2,
>  );
>  
> +sub test_dnsmasq_stale_reservation_replaced {
> +    my $test_name = (split(/::/, (caller(0))[3]))[-1];
> +
> +    my $stale = "da:65:8f:18:9b:01";
> +    my $gone = "da:65:8f:18:9b:02";
> +    my $added = "da:65:8f:18:9b:03";
> +    $test_state->{macdb} = {
> +        macs => {
> +            $stale => { ip4 => '10.0.1.11' },
> +            $added => { ip4 => '10.0.1.13' },
> +        },
> +    };
> +    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,
> +    );
> +
> +    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"],
> +        "$test_name: the stale line is replaced, the unknown one dropped",
> +    );
> +}
> +
> +run_test(\&test_dnsmasq_stale_reservation_replaced);
> +
>  done_testing();