From f89bdce7f0b661c96966caecc6174f3550c2e652 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Joakim=20Frosteg=C3=A5rd?= <joakim.frostegard@gmail.com>
Date: Thu, 9 Mar 2023 21:28:48 +0100
Subject: [PATCH] udp: uring: change SendBuffer unsafe declarations, add
 comments

---
 aquatic_udp/src/workers/socket/uring/mod.rs   |  9 ++-
 .../src/workers/socket/uring/send_buffers.rs  | 59 ++++++++++---------
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/aquatic_udp/src/workers/socket/uring/mod.rs b/aquatic_udp/src/workers/socket/uring/mod.rs
index 7adf062..d4c8374 100644
--- a/aquatic_udp/src/workers/socket/uring/mod.rs
+++ b/aquatic_udp/src/workers/socket/uring/mod.rs
@@ -320,8 +320,13 @@ impl SocketWorker {
                             response_counter.fetch_add(1, Ordering::Relaxed);
                         }
 
-                        self.send_buffers
-                            .mark_index_as_free(send_buffer_index as usize);
+                        // Safety: OK because cqe using buffer has been
+                        // returned and contents will no longer be accessed
+                        // by kernel
+                        unsafe {
+                            self.send_buffers
+                                .mark_index_as_free(send_buffer_index as usize);
+                        }
                     }
                 }
             }
diff --git a/aquatic_udp/src/workers/socket/uring/send_buffers.rs b/aquatic_udp/src/workers/socket/uring/send_buffers.rs
index 38fe6c5..b6fdd21 100644
--- a/aquatic_udp/src/workers/socket/uring/send_buffers.rs
+++ b/aquatic_udp/src/workers/socket/uring/send_buffers.rs
@@ -39,9 +39,9 @@ struct SendBuffer {
     iovec: UnsafeCell<libc::iovec>,
     msghdr: UnsafeCell<libc::msghdr>,
     free: bool,
-    // Only used for statistics
+    /// Only used for statistics
     receiver_is_ipv4: bool,
-    // Only used for statistics
+    /// Only used for statistics
     response_type: ResponseType,
 }
 
@@ -81,32 +81,34 @@ impl SendBuffer {
         }
     }
 
-    /// # Safety
-    ///
-    /// - SendBuffer must be stored at a fixed location in memory
-    unsafe fn setup_pointers(&mut self, socket_is_ipv4: bool) {
-        let iovec = &mut *self.iovec.get();
+    fn setup_pointers(&mut self, socket_is_ipv4: bool) {
+        unsafe {
+            let iovec = &mut *self.iovec.get();
 
-        iovec.iov_base = self.bytes.get() as *mut libc::c_void;
-        iovec.iov_len = (&*self.bytes.get()).len();
+            iovec.iov_base = self.bytes.get() as *mut libc::c_void;
+            iovec.iov_len = (&*self.bytes.get()).len();
 
-        let msghdr = &mut *self.msghdr.get();
+            let msghdr = &mut *self.msghdr.get();
 
-        msghdr.msg_iov = self.iovec.get();
+            msghdr.msg_iov = self.iovec.get();
 
-        if socket_is_ipv4 {
-            msghdr.msg_name = self.name_v4.get() as *mut libc::c_void;
-            msghdr.msg_namelen = core::mem::size_of::<libc::sockaddr_in>() as u32;
-        } else {
-            msghdr.msg_name = self.name_v6.get() as *mut libc::c_void;
-            msghdr.msg_namelen = core::mem::size_of::<libc::sockaddr_in6>() as u32;
+            if socket_is_ipv4 {
+                msghdr.msg_name = self.name_v4.get() as *mut libc::c_void;
+                msghdr.msg_namelen = core::mem::size_of::<libc::sockaddr_in>() as u32;
+            } else {
+                msghdr.msg_name = self.name_v6.get() as *mut libc::c_void;
+                msghdr.msg_namelen = core::mem::size_of::<libc::sockaddr_in6>() as u32;
+            }
         }
     }
 
     /// # Safety
     ///
     /// - SendBuffer must be stored at a fixed location in memory
-    /// - SendBuffer.setup_pointers must have been called previously
+    /// - SendBuffer.setup_pointers must have been called while stored at that
+    ///   fixed location
+    /// - Contents of struct fields wrapped in UnsafeCell can NOT be accessed
+    ///   simultaneously to this function call
     unsafe fn prepare_entry(
         &mut self,
         response: &Response,
@@ -128,6 +130,7 @@ impl SendBuffer {
             name.sin_port = addr.port().to_be();
             name.sin_addr.s_addr = u32::from(*addr.ip()).to_be();
         } else {
+            // Set receiver protocol type before calling addr.get_ipv6_mapped()
             self.receiver_is_ipv4 = addr.is_ipv4();
 
             let addr = if let SocketAddr::V6(addr) = addr.get_ipv6_mapped() {
@@ -153,9 +156,7 @@ impl SendBuffer {
                 self.response_type = ResponseType::from_response(response);
                 self.free = false;
 
-                let sqe = SendMsg::new(SOCKET_IDENTIFIER, self.msghdr.get()).build();
-
-                Ok(sqe)
+                Ok(SendMsg::new(SOCKET_IDENTIFIER, self.msghdr.get()).build())
             }
             Err(err) => Err(Error::SerializationFailed(err)),
         }
@@ -178,10 +179,7 @@ impl SendBuffers {
             .into_boxed_slice();
 
         for buffer in buffers.iter_mut() {
-            // Safety: OK because buffers are stored in fixed memory location
-            unsafe {
-                buffer.setup_pointers(socket_is_ipv4);
-            }
+            buffer.setup_pointers(socket_is_ipv4);
         }
 
         Self {
@@ -197,7 +195,11 @@ impl SendBuffers {
         (buffer.response_type, buffer.receiver_is_ipv4)
     }
 
-    pub fn mark_index_as_free(&mut self, index: usize) {
+    /// # Safety
+    ///
+    /// Only safe to call once buffer is no longer referenced by in-flight
+    /// io_uring queue entries
+    pub unsafe fn mark_index_as_free(&mut self, index: usize) {
         self.buffers[index].free = true;
     }
 
@@ -215,8 +217,9 @@ impl SendBuffers {
 
         let buffer = self.buffers.index_mut(index);
 
-        // Safety: OK because buffers are stored in fixed memory location
-        // and buffer pointers were set up in SendBuffers::new()
+        // Safety: OK because buffers are stored in fixed memory location,
+        // buffer pointers were set up in SendBuffers::new() and pointers to
+        // SendBuffer UnsafeCell contents are not accessed elsewhere
         unsafe {
             match buffer.prepare_entry(response, addr, self.socket_is_ipv4) {
                 Ok(entry) => {