latest work

Signed-off-by: aerosouund <aerosound161@gmail.com>

Author: aerosouund

src/vmm/src/devices/virtio/net/device.rs

@@ -63,6 +63,7 @@ const fn frame_hdr_len() -> usize {
 
 // ammar: maybe reduce visibility of those types when done
 #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)]
+#[serde(rename_all = "snake_case")]
 pub enum NetDevBackendType {
     Socket(String), // the string denotes the socket path
     Tap(String),    // denotes the tap device name
@@ -245,7 +246,7 @@ pub struct Net {
     pub(crate) id: String,
 
     /// The backend for this device: a tap.
-    pub tap: Box<dyn NetDevBackend>,
+    pub backend: Box<dyn NetDevBackend>,
 
     pub(crate) avail_features: u64,
     pub(crate) acked_features: u64,
@@ -285,6 +286,7 @@ impl Net {
         tx_rate_limiter: RateLimiter,
     ) -> Result<Self, NetError> {
         // ammar: what are those features ?
+        // what should we advertise for the stream backend ? the same stuff ?
         let mut avail_features = (1 << VIRTIO_F_VERSION_1)
             | (1 << VIRTIO_NET_F_MRG_RXBUF)
             | (1 << VIRTIO_RING_F_EVENT_IDX);
@@ -306,7 +308,7 @@ impl Net {
 
         Ok(Net {
             id: id.clone(),
-            tap: backend,
+            backend: backend,
             avail_features,
             acked_features: 0u64,
             queues,
@@ -337,12 +339,7 @@ impl Net {
         tx_rate_limiter: RateLimiter,
         backend_type: NetDevBackendType,
     ) -> Result<Self, NetError> {
-        // ammar: here we should do an enum check on the type of the backend to see how to construct the netdev
-
         let vnet_hdr_size = i32::try_from(vnet_hdr_len()).unwrap();
-        // ammar: there is a set header length size. so the backend we will implement should respect that and only return to us bytes
-        // per read as much as this header size
-
         let backend: Box<dyn NetDevBackend> = match backend_type {
             // ammar: use something other than io error as the return of socket backend creation
             NetDevBackendType::Socket(path) => {
@@ -366,9 +363,9 @@ impl Net {
     }
 
     /// Provides the host IFACE name of this net device.
+    /// ammar: maybe this function should be called something else ?
     pub fn iface_name(&self) -> String {
-        // ammar: what should iface name in the non tap case ? this will matter in persistence
-        self.tap.identifier().to_string()
+        self.backend.identifier().to_string()
     }
 
     /// Provides the MmdsNetworkStack of this net device.
@@ -762,7 +759,7 @@ impl Net {
                 &mut self.tx_rate_limiter,
                 &mut self.tx_frame_headers,
                 &self.tx_buffer,
-                &mut self.tap,
+                &mut self.backend,
                 self.guest_mac,
                 &self.metrics,
             )
@@ -856,7 +853,7 @@ impl Net {
         } else {
             self.rx_buffer.single_chain_slice_mut()
         };
-        self.tap.read_iovec(slice)
+        self.backend.read_iovec(slice)
     }
 
     fn write_backend(
@@ -1058,8 +1055,8 @@ impl VirtioDevice for Net {
 
         // ammar: we should try to fully understand offload
         let supported_flags: u32 = Net::build_tap_offload_features(self.acked_features);
-        if let NetDevBackendType::Tap(_) = self.tap.save() {
-            self.tap
+        if let NetDevBackendType::Tap(_) = self.backend.save() {
+            self.backend
                 .set_offload(supported_flags)
                 .map_err(super::super::ActivateError::TapSetOffload)?;
         }

src/vmm/src/devices/virtio/net/event_handler.rs

@@ -46,10 +46,10 @@ impl Net {
         )) {
             error!("Failed to register tx queue event: {}", err);
         }
-        let fd = &self.tap.as_raw_fd();
+        let fd = &self.backend.as_raw_fd();
         if let Err(err) = ops.add(Events::with_data(
             fd,
-            Self::PROCESS_TAP_RX,
+            Self::PROCESS_TAP_RX, // ammar: should we just keep calling this tap ?
             EventSet::IN | EventSet::EDGE_TRIGGERED,
         )) {
             error!("Failed to register tap event: {}", err);

src/vmm/src/devices/virtio/net/persist.rs

@@ -86,7 +86,7 @@ impl Persist<'_> for Net {
                 guest_mac: self.guest_mac,
             },
             virtio_state: VirtioDeviceState::from_device(self),
-            backend_type: self.tap.save(),
+            backend_type: self.backend.save(),
         }
     }
 

src/vmm/src/devices/virtio/net/tap.rs

@@ -123,7 +123,6 @@ pub trait NetDevBackend: Debug + Send + Sync + AsRawFd {
     // if name as str no. but there should be a way to store and retrieve the interface name for persistence
     // set_offload ? idk, don't really understand
     // read and write iovec ? for sure
-    fn set_vnet_hdr_size(&mut self, size: c_int) -> Result<(), TapError>;
     // check if we can make them default implementations
     fn read_iovec(&mut self, buffer: &mut [libc::iovec]) -> Result<usize, IoError>;
     // heyy
@@ -133,6 +132,8 @@ pub trait NetDevBackend: Debug + Send + Sync + AsRawFd {
     // fetch back the NetDevBackendType from the trait
     fn save(&self) -> NetDevBackendType;
     // set offload
+    // ammar: whats the fate of offload on the socket backend ? it should be: if you know your socket backend will
+    // handle offload, let it do that. otherwise, tell the vmm to do it itself. does qemu do that ?
     fn set_offload(&self, flags: c_uint) -> Result<(), TapError>;
 }
 
@@ -144,7 +145,7 @@ pub struct SocketBacked {
 }
 
 impl SocketBacked {
-    pub fn new(path: String) -> Result<Self, std::io::Error> {
+    pub fn new(path: String) -> Result<Self, IoError> {
         // open a socket and set its path to path
         let stream = UnixStream::connect(path.clone())?;
         stream.set_nonblocking(true)?;
@@ -163,15 +164,6 @@ impl AsRawFd for SocketBacked {
 }
 
 impl NetDevBackend for SocketBacked {
-    // if i need to make it change the header size then won't i need a mutable reference ? if i
-    // make the tap function take a mutable reference would this fuck up ownership somewhere ?
-    // nah didn't cause much damage, just needed to declare the tap as mut
-    // maybe we don't need that
-    fn set_vnet_hdr_size(&mut self, size: c_int) -> Result<(), TapError> {
-        self.hdr_size = size;
-        Ok(())
-    }
-
     fn read_iovec(&mut self, buffer: &mut [libc::iovec]) -> Result<usize, IoError> {
         let iov = buffer.as_mut_ptr();
 
@@ -181,6 +173,9 @@ impl NetDevBackend for SocketBacked {
             iov_len: unsafe { (*iov).iov_len - vnet_hdr_len() },
         };
 
+        // ammar: we are saying this is a "Socket" backend. not a passt backend. so should we really
+        // be having passt specific handling ? also on some level how well do we even understand passt ?
+        // is using it as in the test the standard way even ?
         // create a 4 byte buffer, read the size that passt prepends into the packet and discard those
         // bytes. we only care about the data packets
         let mut len_buf = [0u8; 4];
@@ -220,13 +215,16 @@ impl NetDevBackend for SocketBacked {
         };
 
         let data_iov = unsafe { *(buffer.as_iovec_ptr()) };
+        // ammar: we need to check if we have a mininum of vnet hdr len bytes
         let data_iov = unsafe {
             libc::iovec {
-                iov_base: (data_iov.iov_base as *mut u8).add(12) as *mut core::ffi::c_void,
-                iov_len: data_iov.iov_len - 12,
+                iov_base: (data_iov.iov_base as *mut u8).add(vnet_hdr_len())
+                    as *mut core::ffi::c_void,
+                iov_len: data_iov.iov_len - vnet_hdr_len(),
             }
         };
 
+        // ammar: can this function absolutely guarantee it will get one iov in the buffer
         let iovs: [libc::iovec; 2] = [size_iov, data_iov];
         iovcnt += 1;
 
@@ -238,8 +236,7 @@ impl NetDevBackend for SocketBacked {
     }
 
     fn identifier(&self) -> String {
-        // ammar: fix. this sucks
-        self.path.to_str().unwrap().to_string()
+        self.path.display().to_string()
     }
 
     fn save(&self) -> NetDevBackendType {
@@ -252,19 +249,7 @@ impl NetDevBackend for SocketBacked {
     }
 }
 
-// should it be a trait or just an enum that contains both?
 impl NetDevBackend for Tap {
-    /// Set the size of the vnet hdr.
-    // ammar: on a socket backend this would just set a field on the backing struct
-    fn set_vnet_hdr_size(&mut self, size: c_int) -> Result<(), TapError> {
-        // SAFETY: ioctl is safe. Called with a valid tap fd, and we check the return.
-        if unsafe { ioctl_with_ref(&self.tap_file, TUNSETVNETHDRSZ(), &size) } < 0 {
-            return Err(TapError::SetSizeOfVnetHdr(IoError::last_os_error()));
-        }
-
-        Ok(())
-    }
-
     /// Write an `IoVecBuffer` to tap
     fn write_iovec(&mut self, buffer: &IoVecBuffer) -> Result<usize, IoError> {
         let iovcnt = i32::try_from(buffer.iovec_count()).unwrap();
@@ -359,6 +344,16 @@ impl Tap {
             .unwrap_or(IFACE_NAME_MAX_LEN);
         std::str::from_utf8(&self.if_name[..len]).unwrap_or("")
     }
+
+    /// Set the size of the vnet hdr.
+    pub fn set_vnet_hdr_size(&mut self, size: c_int) -> Result<(), TapError> {
+        // SAFETY: ioctl is safe. Called with a valid tap fd, and we check the return.
+        if unsafe { ioctl_with_ref(&self.tap_file, TUNSETVNETHDRSZ(), &size) } < 0 {
+            return Err(TapError::SetSizeOfVnetHdr(IoError::last_os_error()));
+        }
+
+        Ok(())
+    }
 }
 
 impl AsRawFd for Tap {

src/vmm/src/vmm_config/net.rs

@@ -45,7 +45,7 @@ impl From<&Net> for NetworkInterfaceConfig {
             guest_mac: net.guest_mac().copied(),
             rx_rate_limiter: rx_rl.into_option(),
             tx_rate_limiter: tx_rl.into_option(),
-            backend_type: net.tap.save(),
+            backend_type: net.backend.save(),
         }
     }
 }