From 6e36b58a5d7bf7fd0e55a66e5d318b8bb6ca6f7c Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 2 Oct 2026 21:16:01 +0800 Subject: [PATCH] fix(compute): refuse a duplicate port NIO and wire before bookkeeping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compute-side backstop of the controller's duplicate-port guard (keep-port-links + live-port bind made the controller guard reliable; this is the layer under it). IOU and Dynamips bound the adapter NIO before the uBridge/hypervisor attach: a failed attach left the port's binding replaced — the link that still owned the port lost its handle and its host state leaked (the IOU EBUSY incident's 8 orphan TAPs). Dynamips was worse: its wiring idempotence check silently skipped the attach on a second binding and rebound only the bookkeeping. A port that already carries an NIO is now refused, and both types bind the NIO only after the wiring succeeded. --- gns3server/compute/dynamips/nodes/router.py | 16 ++++++- gns3server/compute/iou/iou_vm.py | 44 ++++++++++++------- .../dynamips/test_dynamips_kernel_datapath.py | 42 ++++++++++++++++++ tests/compute/iou/test_iou_kernel_datapath.py | 40 +++++++++++++++++ 4 files changed, 125 insertions(+), 17 deletions(-) diff --git a/gns3server/compute/dynamips/nodes/router.py b/gns3server/compute/dynamips/nodes/router.py index 4cb6cf964..f9e4e8bae 100644 --- a/gns3server/compute/dynamips/nodes/router.py +++ b/gns3server/compute/dynamips/nodes/router.py @@ -1312,12 +1312,26 @@ class Router(KernelDatapathMixin, BaseNode): if not adapter.port_exists(port_number): raise DynamipsError(f"Port {port_number} does not exist on adapter {adapter}") + # The compute-side backstop of the controller's duplicate-port guard: + # a port carries at most one NIO, and overwriting the one a link + # still owns orphans that link's teardown (its host state leaks). + # Refusing also keeps the kernel wiring's own idempotence check from + # silently skipping the attach while rebinding the bookkeeping. + if adapter.get_nio(port_number) is not None: + raise DynamipsError( + "Port {port_number} on slot {slot_number} of router '{name}' already has a link".format( + name=self._name, slot_number=slot_number, port_number=port_number + ) + ) + if isinstance(nio, NIOBridge): # Kernel datapath: the port's anchor TAP is opened in the # hypervisor and enslaved into the NIO's per-link bridge (the # wiring is deferred to the node's start when no anchors exist). - adapter.add_nio(port_number, nio) + # Wire before bookkeeping: an attach that fails mid-way leaves + # the port unbound instead of half-wired. await self._attach_kernel_nio(slot_number, port_number, nio) + adapter.add_nio(port_number, nio) log.debug( 'Router "{name}" [{id}]: {nio} bound to port {slot_number}/{port_number}'.format( name=self._name, id=self._id, nio=nio, slot_number=slot_number, port_number=port_number diff --git a/gns3server/compute/iou/iou_vm.py b/gns3server/compute/iou/iou_vm.py index 676d46e3a..af3d82c1a 100644 --- a/gns3server/compute/iou/iou_vm.py +++ b/gns3server/compute/iou/iou_vm.py @@ -1345,27 +1345,39 @@ class IOUVM(KernelDatapathMixin, BaseNode): if not adapter.port_exists(port_number): raise IOUError(f"Port {port_number} does not exist on adapter {adapter}") - adapter.add_nio(port_number, nio) - log.debug(f'IOU "{self._name}" [{self._id}]: {nio} added to {adapter_number}/{port_number}') + # The compute-side backstop of the controller's duplicate-port guard: + # a port carries at most one NIO, and overwriting the one a link + # still owns orphans that link's teardown (its host state leaks). + if adapter.get_nio(port_number) is not None: + raise IOUError( + "Port {port_number} on adapter {adapter_number} of IOU '{name}' already has a link".format( + name=self._name, adapter_number=adapter_number, port_number=port_number + ) + ) + # Wire before bookkeeping: an attach that fails mid-way leaves the + # port unbound instead of half-wired. if self.ubridge: if isinstance(nio, NIOBridge): await self._attach_kernel_nio(adapter_number, port_number, nio) - return - await self._ubridge_send( - "iol_bridge add_nio_udp {name} {iol_id} {bay} {unit} {lport} {rhost} {rport}".format( - name=self._iol_bridge_name(), - iol_id=self.application_id, - bay=adapter_number, - unit=port_number, - lport=nio.lport, - rhost=nio.rhost, - rport=nio.rport, + else: + await self._ubridge_send( + "iol_bridge add_nio_udp {name} {iol_id} {bay} {unit} {lport} {rhost} {rport}".format( + name=self._iol_bridge_name(), + iol_id=self.application_id, + bay=adapter_number, + unit=port_number, + lport=nio.lport, + rhost=nio.rhost, + rport=nio.rport, + ) ) - ) - location = self._iol_location(adapter_number, port_number) - await self._ubridge_apply_filters(location, nio.filters) - await self._ubridge_apply_markers(location, nio) + location = self._iol_location(adapter_number, port_number) + await self._ubridge_apply_filters(location, nio.filters) + await self._ubridge_apply_markers(location, nio) + + adapter.add_nio(port_number, nio) + log.debug(f'IOU "{self._name}" [{self._id}]: {nio} added to {adapter_number}/{port_number}') async def adapter_update_nio_binding(self, adapter_number, port_number, nio): """ diff --git a/tests/compute/dynamips/test_dynamips_kernel_datapath.py b/tests/compute/dynamips/test_dynamips_kernel_datapath.py index b241be526..885780524 100644 --- a/tests/compute/dynamips/test_dynamips_kernel_datapath.py +++ b/tests/compute/dynamips/test_dynamips_kernel_datapath.py @@ -270,6 +270,48 @@ async def test_attach_kernel_nio_opens_the_anchor_in_the_hypervisor_and_enslaves router._ubridge_send.assert_any_call(f'link set "{TAP00}" up') +@pytest.mark.asyncio +async def test_slot_add_nio_binding_refuses_an_occupied_port(router): + """ + The compute-side backstop of the duplicate-port guard: a second NIO for + a port that already carries one is refused here instead of overwriting + (the wiring's own idempotence check would silently skip the attach and + rebind the bookkeeping) — the occupying link's teardown needs its own + NIO handle, and its host state with it. + """ + + router.status = "started" + router._kernel_taps[(0, 0)] = TAP00 + first = await _nio(router) + await router.slot_add_nio_binding(0, 0, first) + assert router._slots[0].get_nio(0) is first + + second = await _nio(router, bridge="gns3other") + with pytest.raises(DynamipsError, match="already has a link"): + await router.slot_add_nio_binding(0, 0, second) + + assert router._slots[0].get_nio(0) is first + + +@pytest.mark.asyncio +async def test_slot_add_nio_binding_attach_failure_leaves_the_port_unbound(router): + """ + The bookkeeping happens last: a kernel attach that fails mid-way (the + anchor is another link's bridge port — uBridge EBUSY) must leave the + port without a binding, so the failed link creates no half-wired state. + """ + + router.status = "started" + router._kernel_taps[(0, 0)] = TAP00 + router._ubridge_send.side_effect = UbridgeError("208-Device or resource busy") + + nio = await _nio(router) + with pytest.raises(UbridgeError): + await router.slot_add_nio_binding(0, 0, nio) + + assert router._slots[0].get_nio(0) is None + + @pytest.mark.asyncio async def test_attach_kernel_nio_without_anchor_on_a_running_node_raises(router): """ diff --git a/tests/compute/iou/test_iou_kernel_datapath.py b/tests/compute/iou/test_iou_kernel_datapath.py index 9a49853a7..40645b279 100644 --- a/tests/compute/iou/test_iou_kernel_datapath.py +++ b/tests/compute/iou/test_iou_kernel_datapath.py @@ -33,6 +33,7 @@ from gns3server.compute.iou import IOU from gns3server.compute.iou.iou_vm import IOUVM from gns3server.compute.iou.iou_error import IOUError from gns3server.compute.nios.nio_bridge import NIOBridge +from gns3server.compute.ubridge.ubridge_error import UbridgeError NODE_ID = "00010203-0405-0607-0809-0a0b0c0d0e0f" @@ -168,6 +169,45 @@ async def test_attach_kernel_nio_without_anchor_raises(vm): await vm._attach_kernel_nio(2, 0, nio) +@pytest.mark.asyncio +async def test_add_nio_binding_refuses_an_occupied_port(vm): + """ + The compute-side backstop of the duplicate-port guard: a second NIO for + a port that already carries one is refused here instead of overwriting + — the occupying link's teardown needs its own NIO handle, and its host + state with it. + """ + + vm._kernel_taps[(0, 0)] = TAP00 + first = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + await vm.adapter_add_nio_binding(0, 0, first) + assert vm._ethernet_adapters[0].get_nio(0) is first + + second = vm.manager.create_nio({"type": "nio_bridge", "bridge": "gns3other"}) + with pytest.raises(IOUError, match="already has a link"): + await vm.adapter_add_nio_binding(0, 0, second) + + assert vm._ethernet_adapters[0].get_nio(0) is first + + +@pytest.mark.asyncio +async def test_add_nio_binding_attach_failure_leaves_the_port_unbound(vm): + """ + The bookkeeping happens last: an attach that fails mid-way (uBridge + EBUSY on a held anchor) must leave the port without a binding, so the + failed link creates no half-wired state. + """ + + vm._kernel_taps[(0, 0)] = TAP00 + vm._ubridge_send.side_effect = UbridgeError("208-Device or resource busy") + + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + with pytest.raises(UbridgeError): + await vm.adapter_add_nio_binding(0, 0, nio) + + assert vm._ethernet_adapters[0].get_nio(0) is None + + @pytest.mark.asyncio async def test_add_nio_binding_relay_keeps_the_iol_udp_path(vm): """