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

[spike] Rework rx/tx loop in pkg/tap/switch.go? #390

Open
cfergeau opened this issue Aug 28, 2024 · 0 comments
Open

[spike] Rework rx/tx loop in pkg/tap/switch.go? #390

cfergeau opened this issue Aug 28, 2024 · 0 comments

Comments

@cfergeau
Copy link
Contributor

The rx/tx code in pkg/tap/switch.go is a bit hard to follow, with the protocol.Stream() tests, and the Buf()/Read()/Write() code in the stream case.
What the code is doing is to send data to/from the hypervisor. For some hypervisors (qemu, hyperkit, stdio), the size must be sent before the data, either as a 16 bit little-endian, or as a 32 bit little-endian data. This is what the code calls 'stream', and the Buf()/Read()/Write() methods from the streamProtocol interface are only used to write this 2 or 4 bytes for the size.

I've tried to rework this code in https://github.com/cfergeau/gvisor-tap-vsock/tree/rxtx:

  • the streamProtocol interface is now ReadSize/WriteSize as I believe this makes the code easier to understand
  • it introduces a new hypervisorConnection interface to replace the if conn.protocolImpl.Stream() tests

This seems to simplify the code, but is untested, and I don't want to spend more time on this right away.
What needs to be done to complete this:

  • ideally, have some unit tests to exercise these code paths
  • test that data transfer is still working (at least with one 'stream' (qemu) and one 'packet' (hyperkit) hypervisors)
  • measure performance, similar to what was done in Improve rxStream performance using bufio reader #188

One potential further improvement is to see if we can remove this allocation from rxStream using https://pkg.go.dev/bytes#Buffer

@gbraad gbraad changed the title Rework rx/tx loop in pkg/tap/switch.go? [spike] Rework rx/tx loop in pkg/tap/switch.go? Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant