mctpd: add AssignEndpointPreferred for preferred EID#158
Conversation
a8724a9 to
65b1357
Compare
Add AssignEndpointPreferred to allow callers to request a preferred EID for an endpoint. mctpd checks whether the requested EID is available before assigning it to the device. If the preferred EID is already in use, mctpd falls back to dynamic EID assignment. This is used by the MCTP reactor flow, where dbus-sensors reads the MCTPEndpointID property from Entity Manager and passes it to mctpd as the preferred EID. Update the README, mctpd documentation, endpoint recovery documentation, and tests for the preferred EID assignment flow. Related: https://gerrit.openbmc.org/c/openbmc/entity-manager/+/91151 https://gerrit.openbmc.org/c/openbmc/dbus-sensors/+/91188 Signed-off-by: Henry Wu <Henry_Wu@quantatw.com>
65b1357 to
c7964ff
Compare
You've explained when this might be used, but not why, and I'm not clear on the purpose here. To me, this sounds like a source of subtle bugs: if the client is assuming that the preferred EID is assigned, but in some rare cases is not, then you're unlikely to catch that in regular testing. If it's acceptable that the preferred EID is not assigned, then you could just use dynamic allocations anyway. If we end up having a use for an allocation hint, I would suggest not adding another permutation of AssignEndpoint calls. This would be much better done with a flag. |
|
The reason I initially added AssignEndpointPreferred was to keep the existing AssignEndpoint and AssignEndpointStatic semantics unchanged, while allowing the caller to pass the Entity Manager configured EID to mctpd. I also wanted the fallback decision to stay inside mctpd, since mctpd has the authoritative view of assigned EIDs and bridge pools. That said, I agree with your concern. A preferred EID can be misleading if the caller assumes that the requested EID will actually be assigned, and adding another AssignEndpoint permutation is not ideal. For our use case, the configured EID is intended to provide deterministic EID assignment for known devices. If that EID is unavailable, the fallback to dynamic assignment was meant to keep the endpoint usable rather than failing enumeration. However, I agree that this policy should be explicit in the API rather than hidden behind another method name. I’ll take another look at whether this should be modeled as a strict static assignment or as an explicit preferred-with-fallback policy. In either case, I agree the API should make the policy explicit and avoid implying that the preferred EID is guaranteed. |
That's the problem though, it's no longer deterministic, in that the allocation now depends on prior state. And this still doesn't explain why you want preferred EIDs. |
Add AssignEndpointPreferred to allow MCTP reactor to request a preferred EID for an endpoint.
mctpd checks whether the requested EID is available before assigning it to the device. The policy for choosing preferred EID assignment or dynamic EID assignment is handled by MCTP reactor.
This is used with the MCTP reactor flow that reads the MCTPEndpointID property from Entity Manager and decides whether to request the configured EID or fall back to dynamic EID assignment.
Update the README, mctpd documentation, endpoint recovery documentation, and unit tests for the preferred EID assignment flow.
Related:
https://gerrit.openbmc.org/c/openbmc/entity-manager/+/91151
https://gerrit.openbmc.org/c/openbmc/dbus-sensors/+/91188