ethernet_interface: Refactor object creation args
We want to be able to pass a bunch of interface properties directly into
interface instead of querying them directly. This will be used later for
rtnetlink enabled interface creation.
Change-Id: I93fbd460a8a54515e84c415f085548cb5528f231
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/src/ethernet_interface.cpp b/src/ethernet_interface.cpp
index e4ab5b5..2d07e8b 100644
--- a/src/ethernet_interface.cpp
+++ b/src/ethernet_interface.cpp
@@ -63,20 +63,46 @@
}
return fallback;
}
-EthernetInterface::EthernetInterface(sdbusplus::bus_t& bus,
- stdplus::zstring_view objPath,
+
+InterfaceInfo getInterfaceInfo(stdplus::zstring_view ifname)
+{
+ return InterfaceInfo{.running = system::intfIsRunning(ifname),
+ .index = system::intfIndex(ifname),
+ .name = std::string(ifname),
+ .mac = system::getMAC(ifname),
+ .mtu = system::getMTU(ifname)};
+}
+
+static std::string makeObjPath(std::string_view root, std::string_view intf)
+{
+ auto ret = fmt::format(FMT_COMPILE("{}/{}"), root, intf);
+ std::replace(ret.begin() + ret.size() - intf.size(), ret.end(), '.', '_');
+ return ret;
+}
+
+EthernetInterface::EthernetInterface(sdbusplus::bus_t& bus, Manager& manager,
+ const InterfaceInfo& info,
+ std::string_view objRoot,
const config::Parser& config,
- Manager& parent, bool emitSignal,
+ bool emitSignal,
+ std::optional<bool> enabled) :
+ EthernetInterface(bus, manager, info, makeObjPath(objRoot, info.name),
+ config, emitSignal, enabled)
+{
+}
+
+EthernetInterface::EthernetInterface(sdbusplus::bus_t& bus, Manager& manager,
+ const InterfaceInfo& info,
+ std::string&& objPath,
+ const config::Parser& config,
+ bool emitSignal,
std::optional<bool> enabled) :
Ifaces(bus, objPath.c_str(),
emitSignal ? Ifaces::action::defer_emit
: Ifaces::action::emit_no_signals),
- bus(bus), manager(parent), objPath(objPath)
+ bus(bus), manager(manager), objPath(std::move(objPath)), ifIdx(info.index)
{
- auto intfName = std::string(objPath.substr(objPath.rfind('/') + 1));
- std::replace(intfName.begin(), intfName.end(), '_', '.');
- interfaceName(intfName);
- ifIdx = system::intfIndex(intfName);
+ interfaceName(info.name);
auto dhcpVal = getDHCPValue(config);
EthernetInterfaceIntf::dhcp4(dhcpVal.v4);
EthernetInterfaceIntf::dhcp6(dhcpVal.v6);
@@ -89,7 +115,7 @@
for (const auto& gateway : gatewayList)
{
- if (gateway.first == intfName)
+ if (gateway.first == info.name)
{
defaultGateway = gateway.second;
break;
@@ -98,7 +124,7 @@
for (const auto& gateway6 : gateway6List)
{
- if (gateway6.first == intfName)
+ if (gateway6.first == info.name)
{
defaultGateway6 = gateway6.second;
break;
@@ -107,32 +133,20 @@
EthernetInterfaceIntf::defaultGateway(defaultGateway);
EthernetInterfaceIntf::defaultGateway6(defaultGateway6);
- // Don't get the mac address from the system as the mac address
- // would be same as parent interface.
- if (intfName.find(".") == std::string::npos && ifIdx > 0)
- {
- auto mac = ignoreError("GetMAC", intfName, std::nullopt,
- [&] { return system::getMAC(intfName); });
- if (mac)
- {
- MacAddressIntf::macAddress(mac_address::toString(*mac));
- }
- }
EthernetInterfaceIntf::ntpServers(
config.map.getValueStrings("Network", "NTP"));
- EthernetInterfaceIntf::linkUp(linkUp());
- EthernetInterfaceIntf::mtu(mtu());
-
if (ifIdx > 0)
{
- auto ethInfo = ignoreError("GetEthInfo", intfName, {}, [&] {
- return system::getEthInfo(intfName);
+ auto ethInfo = ignoreError("GetEthInfo", info.name, {}, [&] {
+ return system::getEthInfo(info.name);
});
EthernetInterfaceIntf::autoNeg(ethInfo.autoneg);
EthernetInterfaceIntf::speed(ethInfo.speed);
}
+ updateInfo(info);
+
// Emit deferred signal.
if (emitSignal)
{
@@ -140,6 +154,19 @@
}
}
+void EthernetInterface::updateInfo(const InterfaceInfo& info)
+{
+ EthernetInterfaceIntf::linkUp(info.running);
+ if (info.mac)
+ {
+ MacAddressIntf::macAddress(mac_address::toString(*info.mac));
+ }
+ if (info.mtu)
+ {
+ EthernetInterfaceIntf::mtu(*info.mtu);
+ }
+}
+
static IP::Protocol getProtocol(const InAddrAny& addr)
{
if (std::holds_alternative<in_addr>(addr))
@@ -775,27 +802,19 @@
return servers;
}
-std::string EthernetInterface::vlanIntfName(VlanId id) const
+std::string EthernetInterface::vlanIntfName(uint16_t id) const
{
return fmt::format(FMT_COMPILE("{}.{}"), interfaceName(), id);
}
-std::string EthernetInterface::vlanObjPath(VlanId id) const
-{
- return fmt::format(FMT_COMPILE("{}_{}"), objPath, id);
-}
-
-void EthernetInterface::loadVLAN(VlanId id)
+void EthernetInterface::loadVLAN(std::string_view objRoot, uint16_t id)
{
auto vlanInterfaceName = vlanIntfName(id);
- auto path = vlanObjPath(id);
-
config::Parser config(
config::pathForIntfConf(manager.getConfDir(), vlanInterfaceName));
-
auto vlanIntf = std::make_unique<phosphor::network::VlanInterface>(
- bus, path.c_str(), config, EthernetInterfaceIntf::nicEnabled(), id,
- *this, manager);
+ bus, manager, getInterfaceInfo(vlanInterfaceName), objRoot, config, id,
+ *this);
// Fetch the ip address from the system
// and create the dbus object.
@@ -808,7 +827,7 @@
std::move(vlanIntf));
}
-ObjectPath EthernetInterface::createVLAN(VlanId id)
+ObjectPath EthernetInterface::createVLAN(uint16_t id)
{
auto vlanInterfaceName = vlanIntfName(id);
if (this->vlanInterfaces.find(vlanInterfaceName) !=
@@ -820,13 +839,16 @@
Argument::ARGUMENT_VALUE(std::to_string(id).c_str()));
}
- auto path = vlanObjPath(id);
+ auto objRoot = std::string_view(objPath).substr(0, objPath.rfind('/'));
+ auto info = getInterfaceInfo(interfaceName());
+ info.name = vlanInterfaceName;
// Pass the parents nicEnabled property, so that the child
// VLAN interface can inherit.
auto vlanIntf = std::make_unique<phosphor::network::VlanInterface>(
- bus, path.c_str(), config::Parser(),
- EthernetInterfaceIntf::nicEnabled(), id, *this, manager);
+ bus, manager, info, objRoot, config::Parser(), id, *this, /*emit=*/true,
+ nicEnabled());
+ ObjectPath ret = vlanIntf->objPath;
// write the device file for the vlan interface.
vlanIntf->writeDeviceFile();
@@ -836,7 +858,7 @@
writeConfigurationFile();
manager.reloadConfigs();
- return path;
+ return objPath;
}
ServerList EthernetInterface::staticNTPServers(ServerList value)
diff --git a/src/ethernet_interface.hpp b/src/ethernet_interface.hpp
index b8f73c5..3480a57 100644
--- a/src/ethernet_interface.hpp
+++ b/src/ethernet_interface.hpp
@@ -5,9 +5,11 @@
#include "xyz/openbmc_project/Network/IP/Create/server.hpp"
#include "xyz/openbmc_project/Network/Neighbor/CreateStatic/server.hpp"
+#include <netinet/ether.h>
+
+#include <optional>
#include <sdbusplus/bus.hpp>
#include <sdbusplus/server/object.hpp>
-#include <stdplus/zstring.hpp>
#include <stdplus/zstring_view.hpp>
#include <string>
#include <vector>
@@ -37,8 +39,6 @@
using ServerList = std::vector<std::string>;
using ObjectPath = sdbusplus::message::object_path;
-using VlanId = uint32_t;
-
class Manager;
class TestEthernetInterface;
@@ -50,6 +50,22 @@
class Parser;
}
+/** @class InterfaceInfo
+ * @brief Information about interfaces from the kernel
+ */
+struct InterfaceInfo
+{
+ bool running;
+ unsigned index;
+ std::string name;
+ std::optional<ether_addr> mac;
+ std::optional<unsigned> mtu;
+};
+
+/** @brief Returns an InterfaceInfo for the given string name
+ */
+InterfaceInfo getInterfaceInfo(stdplus::zstring_view ifname);
+
/** @class EthernetInterface
* @brief OpenBMC Ethernet Interface implementation.
* @details A concrete implementation for the
@@ -67,18 +83,22 @@
/** @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] manager - parent object.
+ * @param[in] info - Interface information.
+ * @param[in] objRoot - Path to attach at.
* @param[in] config - The parsed configuation file.
- * @param[in] parent - parent object.
* @param[in] emitSignal - true if the object added signal needs to be
* send.
* @param[in] enabled - Override the lookup of nicEnabled
*/
- EthernetInterface(sdbusplus::bus_t& bus, stdplus::zstring_view objPath,
- const config::Parser& config, Manager& parent,
- bool emitSignal = true,
+ EthernetInterface(sdbusplus::bus_t& bus, Manager& manager,
+ const InterfaceInfo& info, std::string_view objRoot,
+ const config::Parser& config, bool emitSignal = true,
std::optional<bool> enabled = std::nullopt);
+ /** @brief Updates the interface information based on new InterfaceInfo */
+ void updateInfo(const InterfaceInfo& info);
+
/** @brief Function used to load the ntpservers
*/
void loadNTPServers(const config::Parser& config);
@@ -195,13 +215,13 @@
/** @brief create Vlan interface.
* @param[in] id- VLAN identifier.
*/
- ObjectPath createVLAN(VlanId id);
+ ObjectPath createVLAN(uint16_t id);
/** @brief load the vlan info from the system
* and creates the ip address dbus objects.
* @param[in] vlanID- VLAN identifier.
*/
- void loadVLAN(VlanId vlanID);
+ void loadVLAN(std::string_view objRoot, uint16_t vlanID);
/** @brief write the network conf file with the in-memory objects.
*/
@@ -287,6 +307,11 @@
friend class TestEthernetInterface;
private:
+ EthernetInterface(sdbusplus::bus_t& bus, Manager& manager,
+ const InterfaceInfo& info, std::string&& objPath,
+ const config::Parser& config, bool emitSignal,
+ std::optional<bool> enabled);
+
/** @brief Determines if DHCP is active for the IP::Protocol supplied.
* @param[in] protocol - Either IPv4 or IPv6
* @returns true/false value if DHCP is active for the input protocol
@@ -304,8 +329,7 @@
*/
bool queryNicEnabled() const;
- std::string vlanIntfName(VlanId id) const;
- std::string vlanObjPath(VlanId id) const;
+ std::string vlanIntfName(uint16_t id) const;
};
} // namespace network
diff --git a/src/network_manager.cpp b/src/network_manager.cpp
index df9bf9b..9a9067e 100644
--- a/src/network_manager.cpp
+++ b/src/network_manager.cpp
@@ -101,7 +101,6 @@
auto interfaceStrList = system::getInterfaces();
for (auto& interface : interfaceStrList)
{
- fs::path objPath = objectPath;
auto index = interface.find(".");
// interface can be of vlan type or normal ethernet interface.
@@ -130,15 +129,14 @@
log<level::ERR>(msg.c_str());
continue;
}
- it->second->loadVLAN(vlanId);
+ it->second->loadVLAN(objectPath, vlanId);
continue;
}
// normal ethernet interface
- objPath /= interface;
config::Parser config(config::pathForIntfConf(confDir, interface));
auto intf = std::make_unique<phosphor::network::EthernetInterface>(
- bus, objPath.string(), config, *this);
+ bus, *this, getInterfaceInfo(interface), objectPath, config);
intf->createIPAddressObjects();
intf->createStaticNeighborObjects();
diff --git a/src/vlan_interface.cpp b/src/vlan_interface.cpp
index 9f92198..0d7879c 100644
--- a/src/vlan_interface.cpp
+++ b/src/vlan_interface.cpp
@@ -17,19 +17,17 @@
using namespace phosphor::logging;
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-VlanInterface::VlanInterface(sdbusplus::bus_t& bus,
- stdplus::const_zstring objPath,
- const config::Parser& config, bool nicEnabled,
- uint32_t vlanID, EthernetInterface& intf,
- Manager& parent) :
- VlanIface(bus, objPath.c_str()),
- DeleteIface(bus, objPath.c_str()),
- EthernetInterface(bus, objPath, config, parent, true, nicEnabled),
- parentInterface(intf)
+VlanInterface::VlanInterface(sdbusplus::bus_t& bus, Manager& manager,
+ const InterfaceInfo& info,
+ std::string_view objRoot,
+ const config::Parser& config, uint16_t vlanID,
+ EthernetInterface& parent, bool emitSignal,
+ std::optional<bool> enabled) :
+ EthernetInterface(bus, manager, info, objRoot, config, emitSignal, enabled),
+ DeleteIface(bus, objPath.c_str()), VlanIface(bus, objPath.c_str()),
+ parentInterface(parent)
{
id(vlanID);
- MacAddressIntf::macAddress(parentInterface.macAddress());
-
emit_object_added();
}
diff --git a/src/vlan_interface.hpp b/src/vlan_interface.hpp
index bc5a1c8..9d27e30 100644
--- a/src/vlan_interface.hpp
+++ b/src/vlan_interface.hpp
@@ -7,7 +7,6 @@
#include <sdbusplus/bus.hpp>
#include <sdbusplus/server/object.hpp>
-#include <stdplus/zstring.hpp>
#include <xyz/openbmc_project/Object/Delete/server.hpp>
namespace phosphor
@@ -25,9 +24,9 @@
* @brief OpenBMC vlan Interface implementation.
* @details A concrete implementation for the vlan interface
*/
-class VlanInterface : public VlanIface,
+class VlanInterface : public EthernetInterface,
public DeleteIface,
- public EthernetInterface
+ public VlanIface
{
public:
VlanInterface() = delete;
@@ -39,17 +38,23 @@
/** @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] manager - network manager object.
+ * @param[in] info - Interface information.
+ * @param[in] objRoot - Path to attach at.
* @param[in] config - The parsed configuation file.
* @param[in] vlanID - vlan identifier.
- * @param[in] intf - ethernet interface object.
- * @param[in] manager - network manager object.
+ * @param[in] parent - ethernet interface object.
+ * @param[in] emitSignal - true if the object added signal needs to be
+ * send.
+ * @param[in] enabled - Override the lookup of nicEnabled
*
* This constructor is called during loading the VLAN Interface
*/
- VlanInterface(sdbusplus::bus_t& bus, stdplus::const_zstring objPath,
- const config::Parser& config, bool nicEnabled,
- uint32_t vlanID, EthernetInterface& intf, Manager& parent);
+ VlanInterface(sdbusplus::bus_t& bus, Manager& manager,
+ const InterfaceInfo& info, std::string_view objRoot,
+ const config::Parser& config, uint16_t vlanID,
+ EthernetInterface& parent, bool emitSignal = true,
+ std::optional<bool> enabled = std::nullopt);
/** @brief Delete this d-bus object.
*/
diff --git a/test/mock_network_manager.hpp b/test/mock_network_manager.hpp
index 130e806..69a3263 100644
--- a/test/mock_network_manager.hpp
+++ b/test/mock_network_manager.hpp
@@ -30,12 +30,10 @@
auto interfaceStrList = system::getInterfaces();
for (auto& interface : interfaceStrList)
{
- fs::path objPath = objectPath;
// normal ethernet interface
- objPath /= interface;
config::Parser config(config::pathForIntfConf(confDir, interface));
auto intf = std::make_unique<MockEthernetInterface>(
- bus, objPath.string(), config, *this);
+ bus, *this, getInterfaceInfo(interface), objectPath, config);
intf->createIPAddressObjects();
intf->createStaticNeighborObjects();
intf->loadNameServers(config);
diff --git a/test/test_ethernet_interface.cpp b/test/test_ethernet_interface.cpp
index 4fd97a0..f8254b0 100644
--- a/test/test_ethernet_interface.cpp
+++ b/test/test_ethernet_interface.cpp
@@ -45,8 +45,8 @@
{
mock_clear();
mock_addIF("test0", /*idx=*/1);
- return {bus, "/xyz/openbmc_test/network/test0", config::Parser(),
- manager};
+ return {bus, manager, getInterfaceInfo("test0"),
+ "/xyz/openbmc_test/network"sv, config::Parser()};
}
int countIPObjects()
@@ -113,8 +113,8 @@
constexpr unsigned mtu = 150;
mock_addIF("test1", idx, IFF_RUNNING, mac, mtu);
- MockEthernetInterface intf(bus, "/xyz/openbmc_test/network/test1",
- config::Parser(), manager);
+ MockEthernetInterface intf(bus, manager, getInterfaceInfo("test1"),
+ "/xyz/openbmc_test/network"sv, config::Parser());
EXPECT_EQ(mtu, intf.mtu());
EXPECT_EQ(mac_address::toString(mac), intf.macAddress());
diff --git a/test/test_vlan_interface.cpp b/test/test_vlan_interface.cpp
index 9c3a1ea..499ab34 100644
--- a/test/test_vlan_interface.cpp
+++ b/test/test_vlan_interface.cpp
@@ -20,6 +20,7 @@
{
namespace fs = std::filesystem;
+using std::literals::string_view_literals::operator""sv;
class TestVlanInterface : public stdplus::gtest::TestWithTmp
{
@@ -42,14 +43,15 @@
mock_clear();
mock_addIF("test0", /*idx=*/1);
return {bus,
- "/xyz/openbmc_test/network/test0",
- config::Parser(),
manager,
- /*emitSignals=*/false,
+ getInterfaceInfo("test0"),
+ "/xyz/openbmc_test/network"sv,
+ config::Parser(),
+ /*emitSignal=*/false,
/*nicEnabled=*/true};
}
- void createVlan(VlanId id)
+ void createVlan(uint16_t id)
{
std::string ifname = "test0.";
ifname += std::to_string(id);