Skip to content
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

vsock: make the descriptor chain parsing more generic #216

Open
lauralt opened this issue Jan 6, 2023 · 1 comment
Open

vsock: make the descriptor chain parsing more generic #216

lauralt opened this issue Jan 6, 2023 · 1 comment
Labels
help wanted Extra attention is needed

Comments

@lauralt
Copy link
Contributor

lauralt commented Jan 6, 2023

The device implementations that we have are Linux specific in terms of how their messages (packets for vsock, requests for block) are split between descriptors. For example, in the case of the vsock device the packet contents had the following distribution: the header on the first descriptor buffer, and the data on an optional second descriptor buffer. This was valid until Linux 6.2; starting with this version you can have both the header and data in a single descriptor (quick fix for this here). That is because the spec is not enforcing a specific layout for the messages, the only thing the descriptor chain layout needs to satisfy is: The driver MUST place any device-writable descriptor elements after any device-readable descriptor elements..

We should fix at least the vsock packet implementation (since the crate is already published and consumed) so that it can fit any valid message layout, i.e. header on any number of descriptors and same for the data buffer (which in terms of implementation translates to header and data having a Vec<VolatileSlice> as type instead of one VolatileSlice). A discussion about this was already started here.
I'm expecting the VsockPacket abstraction can't be nicely fixed without also introducing abstractions for parsing the descriptor chains, such as the Reader and Writer proposed here, other implementation for them can be found also here. One suggestion that I have is to resume (or at least look over) the PR linked above. By adding such abstractions, any other device implementation can be then easily switched to use them.

@lauralt lauralt added the help wanted Extra attention is needed label Jan 6, 2023
stefano-garzarella added a commit to stefano-garzarella/vhost-device that referenced this issue Jan 16, 2023
The 0.2.1 version of virtio-vsock crate contains a fix [1] needed to
properly work with the virtio-vsock driver provided by Linux v6.3 and
later (originally the new driver was supposed to be in v6.2, but it
was postponed).

The fix was just a quick workaround, but in the future more work will
be needed in the virtio-vsock crate to not have a Linux-only specific
implementation of VsockPacket, as described in this issue [2].

[1] rust-vmm/vm-virtio#207
[2] rust-vmm/vm-virtio#216

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
mathieupoirier pushed a commit to rust-vmm/vhost-device that referenced this issue Jan 19, 2023
The 0.2.1 version of virtio-vsock crate contains a fix [1] needed to
properly work with the virtio-vsock driver provided by Linux v6.3 and
later (originally the new driver was supposed to be in v6.2, but it
was postponed).

The fix was just a quick workaround, but in the future more work will
be needed in the virtio-vsock crate to not have a Linux-only specific
implementation of VsockPacket, as described in this issue [2].

[1] rust-vmm/vm-virtio#207
[2] rust-vmm/vm-virtio#216

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
@stefano-garzarella
Copy link
Member

I think we can use #278 to fix this issue.
I'll try in the next weeks, but any help is more than welcome :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants