From 5dcf41e994e6e568d056c973432a44ed5ffe66d0 Mon Sep 17 00:00:00 2001 From: Kun Yi Date: Tue, 5 Mar 2019 14:02:51 -0800 Subject: 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 Change-Id: I5a55a8eb872b1ebe8730c673231be5037d24fc0b --- utils.cpp | 16 ++++++++++------ utils.hpp | 22 ++++++++++++++++------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/utils.cpp b/utils.cpp index 4aaf9c0..7807e76 100644 --- a/utils.cpp +++ b/utils.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -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("Failed to set property", entry("PROPERTY=%s", property.c_str()), diff --git a/utils.hpp b/utils.hpp index a100718..09bb1b4 100644 --- a/utils.hpp +++ b/utils.hpp @@ -1,12 +1,15 @@ #pragma once #include "types.hpp" +#include #include #include 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. -- cgit v1.2.1