From 21b866aeed5ae79481cb7ac6f36366cbdf66e8ca Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sun, 11 Oct 2026 21:44:23 +0800 Subject: [PATCH 01/10] feat: cross-compute VXLAN kernel datapath Cross-compute links can ride kernel VXLAN tunnels instead of the uBridge UDP relay: each endpoint creates one VXLAN device (uBridge's rtnetlink vxlan module) enslaved into the same per-link Linux bridge the same-host datapath uses, so filters, markers, capture and suspend keep running on the anchor unchanged. Opt-in (Server.enable_vxlan_cross_compute, default off) because the tunnel caps the usable MTU at the underlay minus the outer headers; the relay stays the default and the fallback. The controller derives what the pair must agree on: one VNI per link (upper VNI space, per-link random salt) and deterministic device names (gvl{link}{node}) that make crashed-run leftovers claimable (show -> delete -> create). Same-host compute pairs, switch endpoints and serial/unix-socket containers stay on the relay. A VNI the kernel refuses self-heals on every path it can surface: interactive create retries a fresh salt then degrades to the relay; project open re-dispatches refused batch entries the same way; node start recognises the refused device, stops the half-started node, re-wires both ends on a fresh VNI (or degrades the link) and retries the start. The device MTU is derived from the lower device (dev=) and the per-link bridge inherits it - no configuration. The capability probe (ubridge_vxlan in /capabilities) runs one scratch create/delete on a scratch port that never equals the configured dstport. The e2e scenario runs two computes on one machine (a network namespace joined by a veth pair as the fabric) and needs root. --- docs/features/docker-kernel-datapath.md | 1 + docs/features/vxlan-cross-compute.md | 121 ++++++ gns3server/api/routes/compute/capabilities.py | 7 + gns3server/api/routes/compute/projects.py | 34 +- gns3server/compute/base_manager.py | 5 +- gns3server/compute/dynamips/__init__.py | 4 +- gns3server/compute/kernel_datapath.py | 115 ++++++ gns3server/compute/nios/nio_bridge.py | 25 +- gns3server/compute/ubridge/tc_probe.py | 93 +++++ gns3server/controller/node.py | 87 ++++- gns3server/controller/project.py | 111 +++++- gns3server/controller/udp_link.py | 205 +++++++++- gns3server/schemas/compute/nios.py | 25 ++ gns3server/schemas/config.py | 18 + gns3server/schemas/controller/capabilities.py | 5 + gns3server/utils/vxlan.py | 86 +++++ tests/api/routes/compute/test_capabilities.py | 23 ++ .../docker/test_docker_kernel_datapath.py | 146 ++++++++ tests/compute/ubridge/test_tc_probe.py | 51 +++ tests/controller/test_kernel_datapath_link.py | 352 +++++++++++++++++- tests/e2e/README.md | 3 + tests/e2e/harness.py | 273 +++++++++++++- tests/e2e/test_vxlan_cross_compute.py | 305 +++++++++++++++ tests/utils/test_vxlan.py | 71 ++++ 24 files changed, 2107 insertions(+), 59 deletions(-) create mode 100644 docs/features/vxlan-cross-compute.md create mode 100644 gns3server/utils/vxlan.py create mode 100644 tests/e2e/test_vxlan_cross_compute.py create mode 100644 tests/utils/test_vxlan.py diff --git a/docs/features/docker-kernel-datapath.md b/docs/features/docker-kernel-datapath.md index 6e527dd99..fe324b446 100644 --- a/docs/features/docker-kernel-datapath.md +++ b/docs/features/docker-kernel-datapath.md @@ -182,6 +182,7 @@ All dozen-plus GNS3 filter types ride these three kernel objects — nine of the `UDPLink._kernel_datapath_eligible` decides in the controller at NIO prepare time; the compute side only reacts to the NIO type: * both endpoints on the same compute, both able to anchor — Docker anchors via its veth (any class except unix-socket containers), and QEMU / IOU / Dynamips / Ethernet-switch peers anchor via their own mechanisms, so mixed links take the kernel path too + * cross-compute pairs take the kernel path as a **VXLAN tunnel** when `Server.enable_vxlan_cross_compute` is enabled and both computes can create the device — see [vxlan-cross-compute](vxlan-cross-compute.md); the default there is the relay, because the tunnel caps the usable MTU at the underlay minus 50 bytes * `Server.enable_kernel_datapath` enabled (default) Filters never disqualify the kernel path anymore — every type has a kernel equivalent (netem / cls_bpf / eBPF classifier). diff --git a/docs/features/vxlan-cross-compute.md b/docs/features/vxlan-cross-compute.md new file mode 100644 index 000000000..dc0c8aa3c --- /dev/null +++ b/docs/features/vxlan-cross-compute.md @@ -0,0 +1,121 @@ +# Cross-compute VXLAN links + +## Overview + +A link whose endpoints live on two different computes normally flows through the uBridge UDP relay: each end's anchor is attached to a userspace relay bridge and the frames cross the network as plain UDP payloads pumped by uBridge. This feature adds a second, **kernel-native** cross-compute datapath: each end creates a **VXLAN device** (uBridge's `vxlan` module, rtnetlink) and enslaves it into the same per-link kernel bridge the same-host datapath uses, so the kernel encapsulates and decapsulates and no userspace process is on the data path. + +It is **opt-in and off by default** (`Server.enable_vxlan_cross_compute`). The reason is MTU: a tunnel can only carry what the underlay carries minus the outer headers, so its sweet spot is a fabric that moves jumbo frames. The relay stays the default cross-compute datapath and the fallback whenever a tunnel cannot be wired — it always works, at the price of a userspace hop. + +## Terms + +| Term | Meaning | +|---|---| +| Anchor | The host-side interface an adapter owns (a veth host end, a persistent TAP); links attach *to* it, never create it | +| Relay | The default cross-compute datapath: uBridge userspace bridges pumping UDP tunnels between the computes | +| Tunnel device | The per-end VXLAN device (`gvl…`) carrying one link's frames to the peer compute | +| VNI | The VXLAN network identifier — the kernel's demux key for received frames; both ends of a link must use the same one | +| dstport | The UDP port every tunnel of a deployment listens on; the kernel serves one socket per port and demuxes its traffic by VNI | +| Lower device | The underlay interface the route to the peer resolves to; the kernel derives the tunnel's MTU from it | + +## Architecture + +```mermaid +flowchart LR + subgraph GA["guest A"] + EA["eth0"] + end + subgraph CA["compute A"] + AA["anchor"] --> BA["bridge"] + BA --> VA["tunnel device"] + end + subgraph U["underlay"] + UD["UDP to the peer, demuxed by VNI"] + end + subgraph CB["compute B"] + VB["tunnel device"] --> BB["bridge"] + BB --> AB["anchor"] + end + subgraph GB["guest B"] + EB["eth0"] + end + EA --> AA + VA --> UD + UD --> VB + AB --> EB +``` + +Everything on the anchor side is shared with the same-host kernel datapath: filters (the full tc surface including the netem extensions the relay cannot run), markers, capture and suspend all run on the anchor, so they behave identically on a tunneled link. The controller picks the VNI and the device names and hands the peer's address to each end; the kernel does the rest. The device names are a pure function of the link id (bridge, `gvl` device) and the endpoint's node id (device), so each machine can recognise its own leftovers without asking anyone. + +## Datapath selection + +The controller's `UDPLink._kernel_datapath_eligible` decides at NIO prepare time, on every link (re)creation: + +| Condition | Outcome | +|---|---| +| Both endpoints on one compute, both able to anchor | Kernel link (per-link bridge, unchanged) | +| Cross-compute, `enable_vxlan_cross_compute` on, both computes report `ubridge_vxlan`, both ends anchorable, addresses on a common subnet and distinct | **VXLAN tunnel** | +| Same-host pair (both computes resolve to one address) | Relay — the kernel keeps one socket per dstport, so a tunnel pair collides with itself on one host | +| Ethernet switch on either end (cross-compute), serial / non-Ethernet ports, unix-socket vendor containers | Relay | +| Capability missing, flag off, or the link was already downgraded | Relay | + +On project (re)open every link re-runs the same decision, so flipping the flag upgrades or downgrades links the same way the same-host datapath is chosen. + +## Link creation + +Each end's compute owns its own machine: it resolves the lower device from the peer address locally (a connected UDP socket picks the route without sending anything), creates the device and enslaves it. The two sides never talk to each other; the controller's VNI is what makes them a pair. + +```mermaid +sequenceDiagram + participant C as Controller + participant UA as Compute A uBridge + participant UB as Compute B uBridge + C->>UA: NIO: bridge, VNI, dstport, peer address + C->>UB: NIO: bridge, VNI, dstport, peer address + par compute A + UA->>UA: vxlan show, delete a leftover + UA->>UA: vxlan create with the local lower device + UA->>UA: brctl addif bridge anchor, tunnel + and compute B + UB->>UB: vxlan show, delete a leftover + UB->>UB: vxlan create with the local lower device + UB->>UB: brctl addif bridge anchor, tunnel + end + Note over C: a refused create retries with a fresh VNI, then degrades the link to the relay +``` + +The `vxlan show` step is the idempotency contract: a device found under the link's deterministic name belongs to this link (a crashed run's leftover, the previous instance of a node restart) and is replaced, never refused. A create that fails with the name free can only be the VNI — the kernel refuses one already registered on the shared socket — so the controller bumps the derivation salt and retries, and after the retries run out it marks the link downgraded and rebuilds it on the relay. The same shape covers project open: a refused entry is reported per link by the batch route, retried on fresh VNIs up to the same candidate count an interactive creation gets, then degraded, so a VNI collision can never make a project unopenable. And a link wired while its nodes were stopped meets its collision only at node start — the controller's `Node.start` recognises the refused device (its name names the link), stops the half-started node, re-wires the link on a fresh VNI or, on a second refusal, degrades it to the relay, and retries: no path can fail permanently over a VNI. + +## Interfaces + +| Interface | Where | Notes | +|---|---|---| +| `nio_bridge.vxlan` (name, vni, remote, dstport) | Compute API, NIO create/update body | Present only on cross-compute kernel links; ignored by update (the device persists) | +| `ubridge_vxlan` | `GET /capabilities` and `GET /computes/{id}` | Probed with a scratch create/delete on a scratch port; `null` (unknown) keeps links on the relay | +| `kernel_datapath` | Link REST payload | True for tunneled links, like any kernel link | + +## Configuration + +| Key | Default | Meaning | +|---|---|---| +| `Server.enable_vxlan_cross_compute` | `false` | Wire eligible cross-compute links as VXLAN tunnels | +| `Server.vxlan_dstport` | `14789` | The UDP port all tunnels of the deployment listen on (one kernel socket per port; chosen off the IANA 4789 and the legacy 8472 so other VXLAN software cannot merge demux tables with ours) | + +## Known caveats + +* **MTU is the whole story.** The kernel derives the tunnel device's MTU from the lower device (`underlay − 50` for the outer headers; `− 70` for an IPv6 underlay) and the per-link bridge, which carries no MTU of its own, reports the minimum of its ports — i.e. the derived value. A frame past it is dropped at the bridge egress **silently** (no ICMP, no log), which is why the datapath is opt-in: on a 1500-byte fabric a tunnel caps guests at 1450 while the relay carries 1500 with no tax. Guests set their own interface MTU, exactly like a VM or container raising eth0 on the same-host datapath. +* **Asymmetric underlays give one-way jumbo.** The derivation is per machine: a 9000-MTU compute paired with a 1500-MTU one forwards large frames in one direction only. Keep the fabric's MTU uniform between computes. +* **The MTU is derived once, at device creation.** Changing the underlay's MTU does not follow; a link delete/re-create (or project reopen) re-derives. +* **VNI collisions self-heal, not never happen.** The VNI is drawn from the upper VNI space (1M–16.7M) but two links of one deployment can still collide on a host. Every path where the kernel's refusal can surface carries its own heal: link creation retries a fresh VNI then degrades to the relay, project open re-dispatches refused entries the same way, and node start (a link wired while its nodes were stopped) stops the half-started node, re-wires the link on a fresh VNI or degrades it on a second refusal, and retries the start. A reopen re-rolls every link's VNI, so an unlucky link is not permanently unlucky. +* **Security is the relay's, unchanged.** VXLAN carries no authentication; the VNI and the shared port are the only demux, and anyone who can reach the port can inject into any VNI. Treat the compute fabric as trusted, exactly as for the relay's UDP tunnels. +* **Same-host compute pairs stay on the relay**, as do switch links, serial ports and unix-socket vendor containers — the tunnel is a per-link-bridge shape, and those cases are not (or would collide with themselves on one host). + +## Verification + +* Unit: `tests/controller/test_kernel_datapath_link.py` (selection matrix, tunnel spec derivation, VNI retry, downgrade), `tests/utils/test_vxlan.py`, `tests/compute/docker/test_docker_kernel_datapath.py` (device lifecycle, leftover replacement, teardown ordering), `tests/api/routes/compute/test_capabilities.py`. +* Live: `tests/e2e/test_vxlan_cross_compute.py` — two computes on one machine (a network namespace joined to the host by a veth pair as the second "machine"), real containers per compute: VNI/port agreement, bridge membership, the derived MTU and its hard egress drop, dual-stack traffic, a delay filter, leftover replacement, a VNI squatted at node start (both ends re-wired on a fresh VNI, the start succeeding), residue-free delete and a rebuild through the project-open batch path. + +## Roadmap + +* PMTU probing between computes to pick the datapath (and pin the MTU) per pair instead of a deployment-wide flag. +* Ethernet switch links tunneled through the switch's own bridge (the absorbed-anchor shape) instead of falling back to the relay. +* VNI-space bookkeeping to make collisions impossible by construction rather than self-healing. diff --git a/gns3server/api/routes/compute/capabilities.py b/gns3server/api/routes/compute/capabilities.py index e874b9a95..b0fc30566 100644 --- a/gns3server/api/routes/compute/capabilities.py +++ b/gns3server/api/routes/compute/capabilities.py @@ -30,6 +30,7 @@ from gns3server.compute.ubridge.tc_probe import ( probe_iol_tap_support, probe_tap_support, probe_tc_capabilities, + probe_vxlan_support, ) from gns3server.compute.vpcs.vpcs_probe import probe_vpcs_tap from gns3server.utils.path import get_default_project_directory @@ -75,6 +76,11 @@ async def get_capabilities(request: Request) -> dict: # kernel link. None = unknown (probe failed or binary missing). vpcs_tap = await probe_vpcs_tap() + # VXLAN device creation (the vxlan module): the tunnel a cross-compute + # kernel link rides. None = unknown (probe failed, uBridge too old, or + # the kernel refuses vxlan devices here). + ubridge_vxlan = await probe_vxlan_support() + # record the controller hostname or IP address if request.client: request.app.state.controller_host = request.client.host @@ -91,4 +97,5 @@ async def get_capabilities(request: Request) -> dict: "ubridge_iol_tap": ubridge_iol_tap, "ubridge_bridge_tap": ubridge_bridge_tap, "vpcs_tap": vpcs_tap, + "ubridge_vxlan": ubridge_vxlan, } diff --git a/gns3server/api/routes/compute/projects.py b/gns3server/api/routes/compute/projects.py index 89b25b6b7..e0ed3737d 100644 --- a/gns3server/api/routes/compute/projects.py +++ b/gns3server/api/routes/compute/projects.py @@ -35,6 +35,7 @@ from fastapi.encoders import jsonable_encoder from fastapi.responses import FileResponse from gns3server import schemas +from gns3server.compute.error import NodeError from gns3server.compute.project import Project from gns3server.compute.project_manager import ProjectManager from gns3server.utils.path import is_safe_path @@ -271,16 +272,35 @@ async def create_batch_nios( # count (standard == 1, Dynamips == 2) and await the async variant. sig = inspect.signature(node.manager.create_nio) dynamips_style = len(sig.parameters) >= 2 + failed = [] for entry in entries: nio_settings = jsonable_encoder(entry.nio, exclude_unset=True) - if dynamips_style: - nio = await node.manager.create_nio(node, nio_settings) - else: - nio = node.manager.create_nio(nio_settings) - await _add_nio_binding(node, entry.adapter_number, entry.port_number, nio) + try: + if dynamips_style: + nio = await node.manager.create_nio(node, nio_settings) + else: + nio = node.manager.create_nio(nio_settings) + await _add_nio_binding(node, entry.adapter_number, entry.port_number, nio) + except NodeError: + # A VXLAN entry that cannot be wired (its VNI already + # registered on the host's shared socket, no route to the + # peer) must not fail the whole project open: report it so + # the controller can degrade that link to the uBridge relay. + # Every other NIO type keeps the fail-the-batch semantics. + if not nio_settings.get("vxlan"): + raise + failed.append( + { + "node_id": entry.node_id, + "adapter_number": entry.adapter_number, + "port_number": entry.port_number, + } + ) + return failed - await asyncio.gather(*[_create_one_node(nid, ents) for nid, ents in per_node.items()]) - return {"added": len(batch.nios)} + results = await asyncio.gather(*[_create_one_node(nid, ents) for nid, ents in per_node.items()]) + failed = [item for node_failed in results for item in node_failed] + return {"added": len(batch.nios) - len(failed), "failed": failed} @router.put( diff --git a/gns3server/compute/base_manager.py b/gns3server/compute/base_manager.py index 8df8703b7..5c1511e8e 100644 --- a/gns3server/compute/base_manager.py +++ b/gns3server/compute/base_manager.py @@ -375,7 +375,10 @@ class BaseManager: nio.markers = nio_settings.get("markers", {}) nio.suspend = nio_settings.get("suspend", False) elif nio_settings["type"] == "nio_bridge": - nio = NIOBridge(nio_settings["bridge"]) + # A vxlan sub-spec marks a cross-compute link: the node creates + # the VXLAN device and enslaves it into the same per-link bridge + # next to its anchor (see KernelDatapathMixin._kernel_attach). + nio = NIOBridge(nio_settings["bridge"], nio_settings.get("vxlan")) nio.filters = nio_settings.get("filters", {}) # Markers ride the NIO and attach to the veth host interface via # uBridge's AF_PACKET marker module (marker add_kernel). diff --git a/gns3server/compute/dynamips/__init__.py b/gns3server/compute/dynamips/__init__.py index b9674247f..0b9ad2c91 100644 --- a/gns3server/compute/dynamips/__init__.py +++ b/gns3server/compute/dynamips/__init__.py @@ -369,7 +369,9 @@ class Dynamips(BaseManager): # Kernel-datapath NIO: the router binds the port's anchor TAP to # a hypervisor nio_tap and enslaves it into this per-link bridge # (Router._attach_kernel_nio). No hypervisor NIO is created here. - nio = NIOBridge(nio_settings["bridge"]) + # A vxlan sub-spec (cross-compute link) rides along untouched — + # the router's attach path wires it like any kernel link. + nio = NIOBridge(nio_settings["bridge"], nio_settings.get("vxlan")) nio.filters = nio_settings.get("filters", {}) nio.markers = nio_settings.get("markers", {}) nio.suspend = nio_settings.get("suspend", False) diff --git a/gns3server/compute/kernel_datapath.py b/gns3server/compute/kernel_datapath.py index f49275a45..bbc2317ad 100644 --- a/gns3server/compute/kernel_datapath.py +++ b/gns3server/compute/kernel_datapath.py @@ -48,6 +48,9 @@ import asyncio import contextlib import logging import os +import socket + +import psutil from gns3server.compute.error import NodeError from gns3server.compute.ubridge.ubridge_error import UbridgeError @@ -264,6 +267,86 @@ class KernelDatapathMixin: return not (isinstance(nio, NIOBridge) and nio.bridge is None) + async def _ensure_vxlan_device(self, spec): + """ + Create (or replace) the cross-compute VXLAN device this link end + tunnels through and return the device name. The device name is a + pure function of the link id, so a device found under it belongs to + this link: either a leftover from a run that crashed without + cleanup, or the previous instance of a node restart — deleted and + re-created either way, which makes wiring idempotent. The lower + device is resolved locally from the peer address; the kernel + derives the tunnel MTU from it, which is what keeps the per-link + bridge honest about what the underlay can carry. + + A create failure with the name just cleared can only mean the VNI — + the kernel refuses a VNI already registered on the shared dstport + socket (EEXIST) — so it is surfaced as a wiring error and the + controller retries the link with a fresh VNI. + """ + + name = spec["name"] + remote = spec["remote"] + try: + await self._ubridge_send(f'vxlan show "{name}"') + except UbridgeError: + # Not present (or uBridge cannot answer): the create below is + # the real gate either way. + pass + else: + await self._ubridge_send(f'vxlan delete "{name}"') + dev = await self._vxlan_lowerdev(remote) + try: + await self._ubridge_send( + f'vxlan create "{name}" {spec["vni"]} remote={remote} dstport={spec["dstport"]} dev={dev}' + ) + except UbridgeError as e: + raise self._kernel_error(f"Could not create VXLAN device {name} to {remote} via {dev}: {e}") + return name + + async def _vxlan_lowerdev(self, remote): + """ + The local interface the route to *remote* egresses through — the + ``dev=`` a cross-compute VXLAN device needs so the kernel can derive + its MTU from the underlay (peer MTU - 50) and pick the outer source + address. A connected UDP socket makes the kernel pick the route + without sending anything; the chosen source address is matched back + to its interface. + """ + + try: + info = socket.getaddrinfo(remote, 9, socket.AF_UNSPEC, socket.SOCK_DGRAM) + except socket.gaierror as e: + raise self._kernel_error(f"Cannot resolve VXLAN peer {remote}: {e}") + last_error = None + for af, socktype, proto, _, sa in info: + try: + with socket.socket(af, socktype, proto) as sock: + sock.connect(sa) + local_ip = sock.getsockname()[0] + except OSError as e: + last_error = e + continue + dev = self._interface_with_address(local_ip) + if dev is not None: + return dev + raise self._kernel_error(f"No local interface owns {local_ip}, the route to VXLAN peer {remote}") + raise self._kernel_error(f"No route to VXLAN peer {remote}: {last_error}") + + @staticmethod + def _interface_with_address(address): + """ + The name of the interface owning *address* (scope suffix stripped + for link-local IPv6), or None. + """ + + plain = address.split("%")[0] + for name, addrs in psutil.net_if_addrs().items(): + for addr in addrs: + if addr.address.split("%")[0] == plain: + return name + return None + async def _kernel_attach(self, anchor, nio): """ Attach an anchor to the per-link kernel bridge carried by *nio* @@ -276,6 +359,12 @@ class KernelDatapathMixin: bridge first: an existing bridge is success instead of a create race failure. + A NIO carrying a ``vxlan`` spec is a cross-compute link: this end's + VXLAN device is (re-)created first and enslaved into the same + bridge next to the anchor, so the kernel encapsulates/decapsulates + the frames while filters, markers, capture and carrier keep running + on the anchor exactly like a same-host kernel link. + A NIO whose ``bridge`` is None means the anchor is bridged elsewhere — an Ethernet switch absorbed it into the switch's own bridge — so the membership work is skipped here and only the link @@ -287,6 +376,13 @@ class KernelDatapathMixin: """ if nio.bridge is not None: + vxlan_name = None + if nio.vxlan: + # Replace/refresh the device before the bridge work: a + # leftover device may still be enslaved to a leftover bridge + # (its deletion un-enrolls it), and the fresh device must be + # in place when the bridge's MTU auto-adjust runs. + vxlan_name = await self._ensure_vxlan_device(nio.vxlan) try: await self._ubridge_send(f'brctl create "{nio.bridge}"') except UbridgeError: @@ -294,6 +390,8 @@ class KernelDatapathMixin: await self._ubridge_send(f'brctl show "{nio.bridge}"') await self._ubridge_send(f'link set "{nio.bridge}" up') await self._ubridge_send(f'brctl addif "{nio.bridge}" "{anchor}"') + if vxlan_name is not None: + await self._ubridge_send(f'brctl addif "{nio.bridge}" "{vxlan_name}"') if nio.capturing: # Restore a capture that was active before a node restart # (mirrors the relay path's start_capture). @@ -373,6 +471,11 @@ class KernelDatapathMixin: enslaved fails with EBUSY, so the last endpoint to remove its port wins the deletion and the loser's failure is expected and suppressed. + A NIO carrying a ``vxlan`` spec is a cross-compute link: this end + also deletes its own tunnel device (the peer deletes its own on its + compute); the kernel's unregister path releases the bridge port, so + it goes before the bridge delete, same as the anchor. + A NIO whose ``bridge`` is None owns no bridge here: the anchor is bridged by an Ethernet switch, which tears its own membership down on its side; this end only releases the link state it applied @@ -394,6 +497,9 @@ class KernelDatapathMixin: with contextlib.suppress(UbridgeError): await self._ubridge_send(f'brctl delif "{nio.bridge}" "{host_ifc}"') if nio.bridge is not None: + if nio.vxlan: + with contextlib.suppress(UbridgeError): + await self._ubridge_send(f'vxlan delete "{nio.vxlan["name"]}"') with contextlib.suppress(UbridgeError): await self._ubridge_send(f'brctl delete "{nio.bridge}"') @@ -407,6 +513,12 @@ class KernelDatapathMixin: the same contract as link deletion. A node restart re-creates the bridge from the NIO in the attach path, so nothing is lost by deleting it. + + A cross-compute NIO's tunnel device is swept with its bridge: each + end owns its own device, and a device left behind would hold its + VNI on the host's shared socket until a reopen's attach path + replaces it by name (which it does — the sweep just frees it + sooner). """ if not self.ubridge: @@ -416,6 +528,9 @@ class KernelDatapathMixin: # bridge None = externally bridged (an Ethernet switch owns # the bridge); never delete a bridge this node does not own. if isinstance(nio, NIOBridge) and nio.bridge is not None: + if nio.vxlan: + with contextlib.suppress(UbridgeError): + await self._ubridge_send(f'vxlan delete "{nio.vxlan["name"]}"') with contextlib.suppress(UbridgeError): await self._ubridge_send(f'brctl delete "{nio.bridge}"') diff --git a/gns3server/compute/nios/nio_bridge.py b/gns3server/compute/nios/nio_bridge.py index 987431446..da101865e 100644 --- a/gns3server/compute/nios/nio_bridge.py +++ b/gns3server/compute/nios/nio_bridge.py @@ -31,6 +31,13 @@ veth host end (marker add_kernel / capture start_kernel). Ethernet switch absorbed it into the switch's own kernel bridge — and this node applies only the link state the anchor carries (carrier, markers, impairments), never bridge membership. + +A NIO may additionally carry ``vxlan`` (a BridgeVxlan spec): the link +crosses computes through a VXLAN device this end creates (uBridge's +``vxlan`` module) and enslaves into the same per-link bridge next to the +anchor. The kernel encapsulates/decapsulates the frames; everything else — +filters, markers, capture, carrier, suspend — still runs on the anchor, so +the datapath behaves exactly like a same-host kernel link. """ from .nio import NIO @@ -42,12 +49,16 @@ class NIOBridge(NIO): :param bridge: name of the per-link Linux kernel bridge, or None when the anchor is bridged by an Ethernet switch (see module docstring) + :param vxlan: cross-compute tunnel spec (dict: name/vni/remote/dstport) + this end creates and enslaves into the bridge, or None for a + same-host kernel link """ - def __init__(self, bridge): + def __init__(self, bridge, vxlan=None): super().__init__() self._bridge = bridge + self._vxlan = vxlan @property def bridge(self): @@ -60,6 +71,17 @@ class NIOBridge(NIO): return self._bridge + @property + def vxlan(self): + """ + Returns the cross-compute VXLAN tunnel spec, or None when the link + is wired on this host alone. + + :returns: dict (name/vni/remote/dstport) or None + """ + + return self._vxlan + def __str__(self): return "NIO bridge" @@ -69,6 +91,7 @@ class NIOBridge(NIO): return { "type": "nio_bridge", "bridge": self._bridge, + "vxlan": self._vxlan, "suspend": self._suspended, "filters": self._filters, "markers": self._markers, diff --git a/gns3server/compute/ubridge/tc_probe.py b/gns3server/compute/ubridge/tc_probe.py index 4322a53ee..0f9bc31ef 100644 --- a/gns3server/compute/ubridge/tc_probe.py +++ b/gns3server/compute/ubridge/tc_probe.py @@ -43,6 +43,7 @@ from typing import Optional from gns3server.config import Config from gns3server.utils.tc_capabilities import parse_tc_capabilities +from gns3server.utils.vxlan import VXLAN_VNI_MAX, VXLAN_VNI_MIN from .hypervisor import Hypervisor from .ubridge_error import UbridgeError @@ -59,6 +60,8 @@ _tap_cache: dict[tuple[str, int, int, int], Optional[bool]] = {} _iol_tap_cache: dict[tuple[str, int, int, int], Optional[bool]] = {} # binary identity -> whether the bridge module can release a named TAP NIO (None = unknown) _bridge_tap_cache: dict[tuple[str, int, int, int], Optional[bool]] = {} +# binary identity -> whether the vxlan module can create a device (None = unknown) +_vxlan_cache: dict[tuple[str, int, int, int], Optional[bool]] = {} _lock = asyncio.Lock() @@ -358,3 +361,93 @@ async def _probe_bridge_tap(path, config, timeout): with contextlib.suppress(Exception): await hypervisor.stop() shutil.rmtree(working_dir, ignore_errors=True) + + +async def probe_vxlan_support(timeout: float = 15.0): + """ + Whether this host's uBridge can create a VXLAN device (the ``vxlan`` + module) — the carrier a cross-compute kernel link tunnels through. + Probed on a scratch device with a scratch VNI on a scratch dstport + (never the deployment's configured port, so the probe cannot disturb + live tunnels; a device on the port would still be harmless — the kernel + shares one socket — but the probe has no business joining it) and + deleted again, cached by binary identity like the other probes. Any + failure — old build, missing module, no CAP_NET_ADMIN, an unloadable + kernel module — means unknown, reported as None and cached as such; + the controller reads unknown as "keep this link on the relay". + """ + + config = Config.instance() + path = shutil.which(config.settings.Server.ubridge_path) + if not path: + return None + try: + stat = os.stat(path) + key = (path, stat.st_mtime_ns, stat.st_size, stat.st_ctime_ns) + except OSError: + return None + + async with _lock: + if key in _vxlan_cache: + return _vxlan_cache[key] + supported = await _probe_vxlan(path, config, timeout) + _vxlan_cache[key] = supported + return supported + + +async def _probe_vxlan(path, config, timeout): + """ + Spawn a throwaway uBridge and run one ``vxlan create``/``delete`` cycle. + The scratch name (uuid suffix) cannot collide with a real link's + deterministic name; the scratch VNI and dstport are drawn from uuids so + two concurrent probes on one host (two servers, one uBridge user) cannot + fight over either. No ``dev=``: this probe answers "can this binary and + kernel create a VXLAN device at all" — resolving the real underlay + interface belongs to the link attach path, where an unroutable peer + means this particular link falls back to the relay, not that the + capability is absent. + """ + + working_dir = tempfile.mkdtemp(prefix="gns3-vxlan-probe-") + hypervisor = Hypervisor( + None, + path, + working_dir, + config.settings.Server.ubridge_control_transport, + config.settings.Server.host, + str(uuid.uuid4()), + ) + name = f"gvx{uuid.uuid4().hex[:8]}" + # VNI in the deployment's own derived range (utils.vxlan) so the probe + # exercises exactly the kernel path real links use. + vni = VXLAN_VNI_MIN + uuid.uuid4().int % (VXLAN_VNI_MAX - VXLAN_VNI_MIN + 1) + dstport = 20000 + uuid.uuid4().int % 10000 + # The scratch socket must never be the configured dstport: on it the + # scratch VNI shares a demux table with the live links' devices, and an + # unlucky draw would make the probe report the capability unknown for + # the process's lifetime (the result is cached). + if dstport == config.settings.Server.vxlan_dstport: + dstport = 20000 if dstport == 29999 else dstport + 1 + try: + + async def ask(): + await hypervisor.start() + await hypervisor.connect() + await hypervisor.send(f"vxlan create {name} {vni} remote=127.0.0.1 dstport={dstport}") + try: + await hypervisor.send(f"vxlan delete {name}") + except UbridgeError as e: + log.warning("Probe VXLAN %s could not be deleted: %s", name, e) + return True + + try: + return await asyncio.wait_for(ask(), timeout=timeout) + except (UbridgeError, OSError, asyncio.TimeoutError, ValueError) as e: + # Old build ("Unknown command"), missing module, or no + # CAP_NET_ADMIN: unknown. + log.debug("uBridge vxlan-module probe failed: %s", e) + return None + finally: + with contextlib.suppress(Exception): + await hypervisor.stop() + shutil.rmtree(working_dir, ignore_errors=True) diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index fb36ee8ef..c1036f0ee 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -20,18 +20,33 @@ import copy import html import logging import os +import re import uuid from ..config import Config from ..utils import int_to_macaddress, macaddress_to_int from ..utils.application_id import is_iol_runner_environment from ..utils.images import images_directories +from ..utils.vxlan import vxlan_device_name from .controller_error import ComputeConflictError, ComputeError, ControllerError, ControllerTimeoutError from .node_types import BUILTIN_NODE_TYPES from .ports.port_factory import DynamipsPortFactory, PortFactory, StandardPortFactory log = logging.getLogger(__name__) +# The compute's error when it could not create a VXLAN tunnel device — the +# wording is a same-repo contract with KernelDatapathMixin._kernel_error +# (only computes running this codebase carry VXLAN links: the capability +# gate keeps everything else on the relay), and the captured device name +# identifies the link (a pure function of link id + node id). +_VXLAN_CONFLICT_RE = re.compile(r"Could not create VXLAN device (?P\S+)") + +# Node-start self-heal rounds over a refused VXLAN VNI: the first re-wires +# the link on a fresh VNI, the second degrades it to the relay (whose NIOs +# cannot hit the conflict) — a start must not fail permanently because +# another link already registered the VNI on the shared dstport socket. +_VXLAN_START_HEALS = 2 + # Image properties of a node, mapped by node type. The value is the image # "type" used by the image manager (see utils.images). Docker images are not @@ -955,18 +970,32 @@ class Node: f"Cannot start node '{self._name}': {len(failed_links)} deferred link(s) " "could not be restored. Please try again." ) - try: - # For IOU: we need to send the licence everytime we start a node - if self.node_type == "iou": - license_check = self._project.controller.iou_license.get("license_check", True) - iourc_content = self._project.controller.iou_license.get("iourc_content", None) - await self.post( - "/start", timeout=240, data={"license_check": license_check, "iourc_content": iourc_content} + for heal in range(_VXLAN_START_HEALS + 1): + try: + await self._start_on_compute(data) + break + except asyncio.TimeoutError: + raise ControllerTimeoutError(f"Timeout when starting {self._name}") + except ComputeError as e: + conflicted = self._vxlan_conflicted_link(e) + if conflicted is None or heal == _VXLAN_START_HEALS: + raise + degrade = heal > 0 # a second conflict takes the relay + log.warning( + "Node '%s': start refused over a VXLAN VNI conflict on link %s — %s and retrying", + self._name, + conflicted.id, + "degrading the link to the uBridge relay" if degrade else "re-wiring the link on a fresh VNI", ) - else: - await self.post("/start", data=data, timeout=240) - except asyncio.TimeoutError: - raise ControllerTimeoutError(f"Timeout when starting {self._name}") + # The refused start leaves the node half-started (the + # emulator process runs, the wiring partial) — stop it + # first: a start on a running container early-returns + # without wiring anything. + await self.stop() + if await self._project._redispatch_vxlan_links([conflicted], downgrade=degrade) and not degrade: + # The fresh VNI was already refused before the start + # could even retry — degrade the pair to the relay now. + await self._project._redispatch_vxlan_links([conflicted], downgrade=True) self._confirm_status("started") # A started node may have brought kernel anchors into existence that # a link needs to finish wiring (the Ethernet-switch fast path joins @@ -974,6 +1003,42 @@ class Node: for link in self._links: await link.node_started(self) + async def _start_on_compute(self, data=None): + """ + The compute-side start request (IOU carries its licence along). + """ + + # For IOU: we need to send the licence everytime we start a node + if self.node_type == "iou": + license_check = self._project.controller.iou_license.get("license_check", True) + iourc_content = self._project.controller.iou_license.get("iourc_content", None) + await self.post( + "/start", timeout=240, data={"license_check": license_check, "iourc_content": iourc_content} + ) + else: + await self.post("/start", data=data, timeout=240) + + def _vxlan_conflicted_link(self, error): + """ + The link whose VXLAN device a failed node start could not create, + or None for every other failure. + + The compute's error names the device it could not create and the + name is a pure function of (link id, node id), so it identifies the + link with no extra API surface. An unmatched message — another + failure, a future wording change — simply leaves the error to + surface unchanged, which is exactly the behaviour without the heal. + """ + + match = _VXLAN_CONFLICT_RE.search(str(error)) + if match is None: + return None + device = match.group("device") + for link in self._links: + if vxlan_device_name(link.id, self.id) == device and any(nio.get("vxlan") for nio in link._link_data): + return link + return None + async def stop(self, strict=False): """ Stop a node diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 52f8bc65a..e548d34e6 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -49,7 +49,7 @@ from .link import _UNSET from .node import Node from .snapshot import Snapshot from .topology import load_topology, project_to_topology -from .udp_link import UDPLink +from .udp_link import _VXLAN_ATTEMPTS, UDPLink log = logging.getLogger(__name__) @@ -1021,6 +1021,84 @@ class Project: entries = await link._prepare() return (link, entries) + async def _dispatch_nio_batch(self, compute, nio_entries): + """ + POST one compute's NIO batch for this project, returning the entries + it refused (VXLAN entries a compute could not wire — see the batch + route; other NIO types fail the whole request). + """ + + response = await compute.post( + f"/projects/{self._id}/nios/batch", + data={"nios": nio_entries}, + timeout=300, + ) + return response.json.get("failed") or [] + + async def _redispatch_vxlan_links(self, links, downgrade): + """ + Re-prepare *links* and dispatch their NIOs in a second per-compute + batch, returning the links the computes still refused. + + With ``downgrade=False`` each link's VNI salt is bumped first — a + fresh VNI, since the refused one was already registered on the + host's shared dstport socket. With ``downgrade=True`` the links are + marked degraded to the uBridge relay (whose UDP NIOs cannot fail + this way) before re-preparing. + """ + + for link in links: + if downgrade: + log.warning( + "Project '%s' [%s]: VXLAN wiring failed for link %s — falling back to the uBridge relay datapath", + self._name, + self._id, + link.id, + ) + link._vxlan_downgraded = True + else: + link._vxlan_salt += 1 + per_compute = {} + by_key = {} + for link in links: + for node, adapter_number, port_number, nio_data in await link._prepare(): + by_key[(str(node.id), adapter_number, port_number)] = link + per_compute.setdefault(node.compute, []).append( + { + "node_id": node.id, + "adapter_number": adapter_number, + "port_number": port_number, + "nio": nio_data, + } + ) + if not per_compute: + return [] + results = await asyncio.gather( + *[self._dispatch_nio_batch(compute, entries) for compute, entries in per_compute.items()] + ) + failed = [] + for item in (i for sub in results for i in sub): + link = by_key.get((str(item["node_id"]), item["adapter_number"], item["port_number"])) + if link is not None and link not in failed: + failed.append(link) + return failed + + async def _heal_refused_vxlan_links(self, pending): + """ + Drive the links a compute refused at open onto a working datapath: + fresh-VNI rounds first — the same candidate count an interactive + link creation tries — then, for whatever still fails, the + degradation to the uBridge relay (whose UDP NIOs cannot hit this + refusal). + """ + + for _ in range(_VXLAN_ATTEMPTS - 1): + if not pending: + return + pending = await self._redispatch_vxlan_links(pending, downgrade=False) + if pending: + await self._redispatch_vxlan_links(pending, downgrade=True) + async def restore_deferred_links(self, node): """ Create on the computes the NIOs of the links that were deferred while @@ -1994,15 +2072,30 @@ class Project: } ) - async def _dispatch_batch(compute, nio_entries): - await compute.post( - f"/projects/{self._id}/nios/batch", - data={"nios": nio_entries}, - timeout=300, - ) - if per_compute: - await asyncio.gather(*[_dispatch_batch(c, n) for c, n in per_compute.items()]) + results = await asyncio.gather( + *[self._dispatch_nio_batch(compute, entries) for compute, entries in per_compute.items()] + ) + failed = [item for sub in results for item in sub] + else: + failed = [] + # A VXLAN entry a compute refused (its VNI already registered on + # the host's shared socket, no route to the peer) must not fail + # the whole open: the refused links are healed onto a working + # datapath (fresh VNIs, then the relay) — see + # _heal_refused_vxlan_links. + if failed: + by_key = {} + for link, entries in valid: + for node, adapter_number, port_number, nio_data in entries: + if nio_data.get("vxlan"): + by_key[(str(node.id), adapter_number, port_number)] = link + pending = [] + for item in failed: + link = by_key.get((str(item["node_id"]), item["adapter_number"], item["port_number"])) + if link is not None and link not in pending: + pending.append(link) + await self._heal_refused_vxlan_links(pending) # Finalise every link: wire node/port back-references, mark created, # notify clients, and apply project-level marker definitions. diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index fd2dd8aab..64ed272c8 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -18,6 +18,7 @@ import asyncio import logging +import uuid from gns3server.config import Config from gns3server.utils.application_id import is_iol_runner_environment @@ -26,6 +27,7 @@ from gns3server.utils.packet_filter_validation import ( split_kernel_only_features, validate_bpf_syntax, ) +from gns3server.utils.vxlan import derive_vni, vxlan_device_name from .compute import ComputeError from .controller_error import ControllerError, ControllerNotFoundError @@ -52,6 +54,10 @@ _MARKER_CAPABLE_TYPES = frozenset( log = logging.getLogger(__name__) +# VXLAN wiring attempts per link creation (the first plus the salt-bumped +# retries) before the link degrades to the uBridge relay. +_VXLAN_ATTEMPTS = 3 + def _is_unix_socket_docker(node): """ @@ -92,6 +98,15 @@ class UDPLink(Link): # of its endpoints has a missing image. The NIO creation is retried # once the image is resolved. self._deferred = False + # Cross-compute VXLAN wiring state: the salt is this link object's + # starting point in the VNI space — random per object, so a project + # reopen re-rolls every link's VNI and a link whose VNI collided + # with an unrelated device on a host is not permanently unlucky + # (the device *name*, which cleanup keys on, stays deterministic). + # _commit_vxlan bumps it for an in-flight retry; the downgrade flag + # marks a link degraded to the UDP relay for this object's lifetime. + self._vxlan_salt = uuid.uuid4().int + self._vxlan_downgraded = False @property def debug_link_data(self): @@ -206,9 +221,13 @@ class UDPLink(Link): Every adapter owns an anchor, so the datapath is a runtime decision — links attach to running nodes too: the compute side enslaves the anchor (brctl) or attaches the relay to it (add_nio_ethernet) without - touching the interfaces the node itself uses. Both endpoints must be - on the same compute (there is no kernel link across hosts) and both - must be able to anchor: Docker adapters are born as veth pairs, and + touching the interfaces the node itself uses. Both endpoints must sit + on computes that can carry their end: on one compute the ends meet in + a per-link Linux bridge; across computes each end additionally rides + its own VXLAN device (opt-in via enable_vxlan_cross_compute, uBridge's + vxlan module) enslaved into the same per-link bridge, so everything + below the bridge — filters, markers, capture, carrier — is identical + either way. Both must be able to anchor: Docker adapters are born as veth pairs, and IOL runner containers — whose guest leg is unix sockets, not a veth — anchor on persistent TAPs reached through the bridge module's swappable TAP leg (its own capability pair, see @@ -232,8 +251,21 @@ class UDPLink(Link): if not Config.instance().settings.Server.enable_kernel_datapath: return False - if node1.compute.id != node2.compute.id: - return False + cross_compute = node1.compute.id != node2.compute.id + if cross_compute: + # Cross-compute kernel links tunnel: each endpoint rides a VXLAN + # device into the same per-link bridge the same-host datapath + # uses. Opt-in (the tunnel caps the usable MTU at the underlay + # minus the outer headers, so it pays off on jumbo fabrics) and + # gated on both computes' uBridge being able to create the + # device; a link that cannot tunnel stays on the relay, where it + # always works. The downgrade flag is the runtime fallback: after + # the kernel kept refusing this link's VNI, the link stays on the + # relay for this object's lifetime. + if not Config.instance().settings.Server.enable_vxlan_cross_compute: + return False + if self._vxlan_downgraded: + return False if _is_unix_socket_docker(node1) or _is_unix_socket_docker(node2): return False for side in self._nodes: @@ -249,10 +281,17 @@ class UDPLink(Link): # pair is the only shape a bridge-to-bridge cable can take). switch = self._switch_endpoint() if switch is not None: + if cross_compute: + # The fast path is a one-host shape (the peer's anchor is + # absorbed into the switch's bridge); a switch link across + # computes stays on the relay. + return False if node1.node_type == "ethernet_switch" and node2.node_type == "ethernet_switch": return True peer = node2 if switch[0] == 0 else node1 return self._kernel_endpoint_ready(peer) + if cross_compute: + return self._vxlan_endpoint_ready(node1) and self._vxlan_endpoint_ready(node2) return self._kernel_endpoint_ready(node1) and self._kernel_endpoint_ready(node2) def _switch_endpoint(self): @@ -305,6 +344,23 @@ class UDPLink(Link): return capabilities.get("ubridge_tap") is True and capabilities.get("vpcs_tap") is True return False + @staticmethod + def _vxlan_endpoint_ready(node): + """ + Whether a node can carry its end of a cross-compute VXLAN link: it + must be able to anchor (the same per-node-type capability the + same-host kernel datapath needs — its end of the tunnel is enslaved + into a per-link bridge like any kernel link's) and its compute's + uBridge must be able to create the VXLAN device the tunnel rides. + Strict True on both, like the other capability gates: an unreported + or failed probe keeps the link on the relay, where it always works. + """ + + if not UDPLink._kernel_endpoint_ready(node): + return False + capabilities = node.compute.capabilities or {} + return capabilities.get("ubridge_vxlan") is True + async def _prepare(self): """ Local-only link setup: resolve peer addresses, reserve UDP ports and @@ -330,10 +386,77 @@ class UDPLink(Link): adapter_number2 = self._nodes[1]["adapter_number"] port_number2 = self._nodes[1]["port_number"] - if self._kernel_datapath_eligible(node1, node2): + cross_compute = node1.compute.id != node2.compute.id + # A cross-compute link needs the compute pair's addresses on both + # datapaths (the VXLAN tunnel's outer endpoints, the relay's UDP + # peers): resolve once, and let the no-common-subnet failure stay a + # ControllerError either way — it is a deployment error, not a + # datapath decision. Equal addresses mean both computes share one + # host: the kernel shares one VXLAN socket per dstport, so a tunnel + # pair cannot live there, and such links keep the relay, where the + # two ends are told apart by their UDP ports. + hosts = None + if cross_compute: + try: + hosts = await node1.compute.get_ip_on_same_subnet(node2.compute) + except ValueError as e: + raise ControllerError(f"Cannot get an IP address on same subnet: {e}") + same_host = hosts is not None and hosts[0] == hosts[1] + + if self._kernel_datapath_eligible(node1, node2) and not same_host: node1_filters, node2_filters = self._get_node_filters(node1, node2, kernel=True) node1_markers, node2_markers = self._get_node_markers(node1, node2) switch = self._switch_endpoint() + if hosts is not None: + # Cross-compute VXLAN: each end gets one VXLAN device + # carrying the link's VNI, enslaved into the per-link kernel + # bridge next to the node's anchor (see _kernel_attach on + # the compute side). The controller derives name and VNI — + # both ends must agree on the VNI and neither can ask the + # other — and the compute resolves the underlay interface + # locally from the peer address, so the kernel derives the + # device MTU (underlay - 50) and the bridge reports the + # honest "what this fabric can carry" value. A VNI the + # kernel refuses as already registered on the shared + # dstport socket surfaces as a commit failure; create() + # retries with a bumped salt and degrades to the relay when + # the retries run out. + node1_host, node2_host = hosts + dstport = Config.instance().settings.Server.vxlan_dstport + vni = derive_vni(self._id, self._vxlan_salt) + bridge_name = "gns3" + self._id.replace("-", "")[:11] + self._link_data = [ + { + "type": "nio_bridge", + "bridge": bridge_name, + "vxlan": { + "name": vxlan_device_name(self._id, node1.id), + "vni": vni, + "remote": node2_host, + "dstport": dstport, + }, + "filters": node1_filters, + "markers": node1_markers, + "suspend": self._suspended, + }, + { + "type": "nio_bridge", + "bridge": bridge_name, + "vxlan": { + "name": vxlan_device_name(self._id, node2.id), + "vni": vni, + "remote": node1_host, + "dstport": dstport, + }, + "filters": node2_filters, + "markers": node2_markers, + "suspend": self._suspended, + }, + ] + return [ + (node1, adapter_number1, port_number1, self._link_data[0]), + (node2, adapter_number2, port_number2, self._link_data[1]), + ] if switch is not None: if node1.node_type == "ethernet_switch" and node2.node_type == "ethernet_switch": # Switch-to-switch cascade: one veth pair joins the two @@ -449,11 +572,16 @@ class UDPLink(Link): (node2, adapter_number2, port_number2, self._link_data[1]), ] - # Get an IP allowing communication between both host - try: - (node1_host, node2_host) = await node1.compute.get_ip_on_same_subnet(node2.compute) - except ValueError as e: - raise ControllerError(f"Cannot get an IP address on same subnet: {e}") + # Relay datapath: UDP tunnels between the computes' addresses — + # already resolved for a cross-compute pair, resolved here otherwise + # (a same-compute relay pair resolves to the host's own address on + # both sides, distinguished by their UDP ports). + if hosts is None: + try: + hosts = await node1.compute.get_ip_on_same_subnet(node2.compute) + except ValueError as e: + raise ControllerError(f"Cannot get an IP address on same subnet: {e}") + node1_host, node2_host = hosts # Reserve a UDP port on both sides in parallel. Pre-allocated ports # (used during batch project loading) are popped from memory; otherwise @@ -537,17 +665,70 @@ class UDPLink(Link): raise errors[0] self._created = True + @staticmethod + def _has_vxlan(entries): + """ + Whether any prepared NIO spec of this link carries a VXLAN tunnel. + """ + + return any(nio_data.get("vxlan") for _, _, _, nio_data in entries) + async def create(self): """ Create the link on the nodes (interactive path: prepare + commit). """ entries = await self._prepare() - await self._commit_nios(entries) + if self._has_vxlan(entries): + await self._commit_vxlan(entries) + else: + await self._commit_nios(entries) # New links automatically inherit every active project-level marker # definition so the user doesn't have to reconfigure. await self._project.apply_defs_to_new_link(self) + async def _commit_vxlan(self, entries): + """ + Commit a cross-compute VXLAN link, retrying with a bumped VNI salt + on failure and degrading the link to the UDP relay when the retries + run out. + + The kernel refuses a VNI already registered on the shared dstport + socket at device creation (EEXIST) — the one conflict a freshly + created, deterministically named device can hit: the other end's + device lives on another host, and a leftover under this end's name + is replaced by the attach path, not refused. Any other commit + failure (a stopped node, a missing underlay route) is retried too, + harmlessly — it fails the same way every attempt, and the final + relay attempt surfaces the real error if the link genuinely cannot + be created. VXLAN is opt-in and the relay always works, so a tunnel + that cannot be wired degrades instead of failing the link; the + degradation lasts for this link object's lifetime (a project reopen + retries VXLAN from a clean state). + """ + + last_error = None + for attempt in range(_VXLAN_ATTEMPTS): + if attempt: + self._vxlan_salt += 1 + entries = await self._prepare() + if not self._has_vxlan(entries): + # Eligibility changed under us (a capability disappeared): + # commit the relay entries as prepared. + await self._commit_nios(entries) + return + try: + await self._commit_nios(entries) + return + except (ComputeError, ControllerError) as e: + last_error = e + log.warning( + "Link %s: VXLAN wiring failed (%s) — falling back to the uBridge relay datapath", self._id, last_error + ) + self._vxlan_downgraded = True + entries = await self._prepare() + await self._commit_nios(entries) + async def update(self): """ Update the link on the nodes diff --git a/gns3server/schemas/compute/nios.py b/gns3server/schemas/compute/nios.py index a41b7eda1..634b27dc9 100644 --- a/gns3server/schemas/compute/nios.py +++ b/gns3server/schemas/compute/nios.py @@ -73,6 +73,26 @@ class BridgeNIOType(str, Enum): bridge = "nio_bridge" +class BridgeVxlan(BaseModel): + """ + VXLAN tunnel spec carried by a cross-compute BridgeNIO. Each endpoint + creates one VXLAN device (uBridge ``vxlan create``) and enslaves it into + the link's per-link kernel bridge next to the node's anchor; the kernel + encapsulates/decapsulates the frames. The controller derives the name + and VNI (utils.vxlan) and resolves the peer address, so both endpoints + agree on the VNI without ever talking to each other. + + The device's MTU is derived by the kernel from the underlay interface + the route to ``remote`` picks (MTU - 50 for the outer headers), which is + what the link's bridge then reports as "what this fabric can carry". + """ + + name: str = Field(..., max_length=15, description="VXLAN device name on this endpoint's host (IFNAMSIZ-1)") + vni: int = Field(..., ge=1, le=16777215, description="VXLAN network identifier, identical on both endpoints") + remote: str = Field(..., description="Peer compute's address (unicast tunnel endpoint)") + dstport: int = Field(..., ge=1, le=65535, description="UDP destination port, identical on both endpoints") + + class BridgeNIO(BaseModel): """ Kernel-datapath bridge Network Input/Output properties. The NIO tells the @@ -99,6 +119,11 @@ class BridgeNIO(BaseModel): description="Kernel bridge name e.g. gns3a1b2c3d4e5; None when an Ethernet switch " "owns the anchor's bridge membership (absorbed link)", ) + vxlan: Optional[BridgeVxlan] = Field( + None, + description="Cross-compute tunnel: the VXLAN device this end creates and enslaves into the bridge " + "next to the anchor; absent for same-host kernel links", + ) suspend: Optional[bool] = Field(None, description="Suspend the NIO") filters: Optional[dict] = Field( None, diff --git a/gns3server/schemas/config.py b/gns3server/schemas/config.py index d9bb78295..ed8245777 100644 --- a/gns3server/schemas/config.py +++ b/gns3server/schemas/config.py @@ -192,6 +192,24 @@ class ServerSettings(BaseModel): "or QEMU when the compute's uBridge supports persistent TAPs) through kernel interfaces instead " "of the uBridge UDP relay", ) + enable_vxlan_cross_compute: bool = Field( + False, + description="Wire eligible cross-compute links through kernel VXLAN tunnels (one device per endpoint, " + "uBridge's vxlan module) instead of the uBridge UDP relay. The kernel encapsulates/decapsulates, so " + "no user-space forwarding is involved; the price is MTU: a tunnel can only carry the underlay MTU " + "minus 50 bytes of outer headers, so enable this where the fabric between computes carries jumbo " + "frames. The relay stays the default and the fallback when a tunnel cannot be wired", + ) + vxlan_dstport: int = Field( + 14789, + ge=1, + le=65535, + description="UDP destination port every cross-compute VXLAN tunnel listens on. The kernel shares one " + "socket per port across all VXLAN devices, so all links of a deployment ride this one port — chosen " + "off the IANA 4789 and the legacy 8472 (other VXLAN software's ports: sharing a socket would merge " + "their demux table with ours) and above the ephemeral range. Change it only when it collides with " + "a service already listening on the computes", + ) auto_discover_images: bool = Field(True, description="Automatically discover images in the images directory") image_sync_interval: int = Field(900, ge=10, description="Seconds between automatic image inventory scans") report_errors: bool = Field(True, description="Automatically send crash reports to the GNS3 team") diff --git a/gns3server/schemas/controller/capabilities.py b/gns3server/schemas/controller/capabilities.py index 69fbf5444..f683e805f 100644 --- a/gns3server/schemas/controller/capabilities.py +++ b/gns3server/schemas/controller/capabilities.py @@ -73,3 +73,8 @@ class Capabilities(BaseModel): "keeps the guest from auto-configuring the host side of the anchor TAP); absent when the " "probe failed", ) + ubridge_vxlan: Optional[bool] = Field( + None, + description="Whether uBridge can create VXLAN devices (the vxlan module, kernel support included) — " + "the tunnel a cross-compute kernel link rides; absent when the probe failed", + ) diff --git a/gns3server/utils/vxlan.py b/gns3server/utils/vxlan.py new file mode 100644 index 000000000..7ecfec8ce --- /dev/null +++ b/gns3server/utils/vxlan.py @@ -0,0 +1,86 @@ +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +""" +Identity of a cross-compute VXLAN link: the device name and VNI the +controller derives and ships in both endpoints' NIOs. + +A cross-compute kernel link is carried by one VXLAN device per endpoint +(uBridge's ``vxlan`` module, rtnetlink), enslaved into the link's per-link +kernel bridge next to the node's anchor. Each device is local to its own +host, but the VNI is the kernel's demux key for received frames on the +deployment-wide shared dstport socket — both ends must agree on it while +neither can ask the other, so the controller is the single source for both +values. + +* the device name is a pure function of the link id plus the endpoint's + node id: unique per (link, endpoint), within IFNAMSIZ, and outside the + ``gns3``/anchor name spaces (``gvl`` prefix). Determinism is what makes + a crashed run's leftover device recognisable — the compute deletes a + device it finds under the name before re-creating it; +* the VNI is a pure function of the link id and a controller-side salt — + a random base per link object, bumped when a retry needs a fresh VNI — + drawn from the upper half of the VNI space so it cannot collide with the + low well-known VNIs other software uses (Flannel's 1, Docker overlay + networks from 256, ...). + +The kernel refuses a second device registering a VNI already taken on the +same (dstport, socket) pair with EEXIST — a check that doubles as the +collision detector: the controller retries with a bumped salt and falls +back to the UDP relay when the space stays occupied (the relay needs no +VNI at all). +""" + +import hashlib + +# The VNI range derived links draw from: at or above 0x100000 (1M) to stay +# clear of the low VNIs other VXLAN software commonly registers, at or below +# 0xFFFFFE so every derived value is valid for the module's 1..16777215 +# (0xFFFFFF) check with one value of headroom. +VXLAN_VNI_MIN = 0x100000 +VXLAN_VNI_MAX = 0xFFFFFE + +# IFNAMSIZ (16) - 1 for the terminator: "gvl" (3) + 8 (link) + 4 (node) = 15. +VXLAN_NAME_PREFIX = "gvl" + + +def derive_vni(link_id, salt=0): + """ + The VNI both endpoints of *link_id* tunnel through. + + :param link_id: link UUID (or str) + :param salt: controller-side retry counter — bumped when the kernel + refuses the previous VNI as already registered on the shared + dstport socket; 0 for the first attempt + :returns: VNI in [VXLAN_VNI_MIN, VXLAN_VNI_MAX] + """ + + digest = hashlib.sha256(f"{link_id}:{salt}".encode()).digest() + span = VXLAN_VNI_MAX - VXLAN_VNI_MIN + 1 + return VXLAN_VNI_MIN + int.from_bytes(digest[:8], "big") % span + + +def vxlan_device_name(link_id, node_id): + """ + The VXLAN device name one endpoint of *link_id* creates on its own host, + deterministic from the link id plus the endpoint's node id (the pair + normally lives on two hosts, but a forced same-host pair must not name + both its devices identically). + """ + + link = str(link_id).replace("-", "")[:8] + node = str(node_id).replace("-", "")[:4] + return f"{VXLAN_NAME_PREFIX}{link}{node}" diff --git a/tests/api/routes/compute/test_capabilities.py b/tests/api/routes/compute/test_capabilities.py index c3fdf4eb4..3be162303 100644 --- a/tests/api/routes/compute/test_capabilities.py +++ b/tests/api/routes/compute/test_capabilities.py @@ -45,6 +45,7 @@ class TestCapabilitiesRoutes: monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_iol_tap_support", _no_tap_probe) monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_bridge_tap_support", _no_tap_probe) monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vpcs_tap", _no_tap_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vxlan_support", _no_tap_probe) response = await compute_client.get(app.url_path_for("compute:get_capabilities")) assert response.status_code == status.HTTP_200_OK assert response.json() == { @@ -73,6 +74,7 @@ class TestCapabilitiesRoutes: "ubridge_iol_tap": None, "ubridge_bridge_tap": None, "vpcs_tap": None, + "ubridge_vxlan": None, } async def test_get_on_gns3vm(self, app: FastAPI, compute_client: AsyncClient, on_gns3vm, monkeypatch) -> None: @@ -85,6 +87,7 @@ class TestCapabilitiesRoutes: monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_iol_tap_support", _no_tap_probe) monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_bridge_tap_support", _no_tap_probe) monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vpcs_tap", _no_tap_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vxlan_support", _no_tap_probe) response = await compute_client.get(app.url_path_for("compute:get_capabilities")) assert response.status_code == status.HTTP_200_OK assert response.json() == { @@ -113,6 +116,7 @@ class TestCapabilitiesRoutes: "ubridge_iol_tap": None, "ubridge_bridge_tap": None, "vpcs_tap": None, + "ubridge_vxlan": None, } async def test_get_reports_ubridge_tc(self, app: FastAPI, compute_client: AsyncClient, monkeypatch) -> None: @@ -199,6 +203,25 @@ class TestCapabilitiesRoutes: monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_tap_support", _no_tap_probe) monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_iol_tap_support", _no_tap_probe) monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_bridge_tap_support", _no_tap_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vxlan_support", _no_tap_probe) response = await compute_client.get(app.url_path_for("compute:get_capabilities")) assert response.status_code == status.HTTP_200_OK assert response.json()["vpcs_tap"] is True + + async def test_get_reports_ubridge_vxlan(self, app: FastAPI, compute_client: AsyncClient, monkeypatch) -> None: + """ + The vxlan capability is what makes a cross-compute link tunnel- + eligible, so it travels with the other compute capabilities. + """ + + async def fake_vxlan_probe(): + return True + + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vxlan_support", fake_vxlan_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_tap_support", _no_tap_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_iol_tap_support", _no_tap_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_bridge_tap_support", _no_tap_probe) + monkeypatch.setattr("gns3server.api.routes.compute.capabilities.probe_vpcs_tap", _no_tap_probe) + response = await compute_client.get(app.url_path_for("compute:get_capabilities")) + assert response.status_code == status.HTTP_200_OK + assert response.json()["ubridge_vxlan"] is True diff --git a/tests/compute/docker/test_docker_kernel_datapath.py b/tests/compute/docker/test_docker_kernel_datapath.py index abfc23220..94a816cf6 100644 --- a/tests/compute/docker/test_docker_kernel_datapath.py +++ b/tests/compute/docker/test_docker_kernel_datapath.py @@ -22,6 +22,7 @@ fallback and cleanup. """ import os +import socket import uuid from unittest.mock import MagicMock, call, patch @@ -32,6 +33,7 @@ from gns3server.compute.compute_error import ComputeError from gns3server.compute.docker import Docker from gns3server.compute.docker.docker_error import DockerError, DockerHttp404Error from gns3server.compute.docker.docker_vm import DockerVM +from gns3server.compute.error import NodeError from gns3server.compute.nios.nio_bridge import NIOBridge from gns3server.compute.ubridge.ubridge_error import UbridgeError, UbridgeNamespaceError from tests.utils import AsyncioMagicMock @@ -1468,3 +1470,147 @@ async def test_stop_clears_relay_bridge_registry(vm): await vm.stop() assert vm._bridges == set() assert vm._ubridge_tc_caps is None + + +# --------------------------------------------------------------------------- +# Cross-compute VXLAN tunnel +# --------------------------------------------------------------------------- + +VXLAN_SPEC = {"name": "gvl01234567abcd", "vni": 1048577, "remote": "192.168.1.2", "dstport": 14789} + + +def test_create_nio_bridge_carries_vxlan_spec(vm): + """ + The vxlan sub-spec of a cross-compute NIO survives the factory and the + asdict round-trip (the node replays the NIO from asdict on restart). + """ + + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "vxlan": dict(VXLAN_SPEC)}) + assert isinstance(nio, NIOBridge) + assert nio.vxlan == VXLAN_SPEC + assert nio.asdict()["vxlan"] == VXLAN_SPEC + + +@pytest.mark.asyncio +async def test_add_ubridge_kernel_connection_with_vxlan(vm): + """ + A vxlan-carrying NIO creates this end's tunnel device (lower device + resolved locally from the peer address) and enslaves it into the link's + kernel bridge next to the anchor. + """ + + async def send(command): + if command.startswith("vxlan show"): + raise UbridgeError("VXLAN does not exist") + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_hypervisor.send = AsyncioMagicMock(side_effect=send) + vm._namespace = 42 + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "vxlan": dict(VXLAN_SPEC)}) + + with patch.object(DockerVM, "_vxlan_lowerdev", return_value="eth0"): + await vm._add_ubridge_connection(nio, 0) + + host_ifc, _ = vm._veth_names(0, 0) + sends = [c.args[0] for c in vm._ubridge_hypervisor.send.call_args_list] + show = f'vxlan show "{VXLAN_SPEC["name"]}"' + create = ( + f'vxlan create "{VXLAN_SPEC["name"]}" {VXLAN_SPEC["vni"]} ' + f"remote={VXLAN_SPEC['remote']} dstport={VXLAN_SPEC['dstport']} dev=eth0" + ) + assert sends.index(show) < sends.index(create) + # The tunnel device is enslaved after the node's anchor, and the anchor + # still goes up. + assert sends.index(f'brctl addif "{BRIDGE}" "{host_ifc}"') < sends.index( + f'brctl addif "{BRIDGE}" "{VXLAN_SPEC["name"]}"' + ) + assert f'link set "{host_ifc}" up' in sends + # A failed show (device absent) must not trigger a delete. + assert f'vxlan delete "{VXLAN_SPEC["name"]}"' not in sends + + +@pytest.mark.asyncio +async def test_add_ubridge_kernel_connection_replaces_vxlan_leftover(vm): + """ + A device found under this link's deterministic name is this link's own + leftover (a crashed run, a node restart): deleted and re-created, which + makes wiring idempotent. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._namespace = 42 + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "vxlan": dict(VXLAN_SPEC)}) + + with patch.object(DockerVM, "_vxlan_lowerdev", return_value="eth0"): + await vm._add_ubridge_connection(nio, 0) + + sends = [c.args[0] for c in vm._ubridge_hypervisor.send.call_args_list] + show = f'vxlan show "{VXLAN_SPEC["name"]}"' + delete = f'vxlan delete "{VXLAN_SPEC["name"]}"' + create = ( + f'vxlan create "{VXLAN_SPEC["name"]}" {VXLAN_SPEC["vni"]} ' + f"remote={VXLAN_SPEC['remote']} dstport={VXLAN_SPEC['dstport']} dev=eth0" + ) + assert sends.index(show) < sends.index(delete) < sends.index(create) + + +@pytest.mark.asyncio +async def test_remove_kernel_nio_deletes_vxlan_device(vm): + """ + Link teardown deletes this end's tunnel device (the kernel un-enrolls it + from the bridge on unregister) before the bridge delete. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_send = AsyncioMagicMock() + vm.status = "started" + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + vm._ethernet_adapters[0].add_nio( + 0, vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "vxlan": dict(VXLAN_SPEC)}) + ) + + await vm.adapter_remove_nio_binding(0) + + vm._ubridge_send.assert_has_calls( + [ + call(f'brctl delif "{BRIDGE}" "{host_ifc}"'), + call(f'vxlan delete "{VXLAN_SPEC["name"]}"'), + call(f'brctl delete "{BRIDGE}"'), + ], + any_order=False, + ) + + +@pytest.mark.asyncio +async def test_vxlan_lowerdev_unresolvable_peer_raises(vm): + """ + A peer that cannot be resolved (or routed to) is a wiring error of this + link — the controller retries and then degrades it to the relay. + """ + + with patch("gns3server.compute.kernel_datapath.socket.getaddrinfo", side_effect=socket.gaierror("nope")): + with pytest.raises(NodeError): + await vm._vxlan_lowerdev("192.0.2.1") + + +def test_interface_with_address_matches_the_owning_interface(): + """ + The route's chosen source address is matched back to its interface, + scope suffixes stripped (link-local IPv6). + """ + + class _Addr: + def __init__(self, address): + self.address = address + + with patch( + "gns3server.compute.kernel_datapath.psutil.net_if_addrs", + return_value={ + "eth0": [_Addr("192.168.1.1")], + "eth1": [_Addr("fe80::1%eth1")], + }, + ): + assert DockerVM._interface_with_address("192.168.1.1") == "eth0" + assert DockerVM._interface_with_address("fe80::1%eth1") == "eth1" + assert DockerVM._interface_with_address("10.0.0.1") is None diff --git a/tests/compute/ubridge/test_tc_probe.py b/tests/compute/ubridge/test_tc_probe.py index 45275368c..202a0abfd 100644 --- a/tests/compute/ubridge/test_tc_probe.py +++ b/tests/compute/ubridge/test_tc_probe.py @@ -29,8 +29,10 @@ from gns3server.compute.ubridge.tc_probe import ( probe_iol_tap_support, probe_tap_support, probe_tc_capabilities, + probe_vxlan_support, ) from gns3server.compute.ubridge.ubridge_error import UbridgeError +from gns3server.config import Config pytestmark = pytest.mark.asyncio @@ -92,6 +94,7 @@ def probe_env(monkeypatch, tmp_path): tc_probe._tap_cache.clear() tc_probe._iol_tap_cache.clear() tc_probe._bridge_tap_cache.clear() + tc_probe._vxlan_cache.clear() FakeHypervisor.last = None # a real file on disk: the cache is keyed on its (path, mtime, size, # ctime) @@ -275,3 +278,51 @@ async def test_probe_bridge_tap_support_missing_binary(monkeypatch): monkeypatch.setattr(tc_probe.shutil, "which", lambda name: None) assert await probe_bridge_tap_support() is None assert FakeHypervisor.spawned == 0 + + +class _FixedUuid: + """A uuid4 stand-in whose .int/.hex pin the probe's random draws.""" + + def __init__(self, value): + self.int = value + self.hex = f"{value:032x}" + + +def _probe_create_port(): + """The dstport of the one `vxlan create` the probe issued.""" + + create = next(c for c in FakeHypervisor.last.commands if c.startswith("vxlan create ")) + return int(create.rsplit("dstport=", 1)[1]) + + +async def test_probe_vxlan_avoids_the_configured_dstport(monkeypatch, probe_env): + """ + A scratch draw landing exactly on the configured dstport must move: on + that port the scratch VNI shares a demux table with the live links' + devices, and an unlucky VNI draw would make the probe report the + capability unknown for the process's lifetime (the result is cached). + """ + + # int % 10000 == 9999 -> the scratch window draws its very last value + monkeypatch.setattr(tc_probe.uuid, "uuid4", lambda: _FixedUuid(29999)) + settings = Config.instance().settings.Server + monkeypatch.setattr(settings, "vxlan_dstport", 29999) + + assert await probe_vxlan_support() is True + assert _probe_create_port() == 20000 + + +async def test_probe_vxlan_scratch_port_stays_off_the_configured_one(monkeypatch, probe_env): + """ + Whatever the configured dstport, the probe's socket never lands on it + and stays inside its scratch window. + """ + + settings = Config.instance().settings.Server + for configured in (14789, 20000, 25000, 29999): + monkeypatch.setattr(settings, "vxlan_dstport", configured) + tc_probe._vxlan_cache.clear() + assert await probe_vxlan_support() is True + port = _probe_create_port() + assert 20000 <= port <= 29999, port + assert port != configured, (port, configured) diff --git a/tests/controller/test_kernel_datapath_link.py b/tests/controller/test_kernel_datapath_link.py index 1478cd9f4..1242179ef 100644 --- a/tests/controller/test_kernel_datapath_link.py +++ b/tests/controller/test_kernel_datapath_link.py @@ -20,18 +20,21 @@ Controller-side kernel-datapath link selection: eligibility of Docker-to- Docker links, NIO emission and the filters/markers guards. """ +from contextlib import contextmanager from unittest.mock import MagicMock, patch import pytest from gns3server.config import Config -from gns3server.controller.controller_error import ControllerError +from gns3server.controller.compute import ComputeError +from gns3server.controller.controller_error import ComputeConflictError, ControllerError from gns3server.controller.link import FILTERS from gns3server.controller.node import Node from gns3server.controller.ports.ethernet_port import EthernetPort from gns3server.controller.ports.serial_port import SerialPort from gns3server.controller.udp_link import UDPLink from gns3server.utils.kernel_anchor import kernel_anchor_name, kernel_cascade_names +from gns3server.utils.vxlan import vxlan_device_name from tests.utils import AsyncioMagicMock @@ -1174,3 +1177,350 @@ async def test_node_started_repushes_both_cascade_ends(project): sw1.put.assert_called_once_with("/adapters/0/ports/0/nio", data=entries[0][3], timeout=120) sw2.put.assert_called_once_with("/adapters/0/ports/0/nio", data=entries[1][3], timeout=120) + + +# --------------------------------------------------------------------------- +# Cross-compute VXLAN links +# --------------------------------------------------------------------------- + + +@contextmanager +def _cross_compute_enabled(): + settings = Config.instance().settings.Server + original = settings.enable_vxlan_cross_compute + settings.enable_vxlan_cross_compute = True + try: + yield settings + finally: + settings.enable_vxlan_cross_compute = original + + +def _vxlan_compute(compute_id, vxlan=True): + compute = MagicMock() + compute.id = compute_id + compute.capabilities = {"ubridge_vxlan": vxlan} + return compute + + +async def _cross_compute_link(project, vxlan1=True, vxlan2=True, same_host=False): + """A docker↔docker link whose endpoints live on two computes, with the + peer resolution (and the relay fallback's port reservation) mocked.""" + + compute1 = _vxlan_compute("compute-1", vxlan1) + compute2 = _vxlan_compute("compute-2", vxlan2) + + async def subnet_callback(other): + return ("192.168.1.1", "192.168.1.1" if same_host else "192.168.1.2") + + compute1.get_ip_on_same_subnet.side_effect = subnet_callback + + async def post_callback(path, data=None, **kwargs): + response = MagicMock() + response.json = {"udp_port": 1024} + return response + + compute1.post.side_effect = post_callback + compute2.post.side_effect = post_callback + node1 = _node(project, compute1, "docker1") + node2 = _node(project, compute2, "docker2") + link = UDPLink(project) + await link.add_node(node1, 0, 0, batch=True, dump=False) + await link.add_node(node2, 0, 0, batch=True, dump=False) + return link, node1, node2 + + +@pytest.mark.asyncio +async def test_vxlan_cross_compute_disabled_by_config(project): + """ + Cross-compute links stay on the relay until the deployment opts in — + the VXLAN tunnel caps the usable MTU at the underlay minus the outer + headers, so it is opt-in. + """ + + link, node1, node2 = await _cross_compute_link(project) + assert link._kernel_datapath_eligible(node1, node2) is False + + with _cross_compute_enabled(): + assert link._kernel_datapath_eligible(node1, node2) is True + + +@pytest.mark.asyncio +async def test_vxlan_cross_compute_requires_the_vxlan_capability(project): + """ + Both computes' uBridge must be able to create the tunnel device; an + unreported/failed probe (None) or an old build (False) keeps the link + on the relay, exactly like the other capability gates. + """ + + with _cross_compute_enabled(): + for vxlan1, vxlan2 in ((False, True), (True, False), (True, None), (None, None)): + link, node1, node2 = await _cross_compute_link(project, vxlan1=vxlan1, vxlan2=vxlan2) + assert link._kernel_datapath_eligible(node1, node2) is False + + +@pytest.mark.asyncio +async def test_vxlan_cross_compute_switch_stays_on_relay(project): + """ + The switch fast path absorbs the peer's anchor into the switch's own + bridge — a one-host shape; a switch link across computes keeps the + relay. + """ + + switch = _node(project, _vxlan_compute("compute-1"), "switch1", node_type="ethernet_switch") + peer = _node(project, _vxlan_compute("compute-2"), "docker2") + link = await _link(project, switch, peer) + with _cross_compute_enabled(): + assert link._kernel_datapath_eligible(switch, peer) is False + + +@pytest.mark.asyncio +async def test_prepare_vxlan_link_emits_tunnel_specs(project): + """ + Both ends get one nio_bridge sharing the per-link bridge name, each + carrying the same VNI/dstport (the pair must agree, and neither end can + ask the other) and the peer's address, with per-endpoint deterministic + device names. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _cross_compute_link(project) + entries = await link._prepare() + + assert len(entries) == 2 + expected_bridge = "gns3" + link.id.replace("-", "")[:11] + dstport = Config.instance().settings.Server.vxlan_dstport + specs = [entry[3] for entry in entries] + assert all(spec["type"] == "nio_bridge" for spec in specs) + assert all(spec["bridge"] == expected_bridge for spec in specs) + vxlan1, vxlan2 = specs[0]["vxlan"], specs[1]["vxlan"] + assert vxlan1["vni"] == vxlan2["vni"] + assert vxlan1["dstport"] == vxlan2["dstport"] == dstport + assert vxlan1["remote"] == "192.168.1.2" + assert vxlan2["remote"] == "192.168.1.1" + assert vxlan1["name"] == vxlan_device_name(link.id, node1.id) + assert vxlan2["name"] == vxlan_device_name(link.id, node2.id) + assert vxlan1["name"] != vxlan2["name"] + + +@pytest.mark.asyncio +async def test_prepare_vxlan_same_host_pair_falls_back_to_relay(project): + """ + Equal resolved addresses mean both computes share one host: the kernel + shares one VXLAN socket per dstport, so a tunnel pair cannot live there + — such links keep the relay, where the ends differ by UDP port. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _cross_compute_link(project, same_host=True) + entries = await link._prepare() + + assert all(entry[3]["type"] == "nio_udp" for entry in entries) + + +@pytest.mark.asyncio +async def test_commit_vxlan_retries_with_a_fresh_vni(project): + """ + A refused commit (reactively: the VNI was already registered on the + host's shared socket) re-prepares the link with a bumped salt — a fresh + VNI, identical on both ends — and recommits. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _cross_compute_link(project) + vnis = [] + + async def commit(entries): + vnis.append(entries[0][3]["vxlan"]["vni"]) + if len(vnis) == 1: + raise ComputeError("Could not create vxlan gvl...: File exists") + + with patch.object(link, "_commit_nios", commit): + await link.create() + + assert len(vnis) == 2 + assert vnis[0] != vnis[1] + assert link._vxlan_downgraded is False + + +@pytest.mark.asyncio +async def test_commit_vxlan_degrades_to_relay_after_retries(project): + """ + When every VXLAN attempt fails, the link degrades to the UDP relay — + the opt-in datapath must never make a link uncreatable. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _cross_compute_link(project) + commits = [] + + async def commit(entries): + commits.append(entries) + if any(entry[3].get("vxlan") for entry in entries): + raise ComputeError("Could not create vxlan gvl...: File exists") + + with patch.object(link, "_commit_nios", commit): + await link.create() + + assert link._vxlan_downgraded is True + assert len(commits) == 4 # the three VXLAN attempts, then the relay + assert all(entry[3].get("vxlan") for entry in commits[0]) + assert all(entry[3]["type"] == "nio_udp" for entry in commits[-1]) + + +async def _prepared_vxlan_link(project): + """A cross-compute link with its tunnel NIOs prepared (the shape a node + start replays) — call inside ``_cross_compute_enabled`` so the heal's + re-preparation stays on the tunnel too. _bind_members mirrors the + project-open finalize (the node's link back-reference is what lets the + heal find the conflicted link).""" + + link, node1, node2 = await _cross_compute_link(project) + await link._prepare() + link._bind_members() + link._created = True + return link, node1, node2 + + +def _vni_conflict(node, link): + """The compute's 409 when the kernel refuses the link's VNI on the + shared dstport socket — the wording KernelDatapathMixin emits.""" + + device = vxlan_device_name(link.id, node.id) + return ComputeConflictError( + "http://compute", {"message": f"Could not create VXLAN device {device} to 192.168.1.2 via eth0: File exists"} + ) + + +@pytest.mark.asyncio +async def test_node_start_self_heals_a_vni_conflict(project): + """ + A link wired while its nodes were stopped hits its VNI collision at + node start (the kernel's refusal is the first time anyone asks). The + start stops the half-started node, re-wires the link on a fresh VNI — + both ends, through the per-compute batch — and retries. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _prepared_vxlan_link(project) + salt = link._vxlan_salt + starts = [] + + async def post_callback(path, data=None, **kwargs): + if path.endswith("/start"): + starts.append(path) + if len(starts) == 1: + raise _vni_conflict(node1, link) + response = MagicMock() + response.json = {"udp_port": 1024} + return response + + node1.compute.post.side_effect = post_callback + await node1.start() + + assert len(starts) == 2 + assert link._vxlan_downgraded is False + assert link._vxlan_salt == salt + 1 + # the half-started node was stopped before the retry (a start on a + # running container early-returns without wiring) + assert any(call.args[0].endswith("/stop") for call in node1.compute.post.call_args_list) + # the fresh-VNI NIOs were re-dispatched to both computes + for compute in (node1.compute, node2.compute): + batches = [c for c in compute.post.call_args_list if str(c.args[0]).endswith("/nios/batch")] + assert len(batches) == 1 + + +@pytest.mark.asyncio +async def test_node_start_degrades_the_link_after_a_second_conflict(project): + """ + A conflict that survives the fresh VNI degrades the link to the relay + — the start must succeed no later than that round, the same guarantee + create() and the project-open batch path give. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _prepared_vxlan_link(project) + starts = [] + + async def post_callback(path, data=None, **kwargs): + if path.endswith("/start"): + starts.append(path) + if len(starts) <= 2: + raise _vni_conflict(node1, link) + response = MagicMock() + response.json = {"udp_port": 1024} + return response + + node1.compute.post.side_effect = post_callback + await node1.start() + + assert len(starts) == 3 + assert link._vxlan_downgraded is True + assert all(nio["type"] == "nio_udp" for nio in link._link_data) + + +@pytest.mark.asyncio +async def test_node_start_does_not_retry_other_failures(project): + """ + Only the VXLAN device conflict heals: any other start failure surfaces + unchanged, with a single attempt and no stop/redispatch side effects. + """ + + with _cross_compute_enabled(): + link, node1, node2 = await _prepared_vxlan_link(project) + salt = link._vxlan_salt + + async def post_callback(path, data=None, **kwargs): + if path.endswith("/start"): + raise ComputeError("DockerError: image not found") + response = MagicMock() + response.json = {} + return response + + node1.compute.post.side_effect = post_callback + with pytest.raises(ComputeError): + await node1.start() + + starts = [c for c in node1.compute.post.call_args_list if str(c.args[0]).endswith("/start")] + assert len(starts) == 1 + assert not any(call.args[0].endswith("/stop") for call in node1.compute.post.call_args_list) + assert link._vxlan_salt == salt + assert link._vxlan_downgraded is False + + +@pytest.mark.asyncio +async def test_open_heal_tries_the_full_vni_candidate_count(project): + """ + The project-open heal gives a refused link the same VNI candidate count + an interactive creation gets (fresh-salt rounds), degrading to the + relay only after they run out. + """ + + link, node1, node2 = await _cross_compute_link(project) + rounds = [] + + async def redispatch(links, downgrade): + rounds.append(downgrade) + return links if not downgrade else [] + + with patch.object(project, "_redispatch_vxlan_links", redispatch): + await project._heal_refused_vxlan_links([link]) + assert rounds == [False, False, True] + + +@pytest.mark.asyncio +async def test_open_heal_stops_at_the_first_successful_vni(project): + """ + A link healed by the first fresh-VNI round needs no further rounds — + no pointless second redispatch, no relay degradation. + """ + + link, node1, node2 = await _cross_compute_link(project) + rounds = [] + + async def redispatch(links, downgrade): + rounds.append(downgrade) + return [] + + with patch.object(project, "_redispatch_vxlan_links", redispatch): + await project._heal_refused_vxlan_links([link]) + assert rounds == [False] diff --git a/tests/e2e/README.md b/tests/e2e/README.md index fba5b9671..c53c6ea43 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -27,6 +27,7 @@ Beyond a Linux kernel with the datapath features (bridges, veth/TAP devices, tc * **`tc` (iproute2)** for the filter scenarios: their qdisc and classifier assertions read it from `PATH`. Without it those assertions cannot run. * **A Docker daemon** for the Docker scenarios (the harness's own guest driver talks to the same daemon as the server; socket override: `GNS3_E2E_DOCKER_SOCKET`). * **A C compiler that can link statically** (`cc -static`) for the raw-frame probe of the link-local scenario; without it that probe skips. +* **Root (or passwordless sudo) for the cross-compute scenario** — a second network stack on one machine is a network namespace, and Linux confines creating and entering one to CAP_SYS_ADMIN (uBridge's file capabilities do not help there). The scenario skips with the exact run command otherwise; it also writes the second compute's node files, which the harness's passwordless-sudo path cannot do as the test user. * **Emulator images** — see the matrix below. ## Test images @@ -93,3 +94,5 @@ Every kernel scenario also asserts the L2-anchor spec's server-side guarantees ( * `GNS3_E2E_DOCKER_SOCKET` — the Docker daemon socket the harness's guest driver talks to (default `/var/run/docker.sock` — the same daemon the server uses). * `GNS3_E2E_IDLEPC` — skip idle-PC detection for the Dynamips scenarios and use the given value. * `GNS3_E2E_DEBUG=1` — run the isolated instance with debug logging (the uBridge command stream lands in its `server.log`). + +* `test_vxlan_cross_compute.py` — the cross-compute VXLAN datapath, with two computes on one machine: the isolated instance on the host and a second isolated instance inside a network namespace joined to the host by a veth pair (`harness.cross_compute_fabric` — two computes cannot tunnel from the host namespace, since the kernel keeps one VXLAN socket per (namespace, dstport) and the pair must share one VNI on it). One real Alpine container per compute, linked across the tunnel: both ends agree on the VNI and the shared dstport, each per-link bridge enslaves exactly `[anchor, tunnel]`, the tunnel device's MTU is the underlay minus 50 with the bridges inheriting it (a frame at it crosses, one past it is dropped at the bridge egress), dual-stack ICMP crosses, a `delay 100` filter shapes both directions, a stale device squatting on the link's deterministic name is replaced on node restart, a device squatting the link's VNI at node start self-heals (both ends re-wired on a fresh VNI, the start succeeding), link delete leaves nothing behind on either machine, and a project close/reopen rebuilds the tunnel through the batch path. **Needs root** (see host requirements) — run it with `sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images pytest -m e2e tests/e2e/test_vxlan_cross_compute.py -s`. diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index 2b782c4c0..c1da7aff8 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -67,6 +67,7 @@ import time import urllib.error import urllib.parse import urllib.request +import uuid import pytest from websockets.sync.client import connect as ws_connect @@ -145,14 +146,15 @@ class Compute: {"compute_id": "local", "name": name, "node_type": "dynamips", "properties": properties}, ) - def create_docker_node(self, project_id, name, image, adapters=1): + def create_docker_node(self, project_id, name, image, adapters=1, compute_id="local"): """A standard Docker node (the veth datapath): the scenario addresses - its eth0 from the host and pings through it.""" + its eth0 from the host and pings through it. *compute_id* places it + on a remote compute (the cross-compute VXLAN scenario's peer).""" return self.call( "POST", f"/projects/{project_id}/nodes", { - "compute_id": "local", + "compute_id": compute_id, "name": name, "node_type": "docker", "properties": {"image": image, "adapters": adapters, "console_type": "telnet"}, @@ -295,10 +297,11 @@ default_admin_username = admin default_admin_password = admin [Server] -host = 127.0.0.1 +host = {host} port = {port} ubridge_path = {ubridge} enable_kernel_datapath = {kernel} +enable_vxlan_cross_compute = {vxlan} report_errors = False projects_path = {tmp}/projects configs_path = {tmp}/configs @@ -330,10 +333,14 @@ class Server: instance (own config dir — the controller DB and every path follow it — under a temporary directory removed on stop).""" - def __init__(self, compute, process=None, tmpdir=None): + def __init__(self, compute, process=None, tmpdir=None, privilege=()): self.compute = compute self.process = process self.tmpdir = tmpdir + # Command prefix for cleaning up the temp directory when the process + # ran privileged (a compute spawned inside a netns as root writes + # root-owned files the test user cannot remove itself). + self.privilege = privilege @property def isolated(self): @@ -361,6 +368,11 @@ class Server: self._stop_process() if self.tmpdir: shutil.rmtree(self.tmpdir, ignore_errors=True) + if self.privilege and os.path.exists(self.tmpdir): + # S603: fixed argv (the privilege prefix is sudo -n or empty) + subprocess.run( # noqa: S603 + [*self.privilege, "rm", "-rf", self.tmpdir], capture_output=True, check=False + ) def restart(self, kernel=True): """Restart an isolated instance in place: stop the process — the @@ -376,10 +388,15 @@ class Server: return self -def _spawn_isolated(tmpdir, kernel): +def _spawn_isolated(tmpdir, kernel, host="127.0.0.1", vxlan=False, netns=None, privilege=()): """Start one isolated instance inside *tmpdir* (its config is written there; a fresh free port each boot) and return ``(process, compute)``, - logged in. Shared by the first boot and by ``Server.restart``.""" + logged in. Shared by the first boot and by ``Server.restart``. + + With *netns* the instance is spawned inside that network namespace + (through *privilege*, e.g. ``sudo -n``) and binds *host* — the pattern + the cross-compute VXLAN scenario uses for its second "machine". + """ ubridge = shutil.which("ubridge") dynamips = shutil.which("dynamips") if not ubridge: @@ -402,7 +419,15 @@ def _spawn_isolated(tmpdir, kernel): fork_build = os.path.expanduser("~/myCode/GNS3/vpcs/src/vpcs") vpcs = fork_build if os.path.isfile(fork_build) else (shutil.which("vpcs") or "vpcs") config = ISOLATED_CONFIG.format( - port=port, ubridge=ubridge, dynamips=dynamips, images=images, tmp=tmpdir, kernel=kernel, vpcs=vpcs + port=port, + ubridge=ubridge, + dynamips=dynamips, + images=images, + tmp=tmpdir, + kernel=kernel, + vpcs=vpcs, + vxlan=vxlan, + host=host, ) config_path = os.path.join(tmpdir, "gns3_server.conf") with open(config_path, "w") as f: @@ -410,7 +435,15 @@ def _spawn_isolated(tmpdir, kernel): log = open(os.path.join(tmpdir, "server.log"), "a") # S603: fixed argv, this interpreter, no shell involvement - argv = [sys.executable, "-m", "gns3server", "--config", config_path] + argv = [ + *privilege, + *(["ip", "netns", "exec", netns] if netns else []), + sys.executable, + "-m", + "gns3server", + "--config", + config_path, + ] if os.environ.get("GNS3_E2E_DEBUG"): argv.append("-d") # debug logging — the uBridge command stream lands in server.log process = subprocess.Popen( # noqa: S603 @@ -420,13 +453,13 @@ def _spawn_isolated(tmpdir, kernel): stderr=subprocess.STDOUT, ) process.log = log # keep the file object alive with the process handle - compute = Compute(f"http://127.0.0.1:{port}/v3") + compute = Compute(f"http://{host}:{port}/v3") deadline = time.time() + 60 while time.time() < deadline: try: - # S310: the port is ours, the host loopback - with urllib.request.urlopen(f"http://127.0.0.1:{port}/v3/version", timeout=2): + # S310: the address is ours (loopback or the scenario's veth fabric) + with urllib.request.urlopen(f"http://{host}:{port}/v3/version", timeout=2): break except Exception: if process.poll() is not None: @@ -446,13 +479,13 @@ def _spawn_isolated(tmpdir, kernel): return process, compute -def _start_isolated(kernel): +def _start_isolated(kernel, vxlan=False): tmpdir = tempfile.mkdtemp(prefix="gns3-e2e-") - process, compute = _spawn_isolated(tmpdir, kernel) + process, compute = _spawn_isolated(tmpdir, kernel, vxlan=vxlan) return Server(compute, process, tmpdir) -def live_server(kernel=True): +def live_server(kernel=True, vxlan=False): """The server under test. With ``GNS3_E2E_URL`` set, that server is used for kernel mode (it must @@ -475,7 +508,7 @@ def live_server(kernel=True): ).login() server = Server(compute) else: - server = _start_isolated(kernel) + server = _start_isolated(kernel, vxlan=vxlan) if not wait_until(lambda: (server.compute.capabilities() or {}).get("version"), timeout=45): raise AssertionError( f"{server.compute.url}: the controller's local compute did not report capabilities within 45s" @@ -1505,3 +1538,211 @@ def pcap_icmp_rtt(path, ethertype): deltas.append((ts - pending) * 1000) pending = None return sum(deltas) / len(deltas) if deltas else None + + +# --------------------------------------------------------------------------- +# Cross-compute fabric: a second "machine" for VXLAN links +# --------------------------------------------------------------------------- + +# The /24 both ends of the veth "fabric" are addressed in. RFC 2544's +# benchmarking range: never routed on a real network, so it cannot collide +# with anything the host legitimately talks to. +FABRIC_SUBNET_PREFIX = "198.18.7" + + +def fabric_privilege(): + """The command prefix for network-namespace operations: nothing when the + suite runs as root, ``sudo -n`` when passwordless sudo is configured, + None otherwise (the caller skips with the explanation). + + A second network stack — the thing a cross-compute VXLAN link needs on + a single machine — is a network namespace, and Linux confines both + creating one and entering one to CAP_SYS_ADMIN of its owning user + namespace. uBridge's file capabilities do not help here (they are caps + for netlink *operations*, not for joining a namespace), so either the + whole run is root or sudo carries the two calls. + """ + + if os.geteuid() == 0: + return [] + sudo = shutil.which("sudo") + # S603: resolved binary, fixed argv, no shell + if sudo and subprocess.run([sudo, "-n", "true"], capture_output=True, check=False).returncode == 0: # noqa: S603 + return [sudo, "-n"] + return None + + +class CrossComputeFabric: + """A second "machine" on one host: a network namespace joined to the + host by a veth pair, both ends addressed inside ``198.18.7.0/24``. + + Two computes on one machine cannot tunnel to each other from the host + namespace: the kernel keeps one VXLAN socket per (namespace, dstport) + and both ends must register the same VNI on it — the pair collides by + design. Distinct network namespaces give the pair its two sockets, and + the veth pair is the L2 fabric the tunnels ride. uBridge's file + capabilities make the *devices* work inside the namespace; only the + namespace itself needs privilege (see ``fabric_privilege``). + """ + + def __init__(self, netns, host_ifc, peer_ifc, host_ip, peer_ip, prefix): + self.netns = netns + self.host_ifc = host_ifc + self.peer_ifc = peer_ifc + self.host_ip = host_ip + self.peer_ip = peer_ip + self.prefix = prefix + + def netns_run(self, *argv, **kwargs): + """Run a command inside the namespace (privileged, like every netns + access).""" + kwargs.setdefault("capture_output", True) + kwargs.setdefault("text", True) + # S603: fixed argv (the caller builds the netns-side command) + return subprocess.run( # noqa: S603 + [*self.prefix, "ip", "netns", "exec", self.netns, *argv], check=False, **kwargs + ) + + def iface_mtu(self, name): + """The MTU of an interface inside the namespace (None when absent).""" + out = self.netns_run("ip", "-o", "link", "show", name).stdout + match = re.search(r"\bmtu (\d+)", out) + return int(match.group(1)) if match else None + + def iface_exists(self, name): + return self.netns_run("ip", "link", "show", name).returncode == 0 + + def vxlan_show(self, name): + """``ip -d link show`` details of an interface inside the namespace, + as one string (None when absent) — carries the ``vxlan id + ... dstport `` attributes.""" + result = self.netns_run("ip", "-d", "link", "show", name) + return result.stdout if result.returncode == 0 and name in result.stdout else None + + def bridge_members(self, bridge): + """Interfaces enslaved to a bridge inside the namespace, or None + when the bridge does not exist.""" + result = self.netns_run("ls", f"/sys/class/net/{bridge}/brif") + if result.returncode != 0: + return None + return sorted(result.stdout.split()) + + def brport_state(self, name): + """An interface's bridge-port STP state inside the namespace (None + when it is not enslaved; the kernel reports 3 for forwarding).""" + result = self.netns_run("cat", f"/sys/class/net/{name}/brport/state") + if result.returncode != 0: + return None + return result.stdout.strip() or None + + def teardown(self): + """Delete the namespace (all its devices go with it) and any host + end a half-created pair left behind.""" + # S603: fixed argv, resolved ip binary, no shell + subprocess.run( # noqa: S603 + [*self.prefix, "ip", "netns", "del", self.netns], capture_output=True, check=False + ) + subprocess.run( # noqa: S603 + [*self.prefix, "ip", "link", "del", self.host_ifc], capture_output=True, check=False + ) + + +def cross_compute_fabric(): + """Create the fabric, or skip the calling test when this host cannot. + + The names carry a random suffix so concurrent runs (and leftovers of a + crashed one) cannot collide; the caller must call ``teardown()`` in a + finally.""" + + prefix = fabric_privilege() + if prefix is None: + pytest.skip( + "cross-compute VXLAN needs a second network namespace: run the suite as root, e.g. " + "`sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images pytest -m e2e " + "tests/e2e/test_vxlan_cross_compute.py -s`, or configure passwordless sudo" + ) + + suffix = uuid.uuid4().hex[:6] + fabric = CrossComputeFabric( + netns=f"gns3vx{suffix}", + host_ifc=f"vxh{suffix}", + peer_ifc=f"vxp{suffix}", + host_ip=f"{FABRIC_SUBNET_PREFIX}.1", + peer_ip=f"{FABRIC_SUBNET_PREFIX}.2", + prefix=prefix, + ) + try: + # S603: fixed argv, resolved ip binary, no shell + subprocess.run( # noqa: S603 + [*prefix, "ip", "netns", "add", fabric.netns], check=True, capture_output=True + ) + subprocess.run( # noqa: S603 + [*prefix, "ip", "link", "add", fabric.host_ifc, "type", "veth", "peer", "name", fabric.peer_ifc], + check=True, + capture_output=True, + ) + subprocess.run( # noqa: S603 + [*prefix, "ip", "link", "set", fabric.peer_ifc, "netns", fabric.netns], check=True, capture_output=True + ) + subprocess.run( # noqa: S603 + [*prefix, "ip", "addr", "add", f"{fabric.host_ip}/24", "dev", fabric.host_ifc], + check=True, + capture_output=True, + ) + subprocess.run( # noqa: S603 + [*prefix, "ip", "link", "set", fabric.host_ifc, "up"], check=True, capture_output=True + ) + for argv in ( + ("ip", "addr", "add", f"{fabric.peer_ip}/24", "dev", fabric.peer_ifc), + ("ip", "link", "set", fabric.peer_ifc, "up"), + ("ip", "link", "set", "lo", "up"), + ): + fabric.netns_run(*argv, check=True) + except subprocess.CalledProcessError: + fabric.teardown() + raise + return fabric + + +def start_isolated_peer(fabric, kernel=True): + """An isolated instance on the other side of the fabric: spawned inside + the namespace and bound to its address, so the first server can register + it as a remote compute whose network stack is the second "machine".""" + + tmpdir = tempfile.mkdtemp(prefix="gns3-e2e-peer-") + process, compute = _spawn_isolated(tmpdir, kernel, host=fabric.peer_ip, netns=fabric.netns, privilege=fabric.prefix) + return Server(compute, process, tmpdir, privilege=fabric.prefix) + + +def register_remote_compute(server, peer, name="vxlan-peer"): + """Register the peer's server on the first server's controller, so links + between nodes on the two computes are cross-compute links. Waits until + the controller connected it (capabilities reported — the vxlan probe in + particular: the controller gates tunnel eligibility on it).""" + + compute = server.compute.call( + "POST", + "/computes", + { + "name": name, + "protocol": "http", + "host": peer.compute.url.split("://", 1)[1].split(":", 1)[0], + "port": int(peer.compute.url.rsplit(":", 1)[1].split("/", 1)[0]), + "user": peer.compute.username, + "password": peer.compute.password, + }, + ) + compute_id = compute["compute_id"] + if not wait_until(lambda: (server.compute.call("GET", f"/computes/{compute_id}") or {}).get("version"), timeout=60): + raise AssertionError(f"controller did not connect the remote compute {compute_id} within 60s") + if not wait_until( + lambda: ( + ((server.compute.call("GET", f"/computes/{compute_id}") or {}).get("capabilities") or {}).get( + "ubridge_vxlan" + ) + is True + ), + timeout=60, + ): + raise AssertionError(f"remote compute {compute_id} never reported ubridge_vxlan (its netns probe failed)") + return compute_id diff --git a/tests/e2e/test_vxlan_cross_compute.py b/tests/e2e/test_vxlan_cross_compute.py new file mode 100644 index 000000000..87518da7e --- /dev/null +++ b/tests/e2e/test_vxlan_cross_compute.py @@ -0,0 +1,305 @@ +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +""" +Live end-to-end test for cross-compute VXLAN links (pytest marker ``e2e``): +two computes on one machine — the harness's isolated instance on the host and +a second isolated instance inside a network namespace joined to the host by a +veth pair (``harness.cross_compute_fabric``) — with one real Alpine container +per compute, linked across the tunnel. + +Two computes on one machine cannot tunnel from the host namespace: the kernel +keeps one VXLAN socket per (namespace, dstport), and both ends of a link must +register the same VNI on it, so the pair collides by design (the controller +falls back to the relay for same-host pairs). Separate network namespaces +give the pair its two sockets, and the veth pair is the L2 fabric the tunnels +ride — the closest a single machine gets to two hosts. The same-host +fallback itself is asserted by the controller unit tests; this scenario is +the real-kernel proof of the tunnel. + +Needs root: a network namespace is privileged to create and to enter, and the +second compute's node files (its project directories) are written by the +test. Run it as:: + + sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images pytest -m e2e \\ + tests/e2e/test_vxlan_cross_compute.py -s + +What it asserts: + +* both ends report the same VNI on the shared dstport, each pointing at the + other machine's address, and each per-link bridge enslaves exactly + ``[anchor, tunnel]``; +* the tunnel device's MTU is the underlay MTU minus 50 (the kernel derives + it from the lower device the peer route resolves to), the bridges inherit + it, and a frame past it is dropped at the bridge egress while one at it + crosses — the reason the datapath is opt-in on non-jumbo fabrics; +* real dual-stack ICMP crosses, a ``delay 100`` filter shapes it (one end + per direction), link delete leaves no device or bridge on either machine; +* a stale tunnel device squatting on the link's deterministic name is + replaced by the attach path, and the tunnel recovers; +* a device squatting the link's VNI at node start self-heals: the start + re-wires the link on a fresh VNI — the peer re-wired too — and succeeds. +""" + +import os +import re +import subprocess +import time + +import pytest + +from gns3server.utils.vxlan import VXLAN_VNI_MAX, VXLAN_VNI_MIN, vxlan_device_name +from tests.e2e import harness + +pytestmark = pytest.mark.e2e + +# every standard Docker adapter has a single port +ETH = (0, 0) +G1_IP = "10.66.1.1" +G2_IP = "10.66.1.2" +# the dual-stack half: a ULA mirroring the v4 addressing +G1_IP6 = "fd00:66:1::1" +G2_IP6 = "fd00:66:1::2" + +# ICMP payloads against the derived tunnel MTU (underlay 1500 -> tunnel +# 1450): 1400 + 42 bytes of headers is a 1442-byte frame, under the bridge +# egress gate; 1500 + 42 is a 1542-byte frame, over it — and over the +# guest's own 1500 MTU too, so the IP stack may fragment it; either way the +# pieces cannot cross the 1450-wide egress and the ping must fail silently. +FIT_PAYLOAD = 1400 +TOO_BIG_PAYLOAD = 1500 + + +def _ip(*argv): + """Run an iproute2 command on the host (the scenario runs as root).""" + ip = harness.IP + assert ip, "ip binary not found" + # S603: resolved binary, fixed argv from the test itself + return subprocess.run([ip, *argv], capture_output=True, text=True, check=True) # noqa: S603 + + +def _host_vxlan_attrs(name): + """(vni, dstport, remote) of a host-side tunnel device, from + ``ip -d link show``.""" + details = _ip("-d", "link", "show", name).stdout + vni = re.search(r"\bvxlan id (\d+)", details) + dstport = re.search(r"\bdstport (\d+)", details) + remote = re.search(r"\bremote (\S+)", details) + assert vni and dstport and remote, details + return int(vni.group(1)), int(dstport.group(1)), remote.group(1) + + +def _peer_vxlan_attrs(fabric, name): + details = fabric.vxlan_show(name) + assert details, f"tunnel device {name} missing inside {fabric.netns}" + vni = re.search(r"\bvxlan id (\d+)", details) + dstport = re.search(r"\bdstport (\d+)", details) + remote = re.search(r"\bremote (\S+)", details) + assert vni and dstport and remote, details + return int(vni.group(1)), int(dstport.group(1)), remote.group(1) + + +def _container_id(daemon, node): + container_id = daemon.container_id(f"GNS3.{node['name']}.{node['project_id']}") + assert container_id, f"container for {node['name']} not found on the Docker daemon" + node["container_id"] = container_id + return container_id + + +def _start_pair(compute, pid, daemon, n1, n2): + """Start both containers and wait for their userspace to be reachable: + the v4 address from the interfaces file (init.sh), the v6 twin from the + harness (the daemon's disable_ipv6 on bridge networks makes the + interfaces-file route fail). The container ids are re-resolved after the + starts: a project reopen can replace a stale container with a fresh one + under the same deterministic name (a new id).""" + for node in (n1, n2): + compute.call("POST", f"/projects/{pid}/nodes/{node['node_id']}/start") + _container_id(daemon, node) + for node, address in ((n1, G1_IP), (n2, G2_IP)): + harness.docker_wait_address(daemon, node["container_id"], address) + for node, address6 in ((n1, G1_IP6), (n2, G2_IP6)): + harness.docker_bring_up_ipv6(daemon, node["container_id"], address6) + + +def test_vxlan_cross_compute_link(): + if os.geteuid() != 0: + pytest.skip( + "cross-compute VXLAN needs a second network stack (a netns) and writes the second " + "compute's node files: run as root, e.g. `sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images " + "pytest -m e2e tests/e2e/test_vxlan_cross_compute.py -s`" + ) + + fabric = harness.cross_compute_fabric() + server = harness.live_server(kernel=True, vxlan=True) + peer = harness.start_isolated_peer(fabric) + compute = server.compute + daemon = harness.DockerDaemon() + failed = True + pid = None + try: + peer_id = harness.register_remote_compute(server, peer) + image = harness.ensure_docker_image(server) + project = compute.create_project("vxlan-e2e") + pid = project["project_id"] + + print(".. creating one container per compute") + n1 = compute.create_docker_node(pid, "VXE1", image) + harness.configure_docker_interfaces(n1, G1_IP) + _container_id(daemon, n1) + n2 = compute.create_docker_node(pid, "VXE2", image, compute_id=peer_id) + harness.configure_docker_interfaces(n2, G2_IP) + _container_id(daemon, n2) + assert n1["compute_id"] == "local", n1["compute_id"] + assert n2["compute_id"] == peer_id, n2["compute_id"] + + print(".. linking them across the computes") + link = compute.create_link(pid, (n1["node_id"], *ETH), (n2["node_id"], *ETH)) + lid = link["link_id"] + assert link["kernel_datapath"] is True, link + bridge = harness.link_bridge_name(lid) + a1 = harness.docker_anchor_name(n1["node_id"], *ETH) + a2 = harness.docker_anchor_name(n2["node_id"], *ETH) + vx1 = vxlan_device_name(lid, n1["node_id"]) + vx2 = vxlan_device_name(lid, n2["node_id"]) + assert not harness.tap_exists(vx1), "the tunnel lands with node start" + + print(".. starting the containers (the wiring lands with node start)") + _start_pair(compute, pid, daemon, n1, n2) + + print(".. asserting the tunnel on both machines") + assert harness.tap_exists(vx1), f"tunnel device {vx1} missing on the host" + assert harness.bridge_members(bridge) == sorted([a1, vx1]), harness.bridge_members(bridge) + assert fabric.bridge_members(bridge) == sorted([a2, vx2]), fabric.bridge_members(bridge) + vni1, port1, remote1 = _host_vxlan_attrs(vx1) + vni2, port2, remote2 = _peer_vxlan_attrs(fabric, vx2) + assert vni1 == vni2, (vni1, vni2) + assert VXLAN_VNI_MIN <= vni1 <= VXLAN_VNI_MAX, vni1 + assert port1 == port2, (port1, port2) + assert remote1 == fabric.peer_ip, (remote1, fabric.peer_ip) + assert remote2 == fabric.host_ip, (remote2, fabric.host_ip) + + # MTU: the kernel derives the tunnel device's MTU from the lower + # device the peer route resolves to (underlay - 50 for the outer + # headers), and the per-link bridge — no MTU of its own — reports + # the minimum of its ports, i.e. the honest "what this fabric can + # carry". + underlay_mtu = int(harness.sysfs(f"/sys/class/net/{fabric.host_ifc}/mtu")) + assert underlay_mtu == 1500, underlay_mtu # the veth default; the derived MTU follows from it + assert int(harness.sysfs(f"/sys/class/net/{vx1}/mtu")) == underlay_mtu - 50 + assert int(harness.sysfs(f"/sys/class/net/{bridge}/mtu")) == underlay_mtu - 50 + assert fabric.iface_mtu(vx2) == underlay_mtu - 50 + assert fabric.iface_mtu(bridge) == underlay_mtu - 50 + assert fabric.iface_mtu(fabric.peer_ifc) == underlay_mtu + + # The tunnel device is an L2 anchor like any other: hardened + # (lenient — the shared with plain `ip` in the doc's example) and + # FORWARDING on both bridges. + if harness.l2only_supported(): + harness.assert_pure_l2(vx1) + harness.assert_forwarding(vx1) + assert fabric.brport_state(vx2) == "3", fabric.brport_state(vx2) + + print(".. pinging across the tunnel") + ping = harness.docker_wait_ping(daemon, n1["container_id"], G2_IP) + assert ping["loss"] == 0, ping["raw"] + assert ping["avg"] is not None and ping["avg"] < 50, ping + ping6 = harness.docker_wait_ping(daemon, n1["container_id"], G2_IP6) + assert ping6["loss"] == 0, ping6["raw"] + + # The derived MTU is the hard limit: a frame at it crosses, one past + # it is dropped at the bridge egress — silently, which is why the + # deployment has to size the underlay for the frames its guests send. + fit = harness.docker_ping(daemon, n1["container_id"], G2_IP, size=FIT_PAYLOAD) + assert fit["loss"] == 0, fit["raw"] + too_big = harness.docker_ping(daemon, n1["container_id"], G2_IP, count=2, size=TOO_BIG_PAYLOAD) + assert too_big["loss"] > 0 or too_big["exit"] != 0, too_big["raw"] + + print(".. delay 100 shapes the tunnel on both ends (one direction each)") + compute.call("PUT", f"/projects/{pid}/links/{lid}", {"filters": {"delay": [100]}}) + time.sleep(1) + delayed = harness.docker_ping(daemon, n1["container_id"], G2_IP, count=5, interval=None, timeout=2) + assert delayed["loss"] == 0, delayed["raw"] + assert 150 <= (delayed["avg"] or 0) <= 450, delayed + compute.call("PUT", f"/projects/{pid}/links/{lid}", {"filters": {}}) + + print(".. a stale device on the link's name is replaced on node restart") + compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/stop") + _ip("link", "del", vx1) + # A stale tunnel: same deterministic name, some other VNI/port — + # what a crashed run or a hand-made device would leave behind. + _ip("link", "add", vx1, "type", "vxlan", "id", "4242", "dstport", "47999", "remote", "127.0.0.1") + compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/start") + harness.docker_wait_address(daemon, n1["container_id"], G1_IP) + harness.docker_bring_up_ipv6(daemon, n1["container_id"], G1_IP6) + stale_vni, stale_port, stale_remote = _host_vxlan_attrs(vx1) + assert (stale_vni, stale_port, stale_remote) == (vni1, port1, remote1), (stale_vni, stale_port, stale_remote) + assert harness.bridge_members(bridge) == sorted([a1, vx1]), harness.bridge_members(bridge) + assert int(harness.sysfs(f"/sys/class/net/{bridge}/mtu")) == underlay_mtu - 50 + assert harness.docker_wait_ping(daemon, n1["container_id"], G2_IP, attempts=5)["loss"] == 0 + + print(".. a VNI squatted at node start self-heals on a fresh VNI") + compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/stop") + # A device holding the link's exact VNI on the shared dstport socket + # — what a colliding link (or an operator) would have registered. + # The node's start replays the stored NIO, the kernel refuses the + # VNI, and the controller must re-wire the link on a fresh one and + # retry the start instead of failing it. + _ip("link", "add", "gvxsquat", "type", "vxlan", "id", str(vni1), "dstport", str(port1), "remote", "127.0.0.1") + try: + compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/start") + harness.docker_wait_address(daemon, n1["container_id"], G1_IP) + harness.docker_bring_up_ipv6(daemon, n1["container_id"], G1_IP6) + healed_vni, healed_port, healed_remote = _host_vxlan_attrs(vx1) + assert healed_vni != vni1, (healed_vni, vni1) + assert VXLAN_VNI_MIN <= healed_vni <= VXLAN_VNI_MAX, healed_vni + assert healed_port == port1, (healed_port, port1) + assert healed_remote == remote1, (healed_remote, remote1) + # the peer was re-wired to the fresh VNI too — the pair must agree + peer_vni, _, _ = _peer_vxlan_attrs(fabric, vx2) + assert peer_vni == healed_vni, (peer_vni, healed_vni) + assert harness.docker_wait_ping(daemon, n1["container_id"], G2_IP, attempts=5)["loss"] == 0 + finally: + _ip("link", "del", "gvxsquat") + + print(".. deleting the link leaves no tunnel or bridge behind on either machine") + compute.call("DELETE", f"/projects/{pid}/links/{lid}") + assert not harness.tap_exists(vx1) + assert not harness.tap_exists(bridge) + assert not fabric.iface_exists(vx2), f"tunnel device {vx2} survived on the peer" + assert fabric.bridge_members(bridge) is None + + print(".. closing and reopening the project rebuilds the tunnel through the batch path") + compute.call("POST", f"/projects/{pid}/close") + compute.call("POST", f"/projects/{pid}/open") + _start_pair(compute, pid, daemon, n1, n2) + assert harness.tap_exists(vx1), f"tunnel device {vx1} not rebuilt on reopen" + assert fabric.iface_exists(vx2), f"tunnel device {vx2} not rebuilt on reopen" + assert harness.docker_wait_ping(daemon, n1["container_id"], G2_IP, attempts=5)["loss"] == 0 + + failed = False + finally: + keep = failed and harness.keep_on_failure() + if not keep: + peer.stop() + if pid is not None: + harness.release(server, pid, failed=failed) + else: + server.stop() + if keep: + print(f"\n!! kept the fabric {fabric.netns} (and its peer compute) for inspection") + else: + fabric.teardown() diff --git a/tests/utils/test_vxlan.py b/tests/utils/test_vxlan.py new file mode 100644 index 000000000..3253a972a --- /dev/null +++ b/tests/utils/test_vxlan.py @@ -0,0 +1,71 @@ +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +""" +Cross-compute VXLAN link identity: the derived VNI and device name. +""" + +import uuid + +from gns3server.utils.vxlan import VXLAN_NAME_PREFIX, VXLAN_VNI_MAX, VXLAN_VNI_MIN, derive_vni, vxlan_device_name + + +def test_derive_vni_is_deterministic_and_bounded(): + link_id = str(uuid.uuid4()) + vni = derive_vni(link_id) + assert derive_vni(link_id) == vni + assert VXLAN_VNI_MIN <= vni <= VXLAN_VNI_MAX + + +def test_derive_vni_salt_moves_the_value(): + link_id = str(uuid.uuid4()) + values = {derive_vni(link_id, salt) for salt in range(16)} + # Every salt lands in range; a bumped salt is a fresh point of the space + # (a collision here would defeat the retry, and 16 draws out of ~16.8M + # values collapsing is a hash failure, not bad luck). + assert all(VXLAN_VNI_MIN <= v <= VXLAN_VNI_MAX for v in values) + assert len(values) == 16 + + +def test_derive_vni_avoids_low_well_known_vnis(): + # Flannel starts at 1, Docker overlay networks at 256: derived links + # must never land in the low range whatever the link id. + for _ in range(200): + assert derive_vni(str(uuid.uuid4()), uuid.uuid4().int) >= VXLAN_VNI_MIN + + +def test_vxlan_device_name_deterministic_and_within_ifnamsiz(): + link_id = str(uuid.uuid4()) + node_id = str(uuid.uuid4()) + name = vxlan_device_name(link_id, node_id) + assert name == vxlan_device_name(link_id, node_id) + assert name.startswith(VXLAN_NAME_PREFIX) + # IFNAMSIZ (16) includes the NUL terminator: the kernel accepts 15 chars. + assert len(name) <= 15 + + +def test_vxlan_device_name_unique_per_endpoint_and_link(): + link1 = str(uuid.uuid4()) + link2 = str(uuid.uuid4()) + node1 = str(uuid.uuid4()) + node2 = str(uuid.uuid4()) + names = { + vxlan_device_name(link1, node1), + vxlan_device_name(link1, node2), + vxlan_device_name(link2, node1), + } + assert len(names) == 3 From ca31031fdc37e3478ec27e627beb6b1d170edf7c Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sun, 11 Oct 2026 22:56:42 +0800 Subject: [PATCH 02/10] test: drop the cross-compute VXLAN e2e scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scenario needed root for its network namespace and its fabric subnet turned out host-specific: policy routing (a proxy's table consulted before main) sends any non-RFC1918 destination out the default route, so the peer compute was unreachable behind the veth — unfixable by picking another subnet, and the failure paths leaked namespaces and processes. Too environment-coupled to keep in the suite. Unit coverage is unchanged (selection matrix, tunnel spec derivation, VNI retry/degrade/self-heal, device lifecycle, capability probe); the feature doc's Verification section now carries the manual two-compute checklist instead of the e2e reference. --- docs/features/vxlan-cross-compute.md | 2 +- tests/e2e/README.md | 3 - tests/e2e/harness.py | 273 ++--------------------- tests/e2e/test_vxlan_cross_compute.py | 305 -------------------------- 4 files changed, 17 insertions(+), 566 deletions(-) delete mode 100644 tests/e2e/test_vxlan_cross_compute.py diff --git a/docs/features/vxlan-cross-compute.md b/docs/features/vxlan-cross-compute.md index dc0c8aa3c..1e07e2332 100644 --- a/docs/features/vxlan-cross-compute.md +++ b/docs/features/vxlan-cross-compute.md @@ -112,7 +112,7 @@ The `vxlan show` step is the idempotency contract: a device found under the link ## Verification * Unit: `tests/controller/test_kernel_datapath_link.py` (selection matrix, tunnel spec derivation, VNI retry, downgrade), `tests/utils/test_vxlan.py`, `tests/compute/docker/test_docker_kernel_datapath.py` (device lifecycle, leftover replacement, teardown ordering), `tests/api/routes/compute/test_capabilities.py`. -* Live: `tests/e2e/test_vxlan_cross_compute.py` — two computes on one machine (a network namespace joined to the host by a veth pair as the second "machine"), real containers per compute: VNI/port agreement, bridge membership, the derived MTU and its hard egress drop, dual-stack traffic, a delay filter, leftover replacement, a VNI squatted at node start (both ends re-wired on a fresh VNI, the start succeeding), residue-free delete and a rebuild through the project-open batch path. +* Live: no automated scenario — a second network stack on one machine means a network namespace, which needs root and host-specific routing and proved too environment-coupled for the suite. Validate on a real two-compute deployment (two machines, or a namespace-isolated second server): both ends agree on the VNI and dstport, each per-link bridge enslaves exactly `[anchor, tunnel]`, the derived MTU crosses while one past it is dropped at the bridge egress, traffic and filters behave as on the same-host datapath, and link delete plus project close/reopen leave no devices behind. ## Roadmap diff --git a/tests/e2e/README.md b/tests/e2e/README.md index c53c6ea43..fba5b9671 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -27,7 +27,6 @@ Beyond a Linux kernel with the datapath features (bridges, veth/TAP devices, tc * **`tc` (iproute2)** for the filter scenarios: their qdisc and classifier assertions read it from `PATH`. Without it those assertions cannot run. * **A Docker daemon** for the Docker scenarios (the harness's own guest driver talks to the same daemon as the server; socket override: `GNS3_E2E_DOCKER_SOCKET`). * **A C compiler that can link statically** (`cc -static`) for the raw-frame probe of the link-local scenario; without it that probe skips. -* **Root (or passwordless sudo) for the cross-compute scenario** — a second network stack on one machine is a network namespace, and Linux confines creating and entering one to CAP_SYS_ADMIN (uBridge's file capabilities do not help there). The scenario skips with the exact run command otherwise; it also writes the second compute's node files, which the harness's passwordless-sudo path cannot do as the test user. * **Emulator images** — see the matrix below. ## Test images @@ -94,5 +93,3 @@ Every kernel scenario also asserts the L2-anchor spec's server-side guarantees ( * `GNS3_E2E_DOCKER_SOCKET` — the Docker daemon socket the harness's guest driver talks to (default `/var/run/docker.sock` — the same daemon the server uses). * `GNS3_E2E_IDLEPC` — skip idle-PC detection for the Dynamips scenarios and use the given value. * `GNS3_E2E_DEBUG=1` — run the isolated instance with debug logging (the uBridge command stream lands in its `server.log`). - -* `test_vxlan_cross_compute.py` — the cross-compute VXLAN datapath, with two computes on one machine: the isolated instance on the host and a second isolated instance inside a network namespace joined to the host by a veth pair (`harness.cross_compute_fabric` — two computes cannot tunnel from the host namespace, since the kernel keeps one VXLAN socket per (namespace, dstport) and the pair must share one VNI on it). One real Alpine container per compute, linked across the tunnel: both ends agree on the VNI and the shared dstport, each per-link bridge enslaves exactly `[anchor, tunnel]`, the tunnel device's MTU is the underlay minus 50 with the bridges inheriting it (a frame at it crosses, one past it is dropped at the bridge egress), dual-stack ICMP crosses, a `delay 100` filter shapes both directions, a stale device squatting on the link's deterministic name is replaced on node restart, a device squatting the link's VNI at node start self-heals (both ends re-wired on a fresh VNI, the start succeeding), link delete leaves nothing behind on either machine, and a project close/reopen rebuilds the tunnel through the batch path. **Needs root** (see host requirements) — run it with `sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images pytest -m e2e tests/e2e/test_vxlan_cross_compute.py -s`. diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index c1da7aff8..2b782c4c0 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -67,7 +67,6 @@ import time import urllib.error import urllib.parse import urllib.request -import uuid import pytest from websockets.sync.client import connect as ws_connect @@ -146,15 +145,14 @@ class Compute: {"compute_id": "local", "name": name, "node_type": "dynamips", "properties": properties}, ) - def create_docker_node(self, project_id, name, image, adapters=1, compute_id="local"): + def create_docker_node(self, project_id, name, image, adapters=1): """A standard Docker node (the veth datapath): the scenario addresses - its eth0 from the host and pings through it. *compute_id* places it - on a remote compute (the cross-compute VXLAN scenario's peer).""" + its eth0 from the host and pings through it.""" return self.call( "POST", f"/projects/{project_id}/nodes", { - "compute_id": compute_id, + "compute_id": "local", "name": name, "node_type": "docker", "properties": {"image": image, "adapters": adapters, "console_type": "telnet"}, @@ -297,11 +295,10 @@ default_admin_username = admin default_admin_password = admin [Server] -host = {host} +host = 127.0.0.1 port = {port} ubridge_path = {ubridge} enable_kernel_datapath = {kernel} -enable_vxlan_cross_compute = {vxlan} report_errors = False projects_path = {tmp}/projects configs_path = {tmp}/configs @@ -333,14 +330,10 @@ class Server: instance (own config dir — the controller DB and every path follow it — under a temporary directory removed on stop).""" - def __init__(self, compute, process=None, tmpdir=None, privilege=()): + def __init__(self, compute, process=None, tmpdir=None): self.compute = compute self.process = process self.tmpdir = tmpdir - # Command prefix for cleaning up the temp directory when the process - # ran privileged (a compute spawned inside a netns as root writes - # root-owned files the test user cannot remove itself). - self.privilege = privilege @property def isolated(self): @@ -368,11 +361,6 @@ class Server: self._stop_process() if self.tmpdir: shutil.rmtree(self.tmpdir, ignore_errors=True) - if self.privilege and os.path.exists(self.tmpdir): - # S603: fixed argv (the privilege prefix is sudo -n or empty) - subprocess.run( # noqa: S603 - [*self.privilege, "rm", "-rf", self.tmpdir], capture_output=True, check=False - ) def restart(self, kernel=True): """Restart an isolated instance in place: stop the process — the @@ -388,15 +376,10 @@ class Server: return self -def _spawn_isolated(tmpdir, kernel, host="127.0.0.1", vxlan=False, netns=None, privilege=()): +def _spawn_isolated(tmpdir, kernel): """Start one isolated instance inside *tmpdir* (its config is written there; a fresh free port each boot) and return ``(process, compute)``, - logged in. Shared by the first boot and by ``Server.restart``. - - With *netns* the instance is spawned inside that network namespace - (through *privilege*, e.g. ``sudo -n``) and binds *host* — the pattern - the cross-compute VXLAN scenario uses for its second "machine". - """ + logged in. Shared by the first boot and by ``Server.restart``.""" ubridge = shutil.which("ubridge") dynamips = shutil.which("dynamips") if not ubridge: @@ -419,15 +402,7 @@ def _spawn_isolated(tmpdir, kernel, host="127.0.0.1", vxlan=False, netns=None, p fork_build = os.path.expanduser("~/myCode/GNS3/vpcs/src/vpcs") vpcs = fork_build if os.path.isfile(fork_build) else (shutil.which("vpcs") or "vpcs") config = ISOLATED_CONFIG.format( - port=port, - ubridge=ubridge, - dynamips=dynamips, - images=images, - tmp=tmpdir, - kernel=kernel, - vpcs=vpcs, - vxlan=vxlan, - host=host, + port=port, ubridge=ubridge, dynamips=dynamips, images=images, tmp=tmpdir, kernel=kernel, vpcs=vpcs ) config_path = os.path.join(tmpdir, "gns3_server.conf") with open(config_path, "w") as f: @@ -435,15 +410,7 @@ def _spawn_isolated(tmpdir, kernel, host="127.0.0.1", vxlan=False, netns=None, p log = open(os.path.join(tmpdir, "server.log"), "a") # S603: fixed argv, this interpreter, no shell involvement - argv = [ - *privilege, - *(["ip", "netns", "exec", netns] if netns else []), - sys.executable, - "-m", - "gns3server", - "--config", - config_path, - ] + argv = [sys.executable, "-m", "gns3server", "--config", config_path] if os.environ.get("GNS3_E2E_DEBUG"): argv.append("-d") # debug logging — the uBridge command stream lands in server.log process = subprocess.Popen( # noqa: S603 @@ -453,13 +420,13 @@ def _spawn_isolated(tmpdir, kernel, host="127.0.0.1", vxlan=False, netns=None, p stderr=subprocess.STDOUT, ) process.log = log # keep the file object alive with the process handle - compute = Compute(f"http://{host}:{port}/v3") + compute = Compute(f"http://127.0.0.1:{port}/v3") deadline = time.time() + 60 while time.time() < deadline: try: - # S310: the address is ours (loopback or the scenario's veth fabric) - with urllib.request.urlopen(f"http://{host}:{port}/v3/version", timeout=2): + # S310: the port is ours, the host loopback + with urllib.request.urlopen(f"http://127.0.0.1:{port}/v3/version", timeout=2): break except Exception: if process.poll() is not None: @@ -479,13 +446,13 @@ def _spawn_isolated(tmpdir, kernel, host="127.0.0.1", vxlan=False, netns=None, p return process, compute -def _start_isolated(kernel, vxlan=False): +def _start_isolated(kernel): tmpdir = tempfile.mkdtemp(prefix="gns3-e2e-") - process, compute = _spawn_isolated(tmpdir, kernel, vxlan=vxlan) + process, compute = _spawn_isolated(tmpdir, kernel) return Server(compute, process, tmpdir) -def live_server(kernel=True, vxlan=False): +def live_server(kernel=True): """The server under test. With ``GNS3_E2E_URL`` set, that server is used for kernel mode (it must @@ -508,7 +475,7 @@ def live_server(kernel=True, vxlan=False): ).login() server = Server(compute) else: - server = _start_isolated(kernel, vxlan=vxlan) + server = _start_isolated(kernel) if not wait_until(lambda: (server.compute.capabilities() or {}).get("version"), timeout=45): raise AssertionError( f"{server.compute.url}: the controller's local compute did not report capabilities within 45s" @@ -1538,211 +1505,3 @@ def pcap_icmp_rtt(path, ethertype): deltas.append((ts - pending) * 1000) pending = None return sum(deltas) / len(deltas) if deltas else None - - -# --------------------------------------------------------------------------- -# Cross-compute fabric: a second "machine" for VXLAN links -# --------------------------------------------------------------------------- - -# The /24 both ends of the veth "fabric" are addressed in. RFC 2544's -# benchmarking range: never routed on a real network, so it cannot collide -# with anything the host legitimately talks to. -FABRIC_SUBNET_PREFIX = "198.18.7" - - -def fabric_privilege(): - """The command prefix for network-namespace operations: nothing when the - suite runs as root, ``sudo -n`` when passwordless sudo is configured, - None otherwise (the caller skips with the explanation). - - A second network stack — the thing a cross-compute VXLAN link needs on - a single machine — is a network namespace, and Linux confines both - creating one and entering one to CAP_SYS_ADMIN of its owning user - namespace. uBridge's file capabilities do not help here (they are caps - for netlink *operations*, not for joining a namespace), so either the - whole run is root or sudo carries the two calls. - """ - - if os.geteuid() == 0: - return [] - sudo = shutil.which("sudo") - # S603: resolved binary, fixed argv, no shell - if sudo and subprocess.run([sudo, "-n", "true"], capture_output=True, check=False).returncode == 0: # noqa: S603 - return [sudo, "-n"] - return None - - -class CrossComputeFabric: - """A second "machine" on one host: a network namespace joined to the - host by a veth pair, both ends addressed inside ``198.18.7.0/24``. - - Two computes on one machine cannot tunnel to each other from the host - namespace: the kernel keeps one VXLAN socket per (namespace, dstport) - and both ends must register the same VNI on it — the pair collides by - design. Distinct network namespaces give the pair its two sockets, and - the veth pair is the L2 fabric the tunnels ride. uBridge's file - capabilities make the *devices* work inside the namespace; only the - namespace itself needs privilege (see ``fabric_privilege``). - """ - - def __init__(self, netns, host_ifc, peer_ifc, host_ip, peer_ip, prefix): - self.netns = netns - self.host_ifc = host_ifc - self.peer_ifc = peer_ifc - self.host_ip = host_ip - self.peer_ip = peer_ip - self.prefix = prefix - - def netns_run(self, *argv, **kwargs): - """Run a command inside the namespace (privileged, like every netns - access).""" - kwargs.setdefault("capture_output", True) - kwargs.setdefault("text", True) - # S603: fixed argv (the caller builds the netns-side command) - return subprocess.run( # noqa: S603 - [*self.prefix, "ip", "netns", "exec", self.netns, *argv], check=False, **kwargs - ) - - def iface_mtu(self, name): - """The MTU of an interface inside the namespace (None when absent).""" - out = self.netns_run("ip", "-o", "link", "show", name).stdout - match = re.search(r"\bmtu (\d+)", out) - return int(match.group(1)) if match else None - - def iface_exists(self, name): - return self.netns_run("ip", "link", "show", name).returncode == 0 - - def vxlan_show(self, name): - """``ip -d link show`` details of an interface inside the namespace, - as one string (None when absent) — carries the ``vxlan id - ... dstport `` attributes.""" - result = self.netns_run("ip", "-d", "link", "show", name) - return result.stdout if result.returncode == 0 and name in result.stdout else None - - def bridge_members(self, bridge): - """Interfaces enslaved to a bridge inside the namespace, or None - when the bridge does not exist.""" - result = self.netns_run("ls", f"/sys/class/net/{bridge}/brif") - if result.returncode != 0: - return None - return sorted(result.stdout.split()) - - def brport_state(self, name): - """An interface's bridge-port STP state inside the namespace (None - when it is not enslaved; the kernel reports 3 for forwarding).""" - result = self.netns_run("cat", f"/sys/class/net/{name}/brport/state") - if result.returncode != 0: - return None - return result.stdout.strip() or None - - def teardown(self): - """Delete the namespace (all its devices go with it) and any host - end a half-created pair left behind.""" - # S603: fixed argv, resolved ip binary, no shell - subprocess.run( # noqa: S603 - [*self.prefix, "ip", "netns", "del", self.netns], capture_output=True, check=False - ) - subprocess.run( # noqa: S603 - [*self.prefix, "ip", "link", "del", self.host_ifc], capture_output=True, check=False - ) - - -def cross_compute_fabric(): - """Create the fabric, or skip the calling test when this host cannot. - - The names carry a random suffix so concurrent runs (and leftovers of a - crashed one) cannot collide; the caller must call ``teardown()`` in a - finally.""" - - prefix = fabric_privilege() - if prefix is None: - pytest.skip( - "cross-compute VXLAN needs a second network namespace: run the suite as root, e.g. " - "`sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images pytest -m e2e " - "tests/e2e/test_vxlan_cross_compute.py -s`, or configure passwordless sudo" - ) - - suffix = uuid.uuid4().hex[:6] - fabric = CrossComputeFabric( - netns=f"gns3vx{suffix}", - host_ifc=f"vxh{suffix}", - peer_ifc=f"vxp{suffix}", - host_ip=f"{FABRIC_SUBNET_PREFIX}.1", - peer_ip=f"{FABRIC_SUBNET_PREFIX}.2", - prefix=prefix, - ) - try: - # S603: fixed argv, resolved ip binary, no shell - subprocess.run( # noqa: S603 - [*prefix, "ip", "netns", "add", fabric.netns], check=True, capture_output=True - ) - subprocess.run( # noqa: S603 - [*prefix, "ip", "link", "add", fabric.host_ifc, "type", "veth", "peer", "name", fabric.peer_ifc], - check=True, - capture_output=True, - ) - subprocess.run( # noqa: S603 - [*prefix, "ip", "link", "set", fabric.peer_ifc, "netns", fabric.netns], check=True, capture_output=True - ) - subprocess.run( # noqa: S603 - [*prefix, "ip", "addr", "add", f"{fabric.host_ip}/24", "dev", fabric.host_ifc], - check=True, - capture_output=True, - ) - subprocess.run( # noqa: S603 - [*prefix, "ip", "link", "set", fabric.host_ifc, "up"], check=True, capture_output=True - ) - for argv in ( - ("ip", "addr", "add", f"{fabric.peer_ip}/24", "dev", fabric.peer_ifc), - ("ip", "link", "set", fabric.peer_ifc, "up"), - ("ip", "link", "set", "lo", "up"), - ): - fabric.netns_run(*argv, check=True) - except subprocess.CalledProcessError: - fabric.teardown() - raise - return fabric - - -def start_isolated_peer(fabric, kernel=True): - """An isolated instance on the other side of the fabric: spawned inside - the namespace and bound to its address, so the first server can register - it as a remote compute whose network stack is the second "machine".""" - - tmpdir = tempfile.mkdtemp(prefix="gns3-e2e-peer-") - process, compute = _spawn_isolated(tmpdir, kernel, host=fabric.peer_ip, netns=fabric.netns, privilege=fabric.prefix) - return Server(compute, process, tmpdir, privilege=fabric.prefix) - - -def register_remote_compute(server, peer, name="vxlan-peer"): - """Register the peer's server on the first server's controller, so links - between nodes on the two computes are cross-compute links. Waits until - the controller connected it (capabilities reported — the vxlan probe in - particular: the controller gates tunnel eligibility on it).""" - - compute = server.compute.call( - "POST", - "/computes", - { - "name": name, - "protocol": "http", - "host": peer.compute.url.split("://", 1)[1].split(":", 1)[0], - "port": int(peer.compute.url.rsplit(":", 1)[1].split("/", 1)[0]), - "user": peer.compute.username, - "password": peer.compute.password, - }, - ) - compute_id = compute["compute_id"] - if not wait_until(lambda: (server.compute.call("GET", f"/computes/{compute_id}") or {}).get("version"), timeout=60): - raise AssertionError(f"controller did not connect the remote compute {compute_id} within 60s") - if not wait_until( - lambda: ( - ((server.compute.call("GET", f"/computes/{compute_id}") or {}).get("capabilities") or {}).get( - "ubridge_vxlan" - ) - is True - ), - timeout=60, - ): - raise AssertionError(f"remote compute {compute_id} never reported ubridge_vxlan (its netns probe failed)") - return compute_id diff --git a/tests/e2e/test_vxlan_cross_compute.py b/tests/e2e/test_vxlan_cross_compute.py deleted file mode 100644 index 87518da7e..000000000 --- a/tests/e2e/test_vxlan_cross_compute.py +++ /dev/null @@ -1,305 +0,0 @@ -# -# Copyright (C) 2026 GNS3 Technologies Inc. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -""" -Live end-to-end test for cross-compute VXLAN links (pytest marker ``e2e``): -two computes on one machine — the harness's isolated instance on the host and -a second isolated instance inside a network namespace joined to the host by a -veth pair (``harness.cross_compute_fabric``) — with one real Alpine container -per compute, linked across the tunnel. - -Two computes on one machine cannot tunnel from the host namespace: the kernel -keeps one VXLAN socket per (namespace, dstport), and both ends of a link must -register the same VNI on it, so the pair collides by design (the controller -falls back to the relay for same-host pairs). Separate network namespaces -give the pair its two sockets, and the veth pair is the L2 fabric the tunnels -ride — the closest a single machine gets to two hosts. The same-host -fallback itself is asserted by the controller unit tests; this scenario is -the real-kernel proof of the tunnel. - -Needs root: a network namespace is privileged to create and to enter, and the -second compute's node files (its project directories) are written by the -test. Run it as:: - - sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images pytest -m e2e \\ - tests/e2e/test_vxlan_cross_compute.py -s - -What it asserts: - -* both ends report the same VNI on the shared dstport, each pointing at the - other machine's address, and each per-link bridge enslaves exactly - ``[anchor, tunnel]``; -* the tunnel device's MTU is the underlay MTU minus 50 (the kernel derives - it from the lower device the peer route resolves to), the bridges inherit - it, and a frame past it is dropped at the bridge egress while one at it - crosses — the reason the datapath is opt-in on non-jumbo fabrics; -* real dual-stack ICMP crosses, a ``delay 100`` filter shapes it (one end - per direction), link delete leaves no device or bridge on either machine; -* a stale tunnel device squatting on the link's deterministic name is - replaced by the attach path, and the tunnel recovers; -* a device squatting the link's VNI at node start self-heals: the start - re-wires the link on a fresh VNI — the peer re-wired too — and succeeds. -""" - -import os -import re -import subprocess -import time - -import pytest - -from gns3server.utils.vxlan import VXLAN_VNI_MAX, VXLAN_VNI_MIN, vxlan_device_name -from tests.e2e import harness - -pytestmark = pytest.mark.e2e - -# every standard Docker adapter has a single port -ETH = (0, 0) -G1_IP = "10.66.1.1" -G2_IP = "10.66.1.2" -# the dual-stack half: a ULA mirroring the v4 addressing -G1_IP6 = "fd00:66:1::1" -G2_IP6 = "fd00:66:1::2" - -# ICMP payloads against the derived tunnel MTU (underlay 1500 -> tunnel -# 1450): 1400 + 42 bytes of headers is a 1442-byte frame, under the bridge -# egress gate; 1500 + 42 is a 1542-byte frame, over it — and over the -# guest's own 1500 MTU too, so the IP stack may fragment it; either way the -# pieces cannot cross the 1450-wide egress and the ping must fail silently. -FIT_PAYLOAD = 1400 -TOO_BIG_PAYLOAD = 1500 - - -def _ip(*argv): - """Run an iproute2 command on the host (the scenario runs as root).""" - ip = harness.IP - assert ip, "ip binary not found" - # S603: resolved binary, fixed argv from the test itself - return subprocess.run([ip, *argv], capture_output=True, text=True, check=True) # noqa: S603 - - -def _host_vxlan_attrs(name): - """(vni, dstport, remote) of a host-side tunnel device, from - ``ip -d link show``.""" - details = _ip("-d", "link", "show", name).stdout - vni = re.search(r"\bvxlan id (\d+)", details) - dstport = re.search(r"\bdstport (\d+)", details) - remote = re.search(r"\bremote (\S+)", details) - assert vni and dstport and remote, details - return int(vni.group(1)), int(dstport.group(1)), remote.group(1) - - -def _peer_vxlan_attrs(fabric, name): - details = fabric.vxlan_show(name) - assert details, f"tunnel device {name} missing inside {fabric.netns}" - vni = re.search(r"\bvxlan id (\d+)", details) - dstport = re.search(r"\bdstport (\d+)", details) - remote = re.search(r"\bremote (\S+)", details) - assert vni and dstport and remote, details - return int(vni.group(1)), int(dstport.group(1)), remote.group(1) - - -def _container_id(daemon, node): - container_id = daemon.container_id(f"GNS3.{node['name']}.{node['project_id']}") - assert container_id, f"container for {node['name']} not found on the Docker daemon" - node["container_id"] = container_id - return container_id - - -def _start_pair(compute, pid, daemon, n1, n2): - """Start both containers and wait for their userspace to be reachable: - the v4 address from the interfaces file (init.sh), the v6 twin from the - harness (the daemon's disable_ipv6 on bridge networks makes the - interfaces-file route fail). The container ids are re-resolved after the - starts: a project reopen can replace a stale container with a fresh one - under the same deterministic name (a new id).""" - for node in (n1, n2): - compute.call("POST", f"/projects/{pid}/nodes/{node['node_id']}/start") - _container_id(daemon, node) - for node, address in ((n1, G1_IP), (n2, G2_IP)): - harness.docker_wait_address(daemon, node["container_id"], address) - for node, address6 in ((n1, G1_IP6), (n2, G2_IP6)): - harness.docker_bring_up_ipv6(daemon, node["container_id"], address6) - - -def test_vxlan_cross_compute_link(): - if os.geteuid() != 0: - pytest.skip( - "cross-compute VXLAN needs a second network stack (a netns) and writes the second " - "compute's node files: run as root, e.g. `sudo env GNS3_E2E_IMAGES=$HOME/GNS3/images " - "pytest -m e2e tests/e2e/test_vxlan_cross_compute.py -s`" - ) - - fabric = harness.cross_compute_fabric() - server = harness.live_server(kernel=True, vxlan=True) - peer = harness.start_isolated_peer(fabric) - compute = server.compute - daemon = harness.DockerDaemon() - failed = True - pid = None - try: - peer_id = harness.register_remote_compute(server, peer) - image = harness.ensure_docker_image(server) - project = compute.create_project("vxlan-e2e") - pid = project["project_id"] - - print(".. creating one container per compute") - n1 = compute.create_docker_node(pid, "VXE1", image) - harness.configure_docker_interfaces(n1, G1_IP) - _container_id(daemon, n1) - n2 = compute.create_docker_node(pid, "VXE2", image, compute_id=peer_id) - harness.configure_docker_interfaces(n2, G2_IP) - _container_id(daemon, n2) - assert n1["compute_id"] == "local", n1["compute_id"] - assert n2["compute_id"] == peer_id, n2["compute_id"] - - print(".. linking them across the computes") - link = compute.create_link(pid, (n1["node_id"], *ETH), (n2["node_id"], *ETH)) - lid = link["link_id"] - assert link["kernel_datapath"] is True, link - bridge = harness.link_bridge_name(lid) - a1 = harness.docker_anchor_name(n1["node_id"], *ETH) - a2 = harness.docker_anchor_name(n2["node_id"], *ETH) - vx1 = vxlan_device_name(lid, n1["node_id"]) - vx2 = vxlan_device_name(lid, n2["node_id"]) - assert not harness.tap_exists(vx1), "the tunnel lands with node start" - - print(".. starting the containers (the wiring lands with node start)") - _start_pair(compute, pid, daemon, n1, n2) - - print(".. asserting the tunnel on both machines") - assert harness.tap_exists(vx1), f"tunnel device {vx1} missing on the host" - assert harness.bridge_members(bridge) == sorted([a1, vx1]), harness.bridge_members(bridge) - assert fabric.bridge_members(bridge) == sorted([a2, vx2]), fabric.bridge_members(bridge) - vni1, port1, remote1 = _host_vxlan_attrs(vx1) - vni2, port2, remote2 = _peer_vxlan_attrs(fabric, vx2) - assert vni1 == vni2, (vni1, vni2) - assert VXLAN_VNI_MIN <= vni1 <= VXLAN_VNI_MAX, vni1 - assert port1 == port2, (port1, port2) - assert remote1 == fabric.peer_ip, (remote1, fabric.peer_ip) - assert remote2 == fabric.host_ip, (remote2, fabric.host_ip) - - # MTU: the kernel derives the tunnel device's MTU from the lower - # device the peer route resolves to (underlay - 50 for the outer - # headers), and the per-link bridge — no MTU of its own — reports - # the minimum of its ports, i.e. the honest "what this fabric can - # carry". - underlay_mtu = int(harness.sysfs(f"/sys/class/net/{fabric.host_ifc}/mtu")) - assert underlay_mtu == 1500, underlay_mtu # the veth default; the derived MTU follows from it - assert int(harness.sysfs(f"/sys/class/net/{vx1}/mtu")) == underlay_mtu - 50 - assert int(harness.sysfs(f"/sys/class/net/{bridge}/mtu")) == underlay_mtu - 50 - assert fabric.iface_mtu(vx2) == underlay_mtu - 50 - assert fabric.iface_mtu(bridge) == underlay_mtu - 50 - assert fabric.iface_mtu(fabric.peer_ifc) == underlay_mtu - - # The tunnel device is an L2 anchor like any other: hardened - # (lenient — the shared with plain `ip` in the doc's example) and - # FORWARDING on both bridges. - if harness.l2only_supported(): - harness.assert_pure_l2(vx1) - harness.assert_forwarding(vx1) - assert fabric.brport_state(vx2) == "3", fabric.brport_state(vx2) - - print(".. pinging across the tunnel") - ping = harness.docker_wait_ping(daemon, n1["container_id"], G2_IP) - assert ping["loss"] == 0, ping["raw"] - assert ping["avg"] is not None and ping["avg"] < 50, ping - ping6 = harness.docker_wait_ping(daemon, n1["container_id"], G2_IP6) - assert ping6["loss"] == 0, ping6["raw"] - - # The derived MTU is the hard limit: a frame at it crosses, one past - # it is dropped at the bridge egress — silently, which is why the - # deployment has to size the underlay for the frames its guests send. - fit = harness.docker_ping(daemon, n1["container_id"], G2_IP, size=FIT_PAYLOAD) - assert fit["loss"] == 0, fit["raw"] - too_big = harness.docker_ping(daemon, n1["container_id"], G2_IP, count=2, size=TOO_BIG_PAYLOAD) - assert too_big["loss"] > 0 or too_big["exit"] != 0, too_big["raw"] - - print(".. delay 100 shapes the tunnel on both ends (one direction each)") - compute.call("PUT", f"/projects/{pid}/links/{lid}", {"filters": {"delay": [100]}}) - time.sleep(1) - delayed = harness.docker_ping(daemon, n1["container_id"], G2_IP, count=5, interval=None, timeout=2) - assert delayed["loss"] == 0, delayed["raw"] - assert 150 <= (delayed["avg"] or 0) <= 450, delayed - compute.call("PUT", f"/projects/{pid}/links/{lid}", {"filters": {}}) - - print(".. a stale device on the link's name is replaced on node restart") - compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/stop") - _ip("link", "del", vx1) - # A stale tunnel: same deterministic name, some other VNI/port — - # what a crashed run or a hand-made device would leave behind. - _ip("link", "add", vx1, "type", "vxlan", "id", "4242", "dstport", "47999", "remote", "127.0.0.1") - compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/start") - harness.docker_wait_address(daemon, n1["container_id"], G1_IP) - harness.docker_bring_up_ipv6(daemon, n1["container_id"], G1_IP6) - stale_vni, stale_port, stale_remote = _host_vxlan_attrs(vx1) - assert (stale_vni, stale_port, stale_remote) == (vni1, port1, remote1), (stale_vni, stale_port, stale_remote) - assert harness.bridge_members(bridge) == sorted([a1, vx1]), harness.bridge_members(bridge) - assert int(harness.sysfs(f"/sys/class/net/{bridge}/mtu")) == underlay_mtu - 50 - assert harness.docker_wait_ping(daemon, n1["container_id"], G2_IP, attempts=5)["loss"] == 0 - - print(".. a VNI squatted at node start self-heals on a fresh VNI") - compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/stop") - # A device holding the link's exact VNI on the shared dstport socket - # — what a colliding link (or an operator) would have registered. - # The node's start replays the stored NIO, the kernel refuses the - # VNI, and the controller must re-wire the link on a fresh one and - # retry the start instead of failing it. - _ip("link", "add", "gvxsquat", "type", "vxlan", "id", str(vni1), "dstport", str(port1), "remote", "127.0.0.1") - try: - compute.call("POST", f"/projects/{pid}/nodes/{n1['node_id']}/start") - harness.docker_wait_address(daemon, n1["container_id"], G1_IP) - harness.docker_bring_up_ipv6(daemon, n1["container_id"], G1_IP6) - healed_vni, healed_port, healed_remote = _host_vxlan_attrs(vx1) - assert healed_vni != vni1, (healed_vni, vni1) - assert VXLAN_VNI_MIN <= healed_vni <= VXLAN_VNI_MAX, healed_vni - assert healed_port == port1, (healed_port, port1) - assert healed_remote == remote1, (healed_remote, remote1) - # the peer was re-wired to the fresh VNI too — the pair must agree - peer_vni, _, _ = _peer_vxlan_attrs(fabric, vx2) - assert peer_vni == healed_vni, (peer_vni, healed_vni) - assert harness.docker_wait_ping(daemon, n1["container_id"], G2_IP, attempts=5)["loss"] == 0 - finally: - _ip("link", "del", "gvxsquat") - - print(".. deleting the link leaves no tunnel or bridge behind on either machine") - compute.call("DELETE", f"/projects/{pid}/links/{lid}") - assert not harness.tap_exists(vx1) - assert not harness.tap_exists(bridge) - assert not fabric.iface_exists(vx2), f"tunnel device {vx2} survived on the peer" - assert fabric.bridge_members(bridge) is None - - print(".. closing and reopening the project rebuilds the tunnel through the batch path") - compute.call("POST", f"/projects/{pid}/close") - compute.call("POST", f"/projects/{pid}/open") - _start_pair(compute, pid, daemon, n1, n2) - assert harness.tap_exists(vx1), f"tunnel device {vx1} not rebuilt on reopen" - assert fabric.iface_exists(vx2), f"tunnel device {vx2} not rebuilt on reopen" - assert harness.docker_wait_ping(daemon, n1["container_id"], G2_IP, attempts=5)["loss"] == 0 - - failed = False - finally: - keep = failed and harness.keep_on_failure() - if not keep: - peer.stop() - if pid is not None: - harness.release(server, pid, failed=failed) - else: - server.stop() - if keep: - print(f"\n!! kept the fabric {fabric.netns} (and its peer compute) for inspection") - else: - fabric.teardown() From 577be9b94ee2b5298b16ae650b73473ebab74246 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Wed, 14 Oct 2026 10:17:52 +0800 Subject: [PATCH 03/10] docs: pin down the VXLAN MTU derivation and its re-derivation paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified against the running kernel's sources (7.2.6): the 50-byte headroom is 20 outer IPv4 + 8 UDP + 8 VXLAN + 14 inner Ethernet header (vxlan_headroom(), include/net/vxlan.h); the vxlan driver has no NETDEV_CHANGEMTU handler so a live tunnel never follows its lower device, while the bridge does follow its ports at runtime (NETDEV_CHANGEMTU -> br_mtu_auto_adjust). Document the operational consequence: the wiring path re-creates the device on every node start and link (re)creation, so raising the underlay MTU on both computes and stop/start-ing the nodes is the fix — no manual ip link needed. --- docs/features/vxlan-cross-compute.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/features/vxlan-cross-compute.md b/docs/features/vxlan-cross-compute.md index 1e07e2332..e498432ea 100644 --- a/docs/features/vxlan-cross-compute.md +++ b/docs/features/vxlan-cross-compute.md @@ -102,9 +102,9 @@ The `vxlan show` step is the idempotency contract: a device found under the link ## Known caveats -* **MTU is the whole story.** The kernel derives the tunnel device's MTU from the lower device (`underlay − 50` for the outer headers; `− 70` for an IPv6 underlay) and the per-link bridge, which carries no MTU of its own, reports the minimum of its ports — i.e. the derived value. A frame past it is dropped at the bridge egress **silently** (no ICMP, no log), which is why the datapath is opt-in: on a 1500-byte fabric a tunnel caps guests at 1450 while the relay carries 1500 with no tax. Guests set their own interface MTU, exactly like a VM or container raising eth0 on the same-host datapath. +* **MTU is the whole story.** The kernel derives the tunnel device's MTU from the lower device (`underlay − 50` for an IPv4 underlay, `− 70` for IPv6; the 50 is 20 outer IPv4 + 8 UDP + 8 VXLAN + 14 inner Ethernet header — a VXLAN packet carries the guest's complete inner frame, per `vxlan_headroom()` in `include/net/vxlan.h`) and the per-link bridge, which carries no MTU of its own, reports the minimum of its ports — i.e. the derived value. A frame past it is dropped at the bridge egress **silently** (no ICMP, no log), which is why the datapath is opt-in: on a 1500-byte fabric a tunnel caps guests at 1450 while the relay carries 1500 with no tax. Guests set their own interface MTU, exactly like a VM or container raising eth0 on the same-host datapath. * **Asymmetric underlays give one-way jumbo.** The derivation is per machine: a 9000-MTU compute paired with a 1500-MTU one forwards large frames in one direction only. Keep the fabric's MTU uniform between computes. -* **The MTU is derived once, at device creation.** Changing the underlay's MTU does not follow; a link delete/re-create (or project reopen) re-derives. +* **The MTU is derived at device creation and never tracks the underlay afterwards** — the vxlan driver registers no `NETDEV_CHANGEMTU` handler. Re-creation re-derives, and the wiring path deletes and re-creates the device (`_ensure_vxlan_device`) on every node start and link (re)creation, so the operational fix is: change the underlay MTU on **both** computes, then stop/start the nodes (or re-create the link, or reopen the project). A manual `ip link set mtu` on a live deployment also sticks: the bridge, unlike the tunnel, follows its ports' MTU at runtime (`NETDEV_CHANGEMTU → br_mtu_auto_adjust`), up and down, as long as the bridge MTU was never set by hand — ours never is. * **VNI collisions self-heal, not never happen.** The VNI is drawn from the upper VNI space (1M–16.7M) but two links of one deployment can still collide on a host. Every path where the kernel's refusal can surface carries its own heal: link creation retries a fresh VNI then degrades to the relay, project open re-dispatches refused entries the same way, and node start (a link wired while its nodes were stopped) stops the half-started node, re-wires the link on a fresh VNI or degrades it on a second refusal, and retries the start. A reopen re-rolls every link's VNI, so an unlucky link is not permanently unlucky. * **Security is the relay's, unchanged.** VXLAN carries no authentication; the VNI and the shared port are the only demux, and anyone who can reach the port can inject into any VNI. Treat the compute fabric as trusted, exactly as for the relay's UDP tunnels. * **Same-host compute pairs stay on the relay**, as do switch links, serial ports and unix-socket vendor containers — the tunnel is a per-link-bridge shape, and those cases are not (or would collide with themselves on one host). From 6679774de73da91e53704596c77b743d3402200c Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Wed, 14 Oct 2026 10:34:34 +0800 Subject: [PATCH 04/10] docs: spell out how the VXLAN lower device is chosen Per tunnel, not a host setting: a connected UDP socket runs the kernel's route lookup for the peer, the chosen source address is matched back to its owning interface, and that interface becomes the dev= (and with it the derived MTU and the outer source address). Different peers may ride different lower devices with independently derived MTUs; an unresolvable peer is a wiring error that degrades the link to the relay. Re-creation re-resolves too, so route changes ride the same node-start / link-recreation paths as MTU changes. --- docs/features/vxlan-cross-compute.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/features/vxlan-cross-compute.md b/docs/features/vxlan-cross-compute.md index e498432ea..cb4ed3b09 100644 --- a/docs/features/vxlan-cross-compute.md +++ b/docs/features/vxlan-cross-compute.md @@ -62,7 +62,7 @@ On project (re)open every link re-runs the same decision, so flipping the flag u ## Link creation -Each end's compute owns its own machine: it resolves the lower device from the peer address locally (a connected UDP socket picks the route without sending anything), creates the device and enslaves it. The two sides never talk to each other; the controller's VNI is what makes them a pair. +Each end's compute owns its own machine: it resolves the lower device from the peer address locally, creates the device and enslaves it. The resolution is per tunnel, not a host setting: a connected UDP socket makes the kernel run the route lookup for the peer (nothing is sent), the chosen source address is matched back to the interface that holds it, and that interface becomes the `dev=` — carrying both the derived MTU and the outer source address along the actual routed path, policy routing included. Two peers reachable through different interfaces therefore ride different lower devices with independently derived MTUs; a peer with no route, or a source address no interface owns, is a wiring error and the link degrades to the relay. The two sides never talk to each other; the controller's VNI is what makes them a pair. ```mermaid sequenceDiagram @@ -104,7 +104,7 @@ The `vxlan show` step is the idempotency contract: a device found under the link * **MTU is the whole story.** The kernel derives the tunnel device's MTU from the lower device (`underlay − 50` for an IPv4 underlay, `− 70` for IPv6; the 50 is 20 outer IPv4 + 8 UDP + 8 VXLAN + 14 inner Ethernet header — a VXLAN packet carries the guest's complete inner frame, per `vxlan_headroom()` in `include/net/vxlan.h`) and the per-link bridge, which carries no MTU of its own, reports the minimum of its ports — i.e. the derived value. A frame past it is dropped at the bridge egress **silently** (no ICMP, no log), which is why the datapath is opt-in: on a 1500-byte fabric a tunnel caps guests at 1450 while the relay carries 1500 with no tax. Guests set their own interface MTU, exactly like a VM or container raising eth0 on the same-host datapath. * **Asymmetric underlays give one-way jumbo.** The derivation is per machine: a 9000-MTU compute paired with a 1500-MTU one forwards large frames in one direction only. Keep the fabric's MTU uniform between computes. -* **The MTU is derived at device creation and never tracks the underlay afterwards** — the vxlan driver registers no `NETDEV_CHANGEMTU` handler. Re-creation re-derives, and the wiring path deletes and re-creates the device (`_ensure_vxlan_device`) on every node start and link (re)creation, so the operational fix is: change the underlay MTU on **both** computes, then stop/start the nodes (or re-create the link, or reopen the project). A manual `ip link set mtu` on a live deployment also sticks: the bridge, unlike the tunnel, follows its ports' MTU at runtime (`NETDEV_CHANGEMTU → br_mtu_auto_adjust`), up and down, as long as the bridge MTU was never set by hand — ours never is. +* **The MTU is derived at device creation and never tracks the underlay afterwards** — the vxlan driver registers no `NETDEV_CHANGEMTU` handler. Re-creation re-derives — and re-resolves the lower device, so a route change is picked up with it — and the wiring path deletes and re-creates the device (`_ensure_vxlan_device`) on every node start and link (re)creation, so the operational fix is: change the underlay MTU on **both** computes, then stop/start the nodes (or re-create the link, or reopen the project). A manual `ip link set mtu` on a live deployment also sticks: the bridge, unlike the tunnel, follows its ports' MTU at runtime (`NETDEV_CHANGEMTU → br_mtu_auto_adjust`), up and down, as long as the bridge MTU was never set by hand — ours never is. * **VNI collisions self-heal, not never happen.** The VNI is drawn from the upper VNI space (1M–16.7M) but two links of one deployment can still collide on a host. Every path where the kernel's refusal can surface carries its own heal: link creation retries a fresh VNI then degrades to the relay, project open re-dispatches refused entries the same way, and node start (a link wired while its nodes were stopped) stops the half-started node, re-wires the link on a fresh VNI or degrades it on a second refusal, and retries the start. A reopen re-rolls every link's VNI, so an unlucky link is not permanently unlucky. * **Security is the relay's, unchanged.** VXLAN carries no authentication; the VNI and the shared port are the only demux, and anyone who can reach the port can inject into any VNI. Treat the compute fabric as trusted, exactly as for the relay's UDP tunnels. * **Same-host compute pairs stay on the relay**, as do switch links, serial ports and unix-socket vendor containers — the tunnel is a per-link-bridge shape, and those cases are not (or would collide with themselves on one host). From 0d91d9252bcc34d7dfe0d194f5368f3f62d26959 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Wed, 14 Oct 2026 15:15:38 +0800 Subject: [PATCH 05/10] feat: default vxlan_dstport to the IANA VXLAN port 4789 The custom 14789 was picked to keep our demux socket off the ports other VXLAN software listens on, but a private default surprises operators and blocks interop with gear that pins the standard port. Default to the IANA-assigned 4789 instead; the gns3_server.conf override is unchanged for computes where swarm overlay/flannel-style VXLAN already owns 4789. Mixed-version deployments must pin the port on every compute until they all run the new default, or the two tunnel ends will disagree. --- docs/features/vxlan-cross-compute.md | 2 +- gns3server/schemas/config.py | 9 ++++----- tests/compute/docker/test_docker_kernel_datapath.py | 2 +- tests/compute/ubridge/test_tc_probe.py | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/features/vxlan-cross-compute.md b/docs/features/vxlan-cross-compute.md index cb4ed3b09..8d21a86ff 100644 --- a/docs/features/vxlan-cross-compute.md +++ b/docs/features/vxlan-cross-compute.md @@ -98,7 +98,7 @@ The `vxlan show` step is the idempotency contract: a device found under the link | Key | Default | Meaning | |---|---|---| | `Server.enable_vxlan_cross_compute` | `false` | Wire eligible cross-compute links as VXLAN tunnels | -| `Server.vxlan_dstport` | `14789` | The UDP port all tunnels of the deployment listen on (one kernel socket per port; chosen off the IANA 4789 and the legacy 8472 so other VXLAN software cannot merge demux tables with ours) | +| `Server.vxlan_dstport` | `4789` | The UDP port all tunnels of the deployment listen on (one kernel socket per port; the IANA-assigned VXLAN port — change it when other VXLAN software on the computes already owns it, since a shared socket merges demux tables) | ## Known caveats diff --git a/gns3server/schemas/config.py b/gns3server/schemas/config.py index ed8245777..f54725b67 100644 --- a/gns3server/schemas/config.py +++ b/gns3server/schemas/config.py @@ -201,14 +201,13 @@ class ServerSettings(BaseModel): "frames. The relay stays the default and the fallback when a tunnel cannot be wired", ) vxlan_dstport: int = Field( - 14789, + 4789, ge=1, le=65535, description="UDP destination port every cross-compute VXLAN tunnel listens on. The kernel shares one " - "socket per port across all VXLAN devices, so all links of a deployment ride this one port — chosen " - "off the IANA 4789 and the legacy 8472 (other VXLAN software's ports: sharing a socket would merge " - "their demux table with ours) and above the ephemeral range. Change it only when it collides with " - "a service already listening on the computes", + "socket per port across all VXLAN devices, so all links of a deployment ride this one port — the " + "IANA-assigned VXLAN port by default. Change it when other VXLAN software on the computes already " + "listens on it: a shared socket merges their demux table with ours", ) auto_discover_images: bool = Field(True, description="Automatically discover images in the images directory") image_sync_interval: int = Field(900, ge=10, description="Seconds between automatic image inventory scans") diff --git a/tests/compute/docker/test_docker_kernel_datapath.py b/tests/compute/docker/test_docker_kernel_datapath.py index 94a816cf6..3dc0b4324 100644 --- a/tests/compute/docker/test_docker_kernel_datapath.py +++ b/tests/compute/docker/test_docker_kernel_datapath.py @@ -1476,7 +1476,7 @@ async def test_stop_clears_relay_bridge_registry(vm): # Cross-compute VXLAN tunnel # --------------------------------------------------------------------------- -VXLAN_SPEC = {"name": "gvl01234567abcd", "vni": 1048577, "remote": "192.168.1.2", "dstport": 14789} +VXLAN_SPEC = {"name": "gvl01234567abcd", "vni": 1048577, "remote": "192.168.1.2", "dstport": 4789} def test_create_nio_bridge_carries_vxlan_spec(vm): diff --git a/tests/compute/ubridge/test_tc_probe.py b/tests/compute/ubridge/test_tc_probe.py index 202a0abfd..289ce08a8 100644 --- a/tests/compute/ubridge/test_tc_probe.py +++ b/tests/compute/ubridge/test_tc_probe.py @@ -319,7 +319,7 @@ async def test_probe_vxlan_scratch_port_stays_off_the_configured_one(monkeypatch """ settings = Config.instance().settings.Server - for configured in (14789, 20000, 25000, 29999): + for configured in (4789, 20000, 25000, 29999): monkeypatch.setattr(settings, "vxlan_dstport", configured) tc_probe._vxlan_cache.clear() assert await probe_vxlan_support() is True From 84b4c05b9f00628fbd25dd03a6a18312608e75e8 Mon Sep 17 00:00:00 2001 From: Cristi Date: Sat, 17 Oct 2026 23:08:11 +0300 Subject: [PATCH 06/10] feat(qemu): support two CD/DVD images and consistent ISO handling --- gns3server/compute/qemu/qemu_vm.py | 93 ++++++++++--- gns3server/controller/export_project.py | 58 +++++--- gns3server/controller/node.py | 2 + gns3server/db/models/images.py | 17 +++ gns3server/db/models/templates.py | 1 + gns3server/db/tasks.py | 4 + .../a6c8e1b2d490_add_second_qemu_cdrom.py | 23 ++++ gns3server/schemas/compute/qemu_nodes.py | 6 +- gns3server/schemas/controller/appliances.py | 3 +- gns3server/schemas/controller/images.py | 3 + .../controller/templates/qemu_templates.py | 3 +- gns3server/utils/images.py | 13 +- tests/api/routes/controller/test_images.py | 37 ++++++ tests/compute/qemu/test_qemu_vm.py | 125 ++++++++++++++++++ .../controller/test_appliance_to_template.py | 19 +++ tests/controller/test_export_project.py | 46 +++++++ tests/utils/test_images.py | 50 +++++++ 17 files changed, 462 insertions(+), 41 deletions(-) create mode 100644 gns3server/db_migrations/versions/a6c8e1b2d490_add_second_qemu_cdrom.py diff --git a/gns3server/compute/qemu/qemu_vm.py b/gns3server/compute/qemu/qemu_vm.py index eb2f2eb79..908ce15cb 100644 --- a/gns3server/compute/qemu/qemu_vm.py +++ b/gns3server/compute/qemu/qemu_vm.py @@ -158,6 +158,7 @@ class QemuVM(KernelDatapathMixin, BaseNode): self._hdc_disk_interface = "none" self._hdd_disk_interface = "none" self._cdrom_image = "" + self._cdrom2_image = "" self._bios_image = "" self._boot_priority = "c" self._mac_address = "" @@ -524,33 +525,74 @@ class QemuVM(KernelDatapathMixin, BaseNode): else: self._cdrom_image = "" + @property + def cdrom2_image(self): + return self._cdrom2_image + + @cdrom2_image.setter + def cdrom2_image(self, image): + self._cdrom2_image = self.manager.get_abs_image_path(image, self.working_dir) if image else "" + async def update_property(self, name, value): """ Update Qemu VM properties. """ + previous = getattr(self, name) setattr(self, name, value) - if name == "cdrom_image": - # let the guest know about the new cdrom image - await self._update_cdrom_image() + try: + if name in {"cdrom_image", "cdrom2_image"}: + await self._update_cdrom_image(name) + except (Exception, asyncio.CancelledError): + # Old media may have been deleted while QEMU still has it open. + # Restore the stored path without resolving it again. + setattr(self, f"_{name}", previous) + raise self._command_line_changed = True - async def _update_cdrom_image(self): - """ - Update the cdrom image path for the Qemu guest OS - """ + async def _update_cdrom_image(self, name="cdrom_image"): + """Change media only on an existing drive, validating the affected slot.""" + if not self.is_running(): + return + image = getattr(self, name) + self._cdrom_option(name) + device = "ide1-cd0" if name == "cdrom_image" else "ide1-cd1" + reader, writer = await self._open_qemu_monitor_connection_vm() + if reader is None or writer is None: + raise QemuError("Could not connect to the QEMU monitor to change CD/DVD media") - if self.is_running(): - if self._cdrom_image: - self._cdrom_option() # this will check the cdrom image is accessible - await self._control_vm("eject -f ide1-cd0") - await self._control_vm(f"change ide1-cd0 {self._cdrom_image}") - log.debug( - f'QEMU VM "{self._name}" [{self._id}] has changed the cdrom image path to {self._cdrom_image}' - ) + async def command(text): + writer.write((text + "\n").encode("utf-8")) + await writer.drain() + # Consume the echo first: a filename may itself contain "(qemu) ". + await asyncio.wait_for(reader.readline(), timeout=3) + output = await asyncio.wait_for(reader.readuntil(b"(qemu) "), timeout=3) + response = output.decode("utf-8", errors="replace") + response = response.removesuffix("(qemu) ").strip() + if "Error:" in response or (text != "info block" and response): + raise QemuError(response) + return response + + try: + await asyncio.wait_for(reader.readuntil(b"(qemu) "), timeout=3) + blocks = await command("info block") + if not re.search(rf"(?m)^{device}[: ]", blocks): + raise QemuError("Stop the VM before adding this CD/DVD drive") + if image: + filename = image + if not image.isascii() or any(ord(char) < 32 for char in image): + # HMP's line editor cannot reliably handle non-ASCII bytes. + # A JSON block filename carries them as ASCII escapes, which + # the block layer decodes after the HMP parser has run. + filename = "json:" + json.dumps({"driver": "raw", "file": {"driver": "file", "filename": image}}) + await command(f"change {device} -f {json.dumps(filename)}") else: - await self._control_vm("eject -f ide1-cd0") - log.debug(f'QEMU VM "{self._name}" [{self._id}] has ejected the cdrom image') + await command(f"eject -f {device}") + log.debug('QEMU VM "%s" has changed %s to %s', self._name, name, image) + except (asyncio.TimeoutError, asyncio.IncompleteReadError, OSError) as e: + raise QemuError(f"Could not change CD/DVD media: {e}") from e + finally: + writer.close() @property def bios_image(self): @@ -2387,10 +2429,10 @@ class QemuVM(KernelDatapathMixin, BaseNode): return options - def _cdrom_option(self): + def _cdrom_option(self, name=None): options = [] - if self._cdrom_image: + if name in (None, "cdrom_image") and self._cdrom_image: if not os.path.isfile(self._cdrom_image) or not os.path.exists(self._cdrom_image): if os.path.islink(self._cdrom_image): raise QemuError( @@ -2401,6 +2443,17 @@ class QemuVM(KernelDatapathMixin, BaseNode): if self._hdc_disk_image: raise QemuError("You cannot use a disk image on hdc disk and a CDROM image at the same time") options.extend(["-cdrom", self._cdrom_image.replace(",", ",,")]) + if name in (None, "cdrom2_image") and self._cdrom2_image: + if not os.path.isfile(self._cdrom2_image): + raise QemuError(f"cdrom image '{self._cdrom2_image}' is not accessible") + interface = self._hdd_disk_interface + if self._create_config_disk and interface == "none": + interface = self._hda_disk_interface + if (self._hdd_disk_image or self._create_config_disk) and interface == "ide": + raise QemuError("You cannot use an IDE hdd disk and a second CDROM image at the same time") + options.extend( + ["-drive", f"file={self._cdrom2_image.replace(',', ',,')},if=ide,index=3,media=cdrom,readonly=on"] + ) return options def _bios_option(self): @@ -2868,6 +2921,8 @@ class QemuVM(KernelDatapathMixin, BaseNode): answer["cdrom_image"] = self.manager.get_relative_image_path(self._cdrom_image, self.working_dir) answer["cdrom_image_md5sum"] = md5sum(self._cdrom_image, self.working_dir) + answer["cdrom2_image"] = self.manager.get_relative_image_path(self._cdrom2_image, self.working_dir) + answer["cdrom2_image_md5sum"] = md5sum(self._cdrom2_image, self.working_dir) answer["bios_image"] = self.manager.get_relative_image_path(self._bios_image, self.working_dir) answer["bios_image_md5sum"] = md5sum(self._bios_image, self.working_dir) answer["initrd"] = self.manager.get_relative_image_path(self._initrd, self.working_dir) diff --git a/gns3server/controller/export_project.py b/gns3server/controller/export_project.py index f70da2537..0c34d2ed4 100644 --- a/gns3server/controller/export_project.py +++ b/gns3server/controller/export_project.py @@ -208,6 +208,7 @@ async def _patch_project_file( # image files that we need to include in the exported archive images = [] + portable_references = [] try: with open(path) as f: @@ -249,32 +250,54 @@ async def _patch_project_file( if value is None or value.strip() == "": continue - if not keep_compute_ids: # If we keep the original compute we can keep the image path + if not keep_compute_ids: node["properties"][prop] = os.path.basename(value) - if include_images is True: - images.append({"compute_id": compute_id, "image": value, "image_type": node["node_type"]}) + if include_images: + image = {"compute_id": compute_id, "image": value, "image_type": node["node_type"]} + images.append(image) + if not keep_compute_ids: + portable_references.append((node["properties"], prop, image)) if not keep_compute_ids: topology["topology"][ "computes" ] = [] # Strip compute information because could contain secret info like password - local_images = {i["image"] for i in images if i["compute_id"] == "local"} + destinations = _image_export_names(images) + for properties, prop, image in portable_references: + properties[prop] = destinations[(image["compute_id"], image["image_type"], image["image"])] - for image in local_images: - _export_local_image(image, zstream) - - remote_images = {(i["compute_id"], i["image_type"], i["image"]) for i in images if i["compute_id"] != "local"} - - for compute_id, image_type, image in remote_images: - await _export_remote_images(project, compute_id, image_type, image, zstream, temporary_dir) + for (compute_id, image_type, image), export_name in destinations.items(): + if compute_id == "local": + _export_local_image(image, zstream, export_name, image_type) + else: + await _export_remote_images(project, compute_id, image_type, image, zstream, temporary_dir, export_name) zstream.writestr("project.gns3", json.dumps(topology, indent=4, sort_keys=True).encode()) return images -def _export_local_image(image, zstream): +def _image_export_names(images): + """Allocate distinct portable names without changing ordinary image names.""" + destinations = {} + used = set() + for image in images: + key = (image["compute_id"], image["image_type"], image["image"]) + if key in destinations: + continue + filename = os.path.basename(image["image"]) + stem, extension = os.path.splitext(filename) + index = 1 + while (image["image_type"], filename) in used: + index += 1 + filename = f"{stem}-{index}{extension}" + used.add((image["image_type"], filename)) + destinations[key] = filename + return destinations + + +def _export_local_image(image, zstream, export_name=None, image_type=None): """ Exports a local image to the zip file. @@ -285,6 +308,8 @@ def _export_local_image(image, zstream): from ..compute import MODULES for module in MODULES: + if image_type and getattr(module, "_NODE_TYPE", None) != image_type: + continue try: images_directory = module.instance().get_images_directory() except NotImplementedError: @@ -298,13 +323,15 @@ def _export_local_image(image, zstream): path = os.path.join(images_directory, image) if os.path.exists(path): - arcname = os.path.join("images", directory, os.path.basename(image)) + arcname = os.path.join("images", directory, export_name or os.path.basename(image)) _patch_mtime(path) zstream.write(path, arcname) return -async def _export_remote_images(project, compute_id, image_type, image, project_zipfile, temporary_dir): +async def _export_remote_images( + project, compute_id, image_type, image, project_zipfile, temporary_dir, export_name=None +): """ Export specific image from remote compute. """ @@ -334,5 +361,6 @@ async def _export_remote_images(project, compute_id, image_type, image, project_ break await f.write(data) response.close() - arcname = os.path.join("images", image_type, image) + directory = "QEMU" if image_type == "qemu" else image_type + arcname = os.path.join("images", directory, export_name or image) project_zipfile.write(temp_path, arcname=arcname, compress_type=zipfile.ZIP_DEFLATED) diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index c1036f0ee..5e7d7566c 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -58,6 +58,7 @@ IMAGE_PROPERTIES_BY_NODE_TYPE = { "hdc_disk_image": "qemu", "hdd_disk_image": "qemu", "cdrom_image": "qemu", + "cdrom2_image": "qemu", "initrd": "qemu", "kernel_image": "qemu", "bios_image": "qemu", @@ -900,6 +901,7 @@ class Node: "hdc_disk_image", "hdd_disk_image", "cdrom_image", + "cdrom2_image", "bios_image", "initrd", "kernel_image", diff --git a/gns3server/db/models/images.py b/gns3server/db/models/images.py index 139f9054d..f7b9d96dc 100644 --- a/gns3server/db/models/images.py +++ b/gns3server/db/models/images.py @@ -45,6 +45,23 @@ class Image(BaseTable): last_error = Column(String) templates = relationship("Template", secondary=image_template_map, back_populates="images") + @property + def reference(self): + """An emulator-relative reference, or an absolute additional-root path.""" + import os + + from gns3server.utils import force_unix_path + from gns3server.utils.images import default_images_directory + + root = os.path.abspath(default_images_directory(self.image_type)) + path = os.path.abspath(self.path) + try: + if os.path.commonpath([root, path]) == root: + return force_unix_path(os.path.relpath(path, root)) + except ValueError: # Different drives on Windows. + pass + return force_unix_path(path) + class ImageSyncJob(BaseTable): __tablename__ = "image_sync_jobs" diff --git a/gns3server/db/models/templates.py b/gns3server/db/models/templates.py index 73d18e8b7..ffcaeccef 100644 --- a/gns3server/db/models/templates.py +++ b/gns3server/db/models/templates.py @@ -201,6 +201,7 @@ class QemuTemplate(Template): hdd_disk_image = Column(String) hdd_disk_interface = Column(String) cdrom_image = Column(String) + cdrom2_image = Column(String) initrd = Column(String) kernel_image = Column(String) bios_image = Column(String) diff --git a/gns3server/db/tasks.py b/gns3server/db/tasks.py index bf10fa2f8..1ef935a86 100644 --- a/gns3server/db/tasks.py +++ b/gns3server/db/tasks.py @@ -140,12 +140,16 @@ async def connect_to_db(app: FastAPI) -> None: def upgrade_inventory(connection): from alembic.operations import Operations + from gns3server.db_migrations.versions.a6c8e1b2d490_add_second_qemu_cdrom import ( + upgrade as upgrade_cdrom, + ) from gns3server.db_migrations.versions.d9e8a2b7c401_image_inventory_reconciliation import ( upgrade, ) with Operations.context(MigrationContext.configure(connection)): upgrade() + upgrade_cdrom() await conn.run_sync(upgrade_inventory) await conn.run_sync(run_stamp, alembic_cfg) diff --git a/gns3server/db_migrations/versions/a6c8e1b2d490_add_second_qemu_cdrom.py b/gns3server/db_migrations/versions/a6c8e1b2d490_add_second_qemu_cdrom.py new file mode 100644 index 000000000..262abd8ae --- /dev/null +++ b/gns3server/db_migrations/versions/a6c8e1b2d490_add_second_qemu_cdrom.py @@ -0,0 +1,23 @@ +"""Add a second QEMU CD-ROM image. + +Revision ID: a6c8e1b2d490 +Revises: f7d3a9c2e5b1 +""" + +import sqlalchemy as sa +from alembic import op + +revision = "a6c8e1b2d490" +down_revision = "f7d3a9c2e5b1" +branch_labels = None +depends_on = None + + +def upgrade(): + existing = {column["name"] for column in sa.inspect(op.get_bind()).get_columns("qemu_templates")} + if "cdrom2_image" not in existing: + op.add_column("qemu_templates", sa.Column("cdrom2_image", sa.String())) + + +def downgrade(): + op.drop_column("qemu_templates", "cdrom2_image") diff --git a/gns3server/schemas/compute/qemu_nodes.py b/gns3server/schemas/compute/qemu_nodes.py index 6e26583e2..339f4fdae 100644 --- a/gns3server/schemas/compute/qemu_nodes.py +++ b/gns3server/schemas/compute/qemu_nodes.py @@ -183,8 +183,10 @@ class QemuBase(BaseModel): hdd_disk_image_backing_file: Optional[str] = Field(None, description="QEMU hdd backing file disk image path") hdd_disk_image_md5sum: Optional[str] = Field(None, description="QEMU hdd disk image checksum") hdd_disk_interface: Optional[QemuDiskInterfaceType] = Field(None, description="QEMU hdd interface") - cdrom_image: Optional[str] = Field(None, description="QEMU cdrom image path") - cdrom_image_md5sum: Optional[str] = Field(None, description="QEMU cdrom image checksum") + cdrom_image: Optional[str] = Field(None, description="QEMU first cdrom image path") + cdrom2_image: Optional[str] = Field(None, description="QEMU second cdrom image path") + cdrom_image_md5sum: Optional[str] = Field(None, description="QEMU first cdrom image checksum") + cdrom2_image_md5sum: Optional[str] = Field(None, description="QEMU second cdrom image checksum") bios_image: Optional[str] = Field(None, description="QEMU bios image path") bios_image_md5sum: Optional[str] = Field(None, description="QEMU bios image checksum") initrd: Optional[str] = Field(None, description="QEMU initrd path") diff --git a/gns3server/schemas/controller/appliances.py b/gns3server/schemas/controller/appliances.py index ce1a71b98..d3300a675 100644 --- a/gns3server/schemas/controller/appliances.py +++ b/gns3server/schemas/controller/appliances.py @@ -425,7 +425,8 @@ class ApplianceVersionImages(BaseModel): hdb_disk_image: Optional[str] = Field(None, title="Hdc disk image") hdc_disk_image: Optional[str] = Field(None, title="Hdd disk image") hdd_disk_image: Optional[str] = Field(None, title="Hdd diskimage") - cdrom_image: Optional[str] = Field(None, title="cdrom image") + cdrom_image: Optional[str] = Field(None, title="First cdrom image") + cdrom2_image: Optional[str] = Field(None, title="Second cdrom image") class ApplianceVersion(BaseModel): diff --git a/gns3server/schemas/controller/images.py b/gns3server/schemas/controller/images.py index 17eb73ad3..5d99c296c 100644 --- a/gns3server/schemas/controller/images.py +++ b/gns3server/schemas/controller/images.py @@ -37,6 +37,9 @@ class ImageBase(BaseModel): filename: str = Field(..., description="Image filename") path: str = Field(..., description="Image path") + reference: Optional[str] = Field( + None, description="Image reference relative to its emulator directory, or an absolute additional-root path" + ) image_type: ImageType = Field(..., description="Image type") image_size: int = Field(..., description="Image size in bytes") checksum: str = Field(..., description="Checksum value") diff --git a/gns3server/schemas/controller/templates/qemu_templates.py b/gns3server/schemas/controller/templates/qemu_templates.py index 6f6b29328..399918f55 100644 --- a/gns3server/schemas/controller/templates/qemu_templates.py +++ b/gns3server/schemas/controller/templates/qemu_templates.py @@ -79,7 +79,8 @@ class QemuTemplate(TemplateBase): hdd_disk_interface: Optional[QemuDiskInterfaceType] = Field( QemuDiskInterfaceType.none, description="QEMU hdd interface" ) - cdrom_image: Optional[str] = Field("", description="QEMU cdrom image path") + cdrom_image: Optional[str] = Field("", description="QEMU first cdrom image path") + cdrom2_image: Optional[str] = Field("", description="QEMU second cdrom image path") initrd: Optional[str] = Field("", description="QEMU initrd path") kernel_image: Optional[str] = Field("", description="QEMU kernel image path") bios_image: Optional[str] = Field("", description="QEMU bios image path") diff --git a/gns3server/utils/images.py b/gns3server/utils/images.py index 595c9ec9c..829fe78d2 100644 --- a/gns3server/utils/images.py +++ b/gns3server/utils/images.py @@ -49,6 +49,9 @@ from . import force_unix_path log = logging.getLogger(__name__) +# Include the first ISO 9660 volume descriptor in image type detection. +IMAGE_HEADER_SIZE = 16 * 2048 + 7 + async def list_images(image_type): """ @@ -145,7 +148,7 @@ def inspect_image_file(path, expected_image_type=None, allow_raw_image=False, st info = os.fstat(f.fileno()) if not stat.S_ISREG(info.st_mode) or stat_fingerprint(info) != before: raise ImageChangedError(f"Image changed while opening: {path}") - header = f.read(7) + header = f.read(IMAGE_HEADER_SIZE) if len(header) < 7: raise InvalidImageError(f"Image '{path}' is too small to be valid") image_type = check_valid_image_header(path, header, allow_raw_image) @@ -359,6 +362,8 @@ def check_valid_image_header(path: str, data: bytes, allow_raw_image: bool = Fal elif data[:4] == b"QFI\xfb" or data[:4] == b"KDMV": # for Qemy images: file must be QCOW2 or VMDK return "qemu" + elif data[16 * 2048 + 1 : 16 * 2048 + 6] == b"CD001": + return "qemu" else: if allow_raw_image is True: return "qemu" @@ -380,12 +385,14 @@ async def write_image( if subdirectory is not None: validate_image_upload_name(image_filename) subfolders = validate_image_subdirectory(subdirectory) - # HTTP chunk boundaries need not align with the seven-byte image header. + # HTTP chunk boundaries need not align with the image header or ISO descriptor. iterator = stream.__aiter__() prefix = bytearray() async for chunk in iterator: prefix.extend(chunk) - if len(prefix) >= 7: + # Known disk formats keep their original early detection behavior. + header_size = 7 if prefix.startswith((b"\x7fELF", b"QFI\xfb", b"KDMV")) else IMAGE_HEADER_SIZE + if len(prefix) >= header_size: break if len(prefix) < 7: raise InvalidImageError("The image content is empty or too small to be valid") diff --git a/tests/api/routes/controller/test_images.py b/tests/api/routes/controller/test_images.py index 8e0972b96..adfbcb7ee 100644 --- a/tests/api/routes/controller/test_images.py +++ b/tests/api/routes/controller/test_images.py @@ -476,3 +476,40 @@ class TestImageRoutes: assert len(templates) == 1 assert templates[0].name == "Empty VM" await templates_repo.delete_template(templates[0].template_id) + + +class TestIsoImages: + async def test_iso_uploads_can_be_used_together_in_a_template(self, app, client, config, images_dir): + config.settings.Server.allow_raw_images = False + references = [] + for index, name in enumerate(("installer.ISO", "drivers.iso")): + content = bytes(16 * 2048) + b"\x01CD001\x01" + bytes([index]) * 2041 + response = await client.post( + app.url_path_for("upload_image", image_path=name), + params={"subdirectory": "installers"} if index == 0 else {}, + content=content, + ) + assert response.status_code == status.HTTP_201_CREATED, response.text + assert response.json()["image_type"] == "qemu" + relative = "installers/" + name if index == 0 else name + assert response.json()["path"] == os.path.join(images_dir, "QEMU", relative) + assert response.json()["reference"] == relative + assert Qemu.instance().get_abs_image_path(relative) == response.json()["path"] + references.append(relative) + + response = await client.post( + app.url_path_for("create_template"), + json={ + "name": "Windows installer with drivers", + "template_type": "qemu", + "compute_id": "local", + "cdrom_image": references[0], + "cdrom2_image": references[1], + }, + ) + assert response.status_code == status.HTTP_201_CREATED, response.text + template_id = response.json()["template_id"] + response = await client.get(app.url_path_for("get_template", template_id=template_id)) + assert response.status_code == status.HTTP_200_OK + assert response.json()["cdrom_image"] == references[0] + assert response.json()["cdrom2_image"] == references[1] diff --git a/tests/compute/qemu/test_qemu_vm.py b/tests/compute/qemu/test_qemu_vm.py index 1f4953c27..047a32ad9 100644 --- a/tests/compute/qemu/test_qemu_vm.py +++ b/tests/compute/qemu/test_qemu_vm.py @@ -15,9 +15,11 @@ # along with this program. If not, see . import asyncio +import json import os import stat import sys +from pathlib import Path from unittest import mock from unittest.mock import MagicMock, patch @@ -1039,3 +1041,126 @@ async def test_run_with_kvm_linux_dev_kvm_missing(linux_platform, vm): vm.manager.config.settings.Qemu.require_hardware_acceleration = True with pytest.raises(QemuError): await vm._run_with_hardware_acceleration("qemu-system-x86_64", "") + + +@pytest.mark.asyncio +async def test_two_cdrom_images(vm, images_dir): + installer = Path(images_dir) / "installer.iso" + drivers = Path(images_dir) / "virtio,drivers.iso" + installer.touch() + drivers.touch() + vm.cdrom_image = str(installer) + vm.cdrom2_image = str(drivers) + assert vm._cdrom_option() == [ + "-cdrom", + str(installer), + "-drive", + f"file={str(drivers).replace(',', ',,')},if=ide,index=3,media=cdrom,readonly=on", + ] + assert vm.asdict()["cdrom2_image"].endswith("virtio,drivers.iso") + vm.cdrom2_image = "" + assert vm._cdrom_option() == ["-cdrom", str(installer)] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("config_disk", [False, True]) +async def test_second_cdrom_ide_conflict(vm, images_dir, config_disk): + iso = Path(images_dir) / "drivers.iso" + iso.touch() + vm.cdrom2_image = str(iso) + vm._hdd_disk_interface = "ide" + vm._create_config_disk = config_disk + if not config_disk: + vm._hdd_disk_image = str(iso) + with pytest.raises(QemuError, match="second CDROM"): + vm._cdrom_option() + + +@pytest.fixture +def cdrom_monitor(vm): + reader = asyncio.StreamReader() + reader.feed_data(b"QEMU monitor\r\n(qemu) ") + writer = MagicMock() + writer.drain = AsyncioMagicMock() + state = {"blocks": "ide1-cd0: installer.iso\r\nide1-cd1: drivers.iso\r\n", "error": ""} + + def respond(data): + command = data.decode("utf-8").strip() + output = state["blocks"] if command == "info block" else state["error"] + reader.feed_data(data.rstrip(b"\n") + b"\r\n" + output.encode("utf-8") + b"(qemu) ") + + writer.write.side_effect = respond + with ( + patch.object(vm, "is_running", return_value=True), + patch.object( + vm, "_open_qemu_monitor_connection_vm", new_callable=AsyncioMagicMock, return_value=(reader, writer) + ), + ): + yield state, writer + + +@pytest.mark.asyncio +@pytest.mark.parametrize("filename", ["drivers with spaces.iso", "drivers-雪.iso", "drivers (qemu) .iso"]) +async def test_second_cdrom_update(vm, images_dir, cdrom_monitor, filename): + iso = Path(images_dir) / filename + iso.touch() + state, writer = cdrom_monitor + await vm.update_property("cdrom2_image", str(iso)) + assert writer.write.call_args_list[0] == mock.call(b"info block\n") + command = writer.write.call_args_list[1].args[0].decode("ascii") + encoded_filename = json.loads(command.removeprefix("change ide1-cd1 -f ").strip()) + if encoded_filename.startswith("json:"): + assert json.loads(encoded_filename[5:])["file"]["filename"] == str(iso) + else: + assert encoded_filename == str(iso) + assert vm.cdrom2_image == str(iso) + writer.close.assert_called_once() + + +@pytest.mark.asyncio +async def test_second_cdrom_eject(vm, cdrom_monitor): + state, writer = cdrom_monitor + await vm.update_property("cdrom2_image", "") + writer.write.assert_called_with(b"eject -f ide1-cd1\n") + + +@pytest.mark.asyncio +async def test_second_cdrom_hot_add_requires_restart(vm, images_dir, cdrom_monitor): + iso = Path(images_dir) / "drivers.iso" + iso.touch() + state, writer = cdrom_monitor + state["blocks"] = "ide1-cd0: installer.iso\r\n" + vm._command_line_changed = False + with pytest.raises(QemuError, match="Stop the VM"): + await vm.update_property("cdrom2_image", str(iso)) + assert vm.cdrom2_image == "" + assert not vm._command_line_changed + assert writer.write.call_count == 1 + + +@pytest.mark.asyncio +async def test_missing_other_iso_does_not_block_media_swap(vm, images_dir, cdrom_monitor): + iso = Path(images_dir) / "installer.iso" + iso.touch() + vm._cdrom2_image = str(Path(images_dir) / "deleted.iso") + await vm.update_property("cdrom_image", str(iso)) + assert vm.cdrom_image == str(iso) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("error", ["Error: media change failed", "change: filename expected"]) +@pytest.mark.parametrize("original_deleted", [False, True]) +async def test_cdrom_monitor_error_preserves_previous_image(vm, images_dir, cdrom_monitor, error, original_deleted): + original = Path(images_dir) / "original.iso" + original.touch() + replacement = Path(images_dir) / "replacement.iso" + replacement.touch() + vm.cdrom2_image = str(original) + if original_deleted: + original.unlink() + state, writer = cdrom_monitor + state["error"] = error + "\r\n" + with pytest.raises(QemuError, match=error): + await vm.update_property("cdrom2_image", str(replacement)) + assert vm.cdrom2_image == str(original) + writer.close.assert_called_once() diff --git a/tests/controller/test_appliance_to_template.py b/tests/controller/test_appliance_to_template.py index 9faff12b4..843e52653 100644 --- a/tests/controller/test_appliance_to_template.py +++ b/tests/controller/test_appliance_to_template.py @@ -628,3 +628,22 @@ def test_no_appliance_metadata_when_appliance_has_none(): template = ApplianceToTemplate().new_template(appliance, None, "local") assert "appliance_metadata" not in template + + +@pytest.mark.parametrize("version", [6, 8]) +def test_second_cdrom_survives_appliance_conversion(version): + from copy import deepcopy + + from gns3server.schemas.controller.appliances import ApplianceVersionImages + + slots = {"cdrom_image": "installer.iso", "cdrom2_image": "drivers.iso"} + validated = ApplianceVersionImages.model_validate(slots).model_dump(exclude_none=True) + assert validated == slots + appliance = deepcopy(VYOS_V8) + appliance["versions"][0]["images"] = validated + if version == 6: + appliance["registry_version"] = 6 + appliance["qemu"] = {"arch": "x86_64", "ram": 512, "adapters": 1, "kvm": "allow"} + converted = ApplianceToTemplate().new_template(appliance, appliance["versions"][0], "local") + assert converted["cdrom_image"] == "installer.iso" + assert converted["cdrom2_image"] == "drivers.iso" diff --git a/tests/controller/test_export_project.py b/tests/controller/test_export_project.py index 3f4009c9a..04d8715c0 100644 --- a/tests/controller/test_export_project.py +++ b/tests/controller/test_export_project.py @@ -413,3 +413,49 @@ async def test_export_with_ignoring_snapshots(tmpdir, project): with zipfile.ZipFile(str(tmpdir / "zipfile.zip")) as myzip: assert os.path.join("snapshots", "snap.gns3project") not in [f.filename for f in myzip.filelist] + + +@pytest.mark.asyncio +async def test_export_two_isos_with_identical_filenames(tmpdir, project): + from gns3server.compute.qemu import Qemu + from gns3server.controller.import_project import _import_images + + root = Path(str(tmpdir)) / "QEMU" + installer = root / "windows/install.iso" + drivers = root / "drivers/install.iso" + for image, data in ((installer, b"installer"), (drivers, b"drivers")): + image.parent.mkdir(parents=True) + image.write_bytes(data) + topology = { + "topology": { + "nodes": [ + { + "node_type": "qemu", + "properties": { + "cdrom_image": str(installer), + "cdrom2_image": str(drivers), + }, + } + ] + } + } + Path(project.path, "test.gns3").write_text(json.dumps(topology)) + archive = str(tmpdir / "iso-project.zip") + with aiozipstream.ZipFile() as stream: + with patch.object(Qemu, "get_images_directory", return_value=str(root)): + await export_project(stream, project, str(tmpdir), include_images=True) + await write_file(archive, stream) + with zipfile.ZipFile(archive) as stream: + properties = json.loads(stream.read("project.gns3"))["topology"]["nodes"][0]["properties"] + assert properties["cdrom_image"] != properties["cdrom2_image"] + assert stream.read("images/QEMU/" + properties["cdrom_image"]) == b"installer" + assert stream.read("images/QEMU/" + properties["cdrom2_image"]) == b"drivers" + assert len(stream.namelist()) == len(set(stream.namelist())) + destination = Path(str(tmpdir)) / "unpacked" + stream.extractall(destination) + controller = MagicMock() + imported_images = Path(str(tmpdir)) / "imported-images" + controller.images_path.return_value = str(imported_images) + await _import_images(controller, str(destination / "images")) + assert (imported_images / "QEMU" / properties["cdrom_image"]).read_bytes() == b"installer" + assert (imported_images / "QEMU" / properties["cdrom2_image"]).read_bytes() == b"drivers" diff --git a/tests/utils/test_images.py b/tests/utils/test_images.py index 43568076b..87391d23d 100644 --- a/tests/utils/test_images.py +++ b/tests/utils/test_images.py @@ -187,3 +187,53 @@ async def test_list_images(tmpdir, config): "path": "qemu_image.qcow2", } ] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("subdirectory", [None, "installers/linux"]) +async def test_iso_upload_and_discovery(tmp_path, config, subdirectory): + from unittest.mock import AsyncMock + + from gns3server.utils.images import discover_images, inspect_image_file, write_image + + config.settings.Server.images_path = str(tmp_path) + config.settings.Server.additional_images_paths = [] + # ISO 9660 primary volume descriptor; no raw-image fallback is needed. + content = b"\0" * (16 * 2048) + b"\x01CD001\x01" + b"\0" * 2041 + + async def stream(): + # Exercise a descriptor split across upload chunks. + for offset in range(0, len(content), 13): + yield content[offset : offset + 13] + + repo = AsyncMock() + repo.get_image_by_checksum.return_value = None + repo.save_verified_image.side_effect = lambda image: image + result = await write_image( + "installer.ISO", str(tmp_path / "installer.ISO"), stream(), repo, subdirectory=subdirectory + ) + expected = tmp_path / "QEMU" / (subdirectory or "") / "installer.ISO" + assert result["path"] == str(expected) + assert result["image_type"] == "qemu" + assert expected.read_bytes() == content + assert inspect_image_file(expected)["checksum"] == result["checksum"] + assert [image["path"] for image in await discover_images("qemu")] == [str(expected)] + + +def test_iso_extension_alone_is_not_a_valid_image(): + from gns3server.utils.images import InvalidImageError, check_valid_image_header + + with pytest.raises(InvalidImageError): + check_valid_image_header("fake.iso", b"not an ISO image") + + +@pytest.mark.parametrize("images_root", ["images", "QEMU/images"]) +def test_image_reference_uses_configured_root(tmp_path, config, images_root): + from gns3server.db.models.images import Image + + root = tmp_path / images_root + config.settings.Server.images_path = str(root) + image = Image(image_type="qemu", path=str(root / "QEMU/installers/linux.iso")) + assert image.reference == "installers/linux.iso" + image.path = str(tmp_path / "extra/QEMU/linux.iso") + assert image.reference == image.path From 33b28a3b5f97cfe4d502b826d1772a3762bd6a80 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sun, 18 Oct 2026 00:44:28 +0800 Subject: [PATCH 07/10] fix: fetch remote-compute marker pcaps in tag replay Marker pcaps are written by uBridge on the compute that hosts the capture node. The tag replay only looked in the project folder on the controller. So a source from a remote compute showed count 0 and no frames, with no error. Its frame detail returned 404 and said nothing ever matched (#3010). Each source now gets its pcap through the compute of its capture node. Local files are read as before. Remote files are fetched over HTTP on every acquire into a small cache (32 idle files, cleared at shutdown). A file is replaced only when its bytes changed. This keeps the old mtime, so the sharkd session for that file stays alive. When a compute cannot be reached, its source now shows an error message in the timeline instead of just showing nothing. The detail endpoint now returns an error instead of 404. --- docs/features/marker-tag-replay.md | 22 +- gns3server/controller/marker_replay.py | 341 ++++++++++++++---- .../schemas/controller/marker_replay.py | 3 + .../routes/controller/test_response_models.py | 9 +- tests/controller/test_marker_replay.py | 261 +++++++++++++- 5 files changed, 560 insertions(+), 76 deletions(-) diff --git a/docs/features/marker-tag-replay.md b/docs/features/marker-tag-replay.md index d0dd346f5..f3d4114a5 100644 --- a/docs/features/marker-tag-replay.md +++ b/docs/features/marker-tag-replay.md @@ -41,11 +41,16 @@ graph TB TMP["/tmp scratch copies
(hardened-profile workaround)"] SK["sharkd -
(resident JSON-RPC on stdio)"] + subgraph Remote["Remote compute"] + RFS[("its markers dir")] + end + UI -->|"GET range / frames [?filter=&link=]"| GATE GATE --> SCAN SCAN -->|"columns / filter matches"| SESS SESS --> TMP --> SK SCAN -->|"hex: raw bytes"| FS + RFS -->|"HTTP fetch → local cache"| SCAN UI -->|"GET frame detail (lazy)"| SESS ``` @@ -76,6 +81,8 @@ and the user's home even though the server process can read both. The pcap copy off the event loop, and server shutdown kills every session so no scratch copy leaks into `/tmp` across restarts. +Capture-file locality: a marker's pcap is written by uBridge on the compute hosting the capture node, so only local markers' pcaps sit in the controller's project directory. A remote marker's pcap is fetched through the compute's project-files API on every acquire into a bounded cache (32 idle files under `/tmp`, wiped at shutdown) — a paused marker's pcap holds matched packets only, so the refetch stays cheap. The cached copy is replaced only when the bytes changed, so an unchanged refetch keeps the previous file's mtime and the resident sharkd session keyed by it survives; for remote sources the per-request `(mtime, size)` validation runs against the cache copy. A compute that cannot be reached flags its source (`count: 0` plus an `error` message in `sources`) instead of silently shrinking the timeline; the frame-detail endpoint then answers 409 with the fetch failure. + ### Resource bounds Three independent bounds, easily confused: @@ -171,7 +178,7 @@ without it. "frame_count": 29, "sources": [ { "node_id": "47703cad…", "link_id": "2697a7c6…", "marker": "global-ospf", - "data_link_type": "DLT_EN10MB", "count": 4 } + "data_link_type": "DLT_EN10MB", "count": 4, "error": null } ], "frames": [ { "ts": "1788369209.406812", "len": 114, @@ -218,7 +225,10 @@ design: - **`sources` is the stable inventory of the tag**: every capture source is always listed, with engine-free **total** counts, unaffected by `link` / `filter` — a - source dropdown must not shrink when the view narrows. + source dropdown must not shrink when the view narrows. A source whose capture file + could not be fetched from its compute keeps its entry with `count: 0` and an `error` + message (compute unreachable) — a partial timeline must be visible as partial, never + mistaken for the whole capture. - **An unknown `link_id` matches nothing**: `frame_count: 0`, `start: null`, empty `frames` — the same shape as a zero-match display filter, deliberately not a 404. @@ -308,7 +318,7 @@ All error bodies are `{"message": "…"}` (the app's unified format). | 400 | Invalid display filter (sharkd's filter rejection only; message carries its original text) or filter longer than 2000 chars | | 401 | Not authenticated | | 404 | Tag has no markers in the project; detail source unknown; `ts`/`frame_number` do not match the file (the capture may have been rebuilt); or the pcap vanished mid-request | -| 409 | Tag gate: a marker under the tag is still `enabled: true` (the response lists them) | +| 409 | Tag gate: a marker under the tag is still `enabled: true` (the response lists them); detail endpoint only — the compute hosting the capture file could not be reached (message names the compute) | | 501 | sharkd not installed / unavailable — replay is unavailable, no degraded mode | | 502 | sharkd failed, timed out (10 s per RPC), or answered out of sync — the session is killed and re-spawned on the next request | @@ -348,8 +358,4 @@ All error bodies are `{"message": "…"}` (the app's unified format). directory — nothing accumulates across restarts. - **Tag type.** REST and the `marker.match` WS event both carry `tag` as `int` (the listener normalizes); replay keys on that int value. -- **Follow-ups.** Remote-compute support via the existing capture-file proxy pattern; - convenience APIs (`GET …/markers/tags` to list tags, `POST …/markers/tags/{tag}/pause` - to batch-pause — a one-call path to the replayable state); uBridge-side - `pcap_dump_open_append` (with a linktype-header check on the existing file) so capture - history survives node restarts instead of being truncated on every reinstall. +- **Follow-ups.** Convenience APIs (`GET …/markers/tags` to list tags, `POST …/markers/tags/{tag}/pause` to batch-pause — a one-call path to the replayable state); uBridge-side `pcap_dump_open_append` (with a linktype-header check on the existing file) so capture history survives node restarts instead of being truncated on every reinstall. diff --git a/gns3server/controller/marker_replay.py b/gns3server/controller/marker_replay.py index 716c5cf82..6254d445a 100644 --- a/gns3server/controller/marker_replay.py +++ b/gns3server/controller/marker_replay.py @@ -51,9 +51,17 @@ merge sorts by ``(ts, source file, frame number)`` and index structures must never use ts alone as a dict key, or same-microsecond frames silently overwrite each other. The canonical ts strings travel to clients verbatim and must be round-tripped verbatim. + +Capture files live on the compute hosting the capture node — only a local +marker's pcap sits in the controller's project directory. A remote marker's +pcap is fetched from its compute over HTTP on every acquire into a bounded +cache (paused marker pcaps hold matched packets only, so they stay small); +the previous copy is kept when the bytes are identical so resident sharkd +sessions (validated by ``(mtime, size)``) survive unchanged refetches. """ import asyncio +import filecmp import json import logging import os @@ -63,7 +71,9 @@ import tempfile import time from contextlib import asynccontextmanager -from .controller_error import ControllerBadRequestError, ControllerError, ControllerNotFoundError +import aiofiles + +from .controller_error import ComputeError, ControllerBadRequestError, ControllerError, ControllerNotFoundError log = logging.getLogger(__name__) @@ -202,6 +212,173 @@ def read_frame_bytes(path, frame_number): return f.read(incl_len).hex() +# --------------------------------------------------------------------------- +# Remote capture sources (fetched from their compute over HTTP) +# --------------------------------------------------------------------------- + +# Bounded number of remote marker pcaps kept on the controller. Only idle +# entries are ever evicted (see _remote_pcap), so a request's own sources +# cannot be pulled out from under it. +_REMOTE_CACHE_MAX = 32 + +# (project_id, compute_id, rel_path) -> {"path": str, "last_used": float, "_uses": int} +_remote_cache: dict[tuple[str, str, str], dict] = {} +_remote_locks: dict[tuple[str, str, str], asyncio.Lock] = {} +_remote_cache_dir: str | None = None + + +def _cache_path(project, compute, rel_path): + """Flat, collision-free cache path: project/compute ids are uuids and + basename() strips any path component out of the file name.""" + + global _remote_cache_dir + if _remote_cache_dir is None: + _remote_cache_dir = tempfile.mkdtemp(prefix="gns3-marker-replay-") + os.makedirs(_remote_cache_dir, exist_ok=True) + return os.path.join(_remote_cache_dir, f"{project.id}_{compute.id}_{os.path.basename(rel_path)}") + + +def _drop_cached_file(key): + """Forget a cache entry and delete its file (marker removed on the compute).""" + + entry = _remote_cache.pop(key, None) + if entry and entry["path"]: + try: + os.remove(entry["path"]) + except OSError: + pass + + +def _evict_idle_remote_cache(): + """Trim the cache down to the cap, least-recently-used first. Only idle + entries are evictable; dict access is synchronous, so this cannot + interleave with a concurrent acquire's refcount bump.""" + + while len(_remote_cache) > _REMOTE_CACHE_MAX: + idle = [key for key, entry in _remote_cache.items() if entry["_uses"] == 0] + if not idle: + break + victim = min(idle, key=lambda key: _remote_cache[key]["last_used"]) + entry = _remote_cache.pop(victim) + if entry["path"]: + try: + os.remove(entry["path"]) + except OSError: + pass + + +async def _fetch_to_cache(project, compute, rel_path, path): + """ + Fetch the pcap from its compute, replacing the cached copy only when the + bytes changed — an unchanged copy keeps its mtime, so the resident sharkd + session keyed by that path stays valid. The file is refetched on every + acquire: the gate already pins replay to paused markers, and a marker + resumed and re-paused between two requests rebuilds its pcap. + + :returns: the cache path, or None when the compute has no such file (the + marker never matched anything — not an error). + :raises ComputeError: the compute could not be reached or refused the file. + """ + + try: + response = await compute.http_query("GET", f"/projects/{project.id}/files/{rel_path}", stream=True) + except ControllerNotFoundError: + return None + except ControllerError as e: + raise ComputeError(f"Could not fetch capture file from compute '{compute.id}': {e}") from e + + tmp = f"{path}.tmp" + try: + async with aiofiles.open(tmp, "wb") as f: + async for chunk in response.content.iter_any(): + if chunk: + await f.write(chunk) + await response.release() + if os.path.exists(path) and filecmp.cmp(path, tmp, shallow=False): + os.remove(tmp) # unchanged: keep the old copy, mtime intact + else: + os.replace(tmp, path) + finally: + if os.path.exists(tmp): + try: + os.remove(tmp) + except OSError: + pass + return path + + +@asynccontextmanager +async def _remote_pcap(project, compute, rel_path): + """ + A controller-local copy of a capture file living on a remote compute. + Acquires of the same file are serialized per key; the entry is + refcounted while yielded so eviction only ever removes idle files. + + Yields the local path, or None when the compute has no such file. + """ + + key = (project.id, compute.id, rel_path) + lock = _remote_locks.setdefault(key, asyncio.Lock()) + async with lock: + entry = _remote_cache.get(key) + if entry is None: + entry = {"path": _cache_path(project, compute, rel_path), "last_used": 0.0, "_uses": 0} + _remote_cache[key] = entry + entry["_uses"] += 1 + try: + path = await _fetch_to_cache(project, compute, rel_path, entry["path"]) + entry["last_used"] = time.monotonic() + if path is None: + _drop_cached_file(key) + yield path + finally: + entry["_uses"] -= 1 + _evict_idle_remote_cache() + + +def _capture_node_compute(project, node_id): + """ + The compute hosting a marker's capture node, or None when the capture is + local — or the node is gone, in which case only the controller's own + markers directory can hold a copy and the caller has already checked it. + """ + + get_node = getattr(project, "get_node", None) + if get_node is None: + return None + try: + compute = get_node(node_id).compute + except ControllerError: + return None + if compute is None or compute.id == "local": + return None + return compute + + +@asynccontextmanager +async def _source_pcap(project, node_id, link_id, marker): + """ + Resolve a marker's capture file to a controller-local path. + + Yields None when no capture exists (no packet ever matched on that + marker). Local markers resolve straight to the project's markers + directory; a marker whose capture node runs on another compute is + fetched from that compute (see _remote_pcap). + """ + + filename = f"{node_id}_{link_id}_{marker}.pcap" + path = os.path.join(project.markers_directory, filename) + if os.path.exists(path): + yield path + return + compute = _capture_node_compute(project, node_id) + if compute is None: + yield None + return + async with _remote_pcap(project, compute, f"project-files/markers/{filename}") as fetched: + yield fetched + + # --------------------------------------------------------------------------- # sharkd: process environment and scratch copies # --------------------------------------------------------------------------- @@ -526,13 +703,19 @@ def _get_manager(): async def close_sessions(): """Server shutdown hook: kill every resident session and drop its scratch - copy, so nothing leaks into /tmp across restarts.""" + copy, and remove the remote-capture cache, so nothing leaks into /tmp + across restarts.""" global _manager + global _remote_cache_dir manager = _manager if manager is not None: await manager.close_all() _manager = None + if _remote_cache_dir is not None: + shutil.rmtree(_remote_cache_dir, ignore_errors=True) + _remote_cache_dir = None + _remote_cache.clear() # --------------------------------------------------------------------------- @@ -655,45 +838,68 @@ async def _merged_frames(project, entries, filter_expr=None, link_id=None): ``sources`` is the stable inventory of the tag: EVERY capture source is listed with engine-free total counts, unaffected by ``link_id`` / ``filter_expr`` — the inventory must not shrink when the view narrows. + A capture file that cannot be fetched from its compute keeps its source + entry, with ``count: 0`` and an ``error`` message, instead of silently + vanishing from the timeline. """ - markers_dir = project.markers_directory merged = [] sources = [] for entry in entries: - pcap = os.path.join(markers_dir, f"{entry['node_id']}_{entry['link_id']}_{entry['marker']}.pcap") - frames = scan_pcap_frames(pcap) if os.path.exists(pcap) else [] - # Inventory first: every source, engine-free totals. - sources.append( - {**{k: entry[k] for k in ("node_id", "link_id", "marker", "data_link_type")}, "count": len(frames)} - ) - if link_id and entry["link_id"] != link_id: - continue # link narrows the stream before any engine work - if frames: - columns = await _columns_for(pcap, filter_expr) - else: - columns = {} - source_key = f"{entry['node_id']}_{entry['link_id']}_{entry['marker']}" - for frame_number, (sec, usec, incl_len) in enumerate(frames, start=1): - if filter_expr is not None and frame_number not in columns: - continue - cols = columns.get(frame_number, {}) - merged.append( + try: + async with _source_pcap(project, entry["node_id"], entry["link_id"], entry["marker"]) as pcap: + frames = scan_pcap_frames(pcap) if pcap else [] + # Inventory first: every source, engine-free totals. + sources.append( + { + **{k: entry[k] for k in ("node_id", "link_id", "marker", "data_link_type")}, + "count": len(frames), + } + ) + if link_id and entry["link_id"] != link_id: + continue # link narrows the stream before any engine work + if frames: + columns = await _columns_for(pcap, filter_expr) + else: + columns = {} + source_key = f"{entry['node_id']}_{entry['link_id']}_{entry['marker']}" + for frame_number, (sec, usec, incl_len) in enumerate(frames, start=1): + if filter_expr is not None and frame_number not in columns: + continue + cols = columns.get(frame_number, {}) + merged.append( + { + "ts": _format_ts(sec, usec), + "ts_us": sec * 1_000_000 + usec, + "_source": source_key, + "len": incl_len, + "node_id": entry["node_id"], + "link_id": entry["link_id"], + "marker": entry["marker"], + "frame_number": frame_number, + "src": cols.get("src"), + "dst": cols.get("dst"), + "proto": cols.get("proto"), + "info": cols.get("info"), + "bg": cols.get("bg"), + "fg": cols.get("fg"), + } + ) + except ComputeError as e: + # The capture file lives on a compute that could not be reached: + # keep the source in the inventory, flagged, so a partial timeline + # is never mistaken for the whole capture. + log.warning( + "Capture file of marker '%s' on link %s could not be fetched: %s", + entry["marker"], + entry["link_id"], + e, + ) + sources.append( { - "ts": _format_ts(sec, usec), - "ts_us": sec * 1_000_000 + usec, - "_source": source_key, - "len": incl_len, - "node_id": entry["node_id"], - "link_id": entry["link_id"], - "marker": entry["marker"], - "frame_number": frame_number, - "src": cols.get("src"), - "dst": cols.get("dst"), - "proto": cols.get("proto"), - "info": cols.get("info"), - "bg": cols.get("bg"), - "fg": cols.get("fg"), + **{k: entry[k] for k in ("node_id", "link_id", "marker", "data_link_type")}, + "count": 0, + "error": str(e), } ) merged.sort(key=lambda f: (f["ts_us"], f["_source"], f["frame_number"])) @@ -761,7 +967,10 @@ async def decode_frame(project, tag, ts, node_id, link_id, marker, frame_number= round-tripped ts still matches the file (guards a rebuild between the timeline view and this click), read the raw bytes for the hex view straight from the pcap, and rename the sharkd protocol tree into the - REST contract (closed key set, values untouched). + REST contract (closed key set, values untouched). A marker captured on + another compute decodes from its fetched cache copy; a compute that + cannot be reached raises ComputeError — distinct from the 404s below, + which mean the marker never matched anything or the capture was rebuilt. ``ts`` is not unique within one pcap (same-microsecond frames are kept deliberately); an explicit ``frame_number`` from the frame list entry @@ -778,40 +987,40 @@ async def decode_frame(project, tag, ts, node_id, link_id, marker, frame_number= if entry is None: raise ControllerNotFoundError(f"No marker '{marker}' with tag {tag} on link {link_id} captured by {node_id}") - pcap = os.path.join(project.markers_directory, f"{node_id}_{link_id}_{marker}.pcap") - if not os.path.exists(pcap): - raise ControllerNotFoundError(f"No capture file for marker '{marker}' (nothing ever matched)") + async with _source_pcap(project, node_id, link_id, marker) as pcap: + if pcap is None: + raise ControllerNotFoundError(f"No capture file for marker '{marker}' (nothing ever matched)") - frames = scan_pcap_frames(pcap) - rebuilt_message = f"No frame at ts {ts} in marker '{marker}' (the capture may have been rebuilt)" - if frame_number is None: - # The ts must be the exact string the timeline returned; find the frame - # it identifies rather than trusting any position hint from the client. - frame_number = next( - (i for i, (sec, usec, _len) in enumerate(frames, start=1) if _format_ts(sec, usec) == ts), - None, - ) + frames = scan_pcap_frames(pcap) + rebuilt_message = f"No frame at ts {ts} in marker '{marker}' (the capture may have been rebuilt)" if frame_number is None: - raise ControllerNotFoundError(rebuilt_message) - else: - if not 1 <= frame_number <= len(frames): - raise ControllerNotFoundError(rebuilt_message) - sec, usec, _len = frames[frame_number - 1] - if _format_ts(sec, usec) != ts: + # The ts must be the exact string the timeline returned; find the frame + # it identifies rather than trusting any position hint from the client. + frame_number = next( + (i for i, (sec, usec, _len) in enumerate(frames, start=1) if _format_ts(sec, usec) == ts), + None, + ) + if frame_number is None: + raise ControllerNotFoundError(rebuilt_message) + else: + if not 1 <= frame_number <= len(frames): + raise ControllerNotFoundError(rebuilt_message) + sec, usec, _len = frames[frame_number - 1] + if _format_ts(sec, usec) != ts: + raise ControllerNotFoundError(rebuilt_message) + + raw_hex = read_frame_bytes(pcap, frame_number) + if raw_hex is None: + # The header scan listed it but the bytes are gone — mid-write tail. raise ControllerNotFoundError(rebuilt_message) - raw_hex = read_frame_bytes(pcap, frame_number) - if raw_hex is None: - # The header scan listed it but the bytes are gone — mid-write tail. - raise ControllerNotFoundError(rebuilt_message) - - async with _get_manager().session(pcap) as session: - try: - result = await session.rpc("frame", {"frame": frame_number, "proto": True}) - except _SharkdRpcError as e: - # The frame range was validated against the file above, so an - # engine error here is a real fault (502), not a client 404. - raise SharkdError(f"sharkd frame failed: {e.message}") + async with _get_manager().session(pcap) as session: + try: + result = await session.rpc("frame", {"frame": frame_number, "proto": True}) + except _SharkdRpcError as e: + # The frame range was validated against the file above, so an + # engine error here is a real fault (502), not a client 404. + raise SharkdError(f"sharkd frame failed: {e.message}") tree = _rename_value(result.get("tree", [])) return { diff --git a/gns3server/schemas/controller/marker_replay.py b/gns3server/schemas/controller/marker_replay.py index a07d31cd3..6cf79d31f 100644 --- a/gns3server/schemas/controller/marker_replay.py +++ b/gns3server/schemas/controller/marker_replay.py @@ -29,6 +29,9 @@ class ReplaySource(BaseModel): marker: str data_link_type: str count: int = Field(..., description="Total frames in the capture, unaffected by filters") + error: Optional[str] = Field( + None, description="Set when the capture file could not be fetched from the compute hosting it" + ) class ReplayFrame(BaseModel): diff --git a/tests/api/routes/controller/test_response_models.py b/tests/api/routes/controller/test_response_models.py index 9ea0f6dda..476ac8466 100644 --- a/tests/api/routes/controller/test_response_models.py +++ b/tests/api/routes/controller/test_response_models.py @@ -458,7 +458,14 @@ class TestReplayFrameDetail: "end": None, "frame_count": 0, "sources": [ - {"node_id": "n1", "link_id": link.id, "marker": "icmp", "data_link_type": "DLT_EN10MB", "count": 0} + { + "node_id": "n1", + "link_id": link.id, + "marker": "icmp", + "data_link_type": "DLT_EN10MB", + "count": 0, + "error": None, + } ], "frames": [], } diff --git a/tests/controller/test_marker_replay.py b/tests/controller/test_marker_replay.py index 7939b9a03..84edfd4e2 100644 --- a/tests/controller/test_marker_replay.py +++ b/tests/controller/test_marker_replay.py @@ -30,7 +30,13 @@ Unit tests for the tag-keyed aggregate replay module (sharkd edition): invalidation, LRU bound (idle-only eviction), single spawn under concurrent acquires, timeout/stale-reply aborts — against the real sharkd where installed, plus the frame detail end-to-end (hex + renamed tree + - filter expressions, same-microsecond disambiguation). + filter expressions, same-microsecond disambiguation) +* remote capture sources — markers whose capture node runs on another + compute are fetched over HTTP into the bounded cache: merged timelines + span both computes, a compute with no file is a zero-count source (not an + error), an unreachable compute flags its source instead of shrinking the + inventory, unchanged refetches keep the cached file identity (resident + sharkd sessions survive), and the local compute is never queried. """ import asyncio @@ -38,6 +44,7 @@ import os import shutil import struct import sys +from contextlib import asynccontextmanager from types import SimpleNamespace from unittest.mock import patch @@ -45,6 +52,7 @@ import pytest from gns3server.controller import marker_replay from gns3server.controller.controller_error import ( + ComputeError, ControllerBadRequestError, ControllerError, ControllerNotFoundError, @@ -880,3 +888,254 @@ class TestDecodeFrame: assert seen <= known, f"unknown sharkd keys appeared: {seen - known}" finally: await manager.close_all() + + +# --------------------------------------------------------------------------- +# remote capture sources (fetch from the compute hosting the capture node) +# --------------------------------------------------------------------------- + + +def _pcap_bytes(frames, magic=PCAP_MAGIC_US, snaplen=65535): + """In-memory twin of _write_pcap: the bytes a compute serves for a marker + pcap.""" + + buf = bytearray(struct.pack(" Date: Sun, 18 Oct 2026 22:07:37 +0300 Subject: [PATCH 08/10] feat(qemu): support SATA CD-ROM interfaces - Add per-drive IDE/SATA selection with backward-compatible IDE defaults - Persist interfaces in nodes, templates, and appliance imports - Add database migration support for versioned and unversioned databases - Prevent partial updates when rejecting live interface changes - Validate interfaces and support SATA media changes through the monitor --- gns3server/api/routes/compute/qemu_nodes.py | 4 + gns3server/compute/qemu/qemu_vm.py | 93 ++++++++++++----- gns3server/db/models/templates.py | 2 + gns3server/db/tasks.py | 4 + .../e2a7c9d4b601_add_cdrom_interfaces.py | 22 +++++ gns3server/schemas/compute/qemu_nodes.py | 7 +- gns3server/schemas/controller/appliances.py | 5 + .../controller/templates/qemu_templates.py | 3 + tests/api/routes/compute/test_qemu_nodes.py | 30 ++++++ tests/api/routes/controller/test_templates.py | 22 +++++ tests/compute/qemu/test_qemu_vm.py | 99 +++++++++++++++++++ .../controller/test_appliance_to_template.py | 30 ++++++ tests/controller/test_qemu_cdrom_migration.py | 56 +++++++++++ 13 files changed, 351 insertions(+), 26 deletions(-) create mode 100644 gns3server/db_migrations/versions/e2a7c9d4b601_add_cdrom_interfaces.py create mode 100644 tests/controller/test_qemu_cdrom_migration.py diff --git a/gns3server/api/routes/compute/qemu_nodes.py b/gns3server/api/routes/compute/qemu_nodes.py index ca858d245..bee61b88a 100644 --- a/gns3server/api/routes/compute/qemu_nodes.py +++ b/gns3server/api/routes/compute/qemu_nodes.py @@ -135,6 +135,10 @@ async def update_qemu_node(node_data: schemas.QemuUpdate, node: QemuVM = Depends """ data = jsonable_encoder(node_data, exclude_unset=True) + # Reject interface changes before applying any other property or changing live media. + for name in ("cdrom_interface", "cdrom2_interface"): + if name in data: + node.validate_cdrom_interface(name, data[name]) # update the console first to avoid issue if updating console type node.console = data.pop("console", node.console) for name, value in data.items(): diff --git a/gns3server/compute/qemu/qemu_vm.py b/gns3server/compute/qemu/qemu_vm.py index 908ce15cb..e8ee33140 100644 --- a/gns3server/compute/qemu/qemu_vm.py +++ b/gns3server/compute/qemu/qemu_vm.py @@ -159,6 +159,8 @@ class QemuVM(KernelDatapathMixin, BaseNode): self._hdd_disk_interface = "none" self._cdrom_image = "" self._cdrom2_image = "" + self._cdrom_interface = "ide" + self._cdrom2_interface = "ide" self._bios_image = "" self._boot_priority = "c" self._mac_address = "" @@ -533,6 +535,31 @@ class QemuVM(KernelDatapathMixin, BaseNode): def cdrom2_image(self, image): self._cdrom2_image = self.manager.get_abs_image_path(image, self.working_dir) if image else "" + @property + def cdrom_interface(self): + return self._cdrom_interface + + @cdrom_interface.setter + def cdrom_interface(self, interface): + self.validate_cdrom_interface("cdrom_interface", interface) + self._cdrom_interface = interface + + @property + def cdrom2_interface(self): + return self._cdrom2_interface + + @cdrom2_interface.setter + def cdrom2_interface(self, interface): + self.validate_cdrom_interface("cdrom2_interface", interface) + self._cdrom2_interface = interface + + def validate_cdrom_interface(self, name, interface): + """Validate without mutating the VM, including during API preflight.""" + if interface not in ("ide", "sata"): + raise QemuError("CD/DVD interface must be ide or sata") + if interface != getattr(self, name) and self.is_running(): + raise QemuError("Stop the VM before changing a CD/DVD interface") + async def update_property(self, name, value): """ Update Qemu VM properties. @@ -556,7 +583,9 @@ class QemuVM(KernelDatapathMixin, BaseNode): return image = getattr(self, name) self._cdrom_option(name) - device = "ide1-cd0" if name == "cdrom_image" else "ide1-cd1" + slot = 0 if name == "cdrom_image" else 1 + interface = self.cdrom_interface if slot == 0 else self.cdrom2_interface + device = f"sata-cdrom{slot}" if interface == "sata" else f"ide1-cd{slot}" reader, writer = await self._open_qemu_monitor_connection_vm() if reader is None or writer is None: raise QemuError("Could not connect to the QEMU monitor to change CD/DVD media") @@ -576,7 +605,7 @@ class QemuVM(KernelDatapathMixin, BaseNode): try: await asyncio.wait_for(reader.readuntil(b"(qemu) "), timeout=3) blocks = await command("info block") - if not re.search(rf"(?m)^{device}[: ]", blocks): + if not re.search(rf"(?m)^{re.escape(device)}(?:\s+\([^\r\n]*\))?:", blocks): raise QemuError("Stop the VM before adding this CD/DVD drive") if image: filename = image @@ -2430,30 +2459,44 @@ class QemuVM(KernelDatapathMixin, BaseNode): return options def _cdrom_option(self, name=None): - options = [] - if name in (None, "cdrom_image") and self._cdrom_image: - if not os.path.isfile(self._cdrom_image) or not os.path.exists(self._cdrom_image): - if os.path.islink(self._cdrom_image): - raise QemuError( - f"cdrom image '{self._cdrom_image}' linked to '{os.path.realpath(self._cdrom_image)}' is not accessible" - ) - else: - raise QemuError(f"cdrom image '{self._cdrom_image}' is not accessible") - if self._hdc_disk_image: - raise QemuError("You cannot use a disk image on hdc disk and a CDROM image at the same time") - options.extend(["-cdrom", self._cdrom_image.replace(",", ",,")]) - if name in (None, "cdrom2_image") and self._cdrom2_image: - if not os.path.isfile(self._cdrom2_image): - raise QemuError(f"cdrom image '{self._cdrom2_image}' is not accessible") - interface = self._hdd_disk_interface - if self._create_config_disk and interface == "none": - interface = self._hda_disk_interface - if (self._hdd_disk_image or self._create_config_disk) and interface == "ide": - raise QemuError("You cannot use an IDE hdd disk and a second CDROM image at the same time") - options.extend( - ["-drive", f"file={self._cdrom2_image.replace(',', ',,')},if=ide,index=3,media=cdrom,readonly=on"] - ) + for slot, image_name in enumerate(("cdrom_image", "cdrom2_image")): + image = getattr(self, image_name) + if name not in (None, image_name) or not image: + continue + if not os.path.isfile(image): + if os.path.islink(image): + raise QemuError(f"cdrom image '{image}' linked to '{os.path.realpath(image)}' is not accessible") + raise QemuError(f"cdrom image '{image}' is not accessible") + interface = self.cdrom_interface if slot == 0 else self.cdrom2_interface + if interface == "sata": + # Separate IDs avoid collisions with SATA hard disk controllers. + options.extend( + [ + "-device", + f"ich9-ahci,id=cdrom-ahci{slot}", + "-drive", + f"file={image.replace(',', ',,')},if=none,id=sata-cdrom{slot},media=cdrom,readonly=on", + "-device", + f"ide-cd,drive=sata-cdrom{slot},bus=cdrom-ahci{slot}.0", + ] + ) + elif slot == 0: + if self._hdc_disk_image and self._hdc_disk_interface == "ide": + raise QemuError("You cannot use an IDE hdc disk and a CDROM image at the same time") + options.extend(["-cdrom", image.replace(",", ",,")]) + else: + disk_interface = self._hdd_disk_interface + if self._create_config_disk and disk_interface == "none": + disk_interface = self._hda_disk_interface + if (self._hdd_disk_image or self._create_config_disk) and disk_interface == "ide": + raise QemuError("You cannot use an IDE hdd disk and a second CDROM image at the same time") + options.extend( + [ + "-drive", + f"file={image.replace(',', ',,')},if=ide,index=3,media=cdrom,readonly=on", + ] + ) return options def _bios_option(self): diff --git a/gns3server/db/models/templates.py b/gns3server/db/models/templates.py index ffcaeccef..5d9866c02 100644 --- a/gns3server/db/models/templates.py +++ b/gns3server/db/models/templates.py @@ -202,6 +202,8 @@ class QemuTemplate(Template): hdd_disk_interface = Column(String) cdrom_image = Column(String) cdrom2_image = Column(String) + cdrom_interface = Column(String, nullable=False, default="ide", server_default="ide") + cdrom2_interface = Column(String, nullable=False, default="ide", server_default="ide") initrd = Column(String) kernel_image = Column(String) bios_image = Column(String) diff --git a/gns3server/db/tasks.py b/gns3server/db/tasks.py index 1ef935a86..62de57508 100644 --- a/gns3server/db/tasks.py +++ b/gns3server/db/tasks.py @@ -146,10 +146,14 @@ async def connect_to_db(app: FastAPI) -> None: from gns3server.db_migrations.versions.d9e8a2b7c401_image_inventory_reconciliation import ( upgrade, ) + from gns3server.db_migrations.versions.e2a7c9d4b601_add_cdrom_interfaces import ( + upgrade as upgrade_cdrom_interfaces, + ) with Operations.context(MigrationContext.configure(connection)): upgrade() upgrade_cdrom() + upgrade_cdrom_interfaces() await conn.run_sync(upgrade_inventory) await conn.run_sync(run_stamp, alembic_cfg) diff --git a/gns3server/db_migrations/versions/e2a7c9d4b601_add_cdrom_interfaces.py b/gns3server/db_migrations/versions/e2a7c9d4b601_add_cdrom_interfaces.py new file mode 100644 index 000000000..1f4897378 --- /dev/null +++ b/gns3server/db_migrations/versions/e2a7c9d4b601_add_cdrom_interfaces.py @@ -0,0 +1,22 @@ +"""Add QEMU CD/DVD interfaces.""" + +import sqlalchemy as sa +from alembic import op + +revision = "e2a7c9d4b601" +down_revision = "a6c8e1b2d490" +branch_labels = None +depends_on = None + + +def upgrade(): + existing = {column["name"] for column in sa.inspect(op.get_bind()).get_columns("qemu_templates")} + for name in ("cdrom_interface", "cdrom2_interface"): + if name in existing: + continue + op.add_column("qemu_templates", sa.Column(name, sa.String(), nullable=False, server_default="ide")) + + +def downgrade(): + op.drop_column("qemu_templates", "cdrom2_interface") + op.drop_column("qemu_templates", "cdrom_interface") diff --git a/gns3server/schemas/compute/qemu_nodes.py b/gns3server/schemas/compute/qemu_nodes.py index 339f4fdae..30c3b0f3d 100644 --- a/gns3server/schemas/compute/qemu_nodes.py +++ b/gns3server/schemas/compute/qemu_nodes.py @@ -15,7 +15,7 @@ # along with this program. If not, see . from enum import Enum -from typing import List, Optional +from typing import List, Literal, Optional from uuid import UUID from pydantic import BaseModel, Field @@ -152,6 +152,9 @@ class QemuDiskInterfaceType(str, Enum): none = "none" +QemuCdromInterface = Literal["ide", "sata"] + + class QemuBase(BaseModel): """ Common Qemu node properties. @@ -184,6 +187,8 @@ class QemuBase(BaseModel): hdd_disk_image_md5sum: Optional[str] = Field(None, description="QEMU hdd disk image checksum") hdd_disk_interface: Optional[QemuDiskInterfaceType] = Field(None, description="QEMU hdd interface") cdrom_image: Optional[str] = Field(None, description="QEMU first cdrom image path") + cdrom_interface: QemuCdromInterface = Field("ide", description="First CD/DVD interface") + cdrom2_interface: QemuCdromInterface = Field("ide", description="Second CD/DVD interface") cdrom2_image: Optional[str] = Field(None, description="QEMU second cdrom image path") cdrom_image_md5sum: Optional[str] = Field(None, description="QEMU first cdrom image checksum") cdrom2_image_md5sum: Optional[str] = Field(None, description="QEMU second cdrom image checksum") diff --git a/gns3server/schemas/controller/appliances.py b/gns3server/schemas/controller/appliances.py index d3300a675..a62617398 100644 --- a/gns3server/schemas/controller/appliances.py +++ b/gns3server/schemas/controller/appliances.py @@ -23,6 +23,7 @@ from uuid import UUID from pydantic import AnyUrl, BaseModel, BeforeValidator, Discriminator, EmailStr, Field, Tag from ..common import ExtraConfig +from ..compute.qemu_nodes import QemuCdromInterface # ============================================================================ # Shared Enums @@ -398,6 +399,8 @@ class Qemu(BaseModel): hdc_disk_interface: Optional[QemuDiskInterface] = Field( None, title="Disk interface for the installed hdc_disk_image" ) + cdrom_interface: Optional[QemuCdromInterface] = Field(None, title="First CD/DVD interface") + cdrom2_interface: Optional[QemuCdromInterface] = Field(None, title="Second CD/DVD interface") hdd_disk_interface: Optional[QemuDiskInterface] = Field( None, title="Disk interface for the installed hdd_disk_image" ) @@ -579,6 +582,8 @@ class QemuPropertiesV8(BaseModel): hdc_disk_interface: Optional[QemuDiskInterface] = Field( None, title="Disk interface for the installed hdc_disk_image" ) + cdrom_interface: Optional[QemuCdromInterface] = Field(None, title="First CD/DVD interface") + cdrom2_interface: Optional[QemuCdromInterface] = Field(None, title="Second CD/DVD interface") hdd_disk_interface: Optional[QemuDiskInterface] = Field( None, title="Disk interface for the installed hdd_disk_image" ) diff --git a/gns3server/schemas/controller/templates/qemu_templates.py b/gns3server/schemas/controller/templates/qemu_templates.py index 399918f55..4976dd5fd 100644 --- a/gns3server/schemas/controller/templates/qemu_templates.py +++ b/gns3server/schemas/controller/templates/qemu_templates.py @@ -23,6 +23,7 @@ from gns3server.schemas.compute.qemu_nodes import ( CustomAdapter, QemuAdapterType, QemuBootPriority, + QemuCdromInterface, QemuConsoleType, QemuDiskInterfaceType, QemuOnCloseAction, @@ -80,6 +81,8 @@ class QemuTemplate(TemplateBase): QemuDiskInterfaceType.none, description="QEMU hdd interface" ) cdrom_image: Optional[str] = Field("", description="QEMU first cdrom image path") + cdrom_interface: QemuCdromInterface = Field("ide", description="First CD/DVD interface") + cdrom2_interface: QemuCdromInterface = Field("ide", description="Second CD/DVD interface") cdrom2_image: Optional[str] = Field("", description="QEMU second cdrom image path") initrd: Optional[str] = Field("", description="QEMU initrd path") kernel_image: Optional[str] = Field("", description="QEMU kernel image path") diff --git a/tests/api/routes/compute/test_qemu_nodes.py b/tests/api/routes/compute/test_qemu_nodes.py index 1281f338f..37463fd36 100644 --- a/tests/api/routes/compute/test_qemu_nodes.py +++ b/tests/api/routes/compute/test_qemu_nodes.py @@ -370,6 +370,36 @@ class TestQemuNodesRoutes: assert response.json()["hdb_disk_image"] == "linux载.img" assert response.json()["ram"] == 1024 + async def test_cdrom_interfaces_update_and_read_back( + self, app: FastAPI, compute_client: AsyncClient, qemu_vm: dict + ) -> None: + url = app.url_path_for("compute:update_qemu_node", project_id=qemu_vm["project_id"], node_id=qemu_vm["node_id"]) + response = await compute_client.put(url, json={"cdrom_interface": "sata", "cdrom2_interface": "sata"}) + assert response.status_code == status.HTTP_200_OK + assert response.json()["cdrom_interface"] == "sata" + assert response.json()["cdrom2_interface"] == "sata" + response = await compute_client.get(url) + assert response.status_code == status.HTTP_200_OK + assert response.json()["cdrom_interface"] == "sata" + assert response.json()["cdrom2_interface"] == "sata" + + @pytest.mark.parametrize("name", ["cdrom_interface", "cdrom2_interface"]) + async def test_cdrom_interface_rejection_does_not_partially_update(self, app, compute_client, qemu_vm, name): + url = app.url_path_for("compute:update_qemu_node", project_id=qemu_vm["project_id"], node_id=qemu_vm["node_id"]) + original = (await compute_client.get(url)).json() + with patch("gns3server.compute.qemu.qemu_vm.QemuVM.is_running", return_value=True): + response = await compute_client.put(url, json={"ram": original["ram"] + 128, name: "sata"}) + assert response.status_code == status.HTTP_409_CONFLICT + saved = (await compute_client.get(url)).json() + assert saved["ram"] == original["ram"] + assert saved[name] == original[name] + + @pytest.mark.parametrize("name", ["cdrom_interface", "cdrom2_interface"]) + async def test_cdrom_interface_rejects_null(self, app, compute_client, qemu_vm, name): + url = app.url_path_for("compute:update_qemu_node", project_id=qemu_vm["project_id"], node_id=qemu_vm["node_id"]) + response = await compute_client.put(url, json={name: None}) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT + async def test_qemu_nio_create_udp(self, app: FastAPI, compute_client: AsyncClient, qemu_vm: dict) -> None: params = {"type": "nio_udp", "lport": 4242, "rport": 4343, "rhost": "127.0.0.1"} diff --git a/tests/api/routes/controller/test_templates.py b/tests/api/routes/controller/test_templates.py index 6c127a5c0..0e2f9ce9a 100644 --- a/tests/api/routes/controller/test_templates.py +++ b/tests/api/routes/controller/test_templates.py @@ -1112,6 +1112,28 @@ class TestDockerTemplate: class TestQemuTemplate: + async def test_cdrom_interfaces_persist_on_template_update(self, app, client): + params = { + "name": "SATA installer", + "compute_id": "local", + "template_type": "qemu", + "cdrom_interface": "sata", + "cdrom2_interface": "sata", + } + response = await client.post(app.url_path_for("create_template"), json=params) + assert response.status_code == status.HTTP_201_CREATED + template_id = response.json()["template_id"] + url = app.url_path_for("get_template", template_id=template_id) + saved = (await client.get(url)).json() + assert saved["cdrom_interface"] == saved["cdrom2_interface"] == "sata" + response = await client.put( + app.url_path_for("update_template", template_id=template_id), json={"cdrom_interface": "ide"} + ) + assert response.status_code == status.HTTP_200_OK + saved = (await client.get(url)).json() + assert saved["cdrom_interface"] == "ide" + assert saved["cdrom2_interface"] == "sata" + async def test_qemu_template_create(self, app: FastAPI, client: AsyncClient) -> None: params = { diff --git a/tests/compute/qemu/test_qemu_vm.py b/tests/compute/qemu/test_qemu_vm.py index 047a32ad9..66210e92f 100644 --- a/tests/compute/qemu/test_qemu_vm.py +++ b/tests/compute/qemu/test_qemu_vm.py @@ -1076,6 +1076,40 @@ async def test_second_cdrom_ide_conflict(vm, images_dir, config_disk): vm._cdrom_option() +@pytest.mark.asyncio +async def test_first_cdrom_ide_conflict(vm, images_dir): + iso = Path(images_dir) / "installer.iso" + iso.touch() + vm.cdrom_image = str(iso) + vm._hdc_disk_image = str(iso) + vm._hdc_disk_interface = "ide" + with pytest.raises(QemuError, match="IDE hdc disk"): + vm._cdrom_option() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("interface", ["virtio", "scsi", "sata", "nvme"]) +@pytest.mark.parametrize("cdrom_count", [1, 2]) +async def test_four_non_ide_disks_with_cdroms(vm, images_dir, interface, cdrom_count): + iso = Path(images_dir) / "installer.iso" + iso.touch() + vm.cdrom_image = str(iso) + for drive in "abcd": + disk = Path(images_dir) / f"hd{drive}.qcow2" + disk.touch() + setattr(vm, f"_hd{drive}_disk_image", str(disk)) + setattr(vm, f"_hd{drive}_disk_interface", interface) + expected = ["-cdrom", str(iso)] + if cdrom_count == 2: + vm.cdrom2_image = str(iso) + expected.extend(["-drive", f"file={iso},if=ide,index=3,media=cdrom,readonly=on"]) + assert vm._cdrom_option() == expected + vm.linked_clone = False + with patch.object(vm, "_qemu_img_exec", new_callable=AsyncioMagicMock, return_value=0): + disk_options = await vm._disk_options() + assert sum(option.startswith("file=") for option in disk_options) == 4 + + @pytest.fixture def cdrom_monitor(vm): reader = asyncio.StreamReader() @@ -1164,3 +1198,68 @@ async def test_cdrom_monitor_error_preserves_previous_image(vm, images_dir, cdro await vm.update_property("cdrom2_image", str(replacement)) assert vm.cdrom2_image == str(original) writer.close.assert_called_once() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("slot", [0, 1]) +async def test_sata_cdrom_with_ide_disks(vm, images_dir, slot): + iso = Path(images_dir) / "installer,drivers.iso" + iso.touch() + image_name = "cdrom_image" if slot == 0 else "cdrom2_image" + interface_name = "cdrom_interface" if slot == 0 else "cdrom2_interface" + setattr(vm, image_name, str(iso)) + setattr(vm, interface_name, "sata") + vm._hdc_disk_image = vm._hdd_disk_image = str(iso) + vm._hdc_disk_interface = vm._hdd_disk_interface = "ide" + assert vm._cdrom_option() == [ + "-device", + f"ich9-ahci,id=cdrom-ahci{slot}", + "-drive", + f"file={str(iso).replace(',', ',,')},if=none,id=sata-cdrom{slot},media=cdrom,readonly=on", + "-device", + f"ide-cd,drive=sata-cdrom{slot},bus=cdrom-ahci{slot}.0", + ] + assert vm.asdict()[interface_name] == "sata" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("name", ["cdrom_interface", "cdrom2_interface"]) +async def test_cdrom_interface_change_requires_stopped_vm(vm, name): + with patch.object(vm, "is_running", return_value=True): + with pytest.raises(QemuError, match="Stop the VM"): + await vm.update_property(name, "sata") + assert getattr(vm, name) == "ide" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("slot", [0, 1]) +async def test_sata_cdrom_media_update(vm, images_dir, cdrom_monitor, slot): + state, writer = cdrom_monitor + setattr(vm, "_cdrom_interface" if slot == 0 else "_cdrom2_interface", "sata") + state["blocks"] = f"sata-cdrom{slot} (#block123): old.iso\r\n" + iso = Path(images_dir) / "new.iso" + iso.touch() + await vm.update_property("cdrom_image" if slot == 0 else "cdrom2_image", str(iso)) + writer.write.assert_called_with(f'change sata-cdrom{slot} -f "{iso}"\n'.encode()) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("name", ["cdrom_image", "cdrom2_image"]) +async def test_cdrom_broken_symlink_error_includes_target(vm, images_dir, name): + target = Path(images_dir) / "missing.iso" + link = Path(images_dir) / "linked.iso" + link.symlink_to(target) + setattr(vm, f"_{name}", str(link)) + with pytest.raises(QemuError, match="linked to") as error: + vm._cdrom_option() + assert str(target) in str(error.value) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("slot", [0, 1]) +async def test_sata_cdrom_eject(vm, cdrom_monitor, slot): + state, writer = cdrom_monitor + setattr(vm, "_cdrom_interface" if slot == 0 else "_cdrom2_interface", "sata") + state["blocks"] = f"sata-cdrom{slot}: empty\r\n" + await vm.update_property("cdrom_image" if slot == 0 else "cdrom2_image", "") + writer.write.assert_called_with(f"eject -f sata-cdrom{slot}\n".encode()) diff --git a/tests/controller/test_appliance_to_template.py b/tests/controller/test_appliance_to_template.py index 843e52653..a488c2fa0 100644 --- a/tests/controller/test_appliance_to_template.py +++ b/tests/controller/test_appliance_to_template.py @@ -647,3 +647,33 @@ def test_second_cdrom_survives_appliance_conversion(version): converted = ApplianceToTemplate().new_template(appliance, appliance["versions"][0], "local") assert converted["cdrom_image"] == "installer.iso" assert converted["cdrom2_image"] == "drivers.iso" + + +@pytest.mark.parametrize("name", ["cdrom_interface", "cdrom2_interface"]) +def test_v8_cdrom_interface_survives_validation_and_conversion(name): + import copy + + appliance = copy.deepcopy(VYOS_V8) + appliance["settings"][0]["template_properties"][name] = "sata" + validated = ApplianceModel.model_validate(appliance).model_dump(mode="json", exclude_none=True) + template = ApplianceToTemplate().new_template(validated, validated["versions"][1], "local") + assert template[name] == "sata" + + +def test_legacy_cdrom_interfaces_are_qemu_settings(): + from gns3server.schemas.controller.appliances import Qemu + + settings = Qemu.model_validate( + { + "adapter_type": "e1000", + "adapters": 1, + "ram": 512, + "arch": "x86_64", + "console_type": "telnet", + "kvm": "allow", + "cdrom_interface": "sata", + "cdrom2_interface": "sata", + } + ) + assert settings.model_dump()["cdrom_interface"] == "sata" + assert settings.model_dump()["cdrom2_interface"] == "sata" diff --git a/tests/controller/test_qemu_cdrom_migration.py b/tests/controller/test_qemu_cdrom_migration.py new file mode 100644 index 000000000..87aa74a94 --- /dev/null +++ b/tests/controller/test_qemu_cdrom_migration.py @@ -0,0 +1,56 @@ +import pytest +import sqlalchemy as sa +from alembic import command, config +from alembic.operations import Operations +from alembic.runtime.migration import MigrationContext +from fastapi import FastAPI + +from gns3server.db.models import Base +from gns3server.db.tasks import connect_to_db +from gns3server.db_migrations.versions import e2a7c9d4b601_add_cdrom_interfaces as migration + + +def test_cdrom_migration_defaults_idempotence_and_downgrade(): + engine = sa.create_engine("sqlite://") + with engine.begin() as connection: + connection.exec_driver_sql("CREATE TABLE qemu_templates (id INTEGER PRIMARY KEY)") + connection.exec_driver_sql("INSERT INTO qemu_templates (id) VALUES (1)") + with Operations.context(MigrationContext.configure(connection)): + migration.upgrade() + migration.upgrade() + assert connection.exec_driver_sql("SELECT cdrom_interface, cdrom2_interface FROM qemu_templates").one() == ( + "ide", + "ide", + ) + migration.downgrade() + assert [column["name"] for column in sa.inspect(connection).get_columns("qemu_templates")] == ["id"] + engine.dispose() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("versioned", [False, True]) +async def test_database_startup_adds_cdrom_interfaces(tmp_path, monkeypatch, versioned): + path = tmp_path / "controller.db" + engine = sa.create_engine(f"sqlite:///{path}") + with engine.connect() as connection: + Base.metadata.create_all(connection) + connection.exec_driver_sql("ALTER TABLE qemu_templates DROP COLUMN cdrom_interface") + connection.exec_driver_sql("ALTER TABLE qemu_templates DROP COLUMN cdrom2_interface") + if versioned: + cfg = config.Config() + cfg.set_main_option("script_location", "gns3server:db_migrations") + cfg.attributes["connection"] = connection + command.stamp(cfg, migration.down_revision) + connection.commit() + engine.dispose() + monkeypatch.setenv("GNS3_DATABASE_URI", f"sqlite+aiosqlite:///{path}") + app = FastAPI() + await connect_to_db(app) + try: + async with app.state._db_engine.connect() as connection: + columns = await connection.run_sync(lambda conn: sa.inspect(conn).get_columns("qemu_templates")) + assert {"cdrom_interface", "cdrom2_interface"} <= {column["name"] for column in columns} + revision = await connection.scalar(sa.text("SELECT version_num FROM alembic_version")) + assert revision == migration.revision + finally: + await app.state._db_engine.dispose() From a9f5306f16fa44c2728c0766c33f503311ed4c97 Mon Sep 17 00:00:00 2001 From: Cristi Date: Mon, 19 Oct 2026 15:42:18 +0300 Subject: [PATCH 09/10] feat(cloud): attach TAP interfaces to existing Linux bridges --- gns3server/compute/base_manager.py | 7 +- gns3server/compute/builtin/nodes/cloud.py | 48 +++++- gns3server/schemas/compute/cloud_nodes.py | 19 ++- tests/api/routes/compute/test_cloud_nodes.py | 34 ++++ tests/compute/builtin/nodes/test_cloud.py | 154 +++++++++++++++++++ 5 files changed, 253 insertions(+), 9 deletions(-) diff --git a/gns3server/compute/base_manager.py b/gns3server/compute/base_manager.py index 5c1511e8e..4d29f2546 100644 --- a/gns3server/compute/base_manager.py +++ b/gns3server/compute/base_manager.py @@ -297,11 +297,12 @@ class BaseManager: return node @staticmethod - def has_privileged_access(executable): + def has_privileged_access(executable, *, require_net_admin=False): """ Check if an executable have the right to attach to Ethernet and TAP adapters. :param executable: executable path + :param require_net_admin: also require CAP_NET_ADMIN for network configuration :returns: True or False """ @@ -324,8 +325,8 @@ class BaseManager: try: if sys.platform.startswith("linux") and "security.capability" in os.listxattr(executable): caps = os.getxattr(executable, "security.capability") - # test the 2nd byte and check if the 13th bit (CAP_NET_RAW) is set - if struct.unpack(". import logging +import os +import re import subprocess import sys import gns3server.utils.asyncio import gns3server.utils.interfaces from gns3server.compute.ubridge.ubridge_error import UbridgeError +from gns3server.schemas.compute.cloud_nodes import LINUX_TAP_NAME_PATTERN from gns3server.utils.kernel_anchor import is_kernel_datapath_interface from ...base_node import BaseNode @@ -69,7 +72,7 @@ class Cloud(BaseNode): for port in ports: port["port_number"] = port_number port_number += 1 - self._ports_mapping = ports + self._ports_mapping = self._normalize_ports_mapping(ports) @property def nios(self): @@ -101,6 +104,9 @@ class Cloud(BaseNode): } ) + if sys.platform.startswith("linux") and gns3server.utils.interfaces.is_interface_bridge(interface["name"]): + host_interfaces[-1]["is_bridge"] = True + return { "name": self.name, "usage": self.usage, @@ -206,6 +212,11 @@ class Cloud(BaseNode): return self._ports_mapping + @staticmethod + def _normalize_ports_mapping(ports): + # API responses include bridge=None; legacy projects omit it. + return [{key: value for key, value in port.items() if key != "bridge" or value is not None} for port in ports] + @ports_mapping.setter def ports_mapping(self, ports): """ @@ -214,6 +225,7 @@ class Cloud(BaseNode): :param ports: ports info """ + ports = self._normalize_ports_mapping(ports) if ports != self._ports_mapping: if len(self._nios) > 0: raise NodeError("Cannot modify a cloud that is already connected.") @@ -314,7 +326,9 @@ class Cloud(BaseNode): await self._ubridge_apply_filters(bridge_name, nio.filters) await self._ubridge_apply_markers(bridge_name, nio) if port_info["type"] in ("ethernet", "tap"): - if not self.manager.has_privileged_access(self.ubridge_path): + if not self.manager.has_privileged_access( + self.ubridge_path, require_net_admin=bool(port_info.get("bridge")) + ): raise NodeError( "uBridge requires root access or the capability to interact with Ethernet and TAP adapters" ) @@ -334,9 +348,7 @@ class Cloud(BaseNode): await self._add_windows_ethernet(port_info, bridge_name) elif port_info["type"] == "tap": - await self._ubridge_send( - 'bridge add_nio_tap {name} "{interface}"'.format(name=bridge_name, interface=port_info["interface"]) - ) + await self._add_tap_interface(port_info, bridge_name) elif port_info["type"] == "udp": await self._ubridge_send( @@ -350,6 +362,32 @@ class Cloud(BaseNode): await self._ubridge_send(f"bridge start {bridge_name}") + async def _add_tap_interface(self, port_info, bridge_name): + """Open a TAP and restore its optional host bridge assignment.""" + target_bridge = port_info.get("bridge") + if target_bridge: + self._validate_tap_bridge(port_info["interface"], target_bridge) + await self._ubridge_send( + 'bridge add_nio_tap {name} "{interface}"'.format(name=bridge_name, interface=port_info["interface"]) + ) + + if target_bridge: + await self._ubridge_send(f'brctl addif "{target_bridge}" "{port_info["interface"]}"') + + def _validate_tap_bridge(self, tap, target_bridge): + """Validate host resources before creating or attaching a Cloud TAP.""" + if not sys.platform.startswith("linux"): + raise NodeError("TAP bridge assignments are only supported on Linux") + if not all(re.fullmatch(LINUX_TAP_NAME_PATTERN, name) for name in (target_bridge, tap)): + raise NodeError("Invalid TAP or bridge interface name") + if target_bridge not in { + interface["name"] for interface in self._interfaces() + } or not gns3server.utils.interfaces.is_interface_bridge(target_bridge): + raise NodeError(f"Bridge '{target_bridge}' is missing or not allowed on this server") + master_path = os.path.join("/sys/class/net", tap, "master") + if os.path.exists(master_path) and os.path.basename(os.path.realpath(master_path)) != target_bridge: + raise NodeError(f"TAP '{tap}' already belongs to another bridge") + async def _add_linux_ethernet(self, port_info, bridge_name): """ Connects an Ethernet interface on Linux using raw sockets. diff --git a/gns3server/schemas/compute/cloud_nodes.py b/gns3server/schemas/compute/cloud_nodes.py index e2887bbec..274bf23ee 100644 --- a/gns3server/schemas/compute/cloud_nodes.py +++ b/gns3server/schemas/compute/cloud_nodes.py @@ -14,15 +14,18 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import re from enum import Enum from typing import List, Optional, Union from uuid import UUID -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, model_validator from ..common import NodeStatus from ..update import PartialUpdateModel +LINUX_TAP_NAME_PATTERN = r"^[a-zA-Z0-9_.:-]{1,15}$" + class HostInterfaceType(str, Enum): ethernet = "ethernet" @@ -57,6 +60,7 @@ class HostInterface(BaseModel): name: str = Field(..., description="Interface name") type: HostInterfaceType = Field(..., description="Interface type") special: bool = Field(..., description="Whether the interface is non standard") + is_bridge: bool = Field(False, description="Whether this is a Linux bridge") ip_addresses: List[HostInterfaceIPAddress] = Field( default_factory=list, description="All IPv4 and IPv6 addresses on this interface" ) @@ -94,6 +98,19 @@ class TAPPort(BaseModel): port_number: int type: TAPType interface: str + bridge: Optional[str] = Field( + None, + min_length=1, + max_length=15, + pattern=LINUX_TAP_NAME_PATTERN, + description="Existing Linux bridge to attach the TAP to", + ) + + @model_validator(mode="after") + def validate_bridged_interface(self): + if self.bridge and not re.fullmatch(LINUX_TAP_NAME_PATTERN, self.interface): + raise ValueError("A bridged TAP name must be 1-15 letters, digits, underscores, dots, colons or hyphens") + return self class UDPType(str, Enum): diff --git a/tests/api/routes/compute/test_cloud_nodes.py b/tests/api/routes/compute/test_cloud_nodes.py index 0500356a8..fe1d58519 100644 --- a/tests/api/routes/compute/test_cloud_nodes.py +++ b/tests/api/routes/compute/test_cloud_nodes.py @@ -19,6 +19,7 @@ import pytest_asyncio from fastapi import FastAPI, status from httpx import AsyncClient +from gns3server.compute.builtin import Builtin from gns3server.compute.project import Project from tests.utils import asyncio_patch @@ -212,3 +213,36 @@ class TestCloudNodesRoutes: # response = await compute_api.get("/projects/{project_id}/cloud/nodes/{node_id}/adapters/0/ports/0/pcap".format(project_id=compute_project.id, node_id=vm["node_id"])) # assert response.status_code == 200 # + + async def test_unbridged_connected_tap_update(self, app, compute_client, compute_project): + ports = [{"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap"}] + with asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._start_ubridge"): + response = await compute_client.post( + app.url_path_for("compute:create_cloud", project_id=compute_project.id), + json={"name": "Cloud TAP", "ports_mapping": ports}, + ) + assert response.status_code == 201 + data = response.json() + node = Builtin.instance().get_node(data["node_id"], project_id=compute_project.id) + node.nios[0] = object() + response = await compute_client.put( + app.url_path_for("compute:update_cloud", project_id=compute_project.id, node_id=node.id), + json={"usage": "Updated usage", "ports_mapping": data["ports_mapping"]}, + ) + assert response.status_code == 200 + assert response.json()["usage"] == "Updated usage" + + @pytest.mark.parametrize("name", ["tap-interface-too-long", "tap bad", "/dev/tap0"]) + async def test_invalid_bridged_tap_rejected_before_save(self, app, compute_client, compute_project, name): + with asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._start_ubridge") as start: + response = await compute_client.post( + app.url_path_for("compute:create_cloud", project_id=compute_project.id), + json={ + "name": "Invalid TAP", + "ports_mapping": [ + {"interface": name, "name": name, "port_number": 0, "type": "tap", "bridge": "virbr0"} + ], + }, + ) + assert response.status_code == 422 + start.assert_not_called() diff --git a/tests/compute/builtin/nodes/test_cloud.py b/tests/compute/builtin/nodes/test_cloud.py index 180ab167d..89d1b7168 100644 --- a/tests/compute/builtin/nodes/test_cloud.py +++ b/tests/compute/builtin/nodes/test_cloud.py @@ -15,19 +15,34 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import struct import uuid from unittest.mock import MagicMock, call, patch import pytest import pytest_asyncio +from fastapi.encoders import jsonable_encoder +from pydantic import ValidationError +from gns3server.compute.base_manager import BaseManager from gns3server.compute.builtin.nodes.cloud import Cloud +from gns3server.compute.error import NodeError from gns3server.compute.nios.nio_udp import NIOUDP +from gns3server.compute.ubridge.ubridge_error import UbridgeError +from gns3server.schemas.compute.cloud_nodes import Cloud as CloudSchema +from gns3server.schemas.compute.cloud_nodes import CloudUpdate, TAPPort from tests.utils import asyncio_patch pytestmark = pytest.mark.asyncio +@pytest.fixture(autouse=True) +def mock_bridge_detection(): + # Interface inventories are mocked: never mix them with the host's sysfs. + with patch("gns3server.utils.interfaces.is_interface_bridge", return_value=False): + yield + + @pytest.fixture def nio(): @@ -256,3 +271,142 @@ async def test_linux_ethernet_raw_add_nio_bridge(linux_platform, compute_project call(f"bridge start {cloud._id}-0"), ] ) + + +@pytest.mark.parametrize("bridge", [None, "virbr0"]) +@pytest.mark.parametrize("existing_master", [False, True]) +async def test_tap_bridge_recreated(linux_platform, compute_project, manager, nio, bridge, existing_master): + ports = [{"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap"}] + if bridge: + ports[0]["bridge"] = bridge + cloud = Cloud("cloud1", str(uuid.uuid4()), compute_project, manager, ports=ports) + # Round-trip through the API schema, as happens when a project is restored. + restored_ports = CloudSchema(**cloud.asdict()).model_dump()["ports_mapping"] + restored = Cloud("cloud1", cloud.id, compute_project, manager, ports=restored_ports) + with ( + asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._ubridge_send") as send, + asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._start_ubridge"), + patch.object(restored, "_interfaces", return_value=[{"name": "virbr0"}]), + patch("gns3server.utils.interfaces.is_interface_bridge", return_value=True), + patch("gns3server.compute.builtin.nodes.cloud.os.path.exists", return_value=existing_master), + patch("gns3server.compute.builtin.nodes.cloud.os.path.realpath", return_value="/sys/class/net/virbr0"), + ): + restored._nios[0] = nio + await restored.start() + tap_call = call(f'bridge add_nio_tap {cloud.id}-0 "tap0"') + assert tap_call in send.call_args_list + attach_call = call('brctl addif "virbr0" "tap0"') + if bridge: + assert send.call_args_list.index(tap_call) < send.call_args_list.index(attach_call) + else: + assert attach_call not in send.call_args_list + + +@pytest.mark.parametrize("failure", ["missing", "not_bridge", "other_master", "invalid_name", "non_linux"]) +async def test_tap_bridge_validation(linux_platform, compute_project, manager, nio, failure): + ports = [{"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap", "bridge": "virbr0"}] + if failure == "invalid_name": + ports[0]["interface"] = 'tap0"\nbridge delete all' + cloud = Cloud("cloud1", str(uuid.uuid4()), compute_project, manager, ports=ports) + with ( + asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._ubridge_send") as send, + patch.object(cloud, "_interfaces", return_value=[] if failure == "missing" else [{"name": "virbr0"}]), + patch("gns3server.utils.interfaces.is_interface_bridge", return_value=failure != "not_bridge"), + patch("gns3server.compute.builtin.nodes.cloud.os.path.exists", return_value=failure == "other_master"), + patch("gns3server.compute.builtin.nodes.cloud.os.path.realpath", return_value="/sys/class/net/another0"), + patch("gns3server.compute.builtin.nodes.cloud.sys.platform", "darwin" if failure == "non_linux" else "linux"), + ): + with pytest.raises(NodeError): + await cloud._add_ubridge_connection(nio, 0) + assert not any("add_nio_tap" in c.args[0] or "brctl addif" in c.args[0] for c in send.call_args_list) + + +async def test_tap_bridge_attachment_failure(linux_platform, compute_project, manager, nio): + ports = [{"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap", "bridge": "virbr0"}] + cloud = Cloud("cloud1", str(uuid.uuid4()), compute_project, manager, ports=ports) + cloud.status = "started" + + async def send(command): + if command.startswith("brctl addif"): + raise UbridgeError("Operation not permitted") + + with ( + asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._ubridge_send", side_effect=send), + asyncio_patch("gns3server.compute.builtin.nodes.cloud.Cloud._stop_ubridge") as stop, + patch.object(cloud, "_interfaces", return_value=[{"name": "virbr0"}]), + patch("gns3server.utils.interfaces.is_interface_bridge", return_value=True), + patch("gns3server.compute.builtin.nodes.cloud.os.path.exists", return_value=False), + ): + await cloud.add_nio(nio, 0) + stop.assert_called_once() + assert cloud.status == "stopped" + + +async def test_cloud_lists_linux_bridges(linux_platform, compute_project, manager): + cloud = Cloud( + "cloud1", + str(uuid.uuid4()), + compute_project, + manager, + ports=[{"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap"}], + ) + with ( + patch( + "gns3server.utils.interfaces.interfaces", + return_value=[ + {"name": "eth0", "type": "ethernet", "special": False}, + {"name": "virbr0", "type": "ethernet", "special": True}, + ], + ), + patch("gns3server.utils.interfaces.is_interface_bridge", side_effect=lambda name: name == "virbr0"), + ): + interfaces = CloudSchema(**cloud.asdict()).model_dump()["interfaces"] + assert [(interface["name"], interface["is_bridge"]) for interface in interfaces] == [ + ("eth0", False), + ("virbr0", True), + ] + + +@pytest.mark.parametrize("stored_bridge", [None, "omitted"]) +async def test_connected_tap_mapping_round_trip(compute_project, manager, stored_bridge): + port = {"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap"} + if stored_bridge is None: + port["bridge"] = None + cloud = Cloud("cloud1", str(uuid.uuid4()), compute_project, manager, ports=[port]) + response = CloudSchema(**cloud.asdict()).model_dump(mode="json") + cloud._nios[0] = object() + ports = jsonable_encoder(CloudUpdate(ports_mapping=response["ports_mapping"]), exclude_unset=True)["ports_mapping"] + cloud.ports_mapping = ports + assert cloud.ports_mapping == [{"interface": "tap0", "name": "tap0", "port_number": 0, "type": "tap"}] + ports[0]["bridge"] = "virbr0" + with pytest.raises(NodeError, match="already connected"): + cloud.ports_mapping = ports + + +@pytest.mark.parametrize("name", ["tap-interface-too-long", "tap bad", 'tap0"\nbridge delete all', "/dev/tap0"]) +async def test_bridged_tap_name_validation_preserves_legacy_names(name): + port = {"interface": name, "name": name, "port_number": 0, "type": "tap"} + assert TAPPort(**port).interface == name + with pytest.raises(ValidationError, match="bridged TAP name"): + TAPPort(**port, bridge="virbr0") + + +@pytest.mark.parametrize( + "caps,legacy_allowed,bridge_allowed", + [ + (0, False, False), + (1 << 12, False, False), + (1 << 13, True, False), + ((1 << 12) | (1 << 13), True, True), + ], +) +async def test_bridged_tap_requires_net_admin(linux_platform, tmp_path, caps, legacy_allowed, bridge_allowed): + executable = tmp_path / "ubridge" + executable.write_text("") + with ( + patch("os.geteuid", return_value=1000), + patch("os.listxattr", return_value=["security.capability"]), + patch("os.getxattr", return_value=struct.pack(" Date: Mon, 19 Oct 2026 16:02:39 +0300 Subject: [PATCH 10/10] fix(cloud): keep ports_mapping getter and setter adjacent --- gns3server/compute/builtin/nodes/cloud.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gns3server/compute/builtin/nodes/cloud.py b/gns3server/compute/builtin/nodes/cloud.py index 9f9b556a4..7e28c33ae 100644 --- a/gns3server/compute/builtin/nodes/cloud.py +++ b/gns3server/compute/builtin/nodes/cloud.py @@ -202,6 +202,11 @@ class Cloud(BaseNode): self._remote_console_http_path = remote_console_http_path + @staticmethod + def _normalize_ports_mapping(ports): + # API responses include bridge=None; legacy projects omit it. + return [{key: value for key, value in port.items() if key != "bridge" or value is not None} for port in ports] + @property def ports_mapping(self): """ @@ -212,11 +217,6 @@ class Cloud(BaseNode): return self._ports_mapping - @staticmethod - def _normalize_ports_mapping(ports): - # API responses include bridge=None; legacy projects omit it. - return [{key: value for key, value in port.items() if key != "bridge" or value is not None} for port in ports] - @ports_mapping.setter def ports_mapping(self, ports): """