Skip to content

Commit 3a19c4a

Browse files
authored
Fix vsock test flake
This fixes #1197. The test is a simple off by one error which leads to a race condition. We end up waiting for `harness.tx_used_idx()` to reach one less descriptor than we should be, which means calling `reset_tx_cursors` at the top of the loop resets the `QueueWriter` too soon. This opens a window for the poller to not read all the RW packets that we are sending. The log in the test confirms this, and explains why our `read_exact()` fails. ``` Aug 14 02:51:12.719 WARN dropping invalid vsock packet: vsock packet header reported 8192 bytes but the descriptor chain contains 0, component: vsock-test ``` Reviewers: iximeow Reviewed By: iximeow Pull Request: #1198
1 parent c9bae7e commit 3a19c4a

1 file changed

Lines changed: 17 additions & 4 deletions

File tree

lib/propolis/src/vsock/poller.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,6 +1465,10 @@ mod test {
14651465
let mut accepted = listener.accept().unwrap().0;
14661466
accepted.set_nonblocking(false).unwrap();
14671467
accepted.set_read_timeout(Some(Duration::from_secs(5))).unwrap();
1468+
1469+
// Wait for the RESPONSE packet to arrive. The poller takes a desc from
1470+
// the rx_avail ring, writes the packet into it, and publishes it via
1471+
// `push_used` updating rx_used.
14681472
wait_for_condition(|| harness.rx_used_idx() >= 1, 5000);
14691473

14701474
// Send enough data to exceed half the buffer capacity (64KB).
@@ -1473,8 +1477,10 @@ mod test {
14731477
let payload = vec![0xAB_u8; chunk_size];
14741478
let total_sent = num_chunks * chunk_size;
14751479

1476-
for tx_consumed in (1u16..).take(num_chunks) {
1477-
// Reuse descriptor slots each iteration
1480+
for _ in 0..num_chunks {
1481+
// Reuse descriptor slots each iteration. This is only safe because
1482+
// this test ensures that all tx_avail descriptors are pushed back
1483+
// to us via tx_used.
14781484
harness.reset_tx_cursors();
14791485

14801486
let mut rw_hdr = VsockPacketHeader::new();
@@ -1491,11 +1497,18 @@ mod test {
14911497

14921498
let d_hdr = harness.add_tx_readable(hdr_as_bytes(&rw_hdr));
14931499
let d_body = harness.add_tx_readable(&payload);
1500+
1501+
// The vsock poller calls `push_used` which bumps the tx ring's
1502+
// used index after processing the `harness.publish_tx`.
1503+
let expected_tx = harness.tx_used_idx() + 1;
14941504
harness.chain_tx(d_hdr, d_body);
14951505
harness.publish_tx(d_hdr);
1496-
notify.queue_notify(VSOCK_TX_QUEUE).unwrap();
14971506

1498-
wait_for_condition(|| harness.tx_used_idx() >= tx_consumed, 5000);
1507+
// Wait for the poller to release the desc chain back to us via
1508+
// tx_used. This is the running total of the REQUEST and n RW
1509+
// packets.
1510+
notify.queue_notify(VSOCK_TX_QUEUE).unwrap();
1511+
wait_for_condition(|| harness.tx_used_idx() >= expected_tx, 5000);
14991512
}
15001513

15011514
// Drain the data from the accepted socket to confirm it arrived

0 commit comments

Comments
 (0)