Merge pull request #103 from rcore-os/empty_buffer

Avoid adding empty buffers to queue

src/device/net.rs

@@ -243,11 +243,21 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONet<H, T, QUEUE_SIZE>
     /// completed.
     pub fn send(&mut self, tx_buf: TxBuffer) -> Result {
         let header = VirtioNetHdr::default();
-        self.send_queue.add_notify_wait_pop(
-            &[header.as_bytes(), tx_buf.packet()],
-            &mut [],
-            &mut self.transport,
-        )?;
+        if tx_buf.packet_len() == 0 {
+            // Special case sending an empty packet, to avoid adding an empty buffer to the
+            // virtqueue.
+            self.send_queue.add_notify_wait_pop(
+                &[header.as_bytes()],
+                &mut [],
+                &mut self.transport,
+            )?;
+        } else {
+            self.send_queue.add_notify_wait_pop(
+                &[header.as_bytes(), tx_buf.packet()],
+                &mut [],
+                &mut self.transport,
+            )?;
+        }
         Ok(())
     }
 }

src/device/socket/vsock.rs

@@ -411,11 +411,16 @@ impl<H: Hal, T: Transport> VirtIOSocket<H, T> {
     }
 
     fn send_packet_to_tx_queue(&mut self, header: &VirtioVsockHdr, buffer: &[u8]) -> Result {
-        let _len = self.tx.add_notify_wait_pop(
-            &[header.as_bytes(), buffer],
-            &mut [],
-            &mut self.transport,
-        )?;
+        let _len = if buffer.is_empty() {
+            self.tx
+                .add_notify_wait_pop(&[header.as_bytes()], &mut [], &mut self.transport)?
+        } else {
+            self.tx.add_notify_wait_pop(
+                &[header.as_bytes(), buffer],
+                &mut [],
+                &mut self.transport,
+            )?
+        };
         Ok(())
     }
 

src/hal.rs

@@ -118,8 +118,8 @@ pub unsafe trait Hal {
     ///
     /// # Safety
     ///
-    /// The buffer must be a valid pointer to memory which will not be accessed by any other thread
-    /// for the duration of this method call.
+    /// The buffer must be a valid pointer to a non-empty memory range which will not be accessed by
+    /// any other thread for the duration of this method call.
     unsafe fn share(buffer: NonNull<[u8]>, direction: BufferDirection) -> PhysAddr;
 
     /// Unshares the given memory range from the device and (if necessary) copies it back to the
@@ -127,9 +127,9 @@ pub unsafe trait Hal {
     ///
     /// # Safety
     ///
-    /// The buffer must be a valid pointer to memory which will not be accessed by any other thread
-    /// for the duration of this method call. The `paddr` must be the value previously returned by
-    /// the corresponding `share` call.
+    /// The buffer must be a valid pointer to a non-empty memory range which will not be accessed by
+    /// any other thread for the duration of this method call. The `paddr` must be the value
+    /// previously returned by the corresponding `share` call.
     unsafe fn unshare(paddr: PhysAddr, buffer: NonNull<[u8]>, direction: BufferDirection);
 }
 

src/hal/fake.rs

@@ -43,6 +43,7 @@ unsafe impl Hal for FakeHal {
     }
 
     unsafe fn share(buffer: NonNull<[u8]>, direction: BufferDirection) -> PhysAddr {
+        assert_ne!(buffer.len(), 0);
         // To ensure that the driver is handling and unsharing buffers properly, allocate a new
         // buffer and copy to it if appropriate.
         let mut shared_buffer = u8::new_box_slice_zeroed(buffer.len());
@@ -60,6 +61,8 @@ unsafe impl Hal for FakeHal {
     }
 
     unsafe fn unshare(paddr: PhysAddr, buffer: NonNull<[u8]>, direction: BufferDirection) {
+        assert_ne!(buffer.len(), 0);
+        assert_ne!(paddr, 0);
         let vaddr = phys_to_virt(paddr);
         let shared_buffer = unsafe {
             Box::from_raw(ptr::slice_from_raw_parts_mut(

src/queue.rs

@@ -112,6 +112,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
 
     /// Add buffers to the virtqueue, return a token.
     ///
+    /// The buffers must not be empty.
+    ///
     /// Ref: linux virtio_ring.c virtqueue_add
     ///
     /// # Safety
@@ -135,6 +137,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
         let mut last = self.free_head;
 
         for (buffer, direction) in InputOutputIter::new(inputs, outputs) {
+            assert_ne!(buffer.len(), 0);
+
             // Write to desc_shadow then copy.
             let desc = &mut self.desc_shadow[usize::from(self.free_head)];
             // Safe because our caller promises that the buffers live at least until `pop_used`
@@ -183,6 +187,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
     /// them, then pops them.
     ///
     /// This assumes that the device isn't processing any other buffers at the same time.
+    ///
+    /// The buffers must not be empty.
     pub fn add_notify_wait_pop<'a>(
         &mut self,
         inputs: &'a [&'a [u8]],
@@ -281,6 +287,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
         let mut next = Some(head);
 
         for (buffer, direction) in InputOutputIter::new(inputs, outputs) {
+            assert_ne!(buffer.len(), 0);
+
             let desc_index = next.expect("Descriptor chain was shorter than expected.");
             let desc = &mut self.desc_shadow[usize::from(desc_index)];