more feedback

tools/xdp/s2n-quic-xdp/src/task/completion_to_tx.rs

@@ -33,7 +33,7 @@ pub async fn completion_to_tx<P: Poller>(
         frame_size.is_power_of_two(),
         tx_queues.len().is_power_of_two(),
     ) {
-        (0, _, _) => panic!("invalid tx_queues size"),
+        (0, _, _) => panic!("invalid must be non-zero length"),
         (1, _, _) => {
             trace!("using single queue mode");
             CompletionRingToTx {
@@ -101,7 +101,9 @@ pub async fn completion_to_tx<P: Poller>(
 
 /// Polls the completion queue for progress
 pub trait Poller: Unpin {
+    /// Polls the completion queue for progress
     fn poll(&mut self, comp: &mut ring::Completion, cx: &mut Context) -> Poll<Option<u32>>;
+    /// Releases `count` number of entries
     fn release(&mut self, comp: &mut ring::Completion, count: usize);
 }
 
@@ -271,7 +273,7 @@ impl<T: Txs, A: assign::Assign, P: Poller> Future for CompletionRingToTx<T, A, P
                                     .iter()
                                     .take(count as _)
                                     .copied()
-                                    .filter(|desc| assignment.assign(*desc, idx))
+                                    .filter(|desc| assignment.is_assigned(*desc, idx))
                                     .map(|desc| {
                                         trace!("assigning address {} to queue {idx}", desc.address);
 

tools/xdp/s2n-quic-xdp/src/task/completion_to_tx/assign.rs

@@ -9,12 +9,12 @@ use crate::if_xdp::UmemDescriptor;
 /// which TX queues get which descriptors. This trait takes in a descriptor and decides if it
 /// pertains to a worker index or not.
 pub trait Assign: Unpin {
-    fn assign(&self, desc: UmemDescriptor, idx: u64) -> bool;
+    fn is_assigned(&self, desc: UmemDescriptor, idx: u64) -> bool;
 }
 
 impl Assign for () {
     #[inline]
-    fn assign(&self, _desc: UmemDescriptor, idx: u64) -> bool {
+    fn is_assigned(&self, _desc: UmemDescriptor, idx: u64) -> bool {
         debug_assert_eq!(
             idx, 0,
             "assignment mode should only be used for single queue workflows"
@@ -33,7 +33,7 @@ pub struct AssignGeneric<F: FrameToIndex, I: IndexToQueue> {
 
 impl<F: FrameToIndex, I: IndexToQueue> Assign for AssignGeneric<F, I> {
     #[inline]
-    fn assign(&self, desc: UmemDescriptor, idx: u64) -> bool {
+    fn is_assigned(&self, desc: UmemDescriptor, idx: u64) -> bool {
         let v = self.frame.frame_to_index(desc);
         let v = self.index.index_to_queue(v);
         v == idx
@@ -51,7 +51,8 @@ pub struct AlignedFrame {
 
 impl AlignedFrame {
     pub fn new(frame_size: u32) -> Self {
-        debug_assert!(frame_size.is_power_of_two());
+        assert!(frame_size.is_power_of_two());
+        assert!(frame_size > 2, "cannot take the square root of 2");
 
         let shift = frame_size.trailing_zeros();
 
@@ -163,7 +164,7 @@ mod tests {
                 for offset in [0, 1, 2, (frame_size - 1) as _] {
                     let mut desc = desc;
                     desc.address += offset;
-                    let was_assigned = assigner.assign(desc, queue as _);
+                    let was_assigned = assigner.is_assigned(desc, queue as _);
                     assert_eq!(
                         is_expected, was_assigned,
                         "desc: {desc:?}, expected_queue: {expected_queue}, queue: {queue}"
@@ -192,7 +193,7 @@ mod tests {
 
         let is_expected = queue == expected_queue;
 
-        let was_assigned = assigner.assign(desc, queue);
+        let was_assigned = assigner.is_assigned(desc, queue);
         assert_eq!(is_expected, was_assigned,);
     }
 

tools/xdp/s2n-quic-xdp/src/task/rx.rs

@@ -298,6 +298,8 @@ impl<P: Poller, N: Notifier> Future for Rx<P, N> {
                 "the number of actual items should not exceed what was acquired"
             );
 
+            // While we have a `debug_assert` above, this is being overly defensive just in case.
+            // In regular conditions, it's equivalent to just releasing `actual`.
             let len = len.min(actual);
 
             // release the entries back to the RX ring