diff options
Diffstat (limited to 'meta-openbmc-mods/meta-common/recipes-phosphor')
8 files changed, 500 insertions, 1 deletions
diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd/0002-Add-rate-limiting.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd/0002-Add-rate-limiting.patch new file mode 100644 index 000000000..407ea8bbf --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd/0002-Add-rate-limiting.patch @@ -0,0 +1,287 @@ +From 0a8ecbadb73d597da114d77853793e8642102de9 Mon Sep 17 00:00:00 2001 +From: Jonathan Doman <jonathan.doman@intel.com> +Date: Wed, 26 Apr 2023 11:45:39 -0700 +Subject: [PATCH] Add rate limiting + +A host CPU can write POST codes much faster than the BMC can handle +them, considering all the D-Bus/IPC work required. Ideally `dbus-broker` +would apply backpressure when it gets full of unhandled signals, but its +quota mechanism uses a simple per-user accounting that doesn't +differentiate between all the connections from OpenBMC daemons running +as root. So there is no way to configure it to prevent just `snoopd` +from sending too many messages - instead it will disconnect arbitrary +services leading to mass chaos. + +So without a D-Bus policy mechanism to prevent excess memory usage, +there are 2 different failure cases during a POST code storm: +1. `snoopd` continues to send messages faster than `post-code-manager` + can process them, leading to `dbus-broker` consuming all the system + memory. +2. `snoopd` fills up the D-Bus socket buffer. Once sd-bus fails to send + a message across the socket, it starts queuing messages internally + leading to `snoopd` consuming all the system memory. This only + happens because we get stuck in the `snoopd` read loop during a POST + code storm, and we don't process other events that would allow the + write queue to drain. + +As a workaround, introduce configurable rate limiting to `snoopd`. A new +meson option 'rate-limit' sets the corresponding '--rate-limit' +command-line parameter. These options take an integer value representing +the maximum number of POST codes to process per second. The default +meson option value is 1000, and the value of 0 will disable rate limiting. + +Tested: Ran the POST code stress on host for 30 minutes: +``` +[root@sut ~]# stress-ng --ioport 2 +``` + +Watched BMC process memory usage and CPU usage in `top`, verified that +`post-code-manager`, `dbus-broker`, and `snoopd` each used less than 10% +CPU and 2% memory on AST2600 with 512 MiB of DRAM. + +Change-Id: If03a01e0cd62366d188109bb4dff52958346e1db +Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> +--- + lpcsnoop/snoop.hpp | 1 + + main.cpp | 109 +++++++++++++++++++++++++++++++++++++++++---- + meson.build | 5 +++ + meson_options.txt | 8 ++++ + 4 files changed, 115 insertions(+), 8 deletions(-) + +diff --git a/lpcsnoop/snoop.hpp b/lpcsnoop/snoop.hpp +index 68d51b4..c66e421 100644 +--- a/lpcsnoop/snoop.hpp ++++ b/lpcsnoop/snoop.hpp +@@ -24,4 +24,5 @@ class PostReporter : public PostObject + PostObject(bus, objPath, defer) + { + } ++ unsigned int rateLimit = 0; + }; +diff --git a/main.cpp b/main.cpp +index 764c855..11310ba 100644 +--- a/main.cpp ++++ b/main.cpp +@@ -23,6 +23,7 @@ + #include <systemd/sd-event.h> + #include <unistd.h> + ++#include <chrono> + #include <cstdint> + #include <exception> + #include <iostream> +@@ -31,10 +32,13 @@ + #include <sdeventplus/source/event.hpp> + #include <sdeventplus/source/io.hpp> + #include <sdeventplus/source/signal.hpp> ++#include <sdeventplus/source/time.hpp> ++#include <span> + #include <stdplus/signal.hpp> + #include <thread> + + static size_t codeSize = 1; /* Size of each POST code in bytes */ ++static bool verbose = false; + + static void usage(const char* name) + { +@@ -47,15 +51,76 @@ static void usage(const char* name) + name, codeSize); + } + ++/** ++ * Call once for each POST code received. If the number of POST codes exceeds ++ * the configured rate limit, this function will disable the snoop device IO ++ * source until the end of the 1 second interval, then re-enable it. ++ * ++ * @return Whether the rate limit is exceeded. ++ */ ++bool rateLimit(PostReporter& reporter, sdeventplus::source::IO& ioSource) ++{ ++ if (reporter.rateLimit == 0) ++ { ++ // Rate limiting is disabled. ++ return false; ++ } ++ ++ using Clock = sdeventplus::Clock<sdeventplus::ClockId::Monotonic>; ++ ++ static constexpr std::chrono::seconds rateLimitInterval(1); ++ static unsigned int rateLimitCount = 0; ++ static Clock::time_point rateLimitEndTime; ++ ++ const sdeventplus::Event& event = ioSource.get_event(); ++ ++ if (rateLimitCount == 0) ++ { ++ // Initialize the end time when we start a new interval ++ rateLimitEndTime = Clock(event).now() + rateLimitInterval; ++ } ++ ++ if (++rateLimitCount < reporter.rateLimit) ++ { ++ return false; ++ } ++ ++ rateLimitCount = 0; ++ ++ if (rateLimitEndTime < Clock(event).now()) ++ { ++ return false; ++ } ++ ++ if (verbose) ++ { ++ fprintf(stderr, "Hit POST code rate limit - disabling temporarily\n"); ++ } ++ ++ ioSource.set_enabled(sdeventplus::source::Enabled::Off); ++ sdeventplus::source::Time<sdeventplus::ClockId::Monotonic>( ++ event, rateLimitEndTime, std::chrono::milliseconds(100), ++ [&ioSource](auto&, auto) { ++ if (verbose) ++ { ++ fprintf(stderr, "Reenabling POST code handler\n"); ++ } ++ ioSource.set_enabled(sdeventplus::source::Enabled::On); ++ }) ++ .set_floating(true); ++ return true; ++} ++ + /* + * Callback handling IO event from the POST code fd. i.e. there is new + * POST code available to read. + */ +-void PostCodeEventHandler(sdeventplus::source::IO& s, int postFd, uint32_t, +- PostReporter* reporter, bool verbose) ++void PostCodeEventHandler(PostReporter* reporter, sdeventplus::source::IO& s, ++ int postFd, uint32_t) + { + uint64_t code = 0; + ssize_t readb; ++ + while ((readb = read(postFd, &code, codeSize)) > 0) + { + code = le64toh(code); +@@ -72,6 +137,11 @@ void PostCodeEventHandler(sdeventplus::source::IO& s, int postFd, uint32_t, + // read depends on old data being cleared since it doens't always read + // the full code size + code = 0; ++ ++ if (rateLimit(*reporter, s)) ++ { ++ return; ++ } + } + + if (readb < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)) +@@ -103,6 +173,7 @@ int main(int argc, char* argv[]) + int rc = 0; + int opt; + int postFd = -1; ++ unsigned int rateLimit = 0; + + /* + * These string constants are only used in this method within this object +@@ -115,18 +186,19 @@ int main(int argc, char* argv[]) + const char* snoopDbus = SNOOP_BUSNAME; + + bool deferSignals = true; +- bool verbose = false; + + // clang-format off + static const struct option long_options[] = { + {"bytes", required_argument, NULL, 'b'}, + {"device", optional_argument, NULL, 'd'}, ++ {"rate-limit", optional_argument, NULL, 'r'}, + {"verbose", no_argument, NULL, 'v'}, + {0, 0, 0, 0} + }; + // clang-format on + +- while ((opt = getopt_long(argc, argv, "b:d:v", long_options, NULL)) != -1) ++ while ((opt = getopt_long(argc, argv, "h:b:d:r:v", long_options, NULL)) != ++ -1) + { + switch (opt) + { +@@ -153,6 +225,28 @@ int main(int argc, char* argv[]) + } + + break; ++ case 'r': { ++ int argVal = -1; ++ try ++ { ++ argVal = std::stoi(optarg); ++ } ++ catch (...) ++ { ++ } ++ ++ if (argVal < 1) ++ { ++ fprintf(stderr, "Invalid rate limit '%s'. Must be >= 1.\n", ++ optarg); ++ return EXIT_FAILURE; ++ } ++ ++ rateLimit = static_cast<unsigned int>(argVal); ++ fprintf(stderr, "Rate limiting to %d POST codes per second.\n", ++ argVal); ++ break; ++ } + case 'v': + verbose = true; + break; +@@ -178,11 +272,10 @@ int main(int argc, char* argv[]) + std::optional<sdeventplus::source::IO> reporterSource; + if (postFd > 0) + { ++ reporter.rateLimit = rateLimit; + reporterSource.emplace( +- event, postFd, EPOLLIN | EPOLLET, +- std::bind(PostCodeEventHandler, std::placeholders::_1, +- std::placeholders::_2, std::placeholders::_3, +- &reporter, verbose)); ++ event, postFd, EPOLLIN, ++ std::bind_front(PostCodeEventHandler, &reporter)); + } + // Enable bus to handle incoming IO and bus events + bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL); +diff --git a/meson.build b/meson.build +index 2bafd48..f54ee8c 100644 +--- a/meson.build ++++ b/meson.build +@@ -27,7 +27,12 @@ conf_data.set('SYSTEMD_TARGET', get_option('systemd-target')) + snoopd_args = '-b ' + get_option('post-code-bytes').to_string() + if get_option('snoop-device') != '' + snoopd_args += ' -d /dev/' + get_option('snoop-device') ++ rate_limit = get_option('rate-limit') ++ if rate_limit > 0 ++ snoopd_args += ' --rate-limit=' + rate_limit.to_string() ++ endif + endif ++ + conf_data.set('SNOOPD_ARGS', snoopd_args) + + configure_file( +diff --git a/meson_options.txt b/meson_options.txt +index 763c73e..da151e1 100644 +--- a/meson_options.txt ++++ b/meson_options.txt +@@ -20,3 +20,11 @@ option( + option( + 'tests', type: 'feature', description: 'Build tests.', + ) ++option( ++ 'rate-limit', ++ description: 'Maximum number of POST codes to read from snoop device every' ++ + 'second. Value of 0 disables rate limiting.', ++ type: 'integer', ++ min: 0, ++ value: 1000 ++) +-- +2.17.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend index 893f410e8..f21e386bd 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend @@ -7,3 +7,4 @@ DEPENDS += " gtest" SRCREV = "6a5e0a1cba979c3c793e794c41481221da9a4e33" SRC_URI += "file://0001-Avoid-negated-postcode-write-to-D-Bus.patch" +SRC_URI += "file://0002-Add-rate-limiting.patch" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/0037-Fix-certificate-replacement-URI-response-error-code.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/0037-Fix-certificate-replacement-URI-response-error-code.patch new file mode 100644 index 000000000..3d8312961 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/0037-Fix-certificate-replacement-URI-response-error-code.patch @@ -0,0 +1,35 @@ +From 966c2c347b3ab96bcedd362b690e66f39802b660 Mon Sep 17 00:00:00 2001 +From: Manish Baing <manish.baing@intel.com> +Date: Thu, 10 Aug 2023 05:48:48 +0000 +Subject: [PATCH] Fix certificate replacement URI response error code + + +We get 500 Internal Server Error when we try to replace certificate +without providing certificate but expected response is 400 +So fixed the issue by checking for json (body) content before looking +for specific keys and identifying it as 500 Internal Server Error. + +Tested: +Response for attempt to replace certificate without providing +certificate is now 400 (Bad Request response) + +Signed-off-by: Manish Baing <manish.baing@intel.com> +--- + redfish-core/lib/certificate_service.hpp | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/redfish-core/lib/certificate_service.hpp b/redfish-core/lib/certificate_service.hpp +index f0891771..fd4836bd 100644 +--- a/redfish-core/lib/certificate_service.hpp ++++ b/redfish-core/lib/certificate_service.hpp +@@ -691,7 +691,6 @@ inline void requestRoutesCertificateActionsReplaceCertificate(App& app) + certificateType)) + { + BMCWEB_LOG_ERROR << "Required parameters are missing"; +- messages::internalError(asyncResp->res); + return; + } + +-- +2.17.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend index e3bed9eb0..8db6f9c7c 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend @@ -32,6 +32,7 @@ SRC_URI += "file://0001-Firmware-update-configuration-changes.patch \ file://0034-Update-odata.type-version-of-redfish-v1-AccountService.patch \ file://0035-Add-MemoryMetrics-schema-file.patch \ file://0036-PCIeFunctions-not-showing-in-Redfish.patch \ + file://0037-Fix-certificate-replacement-URI-response-error-code.patch \ " # OOB Bios Config: diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/pldmd.bb b/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/pldmd.bb index 2ef0b4e95..a1f3e00c6 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/pldmd.bb +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/pldmd.bb @@ -5,7 +5,7 @@ LICENSE = "Apache-2.0" LIC_FILES_CHKSUM = "file://LICENSE;md5=86d3f3a95c324c9479bd8986968f4327" SRC_URI += "git://git@github.com/Intel-BMC/pldmd.git;protocol=ssh;branch=1-release" -SRCREV = "2c3522ec89828d5abed265b4f460e77b4c580ae4" +SRCREV = "49d1cf2c4c581b9f860af826ef7c044ac12b8591" S = "${WORKDIR}/git" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager/0001-Use-binary-serialization-instead-of-JSON.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager/0001-Use-binary-serialization-instead-of-JSON.patch new file mode 100644 index 000000000..91992e260 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager/0001-Use-binary-serialization-instead-of-JSON.patch @@ -0,0 +1,104 @@ +From 3f362d5e15dd3c20d1026bd814fe52b9793025e5 Mon Sep 17 00:00:00 2001 +From: Jonathan Doman <jonathan.doman@intel.com> +Date: Wed, 23 Nov 2022 15:04:17 -0800 +Subject: [PATCH 1/2] Use binary serialization instead of JSON + +The binary format is much more efficient than JSON in terms of +computational speed and disk space consumption. The former is important +in case the host is sending a constant stream of POST codes. +post-code-manager can fall behind because it takes too long to store +each new POST code on disk, causing D-Bus messages to pile up and +increase memory consumption inside dbus-broker. + +Tested: +Rebooted the host a few times and observed that POST code history is +populated normally in Redfish. After upgrading to this change, old POST +code history stored in JSON format is lost, but remains on disk until it +gets overwritten during subsequent host boots. + +Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> +Change-Id: Id55909a55d950e6e62b78b3333df687b4c582c42 +Signed-off-by: Manish Baing <manish.baing@intel.com> +--- + inc/post_code.hpp | 6 ------ + src/post_code.cpp | 17 ++++++++++++----- + 2 files changed, 12 insertions(+), 11 deletions(-) + +diff --git a/inc/post_code.hpp b/inc/post_code.hpp +index be800f2..3d790b8 100644 +--- a/inc/post_code.hpp ++++ b/inc/post_code.hpp +@@ -18,12 +18,6 @@ + #include <fcntl.h> + #include <unistd.h> + +-#include <cereal/access.hpp> +-#include <cereal/archives/json.hpp> +-#include <cereal/cereal.hpp> +-#include <cereal/types/map.hpp> +-#include <cereal/types/tuple.hpp> +-#include <cereal/types/vector.hpp> + #include <chrono> + #include <filesystem> + #include <fstream> +diff --git a/src/post_code.cpp b/src/post_code.cpp +index 1fcbe55..dfe6ce7 100644 +--- a/src/post_code.cpp ++++ b/src/post_code.cpp +@@ -17,6 +17,13 @@ + + #include "iomanip" + ++#include <cereal/access.hpp> ++#include <cereal/archives/binary.hpp> ++#include <cereal/cereal.hpp> ++#include <cereal/types/map.hpp> ++#include <cereal/types/tuple.hpp> ++#include <cereal/types/vector.hpp> ++ + PostCodeDataHolder* PostCodeDataHolder::instance = 0; + + void PostCode::deleteAll() +@@ -129,18 +136,18 @@ fs::path PostCode::serialize(const std::string& path) + { + fs::path idxPath(path + strCurrentBootCycleIndexName); + std::ofstream osIdx(idxPath.c_str(), std::ios::binary); +- cereal::JSONOutputArchive idxArchive(osIdx); ++ cereal::BinaryOutputArchive idxArchive(osIdx); + idxArchive(currentBootCycleIndex); + + uint16_t count = currentBootCycleCount(); + fs::path cntPath(path + strCurrentBootCycleCountName); + std::ofstream osCnt(cntPath.c_str(), std::ios::binary); +- cereal::JSONOutputArchive cntArchive(osCnt); ++ cereal::BinaryOutputArchive cntArchive(osCnt); + cntArchive(count); + + std::ofstream osPostCodes( + (path + std::to_string(currentBootCycleIndex))); +- cereal::JSONOutputArchive oarchivePostCodes(osPostCodes); ++ cereal::BinaryOutputArchive oarchivePostCodes(osPostCodes); + oarchivePostCodes(postCodes); + } + catch (const cereal::Exception& e) +@@ -163,7 +170,7 @@ bool PostCode::deserialize(const fs::path& path, uint16_t& index) + if (fs::exists(path)) + { + std::ifstream is(path.c_str(), std::ios::in | std::ios::binary); +- cereal::JSONInputArchive iarchive(is); ++ cereal::BinaryInputArchive iarchive(is); + iarchive(index); + return true; + } +@@ -190,7 +197,7 @@ bool PostCode::deserializePostCodes(const fs::path& path, + if (fs::exists(path)) + { + std::ifstream is(path.c_str(), std::ios::in | std::ios::binary); +- cereal::JSONInputArchive iarchive(is); ++ cereal::BinaryInputArchive iarchive(is); + iarchive(codes); + return true; + } +-- +2.17.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager/0002-Max-post-code-file-size-per-cycle-setting.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager/0002-Max-post-code-file-size-per-cycle-setting.patch new file mode 100644 index 000000000..679712d54 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager/0002-Max-post-code-file-size-per-cycle-setting.patch @@ -0,0 +1,63 @@ +From 4415432e32ac8cbc6ec59815a9b9893c2d832c07 Mon Sep 17 00:00:00 2001 +From: Bonnie Lo <Bonnie_Lo@wiwynn.com> +Date: Thu, 27 Oct 2022 17:14:55 +0800 +Subject: [PATCH 2/2] Max post code file size per cycle setting + +Let user could set POST code file size per cycle + +The default size is 512 counts + +Reason: +BMC may crash caused by nonstop saving POST code when BIOS has +some unusual behavior like PXE loop +Thus, BMC should set a limit size to prevent this risk + +Test Case: +Manually send POST code to check the POST code file rotation + +Signed-off-by: Bonnie Lo <Bonnie_Lo@wiwynn.com> +Change-Id: Ic7fbafe532a79123e6ae880a8a3506f9c397d933 +--- + meson.build | 1 + + meson_options.txt | 1 + + src/post_code.cpp | 4 ++++ + 3 files changed, 6 insertions(+) + +diff --git a/meson.build b/meson.build +index 2c44f72..632e07e 100644 +--- a/meson.build ++++ b/meson.build +@@ -16,6 +16,7 @@ conf_data = configuration_data() + conf_data.set_quoted('DBUS_OBJECT_NAME', '/xyz/openbmc_project/State/Boot/PostCode0') + conf_data.set_quoted('DBUS_INTF_NAME','xyz.openbmc_project.State.Boot.PostCode') + conf_data.set('MAX_BOOT_CYCLE_COUNT',get_option('max-boot-cycle-count')) ++conf_data.set('MAX_POST_CODE_SIZE_PER_CYCLE',get_option('max-post-code-size-per-cycle')) + + if get_option('bios-post-code-log').enabled() + add_project_arguments('-DENABLE_BIOS_POST_CODE_LOG',language: 'cpp') +diff --git a/meson_options.txt b/meson_options.txt +index c3d63fd..d877b97 100644 +--- a/meson_options.txt ++++ b/meson_options.txt +@@ -1,2 +1,3 @@ + option('max-boot-cycle-count', type:'integer', min:1, max: 100, description: 'Maximum boot cycles for which the post codes should be persisted', value:100) + option('bios-post-code-log', type:'feature',description:'bios post code log',value:'disabled') ++option('max-post-code-size-per-cycle', type:'integer', min:64, max: 1024, description: 'Maximum post code file size per cycle', value:512) +diff --git a/src/post_code.cpp b/src/post_code.cpp +index dfe6ce7..8411718 100644 +--- a/src/post_code.cpp ++++ b/src/post_code.cpp +@@ -102,6 +102,10 @@ void PostCode::savePostCodes(postcode_t code) + } + + postCodes.insert(std::make_pair(tsUS, code)); ++ if (postCodes.size() > MAX_POST_CODE_SIZE_PER_CYCLE) ++ { ++ postCodes.erase(postCodes.begin()); ++ } + serialize(fs::path(strPostCodeListPath)); + + #ifdef ENABLE_BIOS_POST_CODE_LOG +-- +2.17.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager_git.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager_git.bbappend index f17d24806..3e52f6bde 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager_git.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/state/phosphor-post-code-manager_git.bbappend @@ -1,2 +1,10 @@ +FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:" +PROJECT_SRC_DIR := "${THISDIR}/${PN}" + #SRC_URI = "git://github.com/openbmc/phosphor-post-code-manager.git" SRCREV = "987f91a6536e0330799cc5f4e54740c4023b5ef0" + +SRC_URI += "file://0001-Use-binary-serialization-instead-of-JSON.patch" +SRC_URI += "file://0002-Max-post-code-file-size-per-cycle-setting.patch" + + |