Add mctpreactor for dynamic configuration of MCTP networks
While mctpd[1] may see heavy use in projects such as OpenBMC, it
implements generic functionality necessary to operate MCTP as a
protocol. It therefore should be easy to use in other contexts, and so
it feels unwise to embed OpenBMC-specific details in its implementation.
Conversely, entity-manager's scope is to expose inventory and board
configuration. It externalises all other responsibilities for the sake
of stability and maintenance. While entity-manager is central to
OpenBMC's implementation and has little use in other contexts, embedding
details of how to configure mctpd in entity-manager exceeds its scope.
Thus we reach the design point of mctpreactor, an intermediary process
that encapsulates OpenBMC-specific and mctpd-specific behaviors to
constrain their dispersion in either direction. The design-point was
reached via discussion at [2].
mctpreactor tracks instances of transport-specific MCTP device
configurations[3] appearing as a result of inventory changes, and uses
them to assign endpoint IDs via mctpd.
The lifecycle of an MCTP device can be quite dynamic - mctpd provides
behaviors to recover[4] or remove endpoints from the network. Their
presence cannot be assumed. mctpreactor handles these events: If
a device is removed at the MCTP layer (as it may be unresponsive),
mctpreactor will periodically attempt to re-establish it as an endpoint
so long as the associated configuration on the entity-manager inventory
object remains exposed.
[1]: https://github.com/CodeConstruct/mctp/
[2]: https://github.com/CodeConstruct/mctp/pull/17
[3]: https://gerrit.openbmc.org/c/openbmc/entity-manager/+/70628
[4]: https://github.com/CodeConstruct/mctp/blob/7ec2f8daa3a8948066390aee621d6afa03f6ecd9/docs/endpoint-recovery.md
Change-Id: I5e362cf6e5ce80ce282bab48d912a1038003e236
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/src/tests/meson.build b/src/tests/meson.build
index 0415e96..c477843 100644
--- a/src/tests/meson.build
+++ b/src/tests/meson.build
@@ -56,3 +56,28 @@
include_directories: src_inc,
),
)
+
+test(
+ 'MCTPReactor',
+ executable(
+ 'test_MCTPReactor',
+ 'test_MCTPReactor.cpp',
+ '../mctp/MCTPReactor.cpp',
+ '../mctp/MCTPEndpoint.cpp',
+ dependencies: [ gmock_dep, ut_deps_list, utils_dep ],
+ implicit_include_directories: false,
+ include_directories: '../mctp'
+ )
+)
+
+test(
+ 'MCTPEndpoint',
+ executable(
+ 'test_MCTPEndpoint',
+ 'test_MCTPEndpoint.cpp',
+ '../mctp/MCTPEndpoint.cpp',
+ dependencies: [ gmock_dep, ut_deps_list, utils_dep ],
+ implicit_include_directories: false,
+ include_directories: '../mctp'
+ )
+)
diff --git a/src/tests/test_MCTPEndpoint.cpp b/src/tests/test_MCTPEndpoint.cpp
new file mode 100644
index 0000000..36b7d3c
--- /dev/null
+++ b/src/tests/test_MCTPEndpoint.cpp
@@ -0,0 +1,88 @@
+#include "MCTPEndpoint.hpp"
+#include "Utils.hpp"
+
+#include <stdexcept>
+
+#include <gtest/gtest.h>
+
+TEST(I2CMCTPDDevice, matchEmptyConfig)
+{
+ SensorData config{};
+ EXPECT_FALSE(I2CMCTPDDevice::match(config));
+}
+
+TEST(I2CMCTPDDevice, matchIrrelevantConfig)
+{
+ SensorData config{{"xyz.openbmc_project.Configuration.NVME1000", {}}};
+ EXPECT_FALSE(I2CMCTPDDevice::match(config));
+}
+
+TEST(I2CMCTPDDevice, matchRelevantConfig)
+{
+ SensorData config{{"xyz.openbmc_project.Configuration.MCTPI2CTarget", {}}};
+ EXPECT_TRUE(I2CMCTPDDevice::match(config));
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceNoType)
+{
+ SensorBaseConfigMap iface{{}};
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceWrongType)
+{
+ SensorBaseConfigMap iface{{"Type", "NVME1000"}};
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceNoAddress)
+{
+ SensorBaseConfigMap iface{
+ {"Bus", "0"},
+ {"Name", "test"},
+ {"Type", "MCTPI2CTarget"},
+ };
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceBadAddress)
+{
+ SensorBaseConfigMap iface{
+ {"Address", "not a number"},
+ {"Bus", "0"},
+ {"Name", "test"},
+ {"Type", "MCTPI2CTarget"},
+ };
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceNoBus)
+{
+ SensorBaseConfigMap iface{
+ {"Address", "0x1d"},
+ {"Name", "test"},
+ {"Type", "MCTPI2CTarget"},
+ };
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceBadBus)
+{
+ SensorBaseConfigMap iface{
+ {"Address", "0x1d"},
+ {"Bus", "not a number"},
+ {"Name", "test"},
+ {"Type", "MCTPI2CTarget"},
+ };
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
+
+TEST(I2CMCTPDDevice, fromBadIfaceNoName)
+{
+ SensorBaseConfigMap iface{
+ {"Address", "0x1d"},
+ {"Bus", "0"},
+ {"Type", "MCTPI2CTarget"},
+ };
+ EXPECT_THROW(I2CMCTPDDevice::from({}, iface), std::invalid_argument);
+}
diff --git a/src/tests/test_MCTPReactor.cpp b/src/tests/test_MCTPReactor.cpp
new file mode 100644
index 0000000..abdd9e9
--- /dev/null
+++ b/src/tests/test_MCTPReactor.cpp
@@ -0,0 +1,250 @@
+#include "MCTPEndpoint.hpp"
+#include "MCTPReactor.hpp"
+#include "Utils.hpp"
+
+#include <cstdint>
+#include <functional>
+#include <memory>
+#include <string>
+#include <system_error>
+#include <vector>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+class MockMCTPDevice : public MCTPDevice
+{
+ public:
+ ~MockMCTPDevice() override = default;
+
+ MOCK_METHOD(void, setup,
+ (std::function<void(const std::error_code& ec,
+ const std::shared_ptr<MCTPEndpoint>& ep)> &&
+ added),
+ (override));
+ MOCK_METHOD(void, remove, (), (override));
+ MOCK_METHOD(std::string, describe, (), (const, override));
+};
+
+class MockMCTPEndpoint : public MCTPEndpoint
+{
+ public:
+ ~MockMCTPEndpoint() override = default;
+
+ MOCK_METHOD(int, network, (), (const, override));
+ MOCK_METHOD(uint8_t, eid, (), (const, override));
+ MOCK_METHOD(void, subscribe,
+ (Event && degraded, Event&& available, Event&& removed),
+ (override));
+ MOCK_METHOD(void, remove, (), (override));
+ MOCK_METHOD(std::string, describe, (), (const, override));
+ MOCK_METHOD(std::shared_ptr<MCTPDevice>, device, (), (const, override));
+};
+
+class MockAssociationServer : public AssociationServer
+{
+ public:
+ ~MockAssociationServer() override = default;
+
+ MOCK_METHOD(void, associate,
+ (const std::string& path,
+ const std::vector<Association>& associations),
+ (override));
+ MOCK_METHOD(void, disassociate, (const std::string& path), (override));
+};
+
+class MCTPReactorFixture : public testing::Test
+{
+ protected:
+ void SetUp() override
+ {
+ reactor = std::make_shared<MCTPReactor>(assoc);
+ device = std::make_shared<MockMCTPDevice>();
+ EXPECT_CALL(*device, describe())
+ .WillRepeatedly(testing::Return("mock device"));
+
+ endpoint = std::make_shared<MockMCTPEndpoint>();
+ EXPECT_CALL(*endpoint, device())
+ .WillRepeatedly(testing::Return(device));
+ EXPECT_CALL(*endpoint, describe())
+ .WillRepeatedly(testing::Return("mock endpoint"));
+ EXPECT_CALL(*endpoint, eid()).WillRepeatedly(testing::Return(9));
+ EXPECT_CALL(*endpoint, network()).WillRepeatedly(testing::Return(1));
+ }
+
+ void TearDown() override
+ {
+ // https://stackoverflow.com/a/10289205
+ EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(endpoint.get()));
+ EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(device.get()));
+ }
+
+ MockAssociationServer assoc;
+ std::shared_ptr<MCTPReactor> reactor;
+ std::shared_ptr<MockMCTPDevice> device;
+ std::shared_ptr<MockMCTPEndpoint> endpoint;
+};
+
+TEST_F(MCTPReactorFixture, manageNullDevice)
+{
+ reactor->manageMCTPDevice("/test", {});
+ reactor->unmanageMCTPDevice("/test");
+}
+
+TEST_F(MCTPReactorFixture, manageMockDeviceSetupFailure)
+{
+ EXPECT_CALL(*device, remove());
+ EXPECT_CALL(*device, setup(testing::_))
+ .WillOnce(testing::InvokeArgument<0>(
+ std::make_error_code(std::errc::permission_denied), endpoint));
+
+ reactor->manageMCTPDevice("/test", device);
+ reactor->unmanageMCTPDevice("/test");
+}
+
+TEST_F(MCTPReactorFixture, manageMockDevice)
+{
+ std::function<void(const std::shared_ptr<MCTPEndpoint>& ep)> removeHandler;
+
+ std::vector<Association> requiredAssociation{
+ {"configured_by", "configures", "/test"}};
+ EXPECT_CALL(assoc, associate("/xyz/openbmc_project/mctp/1/9",
+ requiredAssociation));
+ EXPECT_CALL(assoc, disassociate("/xyz/openbmc_project/mctp/1/9"));
+
+ EXPECT_CALL(*endpoint, remove()).WillOnce(testing::Invoke([&]() {
+ removeHandler(endpoint);
+ }));
+ EXPECT_CALL(*endpoint, subscribe(testing::_, testing::_, testing::_))
+ .WillOnce(testing::SaveArg<2>(&removeHandler));
+
+ EXPECT_CALL(*device, remove()).WillOnce(testing::Invoke([&]() {
+ endpoint->remove();
+ }));
+ EXPECT_CALL(*device, setup(testing::_))
+ .WillOnce(testing::InvokeArgument<0>(std::error_code(), endpoint));
+
+ reactor->manageMCTPDevice("/test", device);
+ reactor->unmanageMCTPDevice("/test");
+}
+
+TEST_F(MCTPReactorFixture, manageMockDeviceDeferredSetup)
+{
+ std::function<void(const std::shared_ptr<MCTPEndpoint>& ep)> removeHandler;
+
+ std::vector<Association> requiredAssociation{
+ {"configured_by", "configures", "/test"}};
+ EXPECT_CALL(assoc, associate("/xyz/openbmc_project/mctp/1/9",
+ requiredAssociation));
+ EXPECT_CALL(assoc, disassociate("/xyz/openbmc_project/mctp/1/9"));
+
+ EXPECT_CALL(*endpoint, remove()).WillOnce(testing::Invoke([&]() {
+ removeHandler(endpoint);
+ }));
+ EXPECT_CALL(*endpoint, subscribe(testing::_, testing::_, testing::_))
+ .WillOnce(testing::SaveArg<2>(&removeHandler));
+
+ EXPECT_CALL(*device, remove()).WillOnce(testing::Invoke([&]() {
+ endpoint->remove();
+ }));
+ EXPECT_CALL(*device, setup(testing::_))
+ .WillOnce(testing::InvokeArgument<0>(
+ std::make_error_code(std::errc::permission_denied), endpoint))
+ .WillOnce(testing::InvokeArgument<0>(std::error_code(), endpoint));
+
+ reactor->manageMCTPDevice("/test", device);
+ reactor->tick();
+ reactor->unmanageMCTPDevice("/test");
+}
+
+TEST_F(MCTPReactorFixture, manageMockDeviceRemoved)
+{
+ std::function<void(const std::shared_ptr<MCTPEndpoint>& ep)> removeHandler;
+
+ std::vector<Association> requiredAssociation{
+ {"configured_by", "configures", "/test"}};
+ EXPECT_CALL(assoc,
+ associate("/xyz/openbmc_project/mctp/1/9", requiredAssociation))
+ .Times(2);
+ EXPECT_CALL(assoc, disassociate("/xyz/openbmc_project/mctp/1/9")).Times(2);
+
+ EXPECT_CALL(*endpoint, remove()).WillOnce(testing::Invoke([&]() {
+ removeHandler(endpoint);
+ }));
+ EXPECT_CALL(*endpoint, subscribe(testing::_, testing::_, testing::_))
+ .Times(2)
+ .WillRepeatedly(testing::SaveArg<2>(&removeHandler));
+
+ EXPECT_CALL(*device, remove()).WillOnce(testing::Invoke([&]() {
+ endpoint->remove();
+ }));
+ EXPECT_CALL(*device, setup(testing::_))
+ .Times(2)
+ .WillRepeatedly(
+ testing::InvokeArgument<0>(std::error_code(), endpoint));
+
+ reactor->manageMCTPDevice("/test", device);
+ removeHandler(endpoint);
+ reactor->tick();
+ reactor->unmanageMCTPDevice("/test");
+}
+
+TEST(MCTPReactor, replaceConfiguration)
+{
+ MockAssociationServer assoc{};
+ auto reactor = std::make_shared<MCTPReactor>(assoc);
+ std::function<void(const std::shared_ptr<MCTPEndpoint>& ep)> removeHandler;
+
+ std::vector<Association> requiredAssociation{
+ {"configured_by", "configures", "/test"}};
+
+ EXPECT_CALL(assoc,
+ associate("/xyz/openbmc_project/mctp/1/9", requiredAssociation))
+ .Times(2);
+ EXPECT_CALL(assoc, disassociate("/xyz/openbmc_project/mctp/1/9")).Times(2);
+
+ auto endpoint = std::make_shared<MockMCTPEndpoint>();
+ EXPECT_CALL(*endpoint, describe())
+ .WillRepeatedly(testing::Return("mock endpoint"));
+ EXPECT_CALL(*endpoint, eid()).WillRepeatedly(testing::Return(9));
+ EXPECT_CALL(*endpoint, network()).WillRepeatedly(testing::Return(1));
+ EXPECT_CALL(*endpoint, remove())
+ .Times(2)
+ .WillRepeatedly(testing::Invoke([&]() { removeHandler(endpoint); }));
+ EXPECT_CALL(*endpoint, subscribe(testing::_, testing::_, testing::_))
+ .Times(2)
+ .WillRepeatedly(testing::SaveArg<2>(&removeHandler));
+
+ auto initial = std::make_shared<MockMCTPDevice>();
+ EXPECT_CALL(*initial, describe())
+ .WillRepeatedly(testing::Return("mock device: initial"));
+ EXPECT_CALL(*initial, setup(testing::_))
+ .WillOnce(testing::InvokeArgument<0>(std::error_code(), endpoint));
+ EXPECT_CALL(*initial, remove()).WillOnce(testing::Invoke([&]() {
+ endpoint->remove();
+ }));
+
+ auto replacement = std::make_shared<MockMCTPDevice>();
+ EXPECT_CALL(*replacement, describe())
+ .WillRepeatedly(testing::Return("mock device: replacement"));
+ EXPECT_CALL(*replacement, setup(testing::_))
+ .WillOnce(testing::InvokeArgument<0>(std::error_code(), endpoint));
+ EXPECT_CALL(*replacement, remove()).WillOnce(testing::Invoke([&]() {
+ endpoint->remove();
+ }));
+
+ EXPECT_CALL(*endpoint, device())
+ .WillOnce(testing::Return(initial))
+ .WillOnce(testing::Return(initial))
+ .WillOnce(testing::Return(replacement))
+ .WillOnce(testing::Return(replacement));
+
+ reactor->manageMCTPDevice("/test", initial);
+ reactor->manageMCTPDevice("/test", replacement);
+ reactor->tick();
+ reactor->unmanageMCTPDevice("/test");
+
+ EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(initial.get()));
+ EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(replacement.get()));
+ EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(endpoint.get()));
+}