inventory_mac: fix inventory sync
The inventory mac syncing feature was not working correctly. Fixed a
few issues with it.
1. The code waited for signals for ethernet interfaces, but those
signals were likely already emitted. We need to also scan the
existing interfaces for potential inventory lookups.
2. The code to register for inventory signals was after the
inventory lookup. This leaves a race condition where inventory
could be emitted and missed. Register the signal match before
doing the inventory lookup and cancel the match if the lookup
was successful.
3. Some places were needlessly re-reading the configuration file.
4. Some places were saving a reference to an on-stack representation
of the configuration file into a callback (for signals). Move
this to a global variable to avoid segfaults.
Tested:
Booted up in QEMU without an inventory EEPROM. Observed expected
behavior from daemon. Issued a manual inventory Notify call to create
a fake MAC address. Observed signal was reacted to appropriately.
Restarted the service and observed that 'force-sync' occurs immediately.
(trimmed duplicate journal entries from various journalctl calls)
```
root@bletchley:~# journalctl -u xyz.openbmc_project.Network
Feb 10 13:46:10 bletchley systemd[1]: Starting Phosphor Network Manager...
Feb 10 13:46:12 bletchley phosphor-network-manager[537]: Force sync enabled, check VPD for MAC
Feb 10 13:46:12 bletchley phosphor-network-manager[537]: Registering the Inventory Signals Matcher
Feb 10 13:46:15 bletchley phosphor-network-manager[537]: No Object has implemented the interface
Feb 10 13:46:15 bletchley phosphor-network-manager[537]: The operation failed internally.
Feb 10 13:46:15 bletchley phosphor-network-manager[537]: Exception occurred during getting of MAC address from Inventory
Feb 10 13:46:15 bletchley systemd[1]: Started Phosphor Network Manager.
root@bletchley:~# busctl call xyz.openbmc_project.Inventory.Manager \
/xyz/openbmc_project/inventory \
xyz.openbmc_project.Inventory.Manager Notify \
a{oa{sa{sv}}} 1 \
/xyz/openbmc_project/inventory/system/chassis/bmc/ethernet \
1 xyz.openbmc_project.Inventory.Item.NetworkInterface \
1 MACAddress s "C01850F1D796"
root@bletchley:~# journalctl -u xyz.openbmc_project.Network
Feb 10 13:48:31 bletchley phosphor-network-manager[537]: Wrote networkd file: /etc/systemd/network/00-bmc-eth0.network
Feb 10 13:48:31 bletchley phosphor-network-manager[537]: Setting MAC on eth0
root@bletchley:~# [ 185.469811] ftgmac100 1e670000.ftgmac eth0: Link is Up - 1Gbps/Full - flow control off
[ 185.471607] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
ifconfig
eth0 Link encap:Ethernet HWaddr C0:18:50:F1:D7:96
inet addr:10.0.2.16 Bcast:10.0.2.255 Mask:255.255.255.0
root@bletchley:~# systemctl restart xyz.openbmc_project.Network
root@bletchley:~# journalctl -u xyz.openbmc_project.Network
Feb 10 13:48:34 bletchley phosphor-network-manager[537]: Reloaded systemd-networkd
Feb 10 13:55:24 bletchley phosphor-network-manager[537]: Got TERM, exiting
Feb 10 13:55:24 bletchley systemd[1]: Stopping Phosphor Network Manager...
Feb 10 13:55:24 bletchley systemd[1]: xyz.openbmc_project.Network.service: Deactivated successfully.
Feb 10 13:55:24 bletchley systemd[1]: Stopped Phosphor Network Manager.
Feb 10 13:55:24 bletchley systemd[1]: Starting Phosphor Network Manager...
Feb 10 13:55:25 bletchley phosphor-network-manager[11066]: Using DHCP options from /etc/systemd/network/00-bmc-eth0.network
Feb 10 13:55:25 bletchley phosphor-network-manager[11066]: Force sync enabled, check VPD for MAC
Feb 10 13:55:25 bletchley phosphor-network-manager[11066]: Registering the Inventory Signals Matcher
Feb 10 13:55:25 bletchley phosphor-network-manager[11066]: Setting MAC on eth0
Feb 10 13:55:25 bletchley phosphor-network-manager[11066]: Removing the match for ethernet interfaces
Feb 10 13:55:25 bletchley systemd[1]: Started Phosphor Network Manager.
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Icf18e3d80f6d1f8c4567603fe51d774e9090334c
diff --git a/src/inventory_mac.cpp b/src/inventory_mac.cpp
index 7eaa3c4..4da20ab 100644
--- a/src/inventory_mac.cpp
+++ b/src/inventory_mac.cpp
@@ -46,7 +46,9 @@
Manager* manager = nullptr;
std::unique_ptr<sdbusplus::bus::match_t> EthInterfaceMatch = nullptr;
+std::unique_ptr<sdbusplus::bus::match_t> MacAddressMatch = nullptr;
std::vector<std::string> first_boot_status;
+nlohmann::json configJson;
void setFirstBootMACOnInterface(const std::string& intf, const std::string& mac)
{
@@ -77,13 +79,7 @@
ether_addr getfromInventory(sdbusplus::bus_t& bus, const std::string& intfName)
{
- std::string interfaceName = intfName;
-
- // load the config JSON from the Read Only Path
- std::ifstream in(configFile);
- nlohmann::json configJson;
- in >> configJson;
- interfaceName = configJson[intfName];
+ std::string interfaceName = configJson[intfName];
std::vector<DbusInterface> interfaces;
interfaces.emplace_back(invNetworkIntf);
@@ -165,9 +161,7 @@
return ToAddr<ether_addr>{}(std::get<std::string>(value));
}
-bool setInventoryMACOnSystem(sdbusplus::bus_t& bus,
- const nlohmann::json& configJson,
- const std::string& intfname)
+bool setInventoryMACOnSystem(sdbusplus::bus_t& bus, const std::string& intfname)
{
try
{
@@ -214,12 +208,10 @@
}
// register the macthes to be monitored from inventory manager
-void registerSignals(sdbusplus::bus_t& bus, const nlohmann::json& configJson)
+void registerSignals(sdbusplus::bus_t& bus)
{
log<level::INFO>("Registering the Inventory Signals Matcher");
- static std::unique_ptr<sdbusplus::bus::match_t> MacAddressMatch;
-
auto callback = [&](sdbusplus::message_t& m) {
std::map<DbusObjectPath,
std::map<DbusInterface, std::variant<PropertyValue>>>
@@ -261,10 +253,32 @@
callback);
}
-void watchEthernetInterface(sdbusplus::bus_t& bus,
- const nlohmann::json& configJson)
+void watchEthernetInterface(sdbusplus::bus_t& bus)
{
- auto mycallback = [&](sdbusplus::message_t& m) {
+ auto handle_interface = [&](auto infname) {
+ if (configJson.find(infname) == configJson.end())
+ {
+ // ethernet interface not found in configJSON
+ // check if it is not sit0 interface, as it is
+ // expected.
+ if (infname != "sit0")
+ {
+ log<level::ERR>("Wrong Interface Name in Config Json");
+ }
+ }
+ else
+ {
+ registerSignals(bus);
+ EthInterfaceMatch = nullptr;
+
+ if (setInventoryMACOnSystem(bus, infname))
+ {
+ MacAddressMatch = nullptr;
+ }
+ }
+ };
+
+ auto mycallback = [&, handle_interface](sdbusplus::message_t& m) {
std::map<DbusObjectPath,
std::map<DbusInterface, std::variant<PropertyValue>>>
interfacesProperties;
@@ -272,8 +286,10 @@
sdbusplus::message::object_path objPath;
std::pair<std::string, std::string> ethPair;
m.read(objPath, interfacesProperties);
+
for (const auto& interfaces : interfacesProperties)
{
+ log<level::INFO>(interfaces.first.c_str());
if (interfaces.first ==
"xyz.openbmc_project.Network.EthernetInterface")
{
@@ -281,29 +297,9 @@
{
if (property.first == "InterfaceName")
{
- std::string infname =
- std::get<std::string>(property.second);
+ handle_interface(
+ std::get<std::string>(property.second));
- if (configJson.find(infname) == configJson.end())
- {
- // ethernet interface not found in configJSON
- // check if it is not sit0 interface, as it is
- // expected.
- if (infname != "sit0")
- {
- log<level::ERR>(
- "Wrong Interface Name in Config Json");
- }
- }
- else
- {
- if (!setInventoryMACOnSystem(bus, configJson,
- infname))
- {
- registerSignals(bus, configJson);
- EthInterfaceMatch = nullptr;
- }
- }
break;
}
}
@@ -333,6 +329,14 @@
"member='InterfacesAdded',path='/xyz/openbmc_project/network'",
mycallback);
registeredSignals = true;
+
+ for (const auto& intf : manager->interfaces)
+ {
+ if (intf.first == interfaceString.key())
+ {
+ handle_interface(intf.first);
+ }
+ }
}
}
}
@@ -342,9 +346,8 @@
{
manager = &m.get();
std::ifstream in(configFile);
- nlohmann::json configJson;
in >> configJson;
- watchEthernetInterface(bus, configJson);
+ watchEthernetInterface(bus);
return nullptr;
}