mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-10-22 10:52:21 +03:00
fix(compute): refuse a duplicate port NIO and wire before bookkeeping
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.
This commit is contained in:
parent
6839b279be
commit
6e36b58a5d
@ -1312,12 +1312,26 @@ class Router(KernelDatapathMixin, BaseNode):
|
|||||||
if not adapter.port_exists(port_number):
|
if not adapter.port_exists(port_number):
|
||||||
raise DynamipsError(f"Port {port_number} does not exist on adapter {adapter}")
|
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):
|
if isinstance(nio, NIOBridge):
|
||||||
# Kernel datapath: the port's anchor TAP is opened in the
|
# Kernel datapath: the port's anchor TAP is opened in the
|
||||||
# hypervisor and enslaved into the NIO's per-link bridge (the
|
# hypervisor and enslaved into the NIO's per-link bridge (the
|
||||||
# wiring is deferred to the node's start when no anchors exist).
|
# 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)
|
await self._attach_kernel_nio(slot_number, port_number, nio)
|
||||||
|
adapter.add_nio(port_number, nio)
|
||||||
log.debug(
|
log.debug(
|
||||||
'Router "{name}" [{id}]: {nio} bound to port {slot_number}/{port_number}'.format(
|
'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
|
name=self._name, id=self._id, nio=nio, slot_number=slot_number, port_number=port_number
|
||||||
|
|||||||
@ -1345,27 +1345,39 @@ class IOUVM(KernelDatapathMixin, BaseNode):
|
|||||||
if not adapter.port_exists(port_number):
|
if not adapter.port_exists(port_number):
|
||||||
raise IOUError(f"Port {port_number} does not exist on adapter {adapter}")
|
raise IOUError(f"Port {port_number} does not exist on adapter {adapter}")
|
||||||
|
|
||||||
adapter.add_nio(port_number, nio)
|
# The compute-side backstop of the controller's duplicate-port guard:
|
||||||
log.debug(f'IOU "{self._name}" [{self._id}]: {nio} added to {adapter_number}/{port_number}')
|
# 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 self.ubridge:
|
||||||
if isinstance(nio, NIOBridge):
|
if isinstance(nio, NIOBridge):
|
||||||
await self._attach_kernel_nio(adapter_number, port_number, nio)
|
await self._attach_kernel_nio(adapter_number, port_number, nio)
|
||||||
return
|
else:
|
||||||
await self._ubridge_send(
|
await self._ubridge_send(
|
||||||
"iol_bridge add_nio_udp {name} {iol_id} {bay} {unit} {lport} {rhost} {rport}".format(
|
"iol_bridge add_nio_udp {name} {iol_id} {bay} {unit} {lport} {rhost} {rport}".format(
|
||||||
name=self._iol_bridge_name(),
|
name=self._iol_bridge_name(),
|
||||||
iol_id=self.application_id,
|
iol_id=self.application_id,
|
||||||
bay=adapter_number,
|
bay=adapter_number,
|
||||||
unit=port_number,
|
unit=port_number,
|
||||||
lport=nio.lport,
|
lport=nio.lport,
|
||||||
rhost=nio.rhost,
|
rhost=nio.rhost,
|
||||||
rport=nio.rport,
|
rport=nio.rport,
|
||||||
|
)
|
||||||
)
|
)
|
||||||
)
|
location = self._iol_location(adapter_number, port_number)
|
||||||
location = self._iol_location(adapter_number, port_number)
|
await self._ubridge_apply_filters(location, nio.filters)
|
||||||
await self._ubridge_apply_filters(location, nio.filters)
|
await self._ubridge_apply_markers(location, nio)
|
||||||
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):
|
async def adapter_update_nio_binding(self, adapter_number, port_number, nio):
|
||||||
"""
|
"""
|
||||||
|
|||||||
@ -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')
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_attach_kernel_nio_without_anchor_on_a_running_node_raises(router):
|
async def test_attach_kernel_nio_without_anchor_on_a_running_node_raises(router):
|
||||||
"""
|
"""
|
||||||
|
|||||||
@ -33,6 +33,7 @@ from gns3server.compute.iou import IOU
|
|||||||
from gns3server.compute.iou.iou_vm import IOUVM
|
from gns3server.compute.iou.iou_vm import IOUVM
|
||||||
from gns3server.compute.iou.iou_error import IOUError
|
from gns3server.compute.iou.iou_error import IOUError
|
||||||
from gns3server.compute.nios.nio_bridge import NIOBridge
|
from gns3server.compute.nios.nio_bridge import NIOBridge
|
||||||
|
from gns3server.compute.ubridge.ubridge_error import UbridgeError
|
||||||
|
|
||||||
|
|
||||||
NODE_ID = "00010203-0405-0607-0809-0a0b0c0d0e0f"
|
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)
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_add_nio_binding_relay_keeps_the_iol_udp_path(vm):
|
async def test_add_nio_binding_relay_keeps_the_iol_udp_path(vm):
|
||||||
"""
|
"""
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user