Skip to content

Commit 251417f

Browse files
committed
feat: fix retransmission issues with MRG_RXBUF
Fix retransmission issues introduced by enabling MRG_RXBUF. Do this by tracking total capacity of the `RxBuffer` struct and only return an iov slice if it contains at least 64K bytes. Signed-off-by: Egor Lazarchuk <[email protected]>
1 parent 0a03a33 commit 251417f

File tree

2 files changed

+25
-12
lines changed

2 files changed

+25
-12
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,10 @@ impl Net {
648648

649649
fn read_tap(&mut self) -> std::io::Result<usize> {
650650
if self.has_feature(u64::from(VIRTIO_NET_F_MRG_RXBUF)) {
651-
self.tap.read_iovec(self.rx_buffer.all_chains_mut_slice())
651+
let Some(s) = self.rx_buffer.all_chains_mut_slice() else {
652+
return Err(std::io::Error::from_raw_os_error(EAGAIN));
653+
};
654+
self.tap.read_iovec(s)
652655
} else {
653656
self.tap.read_iovec(self.rx_buffer.one_chain_mut_slice())
654657
}

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

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ pub struct RxBuffer {
8181
pub iovecs: IovRingBuffer,
8282
// Ring buffer of meta data about descriptor chains stored in the `iov_ring`.
8383
pub chain_infos: RingBuffer<ChainInfo>,
84+
// Total capacity this buffer holds.
85+
pub total_capacity: u32,
8486
// Number of descriptor chains we have used to process packets.
8587
pub used_descriptors: u16,
8688
}
@@ -92,6 +94,7 @@ impl RxBuffer {
9294
iovecs: IovRingBuffer::new()?,
9395
chain_infos: RingBuffer::new_with_size(u32::from(FIRECRACKER_MAX_QUEUE_SIZE)),
9496
used_descriptors: 0,
97+
total_capacity: 0,
9598
})
9699
}
97100

@@ -113,8 +116,12 @@ impl RxBuffer {
113116

114117
/// Returns a slice of underlying iovec for the all chains
115118
/// in the buffer.
116-
pub fn all_chains_mut_slice(&mut self) -> &mut [iovec] {
117-
self.iovecs.as_mut_slice()
119+
pub fn all_chains_mut_slice(&mut self) -> Option<&mut [iovec]> {
120+
if self.total_capacity < u32::from(u16::MAX) {
121+
None
122+
} else {
123+
Some(self.iovecs.as_mut_slice())
124+
}
118125
}
119126

120127
/// Add a new `DescriptorChain` that we received from the RX queue in the buffer.
@@ -171,6 +178,7 @@ impl RxBuffer {
171178
chain_len,
172179
chain_capacity,
173180
});
181+
self.total_capacity += chain_capacity;
174182

175183
Ok(())
176184
}
@@ -235,6 +243,7 @@ impl RxBuffer {
235243
.pop_front()
236244
.expect("This should never happen if write to the buffer succeded.");
237245
self.iovecs.pop_front(usize::from(iov_info.chain_len));
246+
self.total_capacity -= iov_info.chain_capacity;
238247

239248
if bytes_written <= iov_info.chain_capacity {
240249
write_used(iov_info.head_index, bytes_written, rx_queue);
@@ -282,7 +291,7 @@ mod tests {
282291

283292
#[test]
284293
fn test_rx_buffer_add_chain() {
285-
let mem = single_region_mem(65562);
294+
let mem = single_region_mem(65562 * 2);
286295
let rxq = VirtQueue::new(GuestAddress(0), &mem, 256);
287296
let mut queue = rxq.create_queue();
288297

@@ -317,9 +326,10 @@ mod tests {
317326
);
318327
}
319328

320-
// 16 chains of len 1
329+
// 64 chains of len 1 and size 1024
330+
// in total 64K
321331
{
322-
let chains = 16;
332+
let chains = 64;
323333
set_dtable_many_chains(&rxq, chains);
324334
queue.next_avail = Wrapping(0);
325335
let mut buff = RxBuffer::new().unwrap();
@@ -329,7 +339,7 @@ mod tests {
329339
buff.add_chain(&mem, desc).unwrap();
330340
}
331341
}
332-
let slice = buff.all_chains_mut_slice();
342+
let slice = buff.all_chains_mut_slice().unwrap();
333343
for i in 0..chains {
334344
assert_eq!(
335345
slice[i].iov_base as u64,
@@ -338,7 +348,7 @@ mod tests {
338348
);
339349
assert_eq!(slice[i].iov_len, 1024);
340350
}
341-
assert_eq!(buff.chain_infos.len(), 16);
351+
assert_eq!(buff.chain_infos.len(), chains as u32);
342352
for (i, ci) in buff.chain_infos.items[0..16].iter().enumerate() {
343353
assert_eq!(
344354
*ci,
@@ -407,11 +417,11 @@ mod tests {
407417

408418
#[test]
409419
fn test_rx_buffer_write_mrg_buf() {
410-
let mem = single_region_mem(65562);
420+
let mem = single_region_mem(65562 * 2);
411421
let rxq = VirtQueue::new(GuestAddress(0), &mem, 256);
412422
let mut queue = rxq.create_queue();
413423

414-
set_dtable_many_chains(&rxq, 2);
424+
set_dtable_many_chains(&rxq, 64);
415425

416426
let mut buff = RxBuffer::new().unwrap();
417427
while let Some(desc) = queue.pop() {
@@ -422,7 +432,7 @@ mod tests {
422432
}
423433

424434
// Initially data should be all zeros
425-
let slice = buff.all_chains_mut_slice();
435+
let slice = buff.all_chains_mut_slice().unwrap();
426436
let data_slice_before: &[u8] =
427437
// SAFETY: safe as iovecs are verified on creation.
428438
unsafe { std::slice::from_raw_parts(slice[0].iov_base.cast(), slice[0].iov_len) };
@@ -435,7 +445,7 @@ mod tests {
435445
// Write should hapepn to all 2 iovecs
436446
buff.write(&[69; 2 * 1024]).unwrap();
437447

438-
let slice = buff.all_chains_mut_slice();
448+
let slice = buff.all_chains_mut_slice().unwrap();
439449
let data_slice_after: &[u8] =
440450
// SAFETY: safe as iovecs are verified on creation.
441451
unsafe { std::slice::from_raw_parts(slice[0].iov_base.cast(), slice[0].iov_len) };

0 commit comments

Comments
 (0)