Bug fixes for UDS vsock device - #495
Merged
Merged
Conversation
A `CONNECT <port>` line from a host client can arrive in several pieces. For example `writeln!()` emits the literal, the port and the newline in three separate writes. Since the accepted socket is non-blocking, `read_line()` then fails with `EAGAIN`, and the error was propagated out of the mio worker loop, tearing down the whole device and closing every connection. Keep the buffered reader and the bytes received so far in the socket map and resume reading once the socket is readable again. Drop and deregister the socket when the client hangs up mid-request or sends an unparsable port. This also fixes flaky failures of vsock_host_close_test, where the `writeln!()` of the test itself raced with the device. Assisted-by: Antigravity:Claude-Opus-5 Signed-off-by: Changyuan Lyu <changyuanl@google.com>
The listener is registered with mio, which uses edge-triggered epoll, but only one connection was accepted per event. When a client connects while another connection is still queued, no new edge is reported and the queued client is never served. In practice every connection was then served one client behind: the pending connection is only accepted once the next client connects. Accept in a loop until `EAGAIN`. While at it, keep the device alive when a client aborts a connection before it is accepted. Assisted-by: Antigravity:Claude-Opus-5 Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Writing to a host socket whose client already closed fails with EPIPE.
The error was propagated out of the mio worker loop, so a single client
hanging up at the wrong moment took down the whole device together with
all other connections. Two paths are affected:
- the "OK <port>" acknowledgement sent when the guest accepts a
connection that the client abandoned in the meantime,
- forwarding guest data of an established connection.
Mark the connection as EOF in both cases and let process_rx_data() send
an RST to the guest, exactly like a client that closes while idle. If
the RX queue has no descriptor at that point, flush_rx_data() delivers
the RST once the guest provides one.
Also downgrade the log for host data arriving before the guest accepts a
connection: it stays buffered in the socket and is not an error.
Assisted-by: Antigravity:Claude-Opus-5
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Coverage Report for CI Build 34744843132Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.03%) to 37.357%Details
Uncovered Changes
Coverage Regressions81 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
Every test repeated the same ~35 lines to set up guest memory, the queues, the device and the worker, and then open-coded the interrupt, descriptor and connection handshake steps. Add a VsockTest fixture that owns the running worker, the queues and the host socket path, with helpers for the recurring steps: offering RX descriptors, waiting for RX messages, sending TX messages, connecting a host client and completing the CONNECT handshake. Move the device metadata assertions of vsock_conn_test into a separate vsock_dev_test, as they need no worker. No change in coverage, the file shrinks from 977 to 571 lines. Assisted-by: Antigravity:Claude-Opus-5 Signed-off-by: Changyuan Lyu <changyuanl@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assisted-by: Antigravity:Claude-Opus-5