smoltcp icon indicating copy to clipboard operation
smoltcp copied to clipboard

RingBuffer exhausts when reading non-aligned chunks of data in bulk

Open JaciBrunning opened this issue 1 year ago • 1 comments

Consider the following socket reader, analogous to an asynchronous codec:

let data = socket.recv(|buffer| {
  let len = buffer.len();
  if len < 13 {
    return (0, None)
  }

  (13, Some(buffer[0..13].to_vec()))
});

With the current implementation of RingBuffer, the buffer will exhaust without wrapping around back to the 0th index. Inside of RingBuffer::deque_many_with, we can see that the implementation will attempt to wrap around only if the read data size is aligned with the total capacity of the buffer:

pub fn dequeue_many_with<'b, R, F>(&'b mut self, f: F) -> (usize, R)
where
    F: FnOnce(&'b mut [T]) -> (usize, R),
{
    let capacity = self.capacity();
    let max_size = cmp::min(self.len(), capacity - self.read_at);
    let (size, result) = f(&mut self.storage[self.read_at..self.read_at + max_size]);
    assert!(size <= max_size);
    self.read_at = if capacity > 0 {
        (self.read_at + size) % capacity
    } else {
        0
    };
    self.length -= size;
    (size, result)
}

With a capacity of 256, after the 19th iteration we get to read_at = 247, but max_size is only 9 - meaning the read function only gets 9 bytes to read, returning 0 for its size, failing to adjust read_at and resulting in the buffer exhausting itself even though there is a full frame of usable data in the buffer.

Unfortunately we can't immediately solve this with losing the contiguous property of the buffer being passed to f, putting to onus of keeping track of full data frames on the user which quickly becomes unwieldy and inefficient when multiple sockets are used. I can think of two reasonable solutions:

  • Implement a bip buffer
  • Pass data to the user in at least one but up to two halves, each of which are contiguous. If the current data is wholely contiguous within the circular buffer, give the user (&buffer[start..end], None). If the data is split across the end of the buffer, give the user (&buffer[start..capacity], Some(&buffer[0..end])). This doesn't fully solve the problem, but it avoids the user having to reserve a buffer per-socket for reading data and it can instead be done per-call if required.

JaciBrunning avatar Jun 11 '23 13:06 JaciBrunning