[dgarske]

Description

Every fwTPM command over the mssim socket transport paid a fixed delay. DispatchAndRespond() sent a reply as three writes - size, response, acknowledgement - and that last four-byte write goes out while the response is still unacknowledged, so Nagle holds it until the peer's delayed-ACK timer expires. The time is spent waiting on a kernel timer, which is why a measurement taken around FWTPM_ProcessCommand() looked normal while the client round trip did not.

The SWTPM client sets TCP_NODELAY on the socket it connects, but that governs the client's own sends. The server never set it on the sockets it accepts.

• SendMssimReply() sends size, response and acknowledgement as one segment with writev(). A custom I/O HAL has no vectored form and keeps the three sequential sends; FWTPM_NO_WRITEV selects that path, as do Windows and Zephyr.
• SetSocketNoDelay() disables Nagle on both accepted sockets, command and platform. The platform port frames its replies the same way.

Either change removes the delay on its own. Both are kept because the fallback path still writes three times, and both are
advisory in the sense SO_REUSEADDR here already is: a failure costs latency, not correctness.

Fixes #663. Thank you @jerohavaz

Testing

test_fwtpm_mssim_reply_framing drives SendMssimReply() over a socketpair and asserts the wire stream is exactly size, response, zero ack; a FWTPM_NO_WRITEV build covers the sequential path through the same test. test_fwtpm_mssim_reply_partial_writes caps writev() at every length so a short write lands in each of the three buffers, and test_fwtpm_mssim_accept_nodelay runs the server loop and checks TCP_NODELAY on the accepted command and platform sockets. make check green against the fwTPM simulator, unit.test and run_examples.sh green against ibmswtpm2, and built over eleven configurations.

[Copilot]

🟢 Approval recommended

No active review issues remain for the changes to fwTPM MSSIM reply handling.

No open issues 🎉

What changed in this PR

This PR addresses #663 by removing the server-side socket behavior responsible for delayed fwTPM MSSIM replies, without changing the wire format.

Changes:

• Uses writev() to send reply framing and payload together, retaining sequential sends for custom HALs and unsupported or disabled configurations.
• Enables TCP_NODELAY on accepted command and platform sockets.
• Adds a reply-framing test shared by vectored and sequential builds.

File	Description
tests/​fwtpm_unit_tests.c	Adds assertions for reply length, payload, and zero acknowledgement.
tests/​fwtpm_io_test_adapter.c	Exposes the reply helper to unit tests.
src/​fwtpm/​fwtpm_io.c	Adds vectored replies with partial-write handling and disables Nagle on accepted sockets.

_{Review effort: Balanced}

[embhorn]

Skoll Code Review

Scan type: review

Overall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [Medium] Test partial vectored writes — tests/fwtpm_unit_tests.c:20424-20425
• [Medium] Test TCP_NODELAY on accepted sockets — tests/fwtpm_unit_tests.c:20423

Review generated by Skoll

[embhorn]

Test partial vectored writes

The new framing test does not force a partial writev() result. A mistake in the PR's iovec advancement logic could therefore drop or duplicate reply bytes while this test passes.

Suggested fix: Add a deterministic test that makes writev() stop within the size, response, and acknowledgement buffers, then checks the complete byte stream.
Basis: The Linux writev(2) manual, RETURN VALUE, states that a successful call may transfer fewer bytes than requested.

[embhorn]

Test TCP_NODELAY on accepted sockets

The new test uses an AF_UNIX socketpair and calls the reply helper directly. It never exercises either new SetSocketNoDelay() call in the TCP accept paths, so a regression in the PR's latency fix would pass.

Suggested fix: Add a TCP server test that checks TCP_NODELAY on accepted command and platform sockets.
Basis: The POSIX getsockopt(3p) manual, DESCRIPTION, specifies that getsockopt() retrieves a socket option's value.