Add VIRTIO_NET_F_MRG_RXBUF to virtio-net #4658
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ShadowCurse
force-pushed
the
net_mrg_buf
branch
5 times, most recently
from
b200d76 to
ccda0b0
Compare
ShadowCurse
force-pushed
the
net_mrg_buf
branch
6 times, most recently
from
cdc6f02 to
fde7478
Compare
ShadowCurse
force-pushed
the
net_mrg_buf
branch
8 times, most recently
from
a4168d1 to
64616a4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4658 +/- ##
==========================================
+ Coverage 84.37% 84.50% +0.12%
==========================================
Files 249 250 +1
Lines 27433 27678 +245
==========================================
+ Hits 23147 23388 +241
- Misses 4286 4290 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ShadowCurse
force-pushed
the
net_mrg_buf
branch
4 times, most recently
from
05923db to
7f94f6a
Compare
ShadowCurse
added
Status: Awaiting review
ShadowCurse
force-pushed
the
net_mrg_buf
branch
5 times, most recently
from
c6ceb46 to
719dbdb
Compare
Now IoVecBufferMut can be reloaded from DescriptorChain same way as IoVecBuffer does it. This is helpful to avoid unnecessary allocations/deallocations when reusing same buffer. Signed-off-by: Egor Lazarchuk <[email protected]>
Replace numbers with more descriptive `size_of` methods. Signed-off-by: Egor Lazarchuk <[email protected]>
`Self::default()` is a bit more compact. Signed-off-by: Egor Lazarchuk <[email protected]>
Move used ring update into a separate method. This will help with preprocessing of descriptor chains in later commits where we will advance used ring once after processing multiple descriptors. Signed-off-by: Egor Lazarchuk <[email protected]>
Now `write_used_ring` makes sure the index is in correct bounds. This removes a need for the caller to make sure the index is in bounds. It will be used in the later commits to discard used descriptors. Signed-off-by: Egor Lazarchuk <[email protected]>
ShadowCurse
force-pushed
the
net_mrg_buf
branch
4 times, most recently
from
cd9e706 to
68db1b0
Compare
Now we will know the index of the head descriptor from which the IoVecBufferMut was build. Signed-off-by: Egor Lazarchuk <[email protected]>
Add methods to obtain head index and pointer to the first buffer from `IoVecBufferMut`. These methods will be used in later commits. Signed-off-by: Egor Lazarchuk <[email protected]>
Add `RingBuffer` data structure. It will be used in later commits. Signed-off-by: Egor Lazarchuk <[email protected]>
Add `read_used_ring` to read used element from a used ring. This is needed for `discard_used`. Add `discard_used` method to discard last `n` used elements in the used ring by setting their `len` to 0. Signed-off-by: Egor Lazarchuk <[email protected]>
Make some structs/fields public as following commits will need this. Signed-off-by: Egor Lazarchuk <[email protected]>
Now virtio-net device can split incoming packets across multiple descriptor chains if VIRTIO_NET_F_MRG_RXBUF is enabled by the guest. The amount of descriptor chains (also known as heads) is written into the `virtio_net_hdr_v1` structure which is located at the very begging of the packet. Virtio spec states that the number of heads used should always be correct: - 1 - if VIRTIO_NET_F_MRG_RXBUF is not negotiated - N - if VIRTIO_NET_F_MRG_RXBUF is negotiated Prior to this commit Firecracker never set the number of used heads to 1, but Linux was fine with it. Now Firecracker always sets correct number of heads. Because of this, some changes were introduced into the unit test code that was generating testing frames. Additionally, because processing of descriptors was taking a big chunk of total time spend in the RX packet processing, move reading of descriptors to the preprocessing step performed when guest notifies Firecracker about new descriptors available for RX queue. Signed-off-by: Egor Lazarchuk <[email protected]>
`read_obj` takes too much time and we don't need it's checks as we do them before the actual reading happens. To help with perpormance, replace it with either `get_slice` or `get_host_address`. Signed-off-by: Egor Lazarchuk <[email protected]>
ShadowCurse
force-pushed
the
net_mrg_buf
branch
from
68db1b0 to
70bce30
Compare
Signed-off-by: Egor Lazarchuk <[email protected]>
ShadowCurse
force-pushed
the
net_mrg_buf
branch
from
26a1394 to
b61f394
Compare
9 tasks
|
Closing in favor of #4834 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Add
VIRTIO_NET_F_MRG_RXBUFto virtio-net.Now virtio-net device can split incoming packets across multiple descriptor chains if
VIRTIO_NET_F_MRG_RXBUFis enabled by the guest. The amount of descriptor chains (also known as heads) is written into thevirtio_net_hdr_v1structure which is located at the very begging of the packet. Virtio spec states that the number of heads used should always be correct:VIRTIO_NET_F_MRG_RXBUFis not negotiatedVIRTIO_NET_F_MRG_RXBUFis negotiatedPrior to this commit Firecracker never set the number of used heads to 1, but Linux was fine with it. Now we always set correct number of heads. Because of this some changes were introduced into the unit test code that was generating testing frames.
Fixes: #1314
Reason
Better performance and guest memory utilization.
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
PR.
CHANGELOG.md.TODOs link to an issue.contribution quality standards.
rust-vmm.