[PATCH pve-network 04/12] sdn: ipam: do not cache negative per-MAC answers, lock the write

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

The lookup cache wrote an entry even when the plugin returned
nothing, and that entry short-circuits every later lookup, so a
record created after the first miss was never seen again. The
read-modify-write also ran without the cluster lock the other cache
writers take, allowing concurrent lookups to drop each other's
entries.

Only cache actual answers and take the lock for the write, keeping
the common cache-hit path lock-free.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/Network/SDN/Ipams.pm | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 179bdf7..9292386 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -140,12 +140,24 @@ sub get_ips_from_mac {
 
     my $plugin_config = get_plugin_config($zone);
     my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-    ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) =
-        $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+    my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
 
-    write_macdb($macdb);
+    # an empty answer is not cached, the record may simply not exist yet
+    return if !defined($ip4) && !defined($ip6);
 
-    return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6});
+    cfs_lock_file(
+        $macdb_filename,
+        undef,
+        sub {
+            my $db = read_macdb();
+            $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+            $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+            write_macdb($db);
+        },
+    );
+    warn "$@" if $@;
+
+    return ($ip4, $ip6);
 }
 
 1;
-- 
2.47.3