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

IPC: Increase SOF_IPC_MSG_MAX_SIZE for testbench and unit test #9402

Merged

Conversation

singalsu
Copy link
Collaborator

The testbench and unit tests use a simplified simulated IPC that currently doesn't support messages split to multiple parts. As workaround to unblock other SOF development the size is increased from 384 bytes to 8192 bytes.

This allows run of current test set with scripts/host-testbench.sh that is also used in SOF CI. The change can be reverted after the capability is added to testbench.

The testbench and unit tests use a simplified simulated IPC that
currently doesn't support messages split to multiple parts. As
workaround to unblock other SOF development the size is increased
from 384 bytes to 8192 bytes.

This allows run of current test set with scripts/host-testbench.sh
that is also used in SOF CI. The change can be reverted after the
capability is added to testbench.

Signed-off-by: Seppo Ingalsuo <[email protected]>
@singalsu
Copy link
Collaborator Author

singalsu commented Aug 23, 2024

@cujomalainey This is other approach to fix the issue found with #9374. I didn't like the loss of tests coverage with #9391, and implementing multi-part binary control IPC to testbench isn't very straightforward. We are missing them from IPC4 testbench and SOF plugin as well. It's coming but likely not in a week or two.

@singalsu singalsu marked this pull request as ready for review August 23, 2024 15:26
@@ -331,6 +331,8 @@ struct ipc_cmd_hdr;
/** Maximum message size for mailbox Tx/Rx */
#if CONFIG_IPC_MAJOR_4
#define SOF_IPC_MSG_MAX_SIZE 0x1000
#elif CONFIG_LIBRARY_STATIC || UNIT_TEST
#define SOF_IPC_MSG_MAX_SIZE 0x2000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much do we care about the unit tests and libraries actually test IPC? If we care a lot then we should make this at best temporary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests and testbench bypass most of the IPC code. The SOF plugin and testbench IPC version (still a draft PR) are using the IPC library code so they would add coverage once they are ready for usage. In short term for unit tests and testbench I don't want to care much about IPC coverage but with the new frameworks I hope it gets better.

@marc-hb
Copy link
Collaborator

marc-hb commented Aug 27, 2024

BTW scripts/fuzz.sh has always printed this message:
https://github.com/thesofproject/sof/actions/runs/10569644939/job/29282813610?pr=9409

INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes

Should this -max_len be increased? It would be a one-line change.

@cujomalainey
Copy link
Contributor

BTW scripts/fuzz.sh has always printed this message: https://github.com/thesofproject/sof/actions/runs/10569644939/job/29282813610?pr=9409

INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes

Should this -max_len be increased? It would be a one-line change.

yes, thanks for pointing that out, we can reduce it to the ipc window size. 4096 i think is ipc4, but 3 is only 384

@marc-hb
Copy link
Collaborator

marc-hb commented Aug 27, 2024

4096 i think is ipc4, but 3 is only 384

Nice coincidence: I'm just adding new -3 / -4 CLI flags in:

@kv2019i kv2019i merged commit 6f61e8a into thesofproject:main Aug 27, 2024
40 of 47 checks passed
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

Successfully merging this pull request may close these issues.

4 participants