From c6829f8efb3750df5e5daefbfaa18b3c08adf87b Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Wed, 16 Aug 2023 23:30:42 +0200 Subject: [PATCH 1/2] add draft to support VIRTIO_NET_F_MRG_RXBUF --- src/drivers/net/virtio_net.rs | 194 +++++++++++--------------- src/drivers/virtio/virtqueue/split.rs | 10 +- 2 files changed, 87 insertions(+), 117 deletions(-) diff --git a/src/drivers/net/virtio_net.rs b/src/drivers/net/virtio_net.rs index c44a77fa29..da559bc667 100644 --- a/src/drivers/net/virtio_net.rs +++ b/src/drivers/net/virtio_net.rs @@ -10,6 +10,7 @@ use core::cmp::Ordering; use core::mem; use core::result::Result; +use align_address::Align; use pci_types::InterruptLine; use smoltcp::phy::{Checksum, ChecksumCapabilities}; use smoltcp::wire::{ETHERNET_HEADER_LEN, IPV4_HEADER_LEN, IPV6_HEADER_LEN}; @@ -191,92 +192,51 @@ impl RxQueues { // Safe virtqueue let rc_vq = Rc::new(vq); let vq = &rc_vq; + let num_buff: u16 = vq.size().into(); - if dev_cfg + let rx_size = if dev_cfg .features - .is_feature(Features::VIRTIO_NET_F_GUEST_TSO4) - | dev_cfg - .features - .is_feature(Features::VIRTIO_NET_F_GUEST_TSO6) - | dev_cfg - .features - .is_feature(Features::VIRTIO_NET_F_GUEST_UFO) + .is_feature(Features::VIRTIO_NET_F_MRG_RXBUF) { - // Receive Buffers must be at least 65562 bytes large with these features set. - // See Virtio specification v1.1 - 5.1.6.3.1 - - // Currently we choose indirect descriptors if possible in order to allow - // as many packages as possible inside the queue. - let buff_def = [ - Bytes::new(mem::size_of::()).unwrap(), - Bytes::new(65550).unwrap(), - ]; - - let spec = if dev_cfg - .features - .is_feature(Features::VIRTIO_F_RING_INDIRECT_DESC) - { - BuffSpec::Indirect(&buff_def) - } else { - BuffSpec::Single(Bytes::new(mem::size_of::() + 65550).unwrap()) - }; - - let num_buff: u16 = vq.size().into(); - - for _ in 0..num_buff { - let buff_tkn = match vq.prep_buffer(Rc::clone(vq), None, Some(spec.clone())) { - Ok(tkn) => tkn, - Err(_vq_err) => { - error!("Setup of network queue failed, which should not happen!"); - panic!("setup of network queue failed!"); - } - }; + //((dev_cfg.raw.get_mtu() as usize + mem::size_of::()) / num_buff as usize) + // .align_up(core::mem::size_of::>()) + 1514 + mem::size_of::() + } else { + dev_cfg.raw.get_mtu() as usize + mem::size_of::() + }; - // BufferTokens are directly provided to the queue - // TransferTokens are directly dispatched - // Transfers will be awaited at the queue - buff_tkn - .provide() - .dispatch_await(Rc::clone(&self.poll_queue), false); - } + // See Virtio specification v1.1 - 5.1.6.3.1 + // + let spec = BuffSpec::Single(Bytes::new(rx_size).unwrap()); + /*let buff_def = [Bytes::new(rx_size).unwrap()]; + let spec = if dev_cfg + .features + .is_feature(Features::VIRTIO_F_RING_INDIRECT_DESC) + && + dev_cfg + .features + .is_feature(Features::VIRTIO_NET_F_MRG_RXBUF) + { + BuffSpec::Indirect(&buff_def) } else { - // If above features not set, buffers must be at least 1526 bytes large. - // See Virtio specification v1.1 - 5.1.6.3.1 - // - let buff_def = [ - Bytes::new(mem::size_of::()).unwrap(), - Bytes::new(dev_cfg.raw.get_mtu() as usize).unwrap(), - ]; - let spec = if dev_cfg - .features - .is_feature(Features::VIRTIO_F_RING_INDIRECT_DESC) - { - BuffSpec::Indirect(&buff_def) - } else { - BuffSpec::Single( - Bytes::new(mem::size_of::() + dev_cfg.raw.get_mtu() as usize) - .unwrap(), - ) + BuffSpec::Single(buff_def[0]) + };*/ + + for _ in 0..num_buff { + let buff_tkn = match vq.prep_buffer(Rc::clone(vq), None, Some(spec.clone())) { + Ok(tkn) => tkn, + Err(_vq_err) => { + error!("Setup of network queue failed, which should not happen!"); + panic!("setup of network queue failed!"); + } }; - let num_buff: u16 = vq.size().into(); - - for _ in 0..num_buff { - let buff_tkn = match vq.prep_buffer(Rc::clone(vq), None, Some(spec.clone())) { - Ok(tkn) => tkn, - Err(_vq_err) => { - error!("Setup of network queue failed, which should not happen!"); - panic!("setup of network queue failed!"); - } - }; - - // BufferTokens are directly provided to the queue - // TransferTokens are directly dispatched - // Transfers will be awaited at the queue - buff_tkn - .provide() - .dispatch_await(Rc::clone(&self.poll_queue), false); - } + // BufferTokens are directly provided to the queue + // TransferTokens are directly dispatched + // Transfers will be awaited at the queue + buff_tkn + .provide() + .dispatch_await(Rc::clone(&self.poll_queue), false); } // Safe virtqueue @@ -631,42 +591,50 @@ impl NetworkDriver for VirtioNetDriver { let (_, recv_data_opt) = transfer.as_slices().unwrap(); let mut recv_data = recv_data_opt.unwrap(); - // If the given length is zero, we currently fail. - if recv_data.len() == 2 { - let recv_payload = recv_data.pop().unwrap(); - /*let header = recv_data.pop().unwrap(); - let header = unsafe { - const HEADER_SIZE: usize = mem::size_of::(); - core::mem::transmute::<[u8; HEADER_SIZE], VirtioNetHdr>( - header[..HEADER_SIZE].try_into().unwrap(), - ) - }; - trace!("Receive data with header {:?}", header);*/ - - let vec_data = recv_payload.to_vec(); - transfer - .reuse() - .unwrap() - .provide() - .dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false); + // If the given length isn't 1, we currently fail. + if recv_data.len() == 1 { + let mut vec_data: Vec = Vec::with_capacity(self.mtu.into()); + let num_buffers = { + let packet = recv_data.pop().unwrap(); + let header = unsafe { + const HEADER_SIZE: usize = mem::size_of::(); + core::mem::transmute::<[u8; HEADER_SIZE], VirtioNetHdr>( + packet[..HEADER_SIZE].try_into().unwrap(), + ) + }; + trace!("Header: {:?}", header); + let num_buffers = header.num_buffers; + + vec_data.extend_from_slice(&packet[mem::size_of::()..]); + transfer + .reuse() + .unwrap() + .provide() + .dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false); - Some((RxToken::new(vec_data), TxToken::new())) - } else if recv_data.len() == 1 { - let packet = recv_data.pop().unwrap(); - /*let header = unsafe { - const HEADER_SIZE: usize = mem::size_of::(); - core::mem::transmute::<[u8; HEADER_SIZE], VirtioNetHdr>( - packet[..HEADER_SIZE].try_into().unwrap(), - ) + num_buffers }; - trace!("Receive data with header {:?}", header);*/ - let vec_data = packet[mem::size_of::()..].to_vec(); - transfer - .reuse() - .unwrap() - .provide() - .dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false); + for _ in 1..num_buffers { + let transfer = + match RxQueues::post_processing(self.recv_vqs.get_next().unwrap()) { + Ok(trf) => trf, + Err(vnet_err) => { + warn!("Post processing failed. Err: {:?}", vnet_err); + return None; + } + }; + + let (_, recv_data_opt) = transfer.as_slices().unwrap(); + let mut recv_data = recv_data_opt.unwrap(); + let packet = recv_data.pop().unwrap(); + vec_data.extend_from_slice(packet); + transfer + .reuse() + .unwrap() + .provide() + .dispatch_await(Rc::clone(&self.recv_vqs.poll_queue), false); + } Some((RxToken::new(vec_data), TxToken::new())) } else { @@ -833,6 +801,8 @@ impl VirtioNetDriver { feats.push(Features::VIRTIO_NET_F_GUEST_CSUM); // Host should avoid the creation of checksums feats.push(Features::VIRTIO_NET_F_CSUM); + // Driver can merge receive buffers + feats.push(Features::VIRTIO_NET_F_MRG_RXBUF); // Currently the driver does NOT support the features below. // In order to provide functionality for these, the driver diff --git a/src/drivers/virtio/virtqueue/split.rs b/src/drivers/virtio/virtqueue/split.rs index 46f86e1329..b5de013e30 100644 --- a/src/drivers/virtio/virtqueue/split.rs +++ b/src/drivers/virtio/virtqueue/split.rs @@ -8,7 +8,6 @@ use alloc::rc::Rc; use alloc::vec::Vec; use core::cell::RefCell; use core::ptr; -use core::sync::atomic::{fence, Ordering}; use align_address::Align; @@ -21,6 +20,7 @@ use super::{ AsSliceU8, BuffSpec, Buffer, BufferToken, Bytes, DescrFlags, MemDescr, MemPool, Pinned, Transfer, TransferState, TransferToken, Virtq, VqIndex, VqSize, }; +use crate::arch::memory_barrier; use crate::arch::mm::paging::{BasePageSize, PageSize}; use crate::arch::mm::{paging, VirtAddr}; @@ -57,7 +57,7 @@ struct AvailRing { struct UsedRing { flags: &'static mut u16, - index: &'static mut u16, + index: *mut u16, ring: &'static mut [UsedElem], event: &'static mut u16, } @@ -193,14 +193,14 @@ impl DescrRing { self.avail_ring.ring[*self.avail_ring.index as usize % self.avail_ring.ring.len()] = index as u16; - fence(Ordering::SeqCst); + memory_barrier(); *self.avail_ring.index = self.avail_ring.index.wrapping_add(1); (pin, 0, 0) } fn poll(&mut self) { - while self.read_idx != *self.used_ring.index { + while self.read_idx != unsafe { ptr::read_volatile(self.used_ring.index) } { let cur_ring_index = self.read_idx as usize % self.used_ring.ring.len(); let used_elem = self.used_ring.ring[cur_ring_index]; @@ -426,7 +426,7 @@ impl SplitVq { let used_ring = unsafe { UsedRing { flags: &mut *(used_raw as *mut u16), - index: &mut *(used_raw.offset(2) as *mut u16), + index: used_raw.offset(2) as *mut u16, ring: core::slice::from_raw_parts_mut( (used_raw.offset(4) as *const _) as *mut UsedElem, size as usize, From 5640f15caf5917f906f385babc3a04638eddc05d Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Thu, 17 Aug 2023 09:43:42 +0200 Subject: [PATCH 2/2] use busy waiting to receive data after 2 seconds RustyHermit switch to a blocking mode --- src/drivers/net/virtio_net.rs | 19 ++----------------- src/drivers/virtio/virtqueue/split.rs | 3 ++- src/fd/socket/tcp.rs | 10 ++++++++-- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/drivers/net/virtio_net.rs b/src/drivers/net/virtio_net.rs index da559bc667..e4d0af591f 100644 --- a/src/drivers/net/virtio_net.rs +++ b/src/drivers/net/virtio_net.rs @@ -198,9 +198,8 @@ impl RxQueues { .features .is_feature(Features::VIRTIO_NET_F_MRG_RXBUF) { - //((dev_cfg.raw.get_mtu() as usize + mem::size_of::()) / num_buff as usize) - // .align_up(core::mem::size_of::>()) - 1514 + mem::size_of::() + (1514 + mem::size_of::()) + .align_up(core::mem::size_of::>()) } else { dev_cfg.raw.get_mtu() as usize + mem::size_of::() }; @@ -208,20 +207,6 @@ impl RxQueues { // See Virtio specification v1.1 - 5.1.6.3.1 // let spec = BuffSpec::Single(Bytes::new(rx_size).unwrap()); - /*let buff_def = [Bytes::new(rx_size).unwrap()]; - let spec = if dev_cfg - .features - .is_feature(Features::VIRTIO_F_RING_INDIRECT_DESC) - && - dev_cfg - .features - .is_feature(Features::VIRTIO_NET_F_MRG_RXBUF) - { - BuffSpec::Indirect(&buff_def) - } else { - BuffSpec::Single(buff_def[0]) - };*/ - for _ in 0..num_buff { let buff_tkn = match vq.prep_buffer(Rc::clone(vq), None, Some(spec.clone())) { Ok(tkn) => tkn, diff --git a/src/drivers/virtio/virtqueue/split.rs b/src/drivers/virtio/virtqueue/split.rs index b5de013e30..308d72e307 100644 --- a/src/drivers/virtio/virtqueue/split.rs +++ b/src/drivers/virtio/virtqueue/split.rs @@ -202,7 +202,7 @@ impl DescrRing { fn poll(&mut self) { while self.read_idx != unsafe { ptr::read_volatile(self.used_ring.index) } { let cur_ring_index = self.read_idx as usize % self.used_ring.ring.len(); - let used_elem = self.used_ring.ring[cur_ring_index]; + let used_elem = unsafe { ptr::read_volatile(&self.used_ring.ring[cur_ring_index]) }; let tkn = unsafe { &mut *(self.ref_ring[used_elem.id as usize]) }; @@ -225,6 +225,7 @@ impl DescrRing { } None => tkn.state = TransferState::Finished, } + memory_barrier(); self.read_idx = self.read_idx.wrapping_add(1); } } diff --git a/src/fd/socket/tcp.rs b/src/fd/socket/tcp.rs index 9fbefe1153..3574a05c15 100644 --- a/src/fd/socket/tcp.rs +++ b/src/fd/socket/tcp.rs @@ -248,7 +248,7 @@ impl Socket { let slice = unsafe { core::slice::from_raw_parts_mut(buf, len) }; if self.nonblocking.load(Ordering::Acquire) { - block_on(self.async_read(slice), Some(Duration::ZERO)).unwrap_or_else(|x| { + poll_on(self.async_read(slice), Some(Duration::ZERO)).unwrap_or_else(|x| { if x == -ETIME { (-EAGAIN).try_into().unwrap() } else { @@ -256,7 +256,13 @@ impl Socket { } }) } else { - block_on(self.async_read(slice), None).unwrap_or_else(|x| x.try_into().unwrap()) + poll_on(self.async_read(slice), Some(Duration::from_secs(2))).unwrap_or_else(|x| { + if x == -ETIME { + block_on(self.async_read(slice), None).unwrap_or_else(|y| y.try_into().unwrap()) + } else { + x.try_into().unwrap() + } + }) } }