-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Indirect desc #4734
Draft
ShadowCurse
wants to merge
16
commits into
firecracker-microvm:main
Choose a base branch
from
ShadowCurse:indirect_desc
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Indirect desc #4734
Conversation
This file contains 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
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]>
Add method to read used element from a used ring. 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 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 `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]>
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]>
VIRTIO_RING_F_INDIRECT_DESC is declared in the virtio_ring.h, so modify bindgen to parse all `VIRTIO_RING_` constants. Signed-off-by: Egor Lazarchuk <[email protected]>
Now IoVecBuffer/Mut can be built from descriptor chains utilizing VIRTQ_DESC_F_INDIRECT flag. The way indirect descriptors work is: - Descriptors from descriptor table instead of pointing to the buffers where data needs to be written to/read from now can point to buffers that contain other descriptor table. That 'indirect' descriptor table contains descriptor which point to actual buffers for data. - All descriptor in the 'indirect' descriptor table are processed sequentially. - The `VIRTQ_DESC_F_WRITE` flag is ignored for the main descriptor (the one from original descriptor table) Signed-off-by: Egor Lazarchuk <[email protected]>
Enable VIRTIO_RING_F_INDIRECT_DESC for virtio-net device. Signed-off-by: Egor Lazarchuk <[email protected]>
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 support for
INDIRECT_DESC
forIoVecBuff/Mut
and enabledVIRTIO_RING_F_INDIRECT_DESC
flag forvirtio-net
device. This feature allows guest to create more buffers for packets which in turn should help with throughput when all available descriptors are used.Reason
Should help with network performance.
Note
This PR is built on top of #4658, so needs to be merged after it.
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
.TODO
s link to an issue.contribution quality standards.
rust-vmm
.