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

FeatureRequest: Async support for send/receive payload #2480

Open
PrithviAPai opened this issue Dec 15, 2023 · 12 comments
Open

FeatureRequest: Async support for send/receive payload #2480

PrithviAPai opened this issue Dec 15, 2023 · 12 comments
Labels
enhancement New feature or request

Comments

@PrithviAPai
Copy link

PrithviAPai commented Dec 15, 2023

Currently in SPDM Requester case libspdm triggers send callback registered when it has payload to send.
Based on return value of the callback the receive callback registered is called subsequently.
In many applications(example: OpenBMC etc) async way of request/response is implemented at transport layer(mctp) where request is sent and once response is received it is handled.
The current implementation of libspdm requires response to be received before returning from send callback.

Can we have async way of handling in SPDM Requester case ?

  1. libspdm will trigger send callback
  2. Once the application receives the response
    application will call for example: process_received_message provided by libspdm to process the response for all the SPDM Request message

Thanks,
Prithvi A Pai

@PrithviAPai PrithviAPai changed the title Async support for send receive payload FeatureRequest: Async support for send receive payload Dec 15, 2023
@PrithviAPai PrithviAPai changed the title FeatureRequest: Async support for send receive payload FeatureRequest: Async support for send/receive payload Dec 15, 2023
@steven-bellock
Copy link
Contributor

See #1049 and #2406.

@PrithviAPai
Copy link
Author

I checked the provided links.
I see the support only for VCA. However, other APIs are still require support is that correct ? what is the plan for supporting all the APIs ?

@jyao1
Copy link
Member

jyao1 commented Dec 17, 2023

We need some one validate and confirm that split is working, before we move too far.

@PrithviAPai , can you help to try the new API?

@PrithviAPai
Copy link
Author

PrithviAPai commented Dec 18, 2023

We need some one validate and confirm that split is working, before we move too far.

@PrithviAPai , can you help to try the new API?

Hi @jyao1,
I did test the APIs provided in the PR.

About my setup:
I have my device acting as SPDM Requester and a i2c simulator acting as SPDM Responder(which internally uses spdm-emu)
The transport used is MCTP over SMBUS.

Findings:

  1. Transport layer encode/decode is NOT supported with the APIs provided. (The integrator has to add/remove Message Type SPMD(5) in case of MCTP).
  2. GET VERSION Requester and Response is obtained ---> Works Fine.
  3. GET CAP Request is sent but Response validation FAILS. The reason is:
    if (spdm_response->header.spdm_version != spdm_request->header.spdm_version) {
        LIBSPDM_DEBUG((LIBSPDM_DEBUG_INFO, "libspdm_process_response_capabilities header request version %02x\n",spdm_request->header.spdm_version)); 
        status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
        goto receive_done;
    }

Prints: 0x00
  1. Once Request packet is formed libspdm_send_spdm_request is called where before adding transport specific header the request is stored in last_message_context. We might need to do that in our case
    /* backup it to last_spdm_request, because the caller wants to compare it with response */
    if (((const spdm_message_header_t *)request)->request_response_code != SPDM_RESPOND_IF_READY
        && ((const spdm_message_header_t *)request)->request_response_code != SPDM_CHUNK_GET
        && ((const spdm_message_header_t*) request)->request_response_code != SPDM_CHUNK_SEND) {
        libspdm_copy_mem (context->last_spdm_request,
                          libspdm_get_scratch_buffer_last_spdm_request_capacity(context),
                          request,
                          request_size
                          );
        context->last_spdm_request_size = request_size;
    }

@PrithviAPai
Copy link
Author

@steven-bellock @jyao1 can this be supported ?
The PR added is NOT helping as transport encoding is not done and Receive part is NOT correctly handled.
BMC is designed to work in asynchronous manner but libspdm requires synchronous approach. Please provide your feedback

@steven-bellock
Copy link
Contributor

I think what you want is a library that completely removes the transport portion. So in this case the Integrator

  1. Calls libspdm to generate a request that is stored in some buffer. This request may be encrypted.
  2. Sends the request to the Responder.
  3. Receives the response from the Responder and puts it in some buffer. This response may be encrypted.
  4. Calls libspdm to parse the Response.

In particular the Integrator has full ownership over 2 and 3. If so then yes, that's doable. @JakubLedworowski does that work for you as well?

@PrithviAPai
Copy link
Author

yes, existing library can work as it is till encode/decode transport messages. Post which we can make sure the integrator takes care of sending and receiving the API.
This helps in allowing integrator send and receive response in async way.

@JakubLedworowski
Copy link

@steven-bellock yes, that approach works for me as long as the integrator has the option to use the transport layer methods directly (eg. libspdm_transport_mctp_encode_message) to avoid code duplication as it is already implemented in libspdm.

So, in case the integrator wants to use the new approach while reusing transport layer the flow would look like this:

  1. Calls libspdm to generate a request that is stored in some buffer. This request may be encrypted.
  2. [Optional] Calls libspdm to encode the request
  3. Sends the request to the Responder.
  4. Receives the response from the Responder and puts it in some buffer. This response may be encrypted.
  5. [Optional] Calls libspdm to decode the response
  6. Calls libspdm to parse the Response.

@PrithviAPai
Copy link
Author

@steven-bellock up to transport layer encoding/decoding can be as we have currently. But, integrator need option to send/receive without having callbacks called for the same.

@PrithviAPai
Copy link
Author

PrithviAPai commented Jan 3, 2024

@steven-bellock can you please tell us some rough timelines when this will be available so that we can plan our deliverables accordingly ?
Thanks a lot for your support and quick reply.

@steven-bellock steven-bellock added the enhancement New feature or request label Jan 4, 2024
@steven-bellock
Copy link
Contributor

If it's at the normal pace of things maybe a year. We are focused on SPDM 1.3 features right now and it's taking a (long) while. That can obviously be accelerated if you, or someone else, implements the feature.

@PrithviAPai
Copy link
Author

PrithviAPai commented Jan 9, 2024

@steven-bellock please suggest if this looks good.
SEND PART

  1. Integrator to acquire SenderBuffer.
  2. Integrator to call libspdm_build_spdm_request
  3. libspdm_build_spdm_request to build spdm request + add transport encoding
  4. Integrator gets the SPDM payload to be sent to spdm endpoint
  5. Integrator to release the SenderBuffer

Questions

  1. In libspdm_handle_large_request we have a looping logic which requires callback to triggered
    How should that be handled when we split the APIs ? Please shed some light

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

No branches or pull requests

4 participants