Skip to content
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

libspdm_device_send_message_func should be called with the timeout parameter #2904

Open
lordaule opened this issue Nov 19, 2024 · 4 comments

Comments

@lordaule
Copy link

Some BMC MCTP implementations are atomic, with send and receive as a single "transaction".

libspdm libspdm_register_device_io_func() splits these into two operations, each of which has a timeout parameter. However, the timeout provided to the send libspdm_device_send_message_func is only the RTT time, whereas the timeout provided to the receive libspdm_device_receive_message_func is the full calculated CT exponent time.

Instead, the full RTT+(ST1 or CTexp) time should be provided to the libspdm_device_send_message_func. For MCTP implementations which split send and receive, this should have no effect, as sends do not even use timeouts.

@steven-bellock
Copy link
Contributor

LIBSPDM_DATA_CAPABILITY_RTT_US needs to be documented in https://github.com/DMTF/libspdm/blob/main/doc/api/common_api.md.

For RTT there appears to be a dependency on SPDM version. In "old" SPDM RTT is

This is the worst case round trip transport timing.
The max value shall be the worst case total time for the complete transmission and
delivery of an SPDM message round trip at the transport layer(s). The actual value for
this parameter is transport/media specific.

whereas in "new" SPDM RTT is

This value shall be the worst-case round-trip transport timing.
The value shall be the worst-case total time for the complete transmission and delivery of an SPDM message round trip at the transport layer(s). The actual value for this parameter is transport- or media-specific. Both the actual value and how an endpoint obtains this value are outside the scope of this specification. A Requester shall measure this timing parameter from the end of a successful transmission of an SPDM request to the beginning of the reception of the corresponding SPDM response less ST1 or CT, depending on the Request.

I filed https://github.com/DMTF/SPDM-WG/issues/3726 to perhaps clarify the new text.

Regardless of the interpretation, libspdm sending RTT to libspdm_device_send_message_func doesn't make sense. For "old" SPDM it looks like it should be RTT / 2. For "new" SPDM RTT begins after the request has been sent and so it does not come into play.

@lordaule for your use case, without any modification to libspdm, libspdm_device_send_message_func can query LIBSPDM_DATA_CAPABILITY_CT_EXPONENT from the SPDM context and presumably assume worst-case timeout from that.

@lordaule
Copy link
Author

libspdm_device_send_message_func can query LIBSPDM_DATA_CAPABILITY_CT_EXPONENT

But that wouldn't actually be correct, because CT_EXPONENT is supposed to only apply to certain transactions, specifically when context->crypto_request true. But external to libspdm, spdm_context is a (void *) and there is no knowledge of whether the request being sent is a crypto request or not.

When sending messages there is actually no such thing as a timeout - timeouts are only for measuring responses. But the API for send includes a timeout, presumably to enable just this type of atomic transaction model - except for the fact that the timeout provided to to context->send_message() differ from the timeout provided to context->receive_message()

@steven-bellock
Copy link
Contributor

When sending messages there is actually no such thing as a timeout - timeouts are only for measuring responses.

There are transports where the receiver acknowledges to the sender that it has received the message. For example PCIE non-posted writes. In such a case the sender can enforce a timeout on just sending the message. But RTT isn't the "right" value for that timeout.

But that wouldn't actually be correct, because CT_EXPONENT is supposed to only apply to certain transactions,

Pragmatically speaking if the value of CT is 1 s, can't you use that as a timeout for all messages? Even more pragmatically speaking, can't you use a fixed timeout value regardless of the value of CT? For example, if the value of CT is 10 minutes the BMC probably won't wait 10 minutes for a reply. It'll wait like, 5 seconds at most.

Maybe libspdm_device_send_message_func and libspdm_device_acquire_sender_buffer_func can include a new parameter that specifies the type of message being sent / received. Then the functions can use that information, as well as CT and ST1 and RTT to figure things out if it wants.

@steven-bellock
Copy link
Contributor

@raghuncstate is doing BMC stuff as well. Maybe he can weigh in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants