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

fix: support for sr-iov passthrough virtual machine network adapters #2133

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

spacegospod
Copy link
Collaborator

@spacegospod spacegospod commented Feb 15, 2024

Description

This change fixes 2 deficiencies in the support for passthrough network adapters on virtual machines.

The support for SR-IOV adapters was added with #2059 which was simply a port of an old change that never made it in #1417

The issues introduced by these changes are:

  1. In-place repacement of NICs is not supported if the source or destination type is "sriov". The reason provided for this is that "it was too fiddly".
  2. The current implementation for SRIOV NICs cannot work with hardware version 20 and onwards, released with vSphere 8

Both problems have the same root cause. A very long time a go someone made the assumption that virtual network adapters are placed on the virtual PCI bus sequentially on slots 7 onwards. More specifically they made the assumption that passthrough adapters are placed on slots 36-45 descending. The entire implementation hangs on its capability to guess the slot number on which the kernel will place a device. We attempt to set the UnitNumber field on each virtual device (even though this is not respected by the kernel) and compute an address for it that we use internally to find the device in the list of devices post-creation.

This already sounds super-fishy and the original developers of the passthrough support decided that the in-place swapping of adapters when SR-IOV is involved is "too fiddly". Can't blame them.

But there is more! The actual problem is that VMware never guaranteed these PCI slot numbers. All of this is based on assumptions.
Starting from hardware version 20 (vSphere 8 and onwards) the kernel creates VMs with a new motherboard layout called ACPI which changes the bus topology (passthrough adapters go to slot 65 and increment from there for example, but again, this is not guaranteed).

My change effectively eliminates all of the guessing when it comes to network adapters. There is no more special handling for SR-IOV. Adapters are instead identified by their position in the list of network devices. This means that the provider will have the same interface for network adapters as the vSphere client, where they are identified by their position.

No changes are being made to devices other than network adapters.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

Ran all tests for virtual nics and added one new case

=== RUN   TestAccResourceVSphereVirtualMachine_cloneMultiNICFromSingleNICTemplate
--- PASS: TestAccResourceVSphereVirtualMachine_cloneMultiNICFromSingleNICTemplate (86.75s)
PASS

=== RUN   TestAccResourceVSphereVirtualMachine_cloneMultiNICSRIOVFromVMXNET3Template
--- PASS: TestAccResourceVSphereVirtualMachine_cloneMultiNICSRIOVFromVMXNET3Template (92.30s)
PASS

=== RUN   TestAccResourceVSphereVirtualMachine_maximumNumberOfNICs
--- PASS: TestAccResourceVSphereVirtualMachine_maximumNumberOfNICs (85.89s)
PASS

=== RUN   TestAccResourceVSphereVirtualMachine_SRIOV
--- PASS: TestAccResourceVSphereVirtualMachine_SRIOV (79.10s)

Release Note

Fix the support for SR-IOV passthrough virtual machine network adapters

References

Closes #2092

Signed-off-by: Stoyan Zhelyazkov <[email protected]>
@spacegospod spacegospod requested a review from a team as a code owner February 15, 2024 12:30
@hashicorp-cla
Copy link

hashicorp-cla commented Feb 15, 2024

CLA assistant check
All committers have signed the CLA.

@tenthirtyam tenthirtyam self-requested a review February 15, 2024 20:24
@tenthirtyam tenthirtyam added area/vm Area: Virtual Machines area/networking Area: Networking labels Feb 15, 2024
@tenthirtyam tenthirtyam added this to the v2.7.0 milestone Feb 15, 2024
@tenthirtyam tenthirtyam changed the title Fix the support for SR-IOV passthrough virtual machine network adapters fix: support for sr-iov passthrough virtual machine network adapters Feb 15, 2024
Copy link
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

LGTM!

@tenthirtyam tenthirtyam merged commit af161f8 into main Feb 15, 2024
5 checks passed
@tenthirtyam tenthirtyam deleted the bugfix/vm-passthrough-nics branch February 15, 2024 20:35
Copy link

github-actions bot commented Mar 6, 2024

This functionality has been released in v2.7.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Copy link

github-actions bot commented Apr 6, 2024

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/networking Area: Networking area/vm Area: Virtual Machines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[sr-iov] Issue cloning template with VMXNET3 adapters – change one to SRIOV and add a SRIOV adapter
4 participants