Skip to content

Commit f7dcab7

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 9d0851e commit f7dcab7

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
@@ -61,6 +61,8 @@ pub struct RxBuffer {
6161
pub iovecs: IovRingBuffer,
6262
// Ring buffer of meta data about descriptor chains stored in the `iov_ring`.
6363
pub chain_infos: RingBuffer<ChainInfo>,
64+
// Total capacity this buffer holds.
65+
pub total_capacity: u32,
6466
// Number of descriptor chains we have used to process packets.
6567
pub used_descriptors: u16,
6668
}
@@ -72,6 +74,7 @@ impl RxBuffer {
7274
iovecs: IovRingBuffer::new()?,
7375
chain_infos: RingBuffer::new_with_size(u32::from(FIRECRACKER_MAX_QUEUE_SIZE)),
7476
used_descriptors: 0,
77+
total_capacity: 0,
7578
})
7679
}
7780

@@ -93,8 +96,12 @@ impl RxBuffer {
9396

9497
/// Returns a slice of underlying iovec for the all chains
9598
/// in the buffer.
96-
pub fn all_chains_mut_slice(&mut self) -> &mut [iovec] {
97-
self.iovecs.as_mut_slice()
99+
pub fn all_chains_mut_slice(&mut self) -> Option<&mut [iovec]> {
100+
if self.total_capacity < u32::from(u16::MAX) {
101+
None
102+
} else {
103+
Some(self.iovecs.as_mut_slice())
104+
}
98105
}
99106

100107
/// Add a new `DescriptorChain` that we received from the RX queue in the buffer.
@@ -151,6 +158,7 @@ impl RxBuffer {
151158
chain_len,
152159
chain_capacity,
153160
});
161+
self.total_capacity += chain_capacity;
154162

155163
Ok(())
156164
}
@@ -210,6 +218,7 @@ impl RxBuffer {
210218
.pop_front()
211219
.expect("This should never happen if write to the buffer succeded.");
212220
self.iovecs.pop_front(usize::from(iov_info.chain_len));
221+
self.total_capacity -= iov_info.chain_capacity;
213222

214223
if bytes_written <= iov_info.chain_capacity {
215224
write_used(iov_info.head_index, bytes_written, rx_queue);
@@ -255,7 +264,7 @@ mod tests {
255264

256265
#[test]
257266
fn test_rx_buffer_add_chain() {
258-
let mem = single_region_mem(65562);
267+
let mem = single_region_mem(65562 * 2);
259268
let rxq = VirtQueue::new(GuestAddress(0), &mem, 256);
260269
let mut queue = rxq.create_queue();
261270

@@ -290,9 +299,10 @@ mod tests {
290299
);
291300
}
292301

293-
// 16 chains of len 1
302+
// 64 chains of len 1 and size 1024
303+
// in total 64K
294304
{
295-
let chains = 16;
305+
let chains = 64;
296306
set_dtable_many_chains(&rxq, chains);
297307
queue.next_avail = Wrapping(0);
298308
let mut buff = RxBuffer::new().unwrap();
@@ -302,7 +312,7 @@ mod tests {
302312
buff.add_chain(&mem, desc).unwrap();
303313
}
304314
}
305-
let slice = buff.all_chains_mut_slice();
315+
let slice = buff.all_chains_mut_slice().unwrap();
306316
for i in 0..chains {
307317
assert_eq!(
308318
slice[i].iov_base as u64,
@@ -311,7 +321,7 @@ mod tests {
311321
);
312322
assert_eq!(slice[i].iov_len, 1024);
313323
}
314-
assert_eq!(buff.chain_infos.len(), 16);
324+
assert_eq!(buff.chain_infos.len(), chains as u32);
315325
for (i, ci) in buff.chain_infos.items[0..16].iter().enumerate() {
316326
assert_eq!(
317327
*ci,
@@ -380,11 +390,11 @@ mod tests {
380390

381391
#[test]
382392
fn test_rx_buffer_write_mrg_buf() {
383-
let mem = single_region_mem(65562);
393+
let mem = single_region_mem(65562 * 2);
384394
let rxq = VirtQueue::new(GuestAddress(0), &mem, 256);
385395
let mut queue = rxq.create_queue();
386396

387-
set_dtable_many_chains(&rxq, 2);
397+
set_dtable_many_chains(&rxq, 64);
388398

389399
let mut buff = RxBuffer::new().unwrap();
390400
while let Some(desc) = queue.pop() {
@@ -395,7 +405,7 @@ mod tests {
395405
}
396406

397407
// Initially data should be all zeros
398-
let slice = buff.all_chains_mut_slice();
408+
let slice = buff.all_chains_mut_slice().unwrap();
399409
let data_slice_before: &[u8] =
400410
// SAFETY: safe as iovecs are verified on creation.
401411
unsafe { std::slice::from_raw_parts(slice[0].iov_base.cast(), slice[0].iov_len) };
@@ -408,7 +418,7 @@ mod tests {
408418
// Write should hapepn to all 2 iovecs
409419
buff.write(&[69; 2 * 1024]).unwrap();
410420

411-
let slice = buff.all_chains_mut_slice();
421+
let slice = buff.all_chains_mut_slice().unwrap();
412422
let data_slice_after: &[u8] =
413423
// SAFETY: safe as iovecs are verified on creation.
414424
unsafe { std::slice::from_raw_parts(slice[0].iov_base.cast(), slice[0].iov_len) };

0 commit comments

Comments
 (0)