neighbor: Refactor object creation

Change-Id: Ic46d39f4717799ecce1c1fa84096f27e674e69a6
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/src/ethernet_interface.cpp b/src/ethernet_interface.cpp
index 76bbe78..1830bcd 100644
--- a/src/ethernet_interface.cpp
+++ b/src/ethernet_interface.cpp
@@ -219,12 +219,11 @@
         {
             continue;
         }
-        auto ip = std::to_string(neighbor.address);
-        auto mac = std::to_string(*neighbor.mac);
-        auto objectPath = generateStaticNeighborObjectPath(ip, mac);
         staticNeighbors.emplace(
-            ip, std::make_unique<Neighbor>(bus, objectPath, *this, ip, mac,
-                                           Neighbor::State::Permanent));
+            neighbor.address,
+            std::make_unique<Neighbor>(bus, std::string_view(objPath), *this,
+                                       neighbor.address, *neighbor.mac,
+                                       Neighbor::State::Permanent));
     }
 }
 
@@ -269,90 +268,58 @@
             Argument::ARGUMENT_VALUE(std::to_string(prefixLength).c_str()));
     }
 
-    auto obj =
+    auto [it, _] = this->addrs.insert_or_assign(
+        ifaddr,
         std::make_unique<IPAddress>(bus, std::string_view(objPath), *this,
-                                    ifaddr, IP::AddressOrigin::Static);
-    auto path = obj->getObjPath();
-    this->addrs.insert_or_assign(ifaddr, std::move(obj));
+                                    ifaddr, IP::AddressOrigin::Static));
 
     writeConfigurationFile();
     manager.reloadConfigs();
 
-    return path;
+    return it->second->getObjPath();
 }
 
 ObjectPath EthernetInterface::neighbor(std::string ipAddress,
                                        std::string macAddress)
 {
-    if (!isValidIP(ipAddress))
+    InAddrAny addr;
+    try
     {
-        log<level::ERR>("Not a valid IP address",
-                        entry("ADDRESS=%s", ipAddress.c_str()));
+        addr = ToAddr<InAddrAny>{}(ipAddress);
+    }
+    catch (const std::exception& e)
+    {
+        auto msg =
+            fmt::format("Not a valid IP address `{}`: {}", ipAddress, e.what());
+        log<level::ERR>(msg.c_str(), entry("ADDRESS=%s", ipAddress.c_str()));
         elog<InvalidArgument>(Argument::ARGUMENT_NAME("ipAddress"),
                               Argument::ARGUMENT_VALUE(ipAddress.c_str()));
     }
-    if (!mac_address::isUnicast(ToAddr<ether_addr>{}(macAddress)))
+
+    ether_addr lladdr;
+    try
     {
-        log<level::ERR>("Not a valid MAC address",
-                        entry("MACADDRESS=%s", ipAddress.c_str()));
+        lladdr = ToAddr<ether_addr>{}(macAddress);
+    }
+    catch (const std::exception& e)
+    {
+        auto msg = fmt::format("Not a valid MAC address `{}`: {}", macAddress,
+                               e.what());
+        log<level::ERR>(msg.c_str(),
+                        entry("MACADDRESS=%s", macAddress.c_str()));
         elog<InvalidArgument>(Argument::ARGUMENT_NAME("macAddress"),
                               Argument::ARGUMENT_VALUE(macAddress.c_str()));
     }
 
-    auto objectPath = generateStaticNeighborObjectPath(ipAddress, macAddress);
-    staticNeighbors.emplace(
-        ipAddress,
-        std::make_unique<Neighbor>(bus, objectPath, *this, ipAddress,
-                                   macAddress, Neighbor::State::Permanent));
+    auto [it, _] = staticNeighbors.emplace(
+        addr,
+        std::make_unique<Neighbor>(bus, std::string_view(objPath), *this, addr,
+                                   lladdr, Neighbor::State::Permanent));
 
     writeConfigurationFile();
     manager.reloadConfigs();
 
-    return objectPath;
-}
-
-void EthernetInterface::deleteStaticNeighborObject(std::string_view ipAddress)
-{
-    auto it = staticNeighbors.find(ipAddress);
-    if (it == staticNeighbors.end())
-    {
-        log<level::ERR>(
-            "DeleteStaticNeighborObject:Unable to find the object.");
-        return;
-    }
-    staticNeighbors.erase(it);
-
-    writeConfigurationFile();
-    manager.reloadConfigs();
-}
-
-std::string EthernetInterface::generateObjectPath(
-    IP::Protocol addressType, std::string_view ipAddress, uint8_t prefixLength,
-    IP::AddressOrigin origin) const
-{
-    std::string_view type;
-    switch (addressType)
-    {
-        case IP::Protocol::IPv4:
-            type = "ipv4"sv;
-            break;
-        case IP::Protocol::IPv6:
-            type = "ipv6"sv;
-            break;
-    }
-    return fmt::format(
-        FMT_COMPILE("{}/{}/{:08x}"), objPath, type,
-        static_cast<uint32_t>(hash_multi(
-            ipAddress, prefixLength,
-            static_cast<std::underlying_type_t<IP::AddressOrigin>>(origin))));
-}
-
-std::string EthernetInterface::generateStaticNeighborObjectPath(
-    std::string_view ipAddress, std::string_view macAddress) const
-{
-    return fmt::format(
-        FMT_COMPILE("{}/static_neighbor/{:08x}"), objPath,
-        static_cast<uint32_t>(hash_multi(ipAddress, macAddress)));
+    return it->second->getObjPath();
 }
 
 bool EthernetInterface::ipv6AcceptRA(bool value)
diff --git a/src/ethernet_interface.hpp b/src/ethernet_interface.hpp
index d7342a8..7d14385 100644
--- a/src/ethernet_interface.hpp
+++ b/src/ethernet_interface.hpp
@@ -97,6 +97,9 @@
     /** @brief Persistent map of IPAddress dbus objects and their names */
     std::unordered_map<IfAddr, std::unique_ptr<IPAddress>> addrs;
 
+    /** @brief Persistent map of Neighbor dbus objects and their names */
+    std::unordered_map<InAddrAny, std::unique_ptr<Neighbor>> staticNeighbors;
+
     /** @brief Updates the interface information based on new InterfaceInfo */
     void updateInfo(const system::InterfaceInfo& info);
 
@@ -123,11 +126,6 @@
      */
     ObjectPath neighbor(std::string ipAddress, std::string macAddress) override;
 
-    /* @brief delete the dbus object of the given ipAddress.
-     * @param[in] ipAddress - IP address.
-     */
-    void deleteStaticNeighborObject(std::string_view ipAddress);
-
     /* @brief creates the dbus object(IPaddres) given in the address list.
      * @param[in] addrs - address list for which dbus objects needs
      *                    to create.
@@ -138,14 +136,6 @@
      */
     void createStaticNeighborObjects();
 
-    /* @brief Gets all the static neighbor entries.
-     * @returns Static neighbor map.
-     */
-    inline const auto& getStaticNeighbors() const
-    {
-        return staticNeighbors;
-    }
-
     /** Set value of DHCPEnabled */
     DHCPConf dhcpEnabled() const override;
     DHCPConf dhcpEnabled(DHCPConf value) override;
@@ -240,23 +230,6 @@
     using EthernetInterfaceIntf::defaultGateway6;
 
   protected:
-    /** @brief construct the ip address dbus object path.
-     *  @param[in] addressType - Type of ip address.
-     *  @param[in] ipAddress - IP address.
-     *  @param[in] prefixLength - Length of prefix.
-     *  @param[in] origin - The origin entry of the IP::Address
-
-     *  @return path of the address object.
-     */
-    std::string generateObjectPath(IP::Protocol addressType,
-                                   std::string_view ipAddress,
-                                   uint8_t prefixLength,
-                                   IP::AddressOrigin origin) const;
-
-    std::string
-        generateStaticNeighborObjectPath(std::string_view ipAddress,
-                                         std::string_view macAddress) const;
-
     /** @brief get the NTP server list from the timsyncd dbus obj
      *
      */
@@ -270,9 +243,6 @@
     /** @brief Persistent sdbusplus DBus bus connection. */
     sdbusplus::bus_t& bus;
 
-    /** @brief Persistent map of Neighbor dbus objects and their names */
-    string_umap<std::unique_ptr<Neighbor>> staticNeighbors;
-
     /** @brief Dbus object path */
     std::string objPath;
 
diff --git a/src/neighbor.cpp b/src/neighbor.cpp
index 58e1a47..c9c1fc9 100644
--- a/src/neighbor.cpp
+++ b/src/neighbor.cpp
@@ -2,6 +2,7 @@
 
 #include "ethernet_interface.hpp"
 #include "netlink.hpp"
+#include "network_manager.hpp"
 #include "util.hpp"
 
 #include <linux/neighbour.h>
@@ -87,14 +88,29 @@
     return neighbors;
 }
 
-Neighbor::Neighbor(sdbusplus::bus_t& bus, stdplus::const_zstring objPath,
-                   EthernetInterface& parent, std::string_view ipAddress,
-                   std::string_view macAddress, State state) :
-    NeighborObj(bus, objPath.c_str(), NeighborObj::action::defer_emit),
-    parent(parent)
+static auto makeObjPath(std::string_view root, InAddrAny addr)
 {
-    NeighborObj::ipAddress(std::string(ipAddress));
-    NeighborObj::macAddress(std::string(macAddress));
+    auto ret = sdbusplus::message::object_path(std::string(root));
+    ret /= std::to_string(addr);
+    return ret;
+}
+
+Neighbor::Neighbor(sdbusplus::bus_t& bus, std::string_view objRoot,
+                   EthernetInterface& parent, InAddrAny addr, ether_addr lladdr,
+                   State state) :
+    Neighbor(bus, makeObjPath(objRoot, addr), parent, addr, lladdr, state)
+{
+}
+
+Neighbor::Neighbor(sdbusplus::bus_t& bus,
+                   sdbusplus::message::object_path objPath,
+                   EthernetInterface& parent, InAddrAny addr, ether_addr lladdr,
+                   State state) :
+    NeighborObj(bus, objPath.str.c_str(), NeighborObj::action::defer_emit),
+    parent(parent), objPath(std::move(objPath))
+{
+    NeighborObj::ipAddress(std::to_string(addr));
+    NeighborObj::macAddress(std::to_string(lladdr));
     NeighborObj::state(state);
 
     // Emit deferred signal.
@@ -103,7 +119,20 @@
 
 void Neighbor::delete_()
 {
-    parent.deleteStaticNeighborObject(ipAddress());
+    auto& neighbors = parent.staticNeighbors;
+    std::unique_ptr<Neighbor> ptr;
+    for (auto it = neighbors.begin(); it != neighbors.end(); ++it)
+    {
+        if (it->second.get() == this)
+        {
+            ptr = std::move(it->second);
+            neighbors.erase(it);
+            break;
+        }
+    }
+
+    parent.writeConfigurationFile();
+    parent.manager.reloadConfigs();
 }
 
 using sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
diff --git a/src/neighbor.hpp b/src/neighbor.hpp
index a29218d..4cd3682 100644
--- a/src/neighbor.hpp
+++ b/src/neighbor.hpp
@@ -7,8 +7,8 @@
 #include <cstdint>
 #include <optional>
 #include <sdbusplus/bus.hpp>
+#include <sdbusplus/message/native_types.hpp>
 #include <sdbusplus/server/object.hpp>
-#include <stdplus/zstring.hpp>
 #include <string_view>
 #include <vector>
 #include <xyz/openbmc_project/Network/Neighbor/server.hpp>
@@ -73,15 +73,15 @@
 
     /** @brief Constructor to put object onto bus at a dbus path.
      *  @param[in] bus - Bus to attach to.
-     *  @param[in] objPath - Path to attach at.
+     *  @param[in] objRoot - Path to attach at.
      *  @param[in] parent - Parent object.
-     *  @param[in] ipAddress - IP address.
-     *  @param[in] macAddress - Low level MAC address.
+     *  @param[in] addr - IP address.
+     *  @param[in] lladdr - Low level MAC address.
      *  @param[in] state - The state of the neighbor entry.
      */
-    Neighbor(sdbusplus::bus_t& bus, stdplus::const_zstring objPath,
-             EthernetInterface& parent, std::string_view ipAddress,
-             std::string_view macAddress, State state);
+    Neighbor(sdbusplus::bus_t& bus, std::string_view objRoot,
+             EthernetInterface& parent, InAddrAny addr, ether_addr lladdr,
+             State state);
 
     /** @brief Delete this d-bus object.
      */
@@ -94,9 +94,21 @@
     using NeighborObj::state;
     State state(State) override;
 
+    inline const auto& getObjPath() const
+    {
+        return objPath;
+    }
+
   private:
     /** @brief Parent Object. */
     EthernetInterface& parent;
+
+    /** @brief Dbus object path */
+    sdbusplus::message::object_path objPath;
+
+    Neighbor(sdbusplus::bus_t& bus, sdbusplus::message::object_path objPath,
+             EthernetInterface& parent, InAddrAny addr, ether_addr lladdr,
+             State state);
 };
 
 namespace detail
diff --git a/test/test_ethernet_interface.cpp b/test/test_ethernet_interface.cpp
index 061e0c9..bb28b22 100644
--- a/test/test_ethernet_interface.cpp
+++ b/test/test_ethernet_interface.cpp
@@ -54,15 +54,6 @@
                 config::Parser()};
     }
 
-    std::string getObjectPath(const std::string& ipaddress, uint8_t subnetMask,
-                              IP::AddressOrigin origin)
-    {
-        IP::Protocol addressType = IP::Protocol::IPv4;
-
-        return interface.generateObjectPath(addressType, ipaddress, subnetMask,
-                                            origin);
-    }
-
     auto createIPObject(IP::Protocol addressType, const std::string& ipaddress,
                         uint8_t subnetMask)
     {