From 738e604bfc9c568a1267fa6ada5f66860d4692b7 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Boric <jblbeurope@gmail.com>
Date: Sun, 18 Jul 2021 10:30:27 +0200
Subject: [PATCH] Kernel: Migrate ARP table locking to ProtectedValue

---
 Kernel/GlobalProcessExposed.cpp |  5 ++---
 Kernel/Net/Routing.cpp          | 38 ++++++++++++++++++++-------------
 Kernel/Net/Routing.h            |  4 ++--
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/Kernel/GlobalProcessExposed.cpp b/Kernel/GlobalProcessExposed.cpp
index 80ae165b3c..c4c6516c41 100644
--- a/Kernel/GlobalProcessExposed.cpp
+++ b/Kernel/GlobalProcessExposed.cpp
@@ -76,12 +76,11 @@ private:
     virtual bool output(KBufferBuilder& builder) override
     {
         JsonArraySerializer array { builder };
-        MutexLocker locker(arp_table().lock(), Mutex::Mode::Shared);
-        for (auto& it : arp_table().resource()) {
+        arp_table().for_each_shared([&](const auto& it) {
             auto obj = array.add_object();
             obj.add("mac_address", it.value.to_string());
             obj.add("ip_address", it.key.to_string());
-        }
+        });
         array.finish();
         return true;
     }
diff --git a/Kernel/Net/Routing.cpp b/Kernel/Net/Routing.cpp
index aaca39bc08..47793f528c 100644
--- a/Kernel/Net/Routing.cpp
+++ b/Kernel/Net/Routing.cpp
@@ -7,6 +7,7 @@
 #include <AK/HashMap.h>
 #include <AK/Singleton.h>
 #include <Kernel/Debug.h>
+#include <Kernel/Locking/ProtectedValue.h>
 #include <Kernel/Net/LoopbackAdapter.h>
 #include <Kernel/Net/NetworkTask.h>
 #include <Kernel/Net/NetworkingManagement.h>
@@ -15,7 +16,7 @@
 
 namespace Kernel {
 
-static AK::Singleton<Lockable<HashMap<IPv4Address, MACAddress>>> s_arp_table;
+static AK::Singleton<ProtectedValue<HashMap<IPv4Address, MACAddress>>> s_arp_table;
 
 class ARPTableBlocker : public Thread::Blocker {
 public:
@@ -70,7 +71,9 @@ protected:
     {
         VERIFY(b.blocker_type() == Thread::Blocker::Type::Routing);
         auto& blocker = static_cast<ARPTableBlocker&>(b);
-        auto val = s_arp_table->resource().get(blocker.ip_addr());
+        auto val = arp_table().with_shared([&](const auto& table) -> auto {
+            return table.get(blocker.ip_addr());
+        });
         if (!val.has_value())
             return true;
         return blocker.unblock(true, blocker.ip_addr(), val.value());
@@ -90,7 +93,9 @@ ARPTableBlocker::ARPTableBlocker(IPv4Address ip_addr, Optional<MACAddress>& addr
 void ARPTableBlocker::not_blocking(bool timeout_in_past)
 {
     VERIFY(timeout_in_past || !m_should_block);
-    auto addr = s_arp_table->resource().get(ip_addr());
+    auto addr = arp_table().with_shared([&](const auto& table) -> auto {
+        return table.get(ip_addr());
+    });
 
     ScopedSpinLock lock(m_lock);
     if (!m_did_unblock) {
@@ -99,25 +104,27 @@ void ARPTableBlocker::not_blocking(bool timeout_in_past)
     }
 }
 
-Lockable<HashMap<IPv4Address, MACAddress>>& arp_table()
+ProtectedValue<HashMap<IPv4Address, MACAddress>>& arp_table()
 {
     return *s_arp_table;
 }
 
 void update_arp_table(const IPv4Address& ip_addr, const MACAddress& addr, UpdateArp update)
 {
-    MutexLocker locker(arp_table().lock());
-    if (update == UpdateArp::Set)
-        arp_table().resource().set(ip_addr, addr);
-    if (update == UpdateArp::Delete)
-        arp_table().resource().remove(ip_addr);
+    arp_table().with_exclusive([&](auto& table) {
+        if (update == UpdateArp::Set)
+            table.set(ip_addr, addr);
+        if (update == UpdateArp::Delete)
+            table.remove(ip_addr);
+    });
     s_arp_table_block_condition->unblock(ip_addr, addr);
 
     if constexpr (ROUTING_DEBUG) {
-        dmesgln("ARP table ({} entries):", arp_table().resource().size());
-        for (auto& it : arp_table().resource()) {
-            dmesgln("{} :: {}", it.value.to_string(), it.key.to_string());
-        }
+        arp_table().with_shared([&](const auto& table) {
+            dmesgln("ARP table ({} entries):", table.size());
+            for (auto& it : table)
+                dmesgln("{} :: {}", it.value.to_string(), it.key.to_string());
+        });
     }
 }
 
@@ -224,8 +231,9 @@ RoutingDecision route_to(const IPv4Address& target, const IPv4Address& source, c
         return { adapter, multicast_ethernet_address(target) };
 
     {
-        MutexLocker locker(arp_table().lock());
-        auto addr = arp_table().resource().get(next_hop_ip);
+        auto addr = arp_table().with_shared([&](const auto& table) -> auto {
+            return table.get(next_hop_ip);
+        });
         if (addr.has_value()) {
             dbgln_if(ROUTING_DEBUG, "Routing: Using cached ARP entry for {} ({})", next_hop_ip, addr.value().to_string());
             return { adapter, addr.value() };
diff --git a/Kernel/Net/Routing.h b/Kernel/Net/Routing.h
index 3c441718a2..edae3464eb 100644
--- a/Kernel/Net/Routing.h
+++ b/Kernel/Net/Routing.h
@@ -6,7 +6,7 @@
 
 #pragma once
 
-#include <Kernel/Locking/Lockable.h>
+#include <Kernel/Locking/ProtectedValue.h>
 #include <Kernel/Net/NetworkAdapter.h>
 #include <Kernel/Thread.h>
 
@@ -27,6 +27,6 @@ enum class UpdateArp {
 void update_arp_table(const IPv4Address&, const MACAddress&, UpdateArp update);
 RoutingDecision route_to(const IPv4Address& target, const IPv4Address& source, const RefPtr<NetworkAdapter> through = nullptr);
 
-Lockable<HashMap<IPv4Address, MACAddress>>& arp_table();
+ProtectedValue<HashMap<IPv4Address, MACAddress>>& arp_table();
 
 }