[vsock] remove buf pointer from Packet

Signed-off-by: Serban Iorga <[email protected]>

Author: Serban Iorga

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

@@ -890,9 +890,16 @@ mod tests {
         }
 
         fn init_data_pkt(&mut self, data: &[u8]) -> &VsockPacket {
-            assert!(data.len() <= self.pkt.buf().unwrap().len());
+            assert!(data.len() <= self.pkt.buf_size());
             self.init_pkt(uapi::VSOCK_OP_RW, data.len() as u32);
-            self.pkt.buf_mut().unwrap()[..data.len()].copy_from_slice(data);
+            self.pkt
+                .read_at_offset_from(
+                    &self._vsock_test_ctx.mem,
+                    0,
+                    &mut std::io::Cursor::new(data.to_vec()),
+                    data.len(),
+                )
+                .unwrap();
             &self.pkt
         }
     }
@@ -960,7 +967,17 @@ mod tests {
         ctx.recv();
         assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RW);
         assert_eq!(ctx.pkt.len() as usize, data.len());
-        assert_eq!(ctx.pkt.buf().unwrap()[..ctx.pkt.len() as usize], *data);
+
+        let mut buf = vec![];
+        ctx.pkt
+            .write_from_offset_to(
+                &ctx._vsock_test_ctx.mem,
+                0,
+                &mut buf,
+                ctx.pkt.len() as usize,
+            )
+            .unwrap();
+        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) {
@@ -1030,7 +1047,17 @@ mod tests {
             ctx.notify_epollin();
             ctx.recv();
             assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RW);
-            assert_eq!(&ctx.pkt.buf().unwrap()[..ctx.pkt.len() as usize], data);
+
+            let mut buf = vec![];
+            ctx.pkt
+                .write_from_offset_to(
+                    &ctx._vsock_test_ctx.mem,
+                    0,
+                    &mut buf,
+                    ctx.pkt.len() as usize,
+                )
+                .unwrap();
+            assert_eq!(&buf, data);
 
             ctx.init_data_pkt(data);
             ctx.send();
@@ -1222,7 +1249,7 @@ mod tests {
         ctx.set_stream(stream);
 
         // Fill up the TX buffer.
-        let data = vec![0u8; ctx.pkt.buf().unwrap().len()];
+        let data = vec![0u8; ctx.pkt.buf_size()];
         ctx.init_data_pkt(data.as_slice());
         for _i in 0..(csm_defs::CONN_TX_BUF_SIZE / data.len() as u32) {
             ctx.send();

src/devices/src/virtio/vsock/packet.rs

@@ -15,10 +15,9 @@
 /// in guest memory. This is done to avoid unnecessarily copying data from guest memory
 /// to temporary buffers, before passing it on to the vsock backend.
 use std::io::{Read, Write};
-use std::result;
 
 use vm_memory::{
-    self, Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryError, GuestMemoryMmap,
+    self, Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap,
     GuestMemoryRegion, GuestRegionMmap, MemoryRegionAddress,
 };
 
@@ -93,18 +92,9 @@ pub struct VsockPacket {
     // 1 write for Rx and 1 read for Tx, plus 1 `check_range` call for each.
     hdr: VsockPacketHeader,
     buf_addr: Option<GuestAddress>,
-    buf: Option<*mut u8>,
     buf_size: usize,
 }
 
-fn get_host_address<T: GuestMemory>(
-    mem: &T,
-    guest_addr: GuestAddress,
-    size: usize,
-) -> result::Result<*mut u8, GuestMemoryError> {
-    Ok(mem.get_slice(guest_addr, size)?.as_ptr())
-}
-
 impl VsockPacket {
     fn check_desc_write_only(desc: &DescriptorChain, expected_write_only: bool) -> Result<()> {
         if desc.is_write_only() != expected_write_only {
@@ -150,10 +140,6 @@ impl VsockPacket {
         }
 
         self.buf_addr = Some(buf_desc.addr);
-        self.buf = Some(
-            get_host_address(buf_desc.mem, buf_desc.addr, buf_size)
-                .map_err(VsockError::GuestMemoryMmap)?,
-        );
         self.buf_size = buf_size;
 
         Ok(())
@@ -181,7 +167,6 @@ impl VsockPacket {
                 .read_obj(hdr_desc.addr)
                 .map_err(VsockError::GuestMemoryMmap)?,
             buf_addr: None,
-            buf: None,
             buf_size: 0,
         };
 
@@ -222,7 +207,6 @@ impl VsockPacket {
             // to the guest memory once, before marking the RX descriptor chain as used.
             hdr: VsockPacketHeader::default(),
             buf_addr: None,
-            buf: None,
             buf_size: 0,
         };
 
@@ -242,36 +226,6 @@ impl VsockPacket {
             .map_err(VsockError::GuestMemoryMmap)
     }
 
-    /// Provides in-place, byte-slice access to the vsock packet data buffer.
-    ///
-    /// Note: control packets (e.g. connection request or reset) have no data buffer associated.
-    ///       For those packets, this method will return `None`.
-    /// Also note: calling `len()` on the returned slice will yield the buffer size, which may be
-    ///            (and often is) larger than the length of the packet data. The packet data length
-    ///            is stored in the packet header, and accessible via `VsockPacket::len()`.
-    pub fn buf(&self) -> Option<&[u8]> {
-        self.buf.map(|ptr| {
-            // This is safe since bound checks have already been performed when creating the packet
-            // from the virtq descriptor.
-            unsafe { std::slice::from_raw_parts(ptr as *const u8, self.buf_size) }
-        })
-    }
-
-    /// Provides in-place, byte-slice, mutable access to the vsock packet data buffer.
-    ///
-    /// Note: control packets (e.g. connection request or reset) have no data buffer associated.
-    ///       For those packets, this method will return `None`.
-    /// Also note: calling `len()` on the returned slice will yield the buffer size, which may be
-    ///            (and often is) larger than the length of the packet data. The packet data length
-    ///            is stored in the packet header, and accessible via `VsockPacket::len()`.
-    pub fn buf_mut(&mut self) -> Option<&mut [u8]> {
-        self.buf.map(|ptr| {
-            // This is safe since bound checks have already been performed when creating the packet
-            // from the virtq descriptor.
-            unsafe { std::slice::from_raw_parts_mut(ptr, self.buf_size) }
-        })
-    }
-
     pub fn buf_size(&self) -> usize {
         self.buf_size
     }
@@ -508,7 +462,7 @@ mod tests {
             )
             .unwrap();
             assert_eq!(
-                pkt.buf().unwrap().len(),
+                pkt.buf_size(),
                 handler_ctx.guest_txvq.dtable[1].len.get() as usize
             );
         }
@@ -535,14 +489,13 @@ mod tests {
         {
             create_context!(test_ctx, handler_ctx);
             set_pkt_len(0, &handler_ctx.guest_txvq.dtable[0], &test_ctx.mem);
-            let mut pkt = VsockPacket::from_tx_virtq_head(
+            let pkt = VsockPacket::from_tx_virtq_head(
                 &handler_ctx.device.queues[TXQ_INDEX]
                     .pop(&test_ctx.mem)
                     .unwrap(),
             )
             .unwrap();
-            assert!(pkt.buf().is_none());
-            assert!(pkt.buf_mut().is_none());
+            assert!(pkt.buf_addr.is_none());
         }
 
         // Test case: TX packet has more data than we can handle.
@@ -597,7 +550,7 @@ mod tests {
             )
             .unwrap();
             assert_eq!(
-                pkt.buf().unwrap().len(),
+                pkt.buf_size,
                 handler_ctx.guest_rxvq.dtable[1].len.get() as usize
             );
         }

src/devices/src/virtio/vsock/test_utils.rs

@@ -57,14 +57,17 @@ impl Default for TestBackend {
 }
 
 impl VsockChannel for TestBackend {
-    fn recv_pkt(&mut self, _pkt: &mut VsockPacket, _mem: &GuestMemoryMmap) -> Result<()> {
+    fn recv_pkt(&mut self, pkt: &mut VsockPacket, mem: &GuestMemoryMmap) -> Result<()> {
         let cool_buf = [0xDu8, 0xE, 0xA, 0xD, 0xB, 0xE, 0xE, 0xF];
         match self.rx_err.take() {
             None => {
-                if let Some(buf) = _pkt.buf_mut() {
-                    for i in 0..buf.len() {
-                        buf[i] = cool_buf[i % cool_buf.len()];
-                    }
+                let buf_size = pkt.buf_size();
+                if buf_size > 0 {
+                    let buf: Vec<u8> = (0..buf_size)
+                        .map(|i| cool_buf[i % cool_buf.len()])
+                        .collect();
+                    pkt.read_at_offset_from(mem, 0, &mut std::io::Cursor::new(buf), buf_size)
+                        .unwrap();
                 }
                 self.rx_ok_cnt += 1;
                 Ok(())

src/devices/src/virtio/vsock/unix/muxer.rs

@@ -841,10 +841,17 @@ mod tests {
             peer_port: u32,
             data: &[u8],
         ) -> &mut VsockPacket {
-            assert!(data.len() <= self.pkt.buf().unwrap().len() as usize);
+            assert!(data.len() <= self.pkt.buf_size());
             self.init_pkt(local_port, peer_port, uapi::VSOCK_OP_RW)
                 .set_len(data.len() as u32);
-            self.pkt.buf_mut().unwrap()[..data.len()].copy_from_slice(data);
+            self.pkt
+                .read_at_offset_from(
+                    &self._vsock_test_ctx.mem,
+                    0,
+                    &mut std::io::Cursor::new(data.to_vec()),
+                    data.len(),
+                )
+                .unwrap();
             &mut self.pkt
         }
 
@@ -1074,10 +1081,20 @@ mod tests {
         assert!(ctx.muxer.has_pending_rx());
         ctx.recv();
         assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RW);
-        assert_eq!(ctx.pkt.buf().unwrap()[..data.len()], data);
         assert_eq!(ctx.pkt.src_port(), LOCAL_PORT);
         assert_eq!(ctx.pkt.dst_port(), PEER_PORT);
 
+        let mut buf = vec![];
+        ctx.pkt
+            .write_from_offset_to(
+                &ctx._vsock_test_ctx.mem,
+                0,
+                &mut buf,
+                ctx.pkt.len() as usize,
+            )
+            .unwrap();
+        assert_eq!(&buf, &data);
+
         assert!(!ctx.muxer.has_pending_rx());
     }
 
@@ -1106,7 +1123,17 @@ mod tests {
         assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RW);
         assert_eq!(ctx.pkt.src_port(), local_port);
         assert_eq!(ctx.pkt.dst_port(), peer_port);
-        assert_eq!(ctx.pkt.buf().unwrap()[..data.len()], data);
+
+        let mut buf = vec![];
+        ctx.pkt
+            .write_from_offset_to(
+                &ctx._vsock_test_ctx.mem,
+                0,
+                &mut buf,
+                ctx.pkt.len() as usize,
+            )
+            .unwrap();
+        assert_eq!(&buf, &data);
     }
 
     #[test]