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_MAX_MESSAGE_VCA_BUFFER_SIZE seems to need larger size. #2302

Open
IamSukwonKim opened this issue Aug 3, 2023 · 6 comments
Open

LIBSPDM_MAX_MESSAGE_VCA_BUFFER_SIZE seems to need larger size. #2302

IamSukwonKim opened this issue Aug 3, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@IamSukwonKim
Copy link

LIBSPDM_MAX_MESSAGE_VCA_BUFFER_SIZE is defined based on a below statement.

Length of the entire request message, in bytes. Length shall be less than or equal to 128 bytes. - Paragraph 252, Table 15 - NEGOTIATE_ALGORITHMS request message format, DSP0274_1.2.1

/*
* +--------------------------+------------------------------------------+---------+
* | GET_VERSION | 4 | 1 |
* | VERSION {1.0, 1.1, 1.2} | 6 + 2 * 3 = 12 | 1 |
* +--------------------------+------------------------------------------+---------+
* | GET_CAPABILITIES 1.2 | 20 | 1 |
* | CAPABILITIES 1.2 | 20 | 1 |
* +--------------------------+------------------------------------------+---------+
* | NEGOTIATE_ALGORITHMS 1.2 | 32 + 4 * 4 = 48 | 2 |
* | ALGORITHMS 1.2 | 36 + 4 * 4 = 52 | 2 |
* +--------------------------+------------------------------------------+---------+
*/
#define LIBSPDM_MAX_MESSAGE_VCA_BUFFER_SIZE (150 + 2 * LIBSPDM_MAX_VERSION_COUNT)

This value assumes that there are no ExtAsymCount and no ExtHashCount. However, in point of responder's view, It doesn't know how long the NEGOTIATION_ALGORITHMS will be until receive it. So, It would be better to allocate additional 80 bytes to reserve Maximum length, 128 bytes.

And also, It can be required additional space for fields which include ExtAsymSel, ExtHashSel and each AlgExternal for ALGORITHMS. However, we can remain these for configurable value, in my opinion.

@jyao1
Copy link
Member

jyao1 commented Aug 3, 2023

@IamSukwonKim, this is valid point. Thanks!

So far, we do not see any Ext use case, and libspdm does not support Ext algorithm at all.

Would you please share with us what use case you will have to support Ext Algorithm?

@IamSukwonKim
Copy link
Author

@jyao1, It’s my pleasure. Thank you :)

Would you please share with us what use case you will have to support Ext Algorithm?

We don' have any use cases for Ext. Algorithm, too. What I'm trying to say is that the responder can't be sure that there will be no Ext Algorithm in the future, because of the specification.

So far, we do not see any Ext use case, and libspdm does not support Ext algorithm at all.

If there are no use case, why don't we get rid of the fields. So, that we can save memory.

@steven-bellock
Copy link
Contributor

Even if an endpoint does not support the ext algorithms they'll still end up in the transcript. The memory allocation for VCA should reflect that, within reason.

@IamSukwonKim
Copy link
Author

Even if an endpoint does not support the ext algorithms they'll still end up in the transcript. The memory allocation for VCA should reflect that, within reason.

Of course, if we're sure by considering backward compatibility, code complexity or something else, we might remove it from the spec. first, and then from the code.

@jyao1
Copy link
Member

jyao1 commented Aug 4, 2023

@IamSukwonKim, can we assign to you, then you can propose a patch?

@IamSukwonKim
Copy link
Author

@IamSukwonKim, can we assign to you, then you can propose a patch?

First, PR is not possible in my work environment. Second, I'm not sure if I'm allowed to PR outside of my workspace, and third, even if I could, it would take a long time because I'm so busy these days.

I want to do the work. If I find out a way, I'll let you know again.

@steven-bellock steven-bellock added the enhancement New feature or request label Aug 14, 2023
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

3 participants