Skip to content

Restructure mctp_estack::Router, add timeouts, add round trip tests #26

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

Merged
merged 22 commits into from
Aug 11, 2025

Conversation

mkj
Copy link
Member

@mkj mkj commented Aug 8, 2025

This series restructures Router and PortTop/PortBottom to have improved ownership/lifetimes, making it easier for using non-static. The impetus is for writing tests, but it should be useful in other situations too.

RouterAsyncReqChannel/RouterAsyncListener now can have a receive timeout set.

The PortLookup API is changed, now taking a const reference, and the MTU for an outgoing message is also taken from PortLookup.

In order to use async closures for tests there is a bump to minimum Rust 1.85.

mkj added 16 commits August 8, 2025 15:09
The stack now defaults to 64 (MCTP protocol minimum MTU). send_message()
calls can provide a larger value if desired.

Signed-off-by: Matt Johnston <[email protected]>
A const reference will make it easier to update the PortLookup
externally. Implementations can use internal mutability with Cell if
needed.

Signed-off-by: Matt Johnston <[email protected]>
This is temporarily copied from embassy-sync, with modifications to add
FixedChannel and Channel sender()/receiver(). It will be replaced
once upstream.
embassy-rs/embassy#4299

Based on upstream 6186d111a5c150946ee5b7e9e68d987a38c1a463
plus
0ab37d4dbdcf ("embassy-sync: zerocopy Channel sender()/receiver()")
e9db18bf5051 ("embassy-sync: Add zerocopy_channel::FixedChannel")

Signed-off-by: Matt Johnston <[email protected]>
This will be required for tests, using std feature.

Signed-off-by: Matt Johnston <[email protected]>
Previously RouterAsyncReqChannel etc used &'r Router<'r>, which is only
usable for static lifetimes. Use a different lifetime for the reference.

Signed-off-by: Matt Johnston <[email protected]>
Now the application provides PortTop mut references to the Router.
Router now has a single lifetime rather than the previous chain of
lifetimes (also for the zero_copy_channel::Channel).

PortStorage has been replaced by PortTop. PortBottom is now renamed to
Port.

This uses zerocopy_channel::FixedChannel.

Signed-off-by: Matt Johnston <[email protected]>
This reverts part of
4cc8969 ("mctp-estack: Minor Fragmenter usage changes")

It is better to break out of the loop immediately rather than waiting
for another loop iteration - is the sender is full it will have to wait
until a packet is sent.

Signed-off-by: Matt Johnston <[email protected]>
Previously a blocking send_message() could also block progress for
forward_packets().

After this change, forward_packets() will wait for access to the channel
(send_message() only holds the lock momentarily), then will drop the
packet and return if the channel is full.

Signed-off-by: Matt Johnston <[email protected]>
Now the lookup function also returns an optional EID, replacing
the fixed port MTU.

Signed-off-by: Matt Johnston <[email protected]>
Stack doesn't allow much customisation, so construct it directly.

Signed-off-by: Matt Johnston <[email protected]>
With non-expiring tags a non-owner response shouldn't remove the flow.

Signed-off-by: Matt Johnston <[email protected]>
A receive timeout can be set with set_timeout(), expiring depending on
udpate_clock().

Signed-off-by: Matt Johnston <[email protected]>
This is slightly more accurate and consistent with other timeouts such as
in Router.

Signed-off-by: Matt Johnston <[email protected]>
cancel_flow is also updated, now has no return (was always Ok) and
removed the debug assertion which was incorrect. A reassembler could be
Done and still existing, but the flow would be removed.

Signed-off-by: Matt Johnston <[email protected]>
Messages now don't expire at all after retain(), it is up to the
application to ensure that they are fetched with get_deferred...() then
dropped.

Signed-off-by: Matt Johnston <[email protected]>
mkj added 5 commits August 11, 2025 11:55
This replaces async_drop(), running a cleanup handler
if needed prior to handing local incoming packets.

This ensures that a dropped ReqChannel/Listener won't hold
Reassembler slots used.

Signed-off-by: Matt Johnston <[email protected]>
Clippy 1.85 warns about these unused lifetimes. Remove them with --fix.
More recently clippy no longer warns, though the code is cleaner
without them.

Signed-off-by: Matt Johnston <[email protected]>
Some lints are unwanted and have been removed in newer versions, for
example operator precedence warnings.

Signed-off-by: Matt Johnston <[email protected]>
Tests will use async closures, so update the version.

Signed-off-by: Matt Johnston <[email protected]>
This is a specialised executor to run async futures with more control.
Can be used to partially run a future such as recv(), then adjust
the clock before proceeding, when testing timeouts.

Signed-off-by: Matt Johnston <[email protected]>
These set up two router peers and test various send/receive scenarios.

Signed-off-by: Matt Johnston <[email protected]>
@mkj mkj merged commit 88c9eba into CodeConstruct:main Aug 11, 2025
4 checks passed
@mkj mkj deleted the matt/roundtrip branch August 11, 2025 04:51
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.

1 participant