Repository: firecracker-microvm/firecracker
File: src/vmm/src/devices/virtio/net/device.rs (Rust)

Code before:
        Ok(())
    }

    /// Returns the total size of available space in the buffer.
    #[inline(always)]
    fn capacity(&self) -> usize {
        self.iovec.len()
    }

    /// Mark the first `size` bytes of available memory as used.
    ///
    /// # Safety:
    ///
    /// * The `RxBuffers` should include at least one parsed `DescriptorChain`.
    /// * `size` needs to be smaller or equal to total length of the first `DescriptorChain` stored
    ///   in the `RxBuffers`.
    unsafe fn mark_used(&mut self, mut bytes_written: u32, rx_queue: &mut Queue) {
        // Since we were able to write a frame in guest memory, we should have at least one
        // descriptor chain here. If not, we have a bug, so fail fast, since the device is
        // fundamentally broken.
        debug_assert!(!self.iovec.is_empty());
        let header_ptr: *mut virtio_net_hdr_v1 = self.iovec.vecs.as_mut_slice()[0].iov_base.cast();
        let header_buff_len = self.iovec.vecs.as_mut_slice()[0].iov_len;
        assert!(
            vnet_hdr_len() <= header_buff_len,
            "Network buffer should be big enough for virtio_net_hdr_v1 object"
        );

        self.used_bytes = bytes_written;

        let mut used_heads: u16 = 0;
        let mut write_used = |head_index: u16, bytes_written: u32, rx_queue: &mut Queue| {
            if let Err(err) = rx_queue.write_used_element(
                (rx_queue.next_used + Wrapping(self.used_descriptors)).0,
                head_index,
                bytes_written,
            ) {
                error!(
                    "net: Failed to add used descriptor {} of length {} to RX queue: {err}",
                    head_index, bytes_written
                );
            }
            used_heads += 1;
            self.used_descriptors += 1;
        };

        loop {
            let parsed_dc = self
                .parsed_descriptors
                .pop_front()
                .expect("This should never happen if write to the buffer succeded.");

Review comment (bug):
As we've discussed offline, this is wrong. I agree that any sensible implementation of the specification will not split the header into multiple `DescriptorChain` objects, but unless we have a very good reason to do this, I would prefer us to be specification compliant.

A very good reason to do this, for me, would be an A/B run where A is this version and B a version that simply uses `write_volatile_at` and finds that A outperforms B.

Code after:
    }

    /// Returns the total size of available space in the buffer.
    #[inline(always)]
    fn capacity(&self) -> u32 {
        self.iovec.len()
    }

    /// Mark the first `size` bytes of available memory as used.
    ///
    /// # Safety:
    ///
    /// * The `RxBuffers` should include at least one parsed `DescriptorChain`.
    /// * `size` needs to be smaller or equal to total length of the first `DescriptorChain` stored
    ///   in the `RxBuffers`.
    unsafe fn mark_used(&mut self, mut bytes_written: u32, rx_queue: &mut Queue) {
        self.used_bytes = bytes_written;

        let mut used_heads: u16 = 0;
        for parsed_dc in self.parsed_descriptors.iter() {
            let used_bytes = bytes_written.min(parsed_dc.length);
            // Safe because we know head_index isn't out of bounds
            rx_queue
                .write_used_element(self.used_descriptors, parsed_dc.head_index, used_bytes)
                .unwrap();
            bytes_written -= used_bytes;
            self.used_descriptors += 1;
            used_heads += 1;

            if bytes_written == 0 {
                break;
            }
        }

        // We need to set num_buffers before dropping chains from `self.iovec`. Otherwise
        // when we set headers, we will iterate over new, yet unused chains instead of the ones
        // we need.
        self.header_set_num_buffers(used_heads);
        for _ in 0..used_heads {
            let parsed_dc = self
                .parsed_descriptors
                .pop_front()
                .expect("This should never happen if write to the buffer succeeded.");
            self.iovec.drop_chain_front(&parsed_dc);
        }
    }

    /// Write the number of descriptors used in VirtIO header
    fn header_set_num_buffers(&mut self, nr_descs: u16) {
        // We can unwrap here, because we have checked before that the `IoVecBufferMut` holds at
        // least one buffer with the proper size, depending on the feature negotiation. In any