Add support for USB NICs #1814

bensmrs · 2025-03-19T14:45:08Z

This PR allows to use USB emulation for NICs through an io.bus device option.
It introduces a UseUSBBus field to the RunConfig struct, which may also help for #1697

bensmrs · 2025-03-19T14:50:12Z

QEMU needs the MAC addresses to start with 40 for usb-net devices; see https://github.com/qemu/qemu/blob/1dae461a913f9da88df05de6e2020d3134356f2e/hw/usb/dev-network.c#L1386

FWIW, I’d love to know why…

stgraber · 2025-03-19T15:01:42Z

internal/server/instance/instance_utils.go

+		// Use a prefix that suits QEMU for USB NICs
+		prefix = "40:xx:xx:xx:xx:xx"
+	} else {
+		// Use the usual prefix
+		prefix = "10:66:6a:xx:xx:xx"


Nit but we like comments to end with a period these days ;)

bensmrs · 2025-03-19T15:08:20Z

internal/server/device/nic.go

@@ -53,6 +55,7 @@ func nicValidationRules(requiredFields []string, optionalFields []string, instCo
 		"security.acls.default.egress.logged":  validate.Optional(validate.IsBool),
 		"security.promiscuous":                 validate.Optional(validate.IsBool),
 		"mode":                                 validate.Optional(validate.IsOneOf("bridge", "vepa", "passthru", "private")),
+		"io.bus":                               validate.Optional(validate.And(func(_ string) error { return nicCheckIsVM(instConf) }, validate.IsOneOf("virtio", "usb"))),


I just noticed Optional is variadic and doesn’t require And. I’ll patch that.

stgraber · 2025-03-19T16:25:45Z

So I've been looking into the QEMU side of things.

As far as I can tell, there are effectively two distinct fields, one that holds the full provided MAC and once that's derived from that MAC minus the first two bytes being switched to 0x40.

When looking in the VM, I don't actually see the MAC address that's set by Incus at all.
Do you have the same behavior in your case?

Stéphane

stgraber · 2025-03-19T16:37:10Z

Oh, testing this stuff is made a bit hardware due to the fact that this branch doesn't seem to implement hotplug correctly yet. So adding a io.bus=usb NIC after the instance has started has it land on virtio-net instead.

bensmrs · 2025-03-19T17:14:59Z

When looking in the VM, I don't actually see the MAC address that's set by Incus at all.
Do you have the same behavior in your case?

You see a NIC with another address? On my machine at least I have the correct address.

this branch doesn't seem to implement hotplug correctly yet

Oh that’s something I haven’t tested yet. I’ll have a look.

stgraber · 2025-03-19T18:24:32Z

You see a NIC with another address? On my machine at least I have the correct address.

I'm pretty sure I got a completely random MAC earlier here, but trying it again now, I do see the "expected" behavior of the MAC address starting with "40:xx".

The problem is that we can't really generate such a MAC. "40:" is not a reserved prefix by the IEEE and I see a lot of entries in "40:xx", so we're effectively violating MAC allocation rules here.

Honestly what I'd do at this point is just keep our MAC addresses the way we do it normally, so feed it a valid MAC and let QEMU mangle the first two bytes to "0x40". We can then leave a note in the doc that this is happening and is out of our control.

I'll ask on QEMU's IRC about what's up with that, see if anyone knows.
I tried a physical USB network adapter and it has a normal MAC, no "40:" prefix.
I also failed to find any standard requiring the 0x40 prefix.

stgraber · 2025-03-19T18:28:47Z

I asked on IRC now. Personally I'm very tempted to just push a one-line patch to QEMU in the Zabbly package that removes that limitation and see what happens ;)

Looking at the libvirt code, they're passing a normal MAC and actually show an example with a MAC starting with ("00:"), I guess they never noticed the rewriting happening.

bensmrs · 2025-03-19T18:39:49Z

Ok I got hotplugging working; I’ll run more tests soon.

I'm pretty sure I got a completely random MAC earlier here, but trying it again now, I do see the "expected" behavior of the MAC address starting with "40:xx".

Ahem now that you’re mentioning it, I just saw this behavior. And it disappeared… Weird.

The problem is that we can't really generate such a MAC. "40:" is not a reserved prefix by the IEEE and I see a lot of entries in "40:xx", so we're effectively violating MAC allocation rules here.

~~Well it’s a free allocation range, so we can define a range for Incus use and it’ll be fine. What problems do you see?~~ <- I’m completely wrong. Just ask for a range prefixed with 40:, how hard and expensive can that be? 🙃

Honestly what I'd do at this point is just keep our MAC addresses the way we do it normally, so feed it a valid MAC and let QEMU mangle the first two bytes to "0x40". We can then leave a note in the doc that this is happening and is out of our control.

Sure, but if you have your network manager configured to get a DHCP lease for all its interfaces, if you don’t explicitly define an hwaddr, you won’t get a lease from Incus. That’s why I modified the address generation logic.

I'll ask on QEMU's IRC about what's up with that, see if anyone knows.

I asked a few months ago when I started tinkering with macOS, but no answer…

I also failed to find any standard requiring the 0x40 prefix.

Yeah I’ve done my research on that and… well I haven’t found anything. But also the commit introducing the 40: prefix is from December 2009…
It’s a pretty old, rarely-touched driver.

Personally I'm very tempted to just push a one-line patch to QEMU in the Zabbly package that removes that limitation and see what happens

Oh my.

stgraber · 2025-03-19T19:06:28Z

Oh my.

I pushed that change to the Zabbly daily package now. Will be easy to test and see if anything obviously bad happens from it :)

bensmrs · 2025-03-19T23:58:19Z

So do you want me to remove all the logic dealing with 40:? I can document that MACs not starting with 40: only work with the Zabbly flavor of QEMU. It’ll simplify the code quite a bit. I’ll do some debugging tomorrow to try to reproduce the strange behavior that we noticed with wrong random MACs appearing.

stgraber · 2025-03-20T02:25:24Z

Yeah, we should remove the check because otherwise we're forcing people into using MAC addresses that they're not supposed to be using...

I haven't yet tested the new QEMU but if it works and nobody sees anything obviously wrong with it, I guess we should just send the patch upstream and see if someone finally tells us what's going on with that thing :)

bensmrs · 2025-03-20T14:30:35Z

Oh btw, do we agree that that line is a leftover from an old logic that doesn’t make sense anymore?

stgraber · 2025-03-20T17:16:50Z

Oh btw, do we agree that that line is a leftover from an old logic that doesn’t make sense anymore?

Github seems a bit confused about what's being linked here.
If it's about the vectors math, then yeah, that code looks a bit odd.

We have a minimum of two queues which then means a minimum of 6 vectors, so the value can never be 0 and should just always enable multi-queue (mq) and expose the number of vectors when on PCI.

bensmrs · 2025-03-21T12:43:02Z

Github seems a bit confused about what's being linked here.
If it's about the vectors math, then yeah, that code looks a bit odd.

The auto-scroll is broken but the highlight is still there. It was indeed about the vector math.

I haven't yet tested the new QEMU but if it works and nobody sees anything obviously wrong with it, I guess we should just send the patch upstream and see if someone finally tells us what's going on with that thing :)

For now at least it works.

I’ll try to wrap the issue up soon, and possibly track the MAC problem we encountered, although I’m afraid I’ll end up debugging QEMU…

bensmrs · 2025-03-21T13:25:25Z

A few questions/remarks.

I could only get the random MAC twice, and twice the same: ca:89:78:d0:7f:76, dev enp1s0f6u4i1.

By any chance, is it the same MAC you get when it goes wrong?

Changing io.type (hot or cold) leads to the device losing its IP, even after a reboot. Reverting io.type works.

I’m pretty sure it’s Linux doing its thing. The sheer idea of doing that feels cursed enough for me to not even consider documenting the behavior. Wdyt?

Pretty often, when I plug 2 usb-net devices, only one of them gets an IP.

Is it something you can reproduce on your side?

bensmrs · 2025-03-21T13:28:31Z

Just now, I got the same wrong MAC. The QMP command is the correct one, so I’m quite afraid to dig deeper.

bensmrs · 2025-03-21T14:21:49Z

Couldn’t find anything in the QEMU driver for usb-net.
The fact that we experienced such a bug on an unpatched QEMU makes me think QEMU may not be the culprit but rather the kernel (because no 0x40 set as the first byte suggests that the problem happens after QEMU defined the device), although I couldn’t find anything inspiring in the code dealing with RNDIS and CDC devices.

stgraber · 2025-03-21T19:39:48Z

Yeah, I agree that it's probably some USB probing issue or something in the kernel.
I believe I actually saw that before with a physical USB network adapter.

So anyway, our side of things is correct so we can move on with that.

Signed-off-by: Benjamin Somers <benjamin.somers@imt-atlantique.fr>

bensmrs · 2025-03-31T12:48:59Z

For the record, when the interface is bugged, I get in dmesg:

[    6.864508] cdc_subset 1-4:1.0: probe with driver cdc_subset failed with error -22
[    6.983942] cdc_subset 1-4:1.1 usb0: register 'cdc_subset' at usb-0000:01:00.6-4, Linux Device, 22:3b:42:e7:ff:eb
[    6.984958] usbcore: registered new interface driver cdc_subset
[    6.985127] cdc_ether 1-4:1.0: probe with driver cdc_ether failed with error -16

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [lxc/incus](https://github.com/lxc/incus) | minor | `v6.10.1` -> `v6.11.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>lxc/incus (lxc/incus)</summary> ### [`v6.11.0`](https://github.com/lxc/incus/releases/tag/v6.11.0): Incus 6.11 [Compare Source](lxc/incus@v6.10.1...v6.11.0) ### Announcement https://discuss.linuxcontainers.org/t/incus-6-11-has-been-released/23322 #### What's Changed - Allow ICMP and low ports for unprivileged users in OCI containers by [@gwenya](https://github.com/gwenya) in lxc/incus#1706 - doc: Clarify virtiofsd requirements by [@stgraber](https://github.com/stgraber) in lxc/incus#1718 - Fix generate-database usage for incusd/db by [@breml](https://github.com/breml) in lxc/incus#1719 - Do not allow mounting of custom block volume snapshots by [@presztak](https://github.com/presztak) in lxc/incus#1720 - generate-database: Abstract db connection / db transaction by [@breml](https://github.com/breml) in lxc/incus#1721 - Fix snapshot size handling in cross-pool copy/move by [@presztak](https://github.com/presztak) in lxc/incus#1717 - generate-database: Accept interface in PrepareStmts by [@breml](https://github.com/breml) in lxc/incus#1725 - Simplify `evaluateShorthandFilter` by reducing nesting levels by [@presztak](https://github.com/presztak) in lxc/incus#1727 - incusd/storage: Don't use sparse writer on thick LVM by [@stgraber](https://github.com/stgraber) in lxc/incus#1729 - generate-database: Add support for marshal to JSON by [@breml](https://github.com/breml) in lxc/incus#1731 - Fixed incus edk2 path overwrite issue by [@nanjj](https://github.com/nanjj) in lxc/incus#1726 - Do not download instance types if cache loadable by [@nanjj](https://github.com/nanjj) in lxc/incus#1732 - Clarify security.secureboot setting by [@gwenya](https://github.com/gwenya) in lxc/incus#1740 - Fix DNS for isolated OVN networks by [@gwenya](https://github.com/gwenya) in lxc/incus#1738 - Allow announcing extra routes through DHCPv4 by [@gwenya](https://github.com/gwenya) in lxc/incus#1734 - Fix link parsing failure on non-ethernet devices by [@stgraber](https://github.com/stgraber) in lxc/incus#1742 - Fix revert on OCI container creation failure by [@gwenya](https://github.com/gwenya) in lxc/incus#1744 - generate-database: Handle non tx DB connections by [@breml](https://github.com/breml) in lxc/incus#1745 - incus file edit extension by [@gwenya](https://github.com/gwenya) in lxc/incus#1746 - Cleanup internal API endpoints by [@stgraber](https://github.com/stgraber) in lxc/incus#1747 - Tweak help message for rebuild by [@stgraber](https://github.com/stgraber) in lxc/incus#1754 - Use lego binary for DNS-01 challenge by [@accuser](https://github.com/accuser) in lxc/incus#1753 - incusd/storage/zfs: Fix ZFS CreateVolume deletes pre-existing data on failure by [@mrstux](https://github.com/mrstux) in lxc/incus#1749 - incus/file: Always use 1MB chunks for SFTP by [@stgraber](https://github.com/stgraber) in lxc/incus#1758 - Use the correct path for ingesting DNS-01 challenge certificate outputs by [@accuser](https://github.com/accuser) in lxc/incus#1759 - incusd/bgp: Rework start/stop logic by [@stgraber](https://github.com/stgraber) in lxc/incus#1761 - incusd/network/ovn: Skip existing static routes by [@stgraber](https://github.com/stgraber) in lxc/incus#1762 - incusd/instance/qemu: Set caching-mode with intel-iommu by [@stgraber](https://github.com/stgraber) in lxc/incus#1772 - incus-agent: Improve SFTP performance by [@stgraber](https://github.com/stgraber) in lxc/incus#1773 - incusd/network/ovn: Keep getting router name when network none by [@diegofernandes](https://github.com/diegofernandes) in lxc/incus#1771 - make `incus copy --device xx,type=none` drop remaining device properties by [@schnoddelbotz](https://github.com/schnoddelbotz) in lxc/incus#1764 - incusd/instance/qemu: rtc base localtime for windows by [@nanjj](https://github.com/nanjj) in lxc/incus#1767 - Add option to configure DNS server for bridge and OVN networks by [@gwenya](https://github.com/gwenya) in lxc/incus#1739 - Use lego binary for http 01 challenge by [@accuser](https://github.com/accuser) in lxc/incus#1770 - Handle live migration between QEMU versions by [@stgraber](https://github.com/stgraber) in lxc/incus#1775 - incusd/instance/qemu: Skip to link nvram to itself by [@nanjj](https://github.com/nanjj) in lxc/incus#1760 - Switch to new MAC address prefix by [@stgraber](https://github.com/stgraber) in lxc/incus#1776 - client: Fix spelling errors found by codespell by [@cjwatson](https://github.com/cjwatson) in lxc/incus#1777 - Add ipv4.dhcp.expiry option for ovn networks by [@gwenya](https://github.com/gwenya) in lxc/incus#1781 - Configure DHCP on existing instance interfaces when it is enabled on a network by [@gwenya](https://github.com/gwenya) in lxc/incus#1780 - incusd/instance/edk2: Select SecureBoot capable firmware on Debian by [@stgraber](https://github.com/stgraber) in lxc/incus#1782 - Fix some `go vet` warnings by [@stgraber](https://github.com/stgraber) in lxc/incus#1784 - Clear gofumpt by [@stgraber](https://github.com/stgraber) in lxc/incus#1803 - Fix some BGP issues by [@stgraber](https://github.com/stgraber) in lxc/incus#1805 - incusd/instance/qemu: bad pid check by [@nanjj](https://github.com/nanjj) in lxc/incus#1806 - Fix spelling errors and run codespell automatically by [@cjwatson](https://github.com/cjwatson) in lxc/incus#1778 - incus/file: Properly handle relative source paths by [@stgraber](https://github.com/stgraber) in lxc/incus#1809 - cmd/storage: incorrect CLI syntax in storage pool creation examples by [@ViniRodrig](https://github.com/ViniRodrig) in lxc/incus#1810 - Improve DB performance by [@stgraber](https://github.com/stgraber) in lxc/incus#1811 - incusd/network/ovn: Fix default DNS IPv4 server by [@stgraber](https://github.com/stgraber) in lxc/incus#1812 - Extend OS detection logic by [@stgraber](https://github.com/stgraber) in lxc/incus#1813 - Add allocated CPU time to instance state by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1807 - incusd/certificates: Properly handle bad PEM data by [@stgraber](https://github.com/stgraber) in lxc/incus#1816 - Extra `generate-database` features by [@masnax](https://github.com/masnax) in lxc/incus#1817 - incusd/network/common: Handle missing BGP peer by [@stgraber](https://github.com/stgraber) in lxc/incus#1818 - incusd/cluster/evacuate: Don't live-migrate stopped instances by [@stgraber](https://github.com/stgraber) in lxc/incus#1819 - Fix generator table pluralization by [@masnax](https://github.com/masnax) in lxc/incus#1823 - incusd/instance/qemu enable s4 by default by [@nanjj](https://github.com/nanjj) in lxc/incus#1820 - Add support for USB NICs by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1814 - incusd/storage/s3 Fixed minio client mc too ambious issue by [@nanjj](https://github.com/nanjj) in lxc/incus#1821 - incusd/networks: Validate configuration on join too by [@stgraber](https://github.com/stgraber) in lxc/incus#1824 - Update gomod for go-jwt vulnerability by [@stgraber](https://github.com/stgraber) in lxc/incus#1825 - cmd/generate-database/db: Fix GetNames spacing by [@masnax](https://github.com/masnax) in lxc/incus#1826 - github: Rework issue templates by [@stgraber](https://github.com/stgraber) in lxc/incus#1827 - Update Debian installation documentation by [@gibmat](https://github.com/gibmat) in lxc/incus#1830 - Extend minio client naming by [@gibmat](https://github.com/gibmat) in lxc/incus#1829 - Various fixes from address set MR by [@stgraber](https://github.com/stgraber) in lxc/incus#1831 - incusd/instance/lxc: Cleanup OCI mount paths by [@stgraber](https://github.com/stgraber) in lxc/incus#1834 - Add `io.bus=usb` for disks by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1835 - golangci: Upgrade to version 2 by [@stgraber](https://github.com/stgraber) in lxc/incus#1836 - golangci: Disable STI005 error checks by [@stgraber](https://github.com/stgraber) in lxc/incus#1841 - Standalone changes from the Linstor branch by [@stgraber](https://github.com/stgraber) in lxc/incus#1842 - incusd/storage/s3 minio client check enhancement by [@nanjj](https://github.com/nanjj) in lxc/incus#1839 - incusd/network/ovn: Remove internal routes to forward/load-balancers by [@stgraber](https://github.com/stgraber) in lxc/incus#1843 - incusd/instance/edk2: Always prefer the EDK2 override by [@stgraber](https://github.com/stgraber) in lxc/incus#1847 - Fixes from Linstor branch by [@stgraber](https://github.com/stgraber) in lxc/incus#1846 - Add `linstor` storage driver by [@luissimas](https://github.com/luissimas) in lxc/incus#1621 - Add `linstor.remove_snapshots` config option by [@luissimas](https://github.com/luissimas) in lxc/incus#1848 - doc/support: Update feature release version by [@bensmrs](https://github.com/bensmrs) in lxc/incus#1853 - incusd/instance: Don't enforce device/config validation on snapshots by [@stgraber](https://github.com/stgraber) in lxc/incus#1854 - OCI entrypoint configuration by [@gwenya](https://github.com/gwenya) in lxc/incus#1845 #### New Contributors - [@mrstux](https://github.com/mrstux) made their first contribution in lxc/incus#1749 - [@diegofernandes](https://github.com/diegofernandes) made their first contribution in lxc/incus#1771 - [@schnoddelbotz](https://github.com/schnoddelbotz) made their first contribution in lxc/incus#1764 - [@cjwatson](https://github.com/cjwatson) made their first contribution in lxc/incus#1777 - [@ViniRodrig](https://github.com/ViniRodrig) made their first contribution in lxc/incus#1810 - [@masnax](https://github.com/masnax) made their first contribution in lxc/incus#1817 **Full Changelog**: lxc/incus@v6.10.1...v6.11.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

bensmrs requested a review from stgraber as a code owner March 19, 2025 14:45

github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Mar 19, 2025

stgraber reviewed Mar 19, 2025

View reviewed changes

bensmrs commented Mar 19, 2025

View reviewed changes

bensmrs mentioned this pull request Mar 20, 2025

Add usb support to io.bus #1697

Closed

bensmrs force-pushed the usb-net branch 2 times, most recently from b3ac399 to a347ece Compare March 21, 2025 14:19

bensmrs added 4 commits March 21, 2025 15:40

api: network_io_bus

feece2d

Signed-off-by: Benjamin Somers <benjamin.somers@imt-atlantique.fr>

incusd/device/config: Add USB bus override

1b04793

Signed-off-by: Benjamin Somers <benjamin.somers@imt-atlantique.fr>

incusd/instance: Add support for USB NICs

8b0f88f

Signed-off-by: Benjamin Somers <benjamin.somers@imt-atlantique.fr>

incusd/device/nic: Add support for io.bus

6dd07f8

Signed-off-by: Benjamin Somers <benjamin.somers@imt-atlantique.fr>

stgraber force-pushed the usb-net branch from a347ece to 5e9e628 Compare March 21, 2025 19:46

stgraber approved these changes Mar 21, 2025

View reviewed changes

stgraber enabled auto-merge March 21, 2025 19:46

doc/device/nic: Add support for io.bus

e724c41

Signed-off-by: Benjamin Somers <benjamin.somers@imt-atlantique.fr>

stgraber force-pushed the usb-net branch from 5e9e628 to e724c41 Compare March 21, 2025 19:51

stgraber merged commit b354ff5 into lxc:main Mar 21, 2025
36 checks passed

bensmrs mentioned this pull request Mar 24, 2025

Add io.bus=usb for disks #1835

Merged

bensmrs deleted the usb-net branch March 27, 2025 09:53

Uh oh!

Add support for USB NICs #1814

Add support for USB NICs #1814

Uh oh!

Conversation

bensmrs commented Mar 19, 2025

Uh oh!

bensmrs commented Mar 19, 2025

Uh oh!

stgraber Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

bensmrs Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

stgraber commented Mar 19, 2025

Uh oh!

stgraber commented Mar 19, 2025

Uh oh!

bensmrs commented Mar 19, 2025

Uh oh!

stgraber commented Mar 19, 2025

Uh oh!

stgraber commented Mar 19, 2025

Uh oh!

bensmrs commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stgraber commented Mar 19, 2025

Uh oh!

bensmrs commented Mar 19, 2025

Uh oh!

stgraber commented Mar 20, 2025

Uh oh!

bensmrs commented Mar 20, 2025

Uh oh!

stgraber commented Mar 20, 2025

Uh oh!

bensmrs commented Mar 21, 2025

Uh oh!

bensmrs commented Mar 21, 2025

Uh oh!

bensmrs commented Mar 21, 2025

Uh oh!

bensmrs commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stgraber commented Mar 21, 2025

Uh oh!

Uh oh!

bensmrs commented Mar 31, 2025

Uh oh!

Uh oh!

bensmrs commented Mar 19, 2025 •

edited

Loading

bensmrs commented Mar 21, 2025 •

edited

Loading