Skip to content

feat: Add Seqpacket support#5595

Open
aerosouund wants to merge 18 commits intofirecracker-microvm:mainfrom
aerosouund:vsock
Open

feat: Add Seqpacket support#5595
aerosouund wants to merge 18 commits intofirecracker-microvm:mainfrom
aerosouund:vsock

Conversation

@aerosouund
Copy link
Copy Markdown
Contributor

@aerosouund aerosouund commented Dec 20, 2025

Changes

There's currently no support for seqpacket based vsock connections in firecracker.
This PR adds it by creating an enum ConnBackend that implements VsockConnectionBackend and make its variants a unix stream or a seqpacket connection. The actual connection is handled by a module for seqpacket based connection that leverages libc calls.
It adds vsock_type as an api parameter to the vsock type.

Reason

Fixes: #4822

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

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

@aerosouund aerosouund force-pushed the vsock branch 22 times, most recently from 9cb7f64 to 4988a5e Compare December 26, 2025 18:31
@aerosouund aerosouund changed the title Vsock feat: Add Seqpacket support Dec 26, 2025
@aerosouund aerosouund marked this pull request as ready for review December 26, 2025 18:38
@JamesC1305 JamesC1305 self-assigned this Jan 8, 2026
@JamesC1305
Copy link
Copy Markdown
Contributor

Hi @aerosouund,

I'm in the process of checking through the full PR again, since it's been a while since I've looked at it. Just a note, I checked out your PR and the tests did not run. It seems like the 'vsock_type' field you introduced is missing from one of the constructors in a test. Could you please fix this in the meantime? Thanks

@aerosouund aerosouund requested a review from micz010 as a code owner April 10, 2026 19:08
@aerosouund
Copy link
Copy Markdown
Contributor Author

@JamesC1305 done

@aerosouund
Copy link
Copy Markdown
Contributor Author

@JamesC1305 fixed the merge conflicts

Copy link
Copy Markdown
Contributor

@JamesC1305 JamesC1305 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aerosouund,

Apologies for the delay in getting back to you, it has been a busy few weeks for us. I've given an initial pass to your PR. I suggest you go through commit-by-commit to read my comments, as that is how I wrote them.

For future revisions, could you please restructure the commits? This PR is very hard to follow at 31 commits. There are some points where you introduce bugs and fix them in a later commit. We want to avoid that scenario entirely, by fixing the bug at the source. Additionally, there are sometimes changes in unrelated commits, which makes it hard to see the final state of the code. If you could try to make everything as self-contained as possible that would be great, because it's quite difficult to follow as is.

Regardless, I've left some feedback on the lower-level implementation details. Once the low-hanging fruit is resolved, we can discuss the architectural flow.

Comment thread src/vmm/src/devices/virtio/vsock/unix/muxer.rs Outdated
Comment thread src/vmm/src/devices/virtio/vsock/unix/seqpacket.rs Outdated
Comment thread src/vmm/src/devices/virtio/vsock/unix/seqpacket.rs Outdated
Comment thread src/vmm/src/devices/virtio/vsock/unix/seqpacket.rs Outdated
Comment thread src/vmm/src/devices/virtio/vsock/unix/seqpacket.rs Outdated
Comment thread src/vmm/src/devices/virtio/vsock/csm/connection.rs Outdated
Comment thread src/firecracker/swagger/firecracker.yaml Outdated
Comment thread src/firecracker/swagger/firecracker.yaml
Comment thread src/vmm/src/devices/virtio/vsock/csm/connection.rs Outdated
Comment thread src/vmm/src/vmm_config/vsock.rs Outdated
The module has two types. SeqpacketConn and SeqpacketListener.
Calling accept() on the listener yields a conn. The types wrap
libc calls that open, bind to, read from seqpacket sockets and
implements traits required by connection consumers.

Signed-off-by: aerosouund <aerosound161@gmail.com>
The enum forwards any calls made from the functions
defined in the VsockConnectionBackend trait to the
backing connection. the types are stream and seqpacket
based connections.

Signed-off-by: aerosouund <aerosound161@gmail.com>
Can be a seqpacket or stream. Refactor vsock.rs methods
to expect it in function calls and use it in building structs.
add a VSOCK_TYPE_SEQPACKET constant to denote seqpacket
connections

Signed-off-by: aerosouund <aerosound161@gmail.com>
- Make the multiplexer use a VsockConnection<ConnBackend> for its
  conn_map values to forward calls to stream or seqpacket types.
- Make the host sock a boxed implementation of the Socket trait which
  exposes the accept() method with different implementations for stream
  and seqpacket connections.
- Globally replace the MuxerConnection type with a VsockConnection
  generic over a ConnBackend.
- Make the init_pkt_hdr method add different type constants based on
  the vsock type.
- pass vsock type to read_local_stream_port. seqpacket sockets
  necessitate that you read the message in one go.

Signed-off-by: aerosouund <aerosound161@gmail.com>
Add a new field to the vsock api and modify functions that interact with
a VsockDeviceConfig or a VsockUnixBackend.

Signed-off-by: aerosouund <aerosound161@gmail.com>
- incoming length is a trait with a default implementation to read the
  size of an incoming message from a connection fd
- read result stores the count of bytes read and whether or no another
  receive should be triggered
- change the VsockChannel trait to return a ReadResult instead of null

Signed-off-by: aerosouund <aerosound161@gmail.com>
- create a set_msg_eom function to set eom in the packet header
- create a set_msg_eor function to set eor in the packet header

Signed-off-by: aerosouund <aerosound161@gmail.com>
- make it start another loop iteration if it received a result with
  should_retrigger

Signed-off-by: aerosouund <aerosound161@gmail.com>
- add connection_buffer and conn_buffer_wrote_bytes fields to the
  connection
- when there's a seqpacket connection with incoming data check the
  length of the incoming message
- if its small enough to fit in the packet buffer (1 memory page size)
  receive directly from the af_unix to the buffer
- if its bigger, receive into a buffer then copy whatever you can fit
  into the buffer and mark that we should retrigger
- if the buffer has data, don't read from the stream. iterate until you
  finish the buffer
- don't use intermediate rx buffering for stream connections
- if the incoming message is larger than the connection buffer
  we return VsockError::MessageTooLong

Signed-off-by: aerosouund <aerosound161@gmail.com>
- propagate conn_buffer_size to connections on creation to determine
  how big the rx intermediate buffer should be
- if the result coming from the connection has should_retrigger push to
  the rx queue to mark that there's still data to be read from the
buffer

Signed-off-by: aerosouund <aerosound161@gmail.com>
- the setting should be persisted across snapshot and restore so the new
  vsock buffers have the same size
- prevent the caller from passing too large buffers. 256 is also the
  limit set by the linux kernel
- prevent the caller from passing too small buffers (less than 4096)
  bytes.
- make the field default to none and add default serialization

Signed-off-by: aerosouund <aerosound161@gmail.com>
- seqpacket sockets necessitate a different flag set than
  whats currently in the seccomp rules. we need to add
  seqpacket with SOCK_CLOEXEC for testing to work

Signed-off-by: aerosouund <aerosound161@gmail.com>
- the functional tests use an echo server which is just a socat command.
  socat doesn't support seqpacket.
- the program listens on a port and handles a connection in a separate
  thread

Signed-off-by: aerosouund <aerosound161@gmail.com>
- can be stream or seqpacket. to facilitate seqpacket testing

Signed-off-by: aerosouund <aerosound161@gmail.com>
- create a fixture to compile the vsock_seq_server program
- create start_seqpacket_echo_server to run the program in
  the vm
- create the check_guest_connections_seqpacket function to
  start a host side seqpacket server and connect to it from
  the guestn and check data hash
- create h2g, g2h and overflow functional tests
- run seqpacket tests on kernel 6.x only because seqpacket
  was introduced in linux 5.14. any previous version would
  fail

Signed-off-by: aerosouund <aerosound161@gmail.com>
- so that the api anticipates it

Signed-off-by: aerosouund <aerosound161@gmail.com>
add changelog entry for the modified field

Signed-off-by: aerosouund <aerosound161@gmail.com>
- Added to the unreleased section

Signed-off-by: aerosouund <aerosound161@gmail.com>
@aerosouund
Copy link
Copy Markdown
Contributor Author

@JamesC1305
addressed/asked follow ups on all points. let me know what you think and what are the next steps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Awaiting author Indicates that an issue or pull request requires author action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add SEQPACKET socket type for vsock

2 participants