devices/vsock: support multi-descriptor TX chains#680
Conversation
slp
requested review from
MatiasVara,
dorindabassey,
jakecorrenti,
mtjhrc and
tylerfanelli
as code owners
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the VsockPacket implementation to support multiple data regions by introducing an owned_buf for cases where packet data is non-contiguous in guest memory. It adds logic to handle single combined descriptors, classic two-descriptor zero-copy packets, and complex multi-descriptor chains that require copying into the new owned buffer. Feedback was provided regarding a behavioral inconsistency in buf_mut(): for multi-descriptor packets, modifications to the returned mutable slice will only affect the local owned_buf and will not be reflected back in guest memory, unlike the zero-copy implementation.
|
Just a nitpick, FYI the
Maybe I'll try creating a CI check for this... |
|
@mtjhrc I was amending one of my commits to use the prescribed format and there's actually an issue that sometimes you might not know what the model was. For example Copilot only puts the harness name to its own commits and it's possible to use model "auto". I'm not sure if you can always retrieve information about the exact model used in that case. |
My .02 cents ... I wonder if we should be a bit elastic around the naming and avoid adding a deep CI check. Idk if you received other direction but AFAIK for statistical measurements they're just detecting the |
from_tx_virtq_head assumed a fixed 2-descriptor layout (header + one data descriptor), which breaks with newer kernels that may combine header and data in a single descriptor (Linux 6.2+) or split data across multiple descriptors. Handle three cases: single combined descriptor (zero-copy), classic two-descriptor (zero-copy, unchanged), and multi-descriptor data (copied into an owned contiguous buffer). The RX path already handled the combined case; this brings the TX path to parity and beyond. Assisted-by: Claude Code:claude-opus-4.6 Signed-off-by: Sergio Lopez <slp@redhat.com>
Fixed, thanks! |
4 participants
from_tx_virtq_head assumed a fixed 2-descriptor layout (header + one data descriptor), which breaks with newer kernels that may combine header and data in a single descriptor (Linux 6.2+) or split data across multiple descriptors.
Handle three cases: single combined descriptor (zero-copy), classic two-descriptor (zero-copy, unchanged), and multi-descriptor data (copied into an owned contiguous buffer). The RX path already handled the combined case; this brings the TX path to parity and beyond.
Assisted-by: <anthropic/claude-opus-4.6>