diff options
author | Kun Yi <kunyi731@gmail.com> | 2019-03-05 14:02:51 -0800 |
---|---|---|
committer | Kun Yi <kunyi731@gmail.com> | 2019-03-12 22:24:38 -0700 |
commit | 5dcf41e994e6e568d056c973432a44ed5ffe66d0 (patch) | |
tree | cf87470f4b5fe9f4d687acff3cddb2b935f86a59 | |
parent | 64a350d3214e8bb9b43c1ff4d49b29ad38ad0d04 (diff) | |
download | phosphor-host-ipmid-5dcf41e994e6e568d056c973432a44ed5ffe66d0.tar.gz phosphor-host-ipmid-5dcf41e994e6e568d056c973432a44ed5ffe66d0.zip |
utils: Use 5s timeout for D-Bus get/set property calls
If the timeout param is not specified in a sdbus call, the default value
of 25s is used, if not configured in the systemd configuration file.
It turns out this timeout is too long than ideal, since the other pieces
in the IPMI stack have much smaller timeout values. For example, the
BTbridge/KCSbridge has a timeout of 5s. It is unnecessary to let ipmid
getting blocked on waiting for D-Bus replies.
This commit changes the timeout to 5 seconds on D-Bus get/set properties,
mainly because these calls are the majority of sensor read/writes which
are most subject to operation timeouts. The other D-Bus operations might
be called for object/service discovering, which might be a one-time
startup procedure.
Resolves: openbmc/openbmc#3494
Signed-off-by: Kun Yi <kunyi731@gmail.com>
Change-Id: I5a55a8eb872b1ebe8730c673231be5037d24fc0b
-rw-r--r-- | utils.cpp | 16 | ||||
-rw-r--r-- | utils.hpp | 22 |
2 files changed, 26 insertions, 12 deletions
@@ -5,6 +5,7 @@ #include <net/if.h> #include <algorithm> +#include <chrono> #include <phosphor-logging/elog-errors.hpp> #include <phosphor-logging/log.hpp> #include <sdbusplus/message/types.hpp> @@ -132,7 +133,8 @@ DbusObjectInfo getIPObject(sdbusplus::bus::bus& bus, Value getDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, const std::string& objPath, const std::string& interface, - const std::string& property) + const std::string& property, + std::chrono::microseconds timeout) { Value value; @@ -142,7 +144,7 @@ Value getDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, method.append(interface, property); - auto reply = bus.call(method); + auto reply = bus.call(method, timeout.count()); if (reply.is_method_error()) { @@ -161,7 +163,8 @@ Value getDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, PropertyMap getAllDbusProperties(sdbusplus::bus::bus& bus, const std::string& service, const std::string& objPath, - const std::string& interface) + const std::string& interface, + std::chrono::microseconds timeout) { PropertyMap properties; @@ -170,7 +173,7 @@ PropertyMap getAllDbusProperties(sdbusplus::bus::bus& bus, method.append(interface); - auto reply = bus.call(method); + auto reply = bus.call(method, timeout.count()); if (reply.is_method_error()) { @@ -209,14 +212,15 @@ ObjectValueTree getManagedObjects(sdbusplus::bus::bus& bus, void setDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, const std::string& objPath, const std::string& interface, - const std::string& property, const Value& value) + const std::string& property, const Value& value, + std::chrono::microseconds timeout) { auto method = bus.new_method_call(service.c_str(), objPath.c_str(), PROP_INTF, METHOD_SET); method.append(interface, property, value); - if (!bus.call(method)) + if (!bus.call(method, timeout.count())) { log<level::ERR>("Failed to set property", entry("PROPERTY=%s", property.c_str()), @@ -1,12 +1,15 @@ #pragma once #include "types.hpp" +#include <chrono> #include <optional> #include <sdbusplus/server.hpp> namespace ipmi { +using namespace std::literals::chrono_literals; + constexpr auto MAPPER_BUS_NAME = "xyz.openbmc_project.ObjectMapper"; constexpr auto MAPPER_OBJ = "/xyz/openbmc_project/object_mapper"; constexpr auto MAPPER_INTF = "xyz.openbmc_project.ObjectMapper"; @@ -21,6 +24,10 @@ constexpr auto METHOD_GET = "Get"; constexpr auto METHOD_GET_ALL = "GetAll"; constexpr auto METHOD_SET = "Set"; +/* Use a value of 5s which aligns with BT/KCS bridged timeouts, rather + * than the default 25s D-Bus timeout. */ +constexpr std::chrono::microseconds IPMI_DBUS_TIMEOUT = 5s; + /** @class ServiceCache * @brief Caches lookups of service names from the object mapper. * @details Most ipmi commands need to talk to other dbus daemons to perform @@ -130,7 +137,8 @@ DbusObjectInfo getIPObject(sdbusplus::bus::bus& bus, */ Value getDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, const std::string& objPath, const std::string& interface, - const std::string& property); + const std::string& property, + std::chrono::microseconds timeout = IPMI_DBUS_TIMEOUT); /** @brief Gets all the properties associated with the given object * and the interface. @@ -140,10 +148,11 @@ Value getDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, * @param[in] interface - Dbus interface. * @return On success returns the map of name value pair. */ -PropertyMap getAllDbusProperties(sdbusplus::bus::bus& bus, - const std::string& service, - const std::string& objPath, - const std::string& interface); +PropertyMap + getAllDbusProperties(sdbusplus::bus::bus& bus, const std::string& service, + const std::string& objPath, + const std::string& interface, + std::chrono::microseconds timeout = IPMI_DBUS_TIMEOUT); /** @brief Gets all managed objects associated with the given object * path and service. @@ -166,7 +175,8 @@ ObjectValueTree getManagedObjects(sdbusplus::bus::bus& bus, */ void setDbusProperty(sdbusplus::bus::bus& bus, const std::string& service, const std::string& objPath, const std::string& interface, - const std::string& property, const Value& value); + const std::string& property, const Value& value, + std::chrono::microseconds timeout = IPMI_DBUS_TIMEOUT); /** @brief Gets all the dbus objects from the given service root * which matches the object identifier. |