From f3d71a18a5049b2d2ea986d6e7a126aa0fc09223 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Tue, 1 Feb 2022 11:27:42 +0000 Subject: [PATCH] redfish: Be more robust by retrying IPMI transactions This fixes creating users when the BMC is otherwise busy. --- plugins/redfish/fu-ipmi-device.c | 130 +++++++++++++++++++++++-------- 1 file changed, 98 insertions(+), 32 deletions(-) diff --git a/plugins/redfish/fu-ipmi-device.c b/plugins/redfish/fu-ipmi-device.c index 01530473d..f46efe6b9 100644 --- a/plugins/redfish/fu-ipmi-device.c +++ b/plugins/redfish/fu-ipmi-device.c @@ -22,6 +22,9 @@ #define FU_IPMI_DEVICE_TIMEOUT 1500 /* ms */ +#define FU_IPMI_TRANSACTION_RETRY_COUNT 5 +#define FU_IPMI_TRANSACTION_RETRY_DELAY 200 /* ms */ + /* not defined in linux/ipmi_msgdefs.h */ #define IPMI_SET_USER_ACCESS 0x43 #define IPMI_SET_USER_NAME 0x45 @@ -234,19 +237,51 @@ fu_ipmi_device_errcode_to_string(guint8 errcode) } static gboolean -fu_ipmi_device_transaction(FuIpmiDevice *self, - guint8 netfn, - guint8 cmd, - const guint8 *req_buf, - gsize req_bufsz, - guint8 *resp_buf, /* optional */ - gsize resp_bufsz, - gsize *resp_len, /* optional, out */ - gint timeout_ms, - GError **error) +fu_ipmi_device_errcode_to_error(guint8 errcode, GError **error) { + /* success */ + if (errcode == IPMI_CC_NO_ERROR) + return TRUE; + + /* data not found, seemingly Lenovo specific */ + if (errcode == IPMI_INVALID_DATA_FIELD_ERR || errcode == IPMI_NOT_FOUND_ERR) { + g_set_error(error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "CC error: %s [0x%02X]", + fu_ipmi_device_errcode_to_string(errcode), + errcode); + return FALSE; + } + + /* fallback */ + g_set_error(error, + G_IO_ERROR, + G_IO_ERROR_FAILED, + "CC error: %s [0x%02X]", + fu_ipmi_device_errcode_to_string(errcode), + errcode); + return FALSE; +} + +typedef struct { + guint8 netfn; + guint8 cmd; + const guint8 *req_buf; + gsize req_bufsz; + guint8 *resp_buf; + gsize resp_bufsz; + gsize *resp_len; + gint timeout_ms; +} FuIpmiDeviceTransactionHelper; + +static gboolean +fu_ipmi_device_transaction_cb(FuDevice *device, gpointer user_data, GError **error) +{ + FuIpmiDevice *self = FU_IPMI_DEVICE(device); + FuIpmiDeviceTransactionHelper *helper = (FuIpmiDeviceTransactionHelper *)user_data; GPollFD pollfds[1]; - gsize resp_buf2sz = resp_bufsz + 1; + gsize resp_buf2sz = helper->resp_bufsz + 1; gsize resp_len2 = 0; g_autoptr(GTimer) timer = g_timer_new(); g_autoptr(FuDeviceLocker) lock = NULL; @@ -256,7 +291,12 @@ fu_ipmi_device_transaction(FuIpmiDevice *self, if (lock == NULL) return FALSE; - if (!fu_ipmi_device_send(self, netfn, cmd, req_buf, req_bufsz, error)) + if (!fu_ipmi_device_send(self, + helper->netfn, + helper->cmd, + helper->req_buf, + helper->req_bufsz, + error)) return FALSE; pollfds[0].fd = fu_udev_device_get_fd(FU_UDEV_DEVICE(self)); @@ -268,7 +308,9 @@ fu_ipmi_device_transaction(FuIpmiDevice *self, glong seq = 0; gint rc; - rc = g_poll(pollfds, 1, timeout_ms - (g_timer_elapsed(timer, NULL) * 1000.f)); + rc = g_poll(pollfds, + 1, + helper->timeout_ms - (g_timer_elapsed(timer, NULL) * 1000.f)); if (rc < 0) { g_set_error(error, G_IO_ERROR, G_IO_ERROR_FAILED, "poll() error %m"); return FALSE; @@ -279,8 +321,8 @@ fu_ipmi_device_transaction(FuIpmiDevice *self, G_IO_ERROR_FAILED, "timeout waiting for response " "(netfn %d, cmd %d)", - netfn, - cmd); + helper->netfn, + helper->cmd); return FALSE; } @@ -307,7 +349,7 @@ fu_ipmi_device_transaction(FuIpmiDevice *self, "expected %ld, got %ld", self->seq, seq); - if (g_timer_elapsed(timer, NULL) * 1000.f >= timeout_ms) { + if (g_timer_elapsed(timer, NULL) * 1000.f >= helper->timeout_ms) { g_set_error_literal(error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -315,33 +357,26 @@ fu_ipmi_device_transaction(FuIpmiDevice *self, return FALSE; } } else { - if (resp_buf2[0] != IPMI_CC_NO_ERROR) { - g_set_error(error, - G_IO_ERROR, - G_IO_ERROR_FAILED, - "CC error: %s [%02x]", - fu_ipmi_device_errcode_to_string(resp_buf2[0]), - resp_buf2[0]); + if (!fu_ipmi_device_errcode_to_error(resp_buf2[0], error)) return FALSE; - } - if (resp_buf != NULL) { - if (!fu_memcpy_safe(resp_buf, - resp_bufsz, + if (helper->resp_buf != NULL) { + if (!fu_memcpy_safe(helper->resp_buf, + helper->resp_bufsz, 0x0, /* dst */ resp_buf2, resp_buf2sz, 0x01, /* src */ - resp_bufsz, + helper->resp_bufsz, error)) return FALSE; } - if (resp_len != NULL) - *resp_len = resp_len2 - 1; + if (helper->resp_len != NULL) + *helper->resp_len = resp_len2 - 1; if (g_getenv("FWUPD_REDFISH_VERBOSE") != NULL) { g_debug("IPMI netfn: %02x->%02x, cmd: %02x->%02x", - netfn, + helper->netfn, resp_netfn, - cmd, + helper->cmd, resp_cmd); } break; @@ -350,6 +385,37 @@ fu_ipmi_device_transaction(FuIpmiDevice *self, return TRUE; } +static gboolean +fu_ipmi_device_transaction(FuIpmiDevice *self, + guint8 netfn, + guint8 cmd, + const guint8 *req_buf, + gsize req_bufsz, + guint8 *resp_buf, /* optional */ + gsize resp_bufsz, + gsize *resp_len, /* optional, out */ + gint timeout_ms, + GError **error) +{ + FuIpmiDeviceTransactionHelper helper = { + .netfn = netfn, + .cmd = cmd, + .req_buf = req_buf, + .req_bufsz = req_bufsz, + .resp_buf = resp_buf, + .resp_bufsz = resp_bufsz, + .resp_len = resp_len, + .timeout_ms = timeout_ms, + }; + fu_device_retry_add_recovery(FU_DEVICE(self), G_IO_ERROR, G_IO_ERROR_NOT_FOUND, NULL); + return fu_device_retry_full(FU_DEVICE(self), + fu_ipmi_device_transaction_cb, + FU_IPMI_TRANSACTION_RETRY_COUNT, + FU_IPMI_TRANSACTION_RETRY_DELAY, + &helper, + error); +} + static gboolean fu_ipmi_device_probe(FuDevice *device, GError **error) {