Update System GUID handling to be more robust
If the service that owns the GUID has not yet started when
netipmid starts, netipmid was throwing an error and failing
to start. If the time difference was too great, it would fail
to re-start and then stay stopped. This makes the error
handling more robust so that the order is not so important.
Tested: started netipmid with and without the UUID service
running and restarted the service to ensure that
netipmid would stay running.
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>
Change-Id: Id464330a2ba8416ff229fa258fff0ad7a1b8f51c
diff --git a/command/guid.cpp b/command/guid.cpp
index 944f34f..4fdece8 100644
--- a/command/guid.cpp
+++ b/command/guid.cpp
@@ -14,14 +14,7 @@
using namespace phosphor::logging;
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-namespace cache
-{
-
-command::Guid guid;
-std::string guidObjService = "";
-std::string guidObjPath = "";
-
-} // namespace cache
+static std::optional<command::Guid> guid;
namespace command
{
@@ -30,27 +23,7 @@
static constexpr auto propInterface = "xyz.openbmc_project.Common.UUID";
static constexpr auto uuidProperty = "UUID";
-static constexpr auto subtreePath = "/xyz/openbmc_project/inventory/";
-
-void getUIDObjectInfo()
-{
- sdbusplus::bus_t bus{ipmid_get_sd_bus_connection()};
- ipmi::DbusObjectInfo bmcObject;
- try
- {
- // Get the Inventory object implementing BMC interface
- bmcObject = ipmi::getDbusObject(bus, propInterface, subtreePath);
- }
- catch (const sdbusplus::exception_t& e)
- {
- lg2::error("Failed in reading BMC UUID property: {ERROR}", "ERROR", e);
- return;
- }
-
- cache::guidObjService = bmcObject.second;
- cache::guidObjPath = bmcObject.first;
- return;
-}
+static constexpr auto subtreePath = "/xyz/openbmc_project/inventory";
static void rfcToGuid(std::string rfc4122, Guid& uuid)
{
@@ -90,44 +63,50 @@
return;
}
-Guid getSystemGUID()
+// Canned System GUID for when the Chassis DBUS object is not populated
+static constexpr Guid fakeGuid = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C,
+ 0x0D, 0x0E, 0x0F, 0x10};
+const Guid& getSystemGUID()
{
- // Canned System GUID for QEMU where the Chassis DBUS object is not
- // populated
- Guid guid = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
- 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10};
+ if (guid.has_value())
+ {
+ return guid.value();
+ }
sdbusplus::bus_t bus{ipmid_get_sd_bus_connection()};
ipmi::Value propValue;
try
{
+ const auto& [objPath, service] = ipmi::getDbusObject(bus, propInterface,
+ subtreePath);
// Read UUID property value from bmcObject
// UUID is in RFC4122 format Ex: 61a39523-78f2-11e5-9862-e6402cfc3223
- propValue = ipmi::getDbusProperty(bus, cache::guidObjService,
- cache::guidObjPath, propInterface,
+ propValue = ipmi::getDbusProperty(bus, service, objPath, propInterface,
uuidProperty);
}
catch (const sdbusplus::exception_t& e)
{
lg2::error("Failed in reading BMC UUID property: {ERROR}", "ERROR", e);
- return guid;
+ return fakeGuid;
}
std::string rfc4122Uuid = std::get<std::string>(propValue);
try
{
// convert to IPMI format
- rfcToGuid(rfc4122Uuid, guid);
+ Guid tmpGuid{};
+ rfcToGuid(rfc4122Uuid, tmpGuid);
+ guid = tmpGuid;
}
catch (const InvalidArgument& e)
{
lg2::error("Failed in parsing BMC UUID property: {VALUE}", "VALUE",
rfc4122Uuid.c_str());
- return guid;
+ return fakeGuid;
}
-
- return guid;
+ return guid.value();
}
void registerGUIDChangeCallback()
@@ -137,9 +116,38 @@
using namespace sdbusplus::bus::match::rules;
sdbusplus::bus_t bus{ipmid_get_sd_bus_connection()};
- matchPtr = std::make_unique<sdbusplus::bus::match_t>(
- bus, propertiesChanged(cache::guidObjPath, propInterface),
- [](sdbusplus::message_t&) { cache::guid = getSystemGUID(); });
+ try
+ {
+ matchPtr = std::make_unique<sdbusplus::bus::match_t>(
+ bus, propertiesChangedNamespace(subtreePath, propInterface),
+ [](sdbusplus::message_t& m) {
+ try
+ {
+ std::string iface{};
+ std::map<std::string, ipmi::Value> pdict{};
+ m.read(iface, pdict);
+ if (iface != propInterface)
+ {
+ return;
+ }
+ auto guidStr = std::get<std::string>(pdict.at("UUID"));
+ Guid tmpGuid{};
+ rfcToGuid(guidStr, tmpGuid);
+ guid = tmpGuid;
+ }
+ catch (const std::exception& e)
+ {
+ // signal contained invalid guid; ignore it
+ lg2::error(
+ "Failed to parse propertiesChanged signal: {ERROR}",
+ "ERROR", e);
+ }
+ });
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error("Failed to create dbus match: {ERROR}", "ERROR", e);
+ }
}
}
diff --git a/command/guid.hpp b/command/guid.hpp
index 60a76a5..3bd8597 100644
--- a/command/guid.hpp
+++ b/command/guid.hpp
@@ -20,19 +20,11 @@
* @return If UUID is successfully read from the Chassis DBUS object, then the
* GUID is returned, else a canned GUID is returned
*/
-Guid getSystemGUID();
+const Guid& getSystemGUID();
/**
* @brief Register the callback to update the cache when the GUID changes
*/
void registerGUIDChangeCallback();
-void getUIDObjectInfo();
} // namespace command
-
-namespace cache
-{
-
-extern command::Guid guid;
-
-} // namespace cache
diff --git a/command/rakp12.cpp b/command/rakp12.cpp
index a6b4f85..2b7a925 100644
--- a/command/rakp12.cpp
+++ b/command/rakp12.cpp
@@ -292,7 +292,8 @@
std::advance(iter, cipher::rakp_auth::BMC_RANDOM_NUMBER_LEN);
// Managed System GUID
- std::copy_n(cache::guid.data(), cache::guid.size(), iter);
+ const Guid& guid = command::getSystemGUID();
+ std::copy_n(guid.data(), guid.size(), iter);
std::advance(iter, BMC_GUID_LEN);
// Requested Privilege Level
@@ -320,8 +321,7 @@
response->managed_system_random_number);
// Copy System GUID to the Response
- std::copy_n(cache::guid.data(), cache::guid.size(),
- response->managed_system_guid);
+ std::copy_n(guid.data(), guid.size(), response->managed_system_guid);
// Insert the HMAC output into the payload
outPayload.insert(outPayload.end(), output.begin(), output.end());
diff --git a/command/rakp34.cpp b/command/rakp34.cpp
index f14430e..7735d8d 100644
--- a/command/rakp34.cpp
+++ b/command/rakp34.cpp
@@ -244,7 +244,8 @@
std::advance(iter, sizeof(bmcSessionID));
// Managed System GUID
- std::copy_n(cache::guid.data(), cache::guid.size(), iter);
+ const Guid& guid = command::getSystemGUID();
+ std::copy_n(guid.data(), guid.size(), iter);
// Integrity Check Value
auto icv = authAlgo->generateICV(input);
diff --git a/main.cpp b/main.cpp
index ca5b7d4..4702d62 100644
--- a/main.cpp
+++ b/main.cpp
@@ -99,9 +99,7 @@
session::Manager::get().managerInit(channel);
// Register callback to update cache for a GUID change and cache the GUID
- command::getUIDObjectInfo();
command::registerGUIDChangeCallback();
- cache::guid = command::getSystemGUID();
// Register callback to update cache for sol conf change
sol::registerSolConfChangeCallbackHandler(channel);