[RFC] vhost-device-vsock: use Reader/Writer#954
[RFC] vhost-device-vsock: use Reader/Writer#954luigix25 wants to merge 8 commits intorust-vmm:mainfrom
Conversation
|
TODOs:
|
| /// - Err(Error::StreamWrite) if there was an error writing to the stream | ||
| fn send_bytes<B: BitmapSlice>(&mut self, buf: &VolatileSlice<B>) -> Result<()> { | ||
| fn send_bytes<B: BitmapSlice>(&mut self, reader: &mut Reader<B>, len: usize) -> Result<()> { | ||
| // TODO: Not sure this is the best way to do it. PTAL |
There was a problem hiding this comment.
I'm not using zerocopy here, as currently Reader/Writer do not support it. I think this probably the most "critical" problem of this PR as I'm introducing a copy in the hot-path.
Reader/Writer need to be modified. Any suggestion is welcome
| impl VsockStreamIo for StreamType { | ||
| /// Read from the stream into the guest buffer (H -> G) | ||
| fn stream_read<B: BitmapSlice>(&mut self, writer: &mut Writer<B>) -> Result<usize> { | ||
| //TODO: no zeropy is possible. |
luigix25
force-pushed
the
fix
branch
6 times, most recently
from
baca8c8 to
4335397
Compare
|
CI is now passing, except a warning on a commit message length. I guess we can ignore it at this stage? I had to bump all vm-memory versions to 0.18, because of a cargo problem. rust-lang/cargo#13594 (thanks @stefano-garzarella) This is because all virtio crates have been already bumped to 0.18. |
TBH I hate that rule we have, but it is what it is, so IMO better to fix now so we have a green CI since now.
IIUC your development branches in vhost/virtio/etc. already have vm-memory 0.18, so you need to update also here, right ? |
Ok!
Correct, this goes back to rust-vmm/vm-virtio#379 |
Add a push_bytes method alongside the existing push to support writing raw byte slices into the ring buffer. This is needed for the migration to the VsockPacketRx/Tx API where data is read into intermediate buffers before being pushed to the tx buffer. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Move HeadParams and prepare_desc_chain_vsock from vsock_conn tests into a shared test_utils module so they can be reused by thread_backend tests. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
All the following changes rely on PRs that are not yet merged. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Replace the unified VsockPacket type with the split VsockPacketTx and VsockPacketRx types from virtio-vsock, which use virtio-queue's Reader/Writer for guest memory access instead of VolatileSlice. Introduce a VsockStreamIo trait with stream_read() and stream_write() methods, replacing the direct ReadVolatile/WriteVolatile usage on StreamType. Update recv_pkt() to take a separate PacketHeader out-parameter so the header can be written back to guest memory by the caller. Rename the UnixRead error variant to StreamRead since it now covers both Unix and Vsock stream backends. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Remove the ReadVolatile and WriteVolatile trait implementations for StreamType and the push method from LocalTxBuf. These are no longer needed after the migration to VsockPacketRx/Tx which uses Reader/Writer instead of VolatileSlice. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Change the test helper to accept a data slice instead of a length, this allows to write a real payload into the descriptor chain memory. This enables some additional tests for send_pkt/recv_pkt. Remove the HeadParams wrapper in favor of passing head_len directly since the data length is now derived from the slice. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Apply cargo fmt suggestions. Not doing this in the "main" commit to make review a little bit easier and this code will be changes anyway. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
2 participants
Summary of the PR
Currently vhost-device-vsock doesn't work with upstream linux using a payload larger than 32kB.
(See #934)
This PR switches to the Reader/Writer removing the burden of handling descriptors.
This PR requires rust-vmm/vm-virtio#392 and rust-vmm/vhost#348
Closes: #934
This PR is very big and I'd like to gather some feedback around it.
Currently the biggest problem I see is about an extra copy I had to introduce because Reader/Writer do not support zerocopy.
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.