-
Notifications
You must be signed in to change notification settings - Fork 31
mctpd: remove_peer if get endpoint id fails #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
mctpd: remove_peer if get endpoint id fails #114
Conversation
While the endpoint should respond to the Get MCTP Message Type Support command (the spec lists it as mandatory), we don't consider this a fatal error for enumeration in general. The spec does call out Get Endpoint ID as the preferred mechanism for liveness detection - we use this in various parts of the recovery process, for example. If we want a liveness check included in the Network1/LearnEndpoint call, I think we should introduce a Get Endpoint ID command here. |
If the bridged endpoint doesn't respond to "Get MCTP Message Type Support command", application layer daemons have no "official" way to know what messages(nvme-mi, pldm or spdm...) this endpoints supports. From this point of view, it doesn't have much value to expose a endpoint which we don't know which messages it supports. Please let me know if you still prefer "Get Endpoint ID" as indication of live scan, I can update the pull request. |
BTW, the original code just uses |
That's true, but there are some use-cases that do not consume the type data for endpoint discovery (ie, they use platform-specific mechanisms for endpoint enumeration). That's not really a primary concern in general, but we do still need to support it. The presence of the endpoint on dbus should reflect whether or not the endpoint exists; the presence of types data in the SupportedMessageTypes property should reflect whether we have type data for that endpoint. I'd prefer to keep those concepts decoupled if possible. (the disadvantage is that we have to issue an extra command in this process, but I think that's justified here) |
58936b7
to
adaf18c
Compare
Updated to use query_get_endpoint_id as liveness detection. |
I have just pushed some fixes for CI, which is likely causing the failures seen above. Are you able to rebase and push? |
method_net_learn_endpoint shall only keep the peer which is alive (responds to the query_get_endpoint_id). Tested: busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/networks/1 au.com.codeconstruct.MCTP.Network1 LearnEndpoint y 8 sb "/au/com/codeconstruct/mctp1/networks/1/endpoints/8" false busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/networks/1 au.com.codeconstruct.MCTP.Network1 LearnEndpoint y 9 Call failed: MCTP Endpoint did not respond busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/networks/1 au.com.codeconstruct.MCTP.Network1 LearnEndpoint y 10 sb "/au/com/codeconstruct/mctp1/networks/1/endpoints/10" true busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/networks/1 au.com.codeconstruct.MCTP.Network1 LearnEndpoint y 15 Call failed: Request failed Signed-off-by: Jinliang Wang <[email protected]>
adaf18c
to
85b5efb
Compare
method_net_learn_endpoint shall only keep the peer which is alive (responds to the query_get_peer_msgtypes).
Tested: