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

Issue in SPDM_RESPOND_IF_READY handling #2507

Open
EddyHanAMI opened this issue Jan 5, 2024 · 6 comments
Open

Issue in SPDM_RESPOND_IF_READY handling #2507

EddyHanAMI opened this issue Jan 5, 2024 · 6 comments
Assignees

Comments

@EddyHanAMI
Copy link

When checking the SPDM functionality using an nVidia CX7 network card,
When executing the challenge and get_measurement commands, I found that the SPDM_RESPOND_IF_READY command always fails.

After checking the source code, I found that the execution process only waits for WT us and does not take WTMax into account.
There was also no SPDM_RESPOND_IF_READY retry.
I think that not every device is in a perfect state like spdm-emu.
The implementation should be improved for further applications.
Any idea about this?

Fig. 1
image
Fig. 2
image

@steven-bellock
Copy link
Contributor

WTMax is a warning to the Requester that the Responder may drop the response if the Requester does not retrieve it by that time. So long as the Requester's response retrieval is WT < ResponseRetrievalTime < WTmax then it is good.

RDTExponent is defined as

Shall be the exponent expressed in logarithmic (base-2 scale) to calculate RDT time in µs after which the Responder can provide successful completion response.

and WT == RDT == 2^RDTExponent, so if the Responder does not have the response within WT/RDT then it has a bug. In that respect I think libspdm is fine.

However, maybe after a request is sent libspdm could wait some amount of time (possibly 0, which is what we do today) before retrieving the response? I think most Responders will block at the transport layer before giving the response. However in your case it seems like the Responder can be pre-empted to return ResponseNotReady error. In that case libspdm and the transport layer should not be pinging/polling constantly.

@EddyHanAMI
Copy link
Author

Thank you for your reply.
Please check the attached log data.
Can I say that the FW implementation of the device is incorrect?
The WT was not sufficient to prepare a response.
I increased the WT to 2^16 and the device returned the required response.
From libspdm's view, when libspdm received SPDM_ERROR(0x7f) again, it didn't wait once more.
Should libspdm handle this situation?

RequesterNonce - 81 10 9f b6 6e b4 ff 42 fb 56 ed a1 f3 2c cd 68 3d 85 4c a3 fa a2 57 a7 41 25 5d 6c 4a ff f5 76
libspdm_send_spdm_request[0] msg SPDM_CHALLENGE(0x83), size (0x24):
0000: 11 83 00 ff 81 10 9f b6 6e b4 ff 42 fb 56 ed a1 f3 2c cd 68 3d 85 4c a3 fa a2 57 a7 41 25 5d 6c
0020: 4a ff f5 76
libspdm_receive_spdm_response[0] msg SPDM_ERROR(0x7f), size (0x8):
0000: 11 7f 42 00 08 83 64 0c
libspdm_send_spdm_request[0] msg SPDM_RESPOND_IF_READY(0xff), size (0x4):
0000: 11 ff 83 64
libspdm_receive_spdm_response[0] msg SPDM_ERROR(0x7f), size (0x8):
0000: 11 7f 42 00 08 83 64 0c

@steven-bellock
Copy link
Contributor

steven-bellock commented Jan 9, 2024

What OS is the Requester running on? https://github.com/DMTF/libspdm/tree/main/os_stub/platform_lib has the reference sleep functions but I don't know if they've ever been tested.

RDTExponent == 8 is pretty small. So yes, if the Requester is waiting 256 us before sending RESPOND_IF_READY and the Responder does not have the message ready, then the Responder has a bug.

Should libspdm handle this situation?

Like send more RESPOND_IF_READYs? I don't think so, but we could allow the Integrator to configure that. We already allow the Integrator to specify how many retries are used when the Responder is BUSY.

@EddyHanAMI
Copy link
Author

We are using Linux.
Please share where to config retry time.
Thanks.

@steven-bellock
Copy link
Contributor

Please share where to config retry time.

LIBSPDM_DATA_REQUEST_RETRY_TIMES and LIBSPDM_DATA_REQUEST_RETRY_DELAY_TIME but that's only for Busy error, not ResponseNotReady.

@EddyHanAMI
Copy link
Author

Update codes in https://github.com/DMTF/libspdm/blob/main/library/spdm_requester_lib/libspdm_req_handle_error_response.c#L186
to following changes for resolving most issues I'm facing.

    size_t retry;
    libspdm_return_t status;
    retry = extend_error_data->rd_tm;
    do{
       
        libspdm_sleep((uint64_t)1 << (extend_error_data->rd_exponent ));
   
        status = libspdm_requester_respond_if_ready(spdm_context, session_id,
                                                 response_size, response,
                                                 expected_response_code);
        if(status == LIBSPDM_STATUS_SUCCESS)
            break;
        if(retry > 0) retry--;
    }while(retry > 0 );
    return status;

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