Skip to content

Conversation

@simonjbeaumont
Copy link
Contributor

@simonjbeaumont simonjbeaumont commented Nov 14, 2024

Motivation:

We have some tests for our Vsock support that only run when we detect the system supports Vsock. These weren't running in CI but, now we've migrated CI to GitHub Actions, they can. In prototyping this though, I discovered that one of our tests was too strict, and will fail for VMs running on Hyper-V, which is the hypervisor used by GitHub actions.

Specifically, we have support for getting the local context ID by a channel option. This is implemented by an ioctl, but the semantics of the return value differ between the various Vsock transport kernel modules:

  • virtio_transport: returns the guest CID, which is a number greater than VMADDR_CID_HOST, but less than VMADDR_CID_ANY, and returns VMADDR_CID_ANY as an error.
  • vsock_loopback: returns VMADDR_CID_LOCAL always.
  • vmci_transport: returns the guest CID if the guest and host transport are active; VMADDR_CID_HOST if only the host transport is active; and VMCI_INVALID_ID otherwise, which happens to be the same value as VMADDR_CID_ANY.
  • hyperv_transport: returns VMADDR_CID_ANY always.

For this reason, we should probably remove any attempts to interpret the validity of the value that comes back from the driver and users will need to know what to do with it.

Modifications:

  • tests: Only run Vsock echo tests on Linux, when vsock_loopback is loaded.
  • tests: Remove asserts on the value of local Vsock CID.
  • ci: Add pipeline that runs Vsock tests.

Result:

  • Vsock tests will no longer run unless vsock_loopback kernel module is loaded.
  • Vsock tests will no longer fail in Hyper-V VMs.
  • Vsock tests will now run in CI.

@simonjbeaumont simonjbeaumont force-pushed the sb/vsock-ci branch 5 times, most recently from be4856b to 134ba74 Compare November 19, 2024 17:29
@simonjbeaumont simonjbeaumont added the semver/none No version bump required. label Nov 19, 2024
@simonjbeaumont simonjbeaumont changed the title Look for vsock_loopback tests: Remove asserts on the value of local Vsock CID Nov 19, 2024
@simonjbeaumont simonjbeaumont marked this pull request as ready for review November 19, 2024 17:52
@simonjbeaumont simonjbeaumont marked this pull request as draft November 19, 2024 18:08
@simonjbeaumont simonjbeaumont marked this pull request as ready for review November 20, 2024 11:31
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Nice cleanup, thanks @simonjbeaumont!

@simonjbeaumont simonjbeaumont enabled auto-merge (squash) November 21, 2024 10:17
@simonjbeaumont simonjbeaumont removed the request for review from FranzBusch November 21, 2024 10:17
@simonjbeaumont simonjbeaumont merged commit 64eb8bf into main Nov 21, 2024
42 of 43 checks passed
@simonjbeaumont simonjbeaumont deleted the sb/vsock-ci branch November 21, 2024 10:33
Lukasa pushed a commit to Lukasa/swift-nio that referenced this pull request Nov 22, 2024
### Motivation:

We have some tests for our Vsock support that only run when we detect
the system supports Vsock. These weren't running in CI but, now we've
migrated CI to GitHub Actions, they can. In prototyping this though, I
discovered that one of our tests was too strict, and will fail for VMs
running on Hyper-V, which is the hypervisor used by GitHub actions.

Specifically, we have support for getting the local context ID by a
channel option. This is implemented by an ioctl, but the semantics of
the return value differ between the various Vsock transport kernel
modules:

- `virtio_transport`: returns the guest CID, which is a number greater
than `VMADDR_CID_HOST`, but less than `VMADDR_CID_ANY`, and returns
`VMADDR_CID_ANY` as an error.
- `vsock_loopback`: returns `VMADDR_CID_LOCAL` always.
- `vmci_transport`: returns the guest CID if the guest and host
transport are active; `VMADDR_CID_HOST` if only the host transport is
active; and `VMCI_INVALID_ID` otherwise, which happens to be the same
value as `VMADDR_CID_ANY`.
- `hyperv_transport`: returns `VMADDR_CID_ANY` always.

For this reason, we should probably remove any attempts to interpret the
validity of the value that comes back from the driver and users will
need to know what to do with it.

### Modifications:

- tests: Only run Vsock echo tests on Linux, when `vsock_loopback` is
loaded.
- tests: Remove asserts on the value of local Vsock CID.
- ci: Add pipeline that runs Vsock tests.

### Result:

- Vsock tests will no longer run unless `vsock_loopback` kernel module
is loaded.
- Vsock tests will no longer fail in Hyper-V VMs.
- Vsock tests will now run in CI.

(cherry picked from commit 64eb8bf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants