Skip to content

Conversation

bchalios
Copy link
Contributor

@bchalios bchalios commented Oct 8, 2024

The changes introduced in IoVecBufferMut that allowed implementing readv support for virtio-net regressed the performance of vsock device. This is because now, when we create IoVecBufferMut objects we do a bunch (1 memfd_create and 3 mmap) of system calls. virtio-net device avoids the performance issue because it creates a single IoVecBufferMut and re-uses it, whereas vsock is creating a new IoVecBufferMut object for every packet it receives.

We have a fix for this, which essentially creates a single IoVecBufferMut that reuses for all the incoming vsock packets during a connection. The problem with the fix is that it makes unit-tests really unhappy and we need a significant amount of work to fix them.

So, revert the PR to have main in a clean state. We will fix the vsock issues out-of-band and re-open the PR.

Reverts #4799

This reverts commit bc0ba43.

Signed-off-by: Babis Chalios <[email protected]>
This reverts commit 667aba4.

Signed-off-by: Babis Chalios <[email protected]>
… objects"

This reverts commit 14e6e33.

Signed-off-by: Babis Chalios <[email protected]>
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 96.55172% with 5 lines in your changes missing coverage. Please review.

Project coverage is 84.35%. Comparing base (6bfe235) to head (080577b).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/vmm/src/devices/virtio/net/device.rs 95.86% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4841      +/-   ##
==========================================
- Coverage   84.42%   84.35%   -0.07%     
==========================================
  Files         250      249       -1     
  Lines       27749    27505     -244     
==========================================
- Hits        23427    23202     -225     
+ Misses       4322     4303      -19     
Flag Coverage Δ
5.10-c5n.metal 84.58% <96.55%> (-0.08%) ⬇️
5.10-m5n.metal 84.57% <96.55%> (-0.08%) ⬇️
5.10-m6a.metal 83.86% <96.55%> (-0.09%) ⬇️
5.10-m6g.metal 80.92% <96.55%> (-0.12%) ⬇️
5.10-m6i.metal 84.57% <96.55%> (-0.08%) ⬇️
5.10-m7g.metal 80.92% <96.55%> (-0.12%) ⬇️
6.1-c5n.metal 84.59% <96.55%> (-0.08%) ⬇️
6.1-m5n.metal 84.57% <96.55%> (-0.07%) ⬇️
6.1-m6a.metal 83.87% <96.55%> (-0.09%) ⬇️
6.1-m6g.metal 80.92% <96.55%> (-0.12%) ⬇️
6.1-m6i.metal 84.56% <96.55%> (-0.08%) ⬇️
6.1-m7g.metal 80.92% <96.55%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bchalios bchalios force-pushed the revert-4799-net_rx_readv branch from bb531e1 to 080577b Compare October 8, 2024 10:22
@bchalios bchalios added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Oct 8, 2024
@bchalios bchalios merged commit 9b136c0 into main Oct 8, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting review Indicates that a pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants