Fixup vsock unit tests to work with iovec-based packet handling

IoVecBuffer[Mut] statically guarantee that a read-only descriptor chain
cannot be written to, and a write-only descriptor chain cannot be read
from. Since RX descriptors are write-only and TX descriptors are
read-only, this causes some problems in the vsock unit test code, which
has to first emulate the guest putting packets into the TX queue (e.g.
write to a read-only queue) and also reading the data that the device
wrote into the RX queue to validate its correctness (which is a
write-only queue). The unit tests however do not differentiate well
between which queue a packet belogns to, using one packet that was
initialized from the RX queue to both mock RX and TX operations. This
commit fixes this somewhat, by differentiating between RX and TX
packets. It does not address the separation between guest- and host-view
of the respective queues, and instead uses a trick: Both queues point
toward the same memory region, which is then interpreted as RX or TX
memory depending on whether the test wants to mock a guest operation, or
validate a device operation.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

src/vmm/src/devices/virtio/vsock/csm/connection.rs

@@ -694,7 +694,7 @@ mod tests {
     use super::super::super::defs::uapi;
     use super::super::defs as csm_defs;
     use super::*;
-    use crate::devices::virtio::vsock::device::RXQ_INDEX;
+    use crate::devices::virtio::vsock::device::{RXQ_INDEX, TXQ_INDEX};
     use crate::devices::virtio::vsock::test_utils;
     use crate::devices::virtio::vsock::test_utils::TestContext;
     use crate::vstate::memory::BitmapSlice;
@@ -850,7 +850,9 @@ mod tests {
     #[derive(Debug)]
     struct CsmTestContext {
         _vsock_test_ctx: TestContext,
-        pkt: VsockPacket,
+        // Two views of the same in-memory packet. rx-view for writing, tx-view for reading
+        rx_pkt: VsockPacket,
+        tx_pkt: VsockPacket,
         conn: VsockConnection<TestStream>,
     }
 
@@ -863,8 +865,14 @@ mod tests {
             let vsock_test_ctx = TestContext::new();
             let mut handler_ctx = vsock_test_ctx.create_event_handler_context();
             let stream = TestStream::new();
-            let mut pkt = VsockPacket::from_rx_virtq_head(
-                &handler_ctx.device.queues[RXQ_INDEX]
+            let mut rx_pkt = VsockPacket::from_rx_virtq_head(
+                handler_ctx.device.queues[RXQ_INDEX]
+                    .pop(&vsock_test_ctx.mem)
+                    .unwrap(),
+            )
+            .unwrap();
+            let tx_pkt = VsockPacket::from_tx_virtq_head(
+                handler_ctx.device.queues[TXQ_INDEX]
                     .pop(&vsock_test_ctx.mem)
                     .unwrap(),
             )
@@ -891,16 +899,17 @@ mod tests {
                         PEER_BUF_ALLOC,
                     );
                     assert!(conn.has_pending_rx());
-                    conn.recv_pkt(&mut pkt, &vsock_test_ctx.mem).unwrap();
-                    assert_eq!(pkt.op(), uapi::VSOCK_OP_RESPONSE);
+                    conn.recv_pkt(&mut rx_pkt, &vsock_test_ctx.mem).unwrap();
+                    assert_eq!(rx_pkt.op(), uapi::VSOCK_OP_RESPONSE);
                     conn
                 }
                 other => panic!("invalid ctx state: {:?}", other),
             };
             assert_eq!(conn.state, conn_state);
             Self {
                 _vsock_test_ctx: vsock_test_ctx,
-                pkt,
+                rx_pkt,
+                tx_pkt,
                 conn,
             }
         }
@@ -918,13 +927,13 @@ mod tests {
 
         fn send(&mut self) {
             self.conn
-                .send_pkt(&self.pkt, &self._vsock_test_ctx.mem)
+                .send_pkt(&self.tx_pkt, &self._vsock_test_ctx.mem)
                 .unwrap();
         }
 
         fn recv(&mut self) {
             self.conn
-                .recv_pkt(&mut self.pkt, &self._vsock_test_ctx.mem)
+                .recv_pkt(&mut self.rx_pkt, &self._vsock_test_ctx.mem)
                 .unwrap();
         }
 
@@ -937,19 +946,17 @@ mod tests {
             self.conn.notify(EventSet::OUT);
         }
 
-        fn init_pkt(&mut self, op: u16, len: u32) -> &mut VsockPacket {
-            init_pkt(&mut self.pkt, op, len)
+        fn init_tx_pkt(&mut self, op: u16, len: u32) -> &mut VsockPacket {
+            init_pkt(&mut self.tx_pkt, op, len)
         }
 
-        fn init_data_pkt(&mut self, mut data: &[u8]) -> &VsockPacket {
-            assert!(data.len() <= self.pkt.buf_size());
-            self.init_pkt(uapi::VSOCK_OP_RW, u32::try_from(data.len()).unwrap());
+        fn init_data_tx_pkt(&mut self, mut data: &[u8]) -> &VsockPacket {
+            assert!(data.len() <= self.tx_pkt.buf_size());
+            self.init_tx_pkt(uapi::VSOCK_OP_RW, u32::try_from(data.len()).unwrap());
 
             let len = data.len();
-            self.pkt
-                .read_at_offset_from(&self._vsock_test_ctx.mem, 0, &mut data, len)
-                .unwrap();
-            &self.pkt
+            self.rx_pkt.read_at_offset_from(&mut data, 0, len).unwrap();
+            &self.tx_pkt
         }
     }
 
@@ -960,13 +967,13 @@ mod tests {
         ctx.recv();
         // For peer-initiated requests, our connection should always yield a vsock reponse packet,
         // in order to establish the connection.
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RESPONSE);
-        assert_eq!(ctx.pkt.src_cid(), LOCAL_CID);
-        assert_eq!(ctx.pkt.dst_cid(), PEER_CID);
-        assert_eq!(ctx.pkt.src_port(), LOCAL_PORT);
-        assert_eq!(ctx.pkt.dst_port(), PEER_PORT);
-        assert_eq!(ctx.pkt.type_(), uapi::VSOCK_TYPE_STREAM);
-        assert_eq!(ctx.pkt.len(), 0);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RESPONSE);
+        assert_eq!(ctx.rx_pkt.src_cid(), LOCAL_CID);
+        assert_eq!(ctx.rx_pkt.dst_cid(), PEER_CID);
+        assert_eq!(ctx.rx_pkt.src_port(), LOCAL_PORT);
+        assert_eq!(ctx.rx_pkt.dst_port(), PEER_PORT);
+        assert_eq!(ctx.rx_pkt.type_(), uapi::VSOCK_TYPE_STREAM);
+        assert_eq!(ctx.rx_pkt.len(), 0);
         // After yielding the response packet, the connection should have transitioned to the
         // established state.
         assert_eq!(ctx.conn.state, ConnState::Established);
@@ -981,11 +988,11 @@ mod tests {
         // armed.
         assert!(!ctx.conn.will_expire());
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_REQUEST);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_REQUEST);
         // Since the request might time-out, the kill timer should now be armed.
         assert!(ctx.conn.will_expire());
         assert!(!ctx.conn.has_expired());
-        ctx.init_pkt(uapi::VSOCK_OP_RESPONSE, 0);
+        ctx.init_tx_pkt(uapi::VSOCK_OP_RESPONSE, 0);
         ctx.send();
         // Upon receiving a connection response, the connection should have transitioned to the
         // established state, and the kill timer should've been disarmed.
@@ -997,7 +1004,7 @@ mod tests {
     fn test_local_request_timeout() {
         let mut ctx = CsmTestContext::new(ConnState::LocalInit);
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_REQUEST);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_REQUEST);
         assert!(ctx.conn.will_expire());
         assert!(!ctx.conn.has_expired());
         std::thread::sleep(std::time::Duration::from_millis(
@@ -1014,14 +1021,14 @@ mod tests {
         assert_eq!(ctx.conn.as_raw_fd(), ctx.conn.stream.as_raw_fd());
         ctx.notify_epollin();
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RW);
-        assert_eq!(ctx.pkt.len() as usize, data.len());
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RW);
+        assert_eq!(ctx.rx_pkt.len() as usize, data.len());
 
-        let buf = test_utils::read_packet_data(&ctx.pkt, &ctx._vsock_test_ctx.mem, 4);
+        let buf = test_utils::read_packet_data(&ctx.tx_pkt, 4);
         assert_eq!(&buf, data);
 
         // There's no more data in the stream, so `recv_pkt` should yield `VsockError::NoData`.
-        match ctx.conn.recv_pkt(&mut ctx.pkt, &ctx._vsock_test_ctx.mem) {
+        match ctx.conn.recv_pkt(&mut ctx.tx_pkt, &ctx._vsock_test_ctx.mem) {
             Err(VsockError::NoData) => (),
             other => panic!("{:?}", other),
         }
@@ -1030,7 +1037,7 @@ mod tests {
         ctx.conn.state = ConnState::LocalClosed;
         ctx.notify_epollin();
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RST);
     }
 
     #[test]
@@ -1044,9 +1051,9 @@ mod tests {
         // When the host-side stream is closed, we can neither send not receive any more data.
         // Therefore, the vsock shutdown packet that we'll deliver to the guest must contain both
         // the no-more-send and the no-more-recv indications.
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_SHUTDOWN);
-        assert_ne!(ctx.pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_SEND, 0);
-        assert_ne!(ctx.pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_RCV, 0);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_SHUTDOWN);
+        assert_ne!(ctx.rx_pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_SEND, 0);
+        assert_ne!(ctx.rx_pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_RCV, 0);
 
         // The kill timer should now be armed.
         assert!(ctx.conn.will_expire());
@@ -1063,14 +1070,14 @@ mod tests {
         {
             let mut ctx = CsmTestContext::new_established();
 
-            ctx.init_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
+            ctx.init_tx_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
                 .set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_RCV);
             ctx.send();
             assert_eq!(ctx.conn.state, ConnState::PeerClosed(true, false));
 
             // Attempting to reset the no-more-recv indication should not work
             // (we are only setting the no-more-send indication here).
-            ctx.pkt.set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_SEND);
+            ctx.tx_pkt.set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_SEND);
             ctx.send();
             assert_eq!(ctx.conn.state, ConnState::PeerClosed(true, true));
         }
@@ -1082,17 +1089,17 @@ mod tests {
             let data = &[1, 2, 3, 4];
             let mut ctx = CsmTestContext::new_established();
             ctx.set_stream(TestStream::new_with_read_buf(data));
-            ctx.init_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
+            ctx.init_tx_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
                 .set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_SEND);
             ctx.send();
             ctx.notify_epollin();
             ctx.recv();
-            assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RW);
+            assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RW);
 
-            let buf = test_utils::read_packet_data(&ctx.pkt, &ctx._vsock_test_ctx.mem, 4);
+            let buf = test_utils::read_packet_data(&ctx.tx_pkt, 4);
             assert_eq!(&buf, data);
 
-            ctx.init_data_pkt(data);
+            ctx.init_data_tx_pkt(data);
             ctx.send();
             assert_eq!(ctx.conn.stream.write_buf.len(), 0);
             assert!(ctx.conn.tx_buf.is_empty());
@@ -1103,29 +1110,29 @@ mod tests {
         // - attempting to read data from it should yield an RST packet.
         {
             let mut ctx = CsmTestContext::new_established();
-            ctx.init_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
+            ctx.init_tx_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
                 .set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_RCV);
             ctx.send();
             let data = &[1, 2, 3, 4];
-            ctx.init_data_pkt(data);
+            ctx.init_data_tx_pkt(data);
             ctx.send();
             assert_eq!(ctx.conn.stream.write_buf, data.to_vec());
 
             ctx.notify_epollin();
             ctx.recv();
-            assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST);
+            assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RST);
         }
 
         // Test case: setting both no-more-send and no-more-recv indications should have the
         // connection confirm termination (i.e. yield an RST).
         {
             let mut ctx = CsmTestContext::new_established();
-            ctx.init_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
+            ctx.init_tx_pkt(uapi::VSOCK_OP_SHUTDOWN, 0)
                 .set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_RCV | uapi::VSOCK_FLAGS_SHUTDOWN_SEND);
             ctx.send();
             assert!(ctx.conn.has_pending_rx());
             ctx.recv();
-            assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST);
+            assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RST);
         }
     }
 
@@ -1137,7 +1144,7 @@ mod tests {
         ctx.set_stream(stream);
         ctx.notify_epollin();
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RST);
     }
 
     #[test]
@@ -1146,19 +1153,19 @@ mod tests {
         ctx.set_peer_credit(0);
         ctx.notify_epollin();
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_CREDIT_REQUEST);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_CREDIT_REQUEST);
     }
 
     #[test]
     fn test_credit_request_from_peer() {
         let mut ctx = CsmTestContext::new_established();
-        ctx.init_pkt(uapi::VSOCK_OP_CREDIT_REQUEST, 0);
+        ctx.init_tx_pkt(uapi::VSOCK_OP_CREDIT_REQUEST, 0);
         ctx.send();
         assert!(ctx.conn.has_pending_rx());
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_CREDIT_UPDATE);
-        assert_eq!(ctx.pkt.buf_alloc(), csm_defs::CONN_TX_BUF_SIZE);
-        assert_eq!(ctx.pkt.fwd_cnt(), ctx.conn.fwd_cnt.0);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_CREDIT_UPDATE);
+        assert_eq!(ctx.rx_pkt.buf_alloc(), csm_defs::CONN_TX_BUF_SIZE);
+        assert_eq!(ctx.rx_pkt.fwd_cnt(), ctx.conn.fwd_cnt.0);
     }
 
     #[test]
@@ -1179,20 +1186,20 @@ mod tests {
         let data = &[1, 2, 3, 4];
 
         // Check that there is no pending RX.
-        ctx.init_data_pkt(data);
+        ctx.init_data_tx_pkt(data);
         ctx.send();
         assert!(!ctx.conn.has_pending_rx());
 
         // Send a packet again.
-        ctx.init_data_pkt(data);
+        ctx.init_data_tx_pkt(data);
         ctx.send();
 
         // The CSM should now have a credit update available for the peer.
         assert!(ctx.conn.has_pending_rx());
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_CREDIT_UPDATE);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_CREDIT_UPDATE);
         assert_eq!(
-            ctx.pkt.fwd_cnt() as usize,
+            ctx.rx_pkt.fwd_cnt() as usize,
             initial_fwd_cnt as usize + data.len() * 2,
         );
         assert_eq!(ctx.conn.fwd_cnt, ctx.conn.last_fwd_cnt_to_peer);
@@ -1214,7 +1221,7 @@ mod tests {
             // Send some data through the connection. The backing stream is set to reject writes,
             // so the data should end up in the TX buffer.
             let data = &[1, 2, 3, 4];
-            ctx.init_data_pkt(data);
+            ctx.init_data_tx_pkt(data);
             ctx.send();
 
             // When there's data in the TX buffer, the connection should ask to be notified when it
@@ -1241,13 +1248,13 @@ mod tests {
             ctx.set_stream(stream);
 
             let data = &[1, 2, 3, 4];
-            ctx.init_data_pkt(data);
+            ctx.init_data_tx_pkt(data);
             ctx.send();
 
             assert_eq!(ctx.conn.state, ConnState::Killed);
             assert!(ctx.conn.has_pending_rx());
             ctx.recv();
-            assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST);
+            assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RST);
         }
 
         // Test case: notifying a connection that it can flush its TX buffer to a broken stream
@@ -1262,7 +1269,7 @@ mod tests {
             // Send some data through the connection. The backing stream is set to reject writes,
             // so the data should end up in the TX buffer.
             let data = &[1, 2, 3, 4];
-            ctx.init_data_pkt(data);
+            ctx.init_data_tx_pkt(data);
             ctx.send();
 
             // Set the backing stream to error out on write.
@@ -1285,8 +1292,8 @@ mod tests {
         ctx.set_stream(stream);
 
         // Fill up the TX buffer.
-        let data = vec![0u8; ctx.pkt.buf_size()];
-        ctx.init_data_pkt(data.as_slice());
+        let data = vec![0u8; ctx.tx_pkt.buf_size()];
+        ctx.init_data_tx_pkt(data.as_slice());
         for _i in 0..(csm_defs::CONN_TX_BUF_SIZE as usize / data.len()) {
             ctx.send();
         }
@@ -1298,6 +1305,6 @@ mod tests {
         assert_eq!(ctx.conn.state, ConnState::Killed);
         assert!(ctx.conn.has_pending_rx());
         ctx.recv();
-        assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST);
+        assert_eq!(ctx.rx_pkt.op(), uapi::VSOCK_OP_RST);
     }
 }