mctp: MCTPReactor: Replace deferred queue with a state machine

This change isn't significant in its own right. Rather it lays the
ground-work to expand the state machine in a subsequent change to
prevent disruption from late propagation of inventory changes relative
to an endpoint being recovered.

Change-Id: I9bb81de48252ca66465489caad8c63c8e2cdea17
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

Author: amboar

src/mctp/MCTPDeviceRepository.hpp

@@ -82,4 +82,14 @@ class MCTPDeviceRepository
         }
         return entry->second;
     }
+
+    auto begin()
+    {
+        return devices.begin();
+    }
+
+    auto end()
+    {
+        return devices.end();
+    }
 };

src/mctp/MCTPReactor.cpp

@@ -21,7 +21,7 @@ void MCTPReactor::deferSetup(const std::shared_ptr<MCTPDevice>& dev)
     debug("Deferring setup for MCTP device at [ {MCTP_DEVICE} ]", "MCTP_DEVICE",
           dev->describe());
 
-    deferred.emplace(dev);
+    states[dev->id()] = MCTPDeviceState::Unassigned;
 }
 
 void MCTPReactor::untrackEndpoint(const std::shared_ptr<MCTPEndpoint>& ep)
@@ -34,6 +34,7 @@ void MCTPReactor::trackEndpoint(const std::shared_ptr<MCTPEndpoint>& ep)
     info("Added MCTP endpoint to device: [ {MCTP_ENDPOINT} ]", "MCTP_ENDPOINT",
          ep->describe());
 
+    states[ep->device()->id()] = MCTPDeviceState::Assigned;
     ep->subscribe(
         // Degraded
         [](const std::shared_ptr<MCTPEndpoint>& ep) {
@@ -57,6 +58,10 @@ void MCTPReactor::trackEndpoint(const std::shared_ptr<MCTPEndpoint>& ep)
                 {
                     self->deferSetup(ep->device());
                 }
+                else
+                {
+                    self->states.erase(ep->device()->id());
+                }
             }
             else
             {
@@ -136,10 +141,12 @@ void MCTPReactor::setupEndpoint(const std::shared_ptr<MCTPDevice>& dev)
 
 void MCTPReactor::tick()
 {
-    auto toSetup = std::exchange(deferred, {});
-    for (const auto& entry : toSetup)
+    for (const auto& entry : devices)
     {
-        setupEndpoint(entry);
+        if (states[entry.second->id()] == MCTPDeviceState::Unassigned)
+        {
+            setupEndpoint(entry.second);
+        }
     }
 }
 
@@ -153,6 +160,7 @@ void MCTPReactor::manageMCTPDevice(const std::string& path,
 
     try
     {
+        states[device->id()] = MCTPDeviceState::Unmanaged;
         devices.add(path, device);
         debug("MCTP device inventory added at '{INVENTORY_PATH}'",
               "INVENTORY_PATH", path);
@@ -202,8 +210,6 @@ void MCTPReactor::unmanageMCTPDevice(const std::string& path)
     debug("MCTP device inventory removed at '{INVENTORY_PATH}'",
           "INVENTORY_PATH", path);
 
-    deferred.erase(device);
-
     // Remove the device from the repository before notifying the device itself
     // of removal so we don't defer its setup
     devices.remove(device);

src/mctp/MCTPReactor.hpp

@@ -8,7 +8,6 @@
 #include <functional>
 #include <memory>
 #include <optional>
-#include <set>
 #include <string>
 #include <vector>
 
@@ -21,6 +20,13 @@ struct AssociationServer
     virtual void disassociate(const std::string& path) = 0;
 };
 
+enum class MCTPDeviceState
+{
+    Unmanaged,
+    Assigned,
+    Unassigned,
+};
+
 class MCTPReactor : public std::enable_shared_from_this<MCTPReactor>
 {
     using MCTPDeviceFactory = std::function<std::shared_ptr<MCTPDevice>(
@@ -47,9 +53,7 @@ class MCTPReactor : public std::enable_shared_from_this<MCTPReactor>
 
     AssociationServer& server;
     MCTPDeviceRepository devices;
-
-    // Tracks MCTP devices that have failed their setup
-    std::set<std::shared_ptr<MCTPDevice>> deferred;
+    std::map<std::size_t, MCTPDeviceState> states;
 
     void deferSetup(const std::shared_ptr<MCTPDevice>& dev);
     void setupEndpoint(const std::shared_ptr<MCTPDevice>& dev);

src/tests/test_MCTPReactor.cpp

@@ -64,6 +64,7 @@ class MCTPReactorFixture : public testing::Test
         device = std::make_shared<MockMCTPDevice>();
         EXPECT_CALL(*device, describe())
             .WillRepeatedly(testing::Return("mock device"));
+        EXPECT_CALL(*device, id()).WillRepeatedly(testing::Return(0UL));
 
         endpoint = std::make_shared<MockMCTPEndpoint>();
         EXPECT_CALL(*endpoint, device())
@@ -233,6 +234,7 @@ TEST(MCTPReactor, replaceConfiguration)
     auto idev = std::make_shared<MockMCTPDevice>();
     EXPECT_CALL(*idev, describe())
         .WillRepeatedly(testing::Return("mock device: initial"));
+    EXPECT_CALL(*idev, id()).WillRepeatedly(testing::Return(0UL));
     EXPECT_CALL(*idev, setup(testing::_))
         .WillOnce(testing::InvokeArgument<0>(std::error_code(), iep));
     EXPECT_CALL(*idev, remove()).WillOnce(testing::Invoke([&]() {
@@ -255,6 +257,7 @@ TEST(MCTPReactor, replaceConfiguration)
     auto rdev = std::make_shared<MockMCTPDevice>();
     EXPECT_CALL(*rdev, describe())
         .WillRepeatedly(testing::Return("mock device: replacement"));
+    EXPECT_CALL(*rdev, id()).WillRepeatedly(testing::Return(0UL));
     EXPECT_CALL(*rdev, setup(testing::_))
         .WillOnce(testing::InvokeArgument<0>(std::error_code(), rep));
     EXPECT_CALL(*rdev, remove()).WillOnce(testing::Invoke([&]() {
@@ -295,6 +298,7 @@ TEST(MCTPReactor, concurrentEndpointSetupReactorTeardown)
 
         EXPECT_CALL(*device, describe())
             .WillRepeatedly(testing::Return("mock device"));
+        EXPECT_CALL(*device, id()).WillRepeatedly(testing::Return(0UL));
         EXPECT_CALL(*device, setup(testing::_))
             .WillOnce(testing::SaveArg<0>(&setupHandler));
 
@@ -328,6 +332,7 @@ TEST(MCTPReactor, manageMockDeviceDelayedSetup)
 
             EXPECT_CALL(*device, describe())
                 .WillRepeatedly(testing::Return("mock device"));
+            EXPECT_CALL(*device, id()).WillRepeatedly(testing::Return(0UL));
             EXPECT_CALL(*device, remove())
                 .WillOnce(testing::Invoke([ep{endpoint}]() { ep->remove(); }));
             EXPECT_CALL(*device, setup(testing::_))