NgTech-LTD/gns3-server
2
0
Fork 0
mirror of https://github.com/GNS3/gns3-server.git synced 2026-08-27 12:30:13 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2850 from yueguobin/fix/marker-batch-reconcile

Browse Source 
fix: marker reconcile scope, batch NIO parallelism, aiohttp WS upgrade
This commit is contained in:
Jeremy Grossmann 2026-08-11 18:04:45 +02:00 committed by GitHub
commit f69098f441
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 221 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gns3server/api/routes/compute/dynamips_nodes.py
View File

@ -64,7 +64,6 @@ async def create_router(project_id: UUID, node_data: schemas.DynamipsCreate) ->
dynamips_manager = Dynamips.instance()
platform = node_data.platform
print(node_data.chassis, platform in DEFAULT_CHASSIS)
if not node_data.chassis and platform in DEFAULT_CHASSIS:
chassis = DEFAULT_CHASSIS[platform]
else:

41
gns3server/api/routes/compute/projects.py
View File

@ -245,21 +245,36 @@ async def create_batch_nios(
binding in memory; started nodes additionally wire uBridge.
"""
added = 0
# Group entries by node so different nodes' uBridge processes are wired
# in parallel (each node has its own AF_UNIX socket). Within a node
# entries are serial to respect the per-node uBridge command lock.
# This is what makes builtin L2 nodes (ethernet_switch/hub/cloud/nat)
# start their uBridge concurrently during project open instead of one
# at a time (~0.5s each for fork + socket connect).
per_node = {}
for entry in batch.nios:
node = project.get_node(entry.node_id)
nio_settings = jsonable_encoder(entry.nio, exclude_unset=True)
# Dynamips.create_nio(self, node, nio_settings) is async and takes an
# extra positional 'node'. Detect via bound-method parameter count:
# standard == 1, Dynamips == 2. Await the async variant.
per_node.setdefault(entry.node_id, []).append(entry)
async def _create_one_node(node_id, entries):
node = project.get_node(node_id)
# Dynamips.create_nio(self, node, nio_settings) is async and takes
# an extra positional 'node'; other managers' create_nio(nio_settings)
# is synchronous. Detect once per node via bound-method parameter
# count (standard == 1, Dynamips == 2) and await the async variant.
sig = inspect.signature(node.manager.create_nio)
if len(sig.parameters) >= 2:
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)
added += 1
return {"added": added}
dynamips_style = len(sig.parameters) >= 2
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)
await asyncio.gather(
*[_create_one_node(nid, ents) for nid, ents in per_node.items()]
)
return {"added": len(batch.nios)}
@router.put(

100
gns3server/compute/base_node.py
View File

@ -103,6 +103,10 @@ class BaseNode:
# marker filter name -> uBridge bridge_name (recorded at apply time so
# _ubridge_set_marker_filter_state can toggle on/off without an NIO rebuild).
self._marker_filter_bridges = {}
# Parallel store of the installed marker spec (bpf/tag/direction/enabled/...)
# keyed by (name, link_id) so _ubridge_apply_markers can reconcile: detect
# deletions and field changes instead of being add-only.
self._marker_specs = {}
if self._console is not None:
# use a previously allocated console port
@ -994,6 +998,7 @@ class BaseNode:
# gone too — clear the map so the next apply re-installs them all rather
# than skipping them as "already installed".
self._marker_filter_bridges.clear()
self._marker_specs.clear()
async def add_ubridge_udp_connection(self, bridge_name, source_nio, destination_nio):
"""
@ -1176,6 +1181,7 @@ class BaseNode:
if nio is not None and getattr(nio, "markers", None):
nio.markers.pop(name, None)
bridge_name = self._marker_filter_bridges.pop((name, link_id), None)
self._marker_specs.pop((name, link_id), None)
if bridge_name is not None:
await self._ubridge_delete_marker_filter(bridge_name, name)
try:
@ -1223,34 +1229,77 @@ class BaseNode:
async def _ubridge_apply_markers(self, bridge_name, nio):
"""
Install the traffic-insight markers carried by *nio* onto bridge
*bridge_name* that aren't already there. uBridge's ``reset_packet_filters``
preserves mark filters (contract), so on an NIO update we add only the new
ones — re-adding an existing marker would either duplicate it or
close/reopen its pcap. Called from ``add_ubridge_udp_connection`` (fresh
bridge, empty map → installs all) and ``update_ubridge_udp_connection``
(incremental).
Reconcile the traffic-insight markers carried by *nio* onto bridge
*bridge_name* with what is already installed there.
uBridge's ``reset_packet_filters`` preserves mark filters (contract), so
a plain re-add would duplicate them; instead this diffs the desired
``nio.markers`` against the installed ``_marker_specs``:
* installed but no longer desired → delete filter + unlink pcap
* desired with changed bpf/tag/direction/data_link_type → rebuild
(delete + add; the marker's own pcap reopens for the new BPF)
* desired with only ``enabled`` changed → instant on/off toggle
(sibling and own pcap stay open)
* desired and unchanged → skip
* desired and new → add
Called from ``add_ubridge_udp_connection`` (fresh bridge, empty maps →
installs all) and ``update_ubridge_udp_connection`` / the batch NIO
update path (incremental reconcile).
"""
from gns3server.compute.marker.marker_manager import MarkerManager
markers = nio.markers if hasattr(nio, 'markers') else {}
if not markers:
return
manager = MarkerManager.instance()
markers_dir = self.project.markers_working_directory()
for name, spec in markers.items():
link_id = spec.get("link_id", "")
# Incremental: skip markers already on this bridge. uBridge keeps mark
# filters across reset_packet_filters, so re-adding would duplicate (or
# reopen the pcap). A fresh bridge has an empty map → installs all.
if (name, link_id) in self._marker_filter_bridges:
desired = {(name, spec.get("link_id", "")): spec for name, spec in markers.items()}
# 1. Remove installed markers that are no longer desired (marker/def delete).
# Scope to THIS bridge: the map is node-wide and also holds markers
# installed on this node's other links/NIOs. Without this guard,
# reconciling one NIO would delete every other link's markers + pcaps
# (desired only carries the current NIO's markers) — a regression.
for key in list(self._marker_filter_bridges):
if self._marker_filter_bridges[key] != bridge_name:
continue
if key not in desired:
mname, link_id = key
installed_bridge = self._marker_filter_bridges.pop(key)
self._marker_specs.pop(key, None)
await self._ubridge_delete_marker_filter(installed_bridge, mname)
try:
os.remove(os.path.join(markers_dir, f"{self._id}_{link_id}_{mname}.pcap"))
except FileNotFoundError:
pass
except OSError as e:
log.warning("Could not remove marker pcap for '%s' on link %s: %s", mname, link_id, e)
manager.unregister(self._id, mname)
# 2. Add newly-desired markers; rebuild ones whose filter fields changed.
rebuild_fields = ("bpf", "tag", "direction", "data_link_type")
for (name, link_id), spec in desired.items():
bpf = spec.get("bpf", "")
tag = spec.get("tag")
pcap_path = os.path.join(
markers_dir, f"{self._id}_{link_id}_{name}.pcap"
)
enabled = spec.get("enabled", True)
if (name, link_id) in self._marker_filter_bridges:
installed_spec = self._marker_specs.get((name, link_id))
if installed_spec is None:
# Installed but no recorded spec (legacy / pre-reconcile state):
# cannot diff, skip to avoid a duplicate add.
continue
if any(installed_spec.get(f) != spec.get(f) for f in rebuild_fields):
# A filter field changed → rebuild (delete + re-add).
installed_bridge = self._marker_filter_bridges.get((name, link_id))
await self._ubridge_delete_marker_filter(installed_bridge, name)
elif installed_spec.get("enabled", True) != enabled:
# Only the on/off state changed → instant toggle, pcap preserved.
await self._ubridge_set_marker_filter_state(name, enabled)
self._marker_specs[(name, link_id)] = spec
continue
else:
continue # unchanged
pcap_path = os.path.join(markers_dir, f"{self._id}_{link_id}_{name}.pcap")
try:
await self._ubridge_add_marker_filter(bridge_name, name, bpf, pcap_path, tag, link_id,
direction=spec.get("direction"),
@ -1268,22 +1317,19 @@ class BaseNode:
# A disabled marker is installed but turned off (a paused tap), not
# dropped — so the UI can flip it back on instantly with
# enable_packet_filter, no NIO rebuild (ubridge contract §3.2).
if not spec.get("enabled", True):
if not enabled:
try:
await self._ubridge_send(f"bridge enable_packet_filter {bridge_name} {name} off")
except UbridgeError as e:
# Old ubridge without enable_packet_filter: leave it installed
# (on) rather than fail the whole link/marker apply.
log.warning(f"Could not turn marker '{name}' off on {bridge_name}: {e}")
manager.register(
str(self.project.id), self._id, name, link_id, tag
)
manager.register(str(self.project.id), self._id, name, link_id, tag)
# Remember which bridge hosts this filter so an instant on/off toggle
# (no NIO rebuild) can resolve it by name alone.
# keyed (name, link_id) so a node that hosts markers for several links
# (e.g. IOU with one IOL-BRIDGE and many bays/units) records each
# copy independently — toggle below iterates all matching entries.
# (no NIO rebuild) can resolve it by name alone, and keep the spec so
# the next reconcile can detect changes.
self._marker_filter_bridges[name, link_id] = bridge_name
self._marker_specs[name, link_id] = spec
async def _ubridge_set_marker_filter_state(self, name, enabled):
"""

73
gns3server/compute/iou/iou_vm.py
View File

@ -1266,7 +1266,9 @@ class IOUVM(BaseNode):
async def _ubridge_apply_markers(self, adapter_number, port_number, nio):
"""
(Re-)apply traffic-insight markers to the IOL bridge.
Reconcile traffic-insight markers on the IOL bridge (diff desired
``nio.markers`` against installed ``_marker_specs``): delete removed,
rebuild changed, toggle on/off-only changes, add new, skip unchanged.
IOU uses ``iol_bridge`` (not ``bridge``) and the ``add_packet_filter``
command carries extra ``{bay} {unit}`` positional arguments between the
@ -1280,41 +1282,60 @@ class IOUVM(BaseNode):
from gns3server.compute.marker.marker_manager import MarkerManager
markers = nio.markers if hasattr(nio, 'markers') else {}
if not markers:
return
manager = MarkerManager.instance()
markers_dir = self.project.markers_working_directory()
bridge_name = f"IOL-BRIDGE-{self.application_id + 512}"
location = "{bridge_name} {bay} {unit}".format(
bridge_name=bridge_name, bay=adapter_number, unit=port_number
)
for name, spec in markers.items():
link_id = spec.get("link_id", "")
# Incremental: skip markers already installed on this port. A NIO
# update carries EVERY marker on the port (e.g. an inherited
# global-* copy plus a newly added private one); uBridge's
# add_packet_filter rejects a duplicate filter name (packet_filter.c),
# so we must not re-add one already here — mirrors the generic
# _ubridge_apply_markers guard. A fresh bridge has an empty map
# (cleared on _stop_ubridge) so all are installed.
if (name, link_id) in self._marker_filter_bridges:
desired = {(name, spec.get("link_id", "")): spec for name, spec in markers.items()}
# 1. Remove installed markers that are no longer desired.
# Scope to THIS port's IOL location — the map is node-wide and also
# holds markers on this IOU's other ports, which must not be deleted
# when reconciling a single NIO (see base_node for the same guard).
for key in list(self._marker_filter_bridges):
if self._marker_filter_bridges[key] != location:
continue
if key not in desired:
mname, link_id = key
installed_location = self._marker_filter_bridges.pop(key)
self._marker_specs.pop(key, None)
await self._ubridge_delete_marker_filter(installed_location, mname)
try:
os.remove(os.path.join(markers_dir, f"{self._id}_{link_id}_{mname}.pcap"))
except FileNotFoundError:
pass
except OSError as e:
log.warning("Could not remove marker pcap for '%s' on link %s: %s", mname, link_id, e)
manager.unregister(self._id, mname)
# 2. Add / reconcile desired markers.
rebuild_fields = ("bpf", "tag", "direction", "data_link_type")
for (name, link_id), spec in desired.items():
bpf = spec.get("bpf", "")
tag = spec.get("tag")
pcap_path = os.path.join(
markers_dir, f"{self._id}_{link_id}_{name}.pcap"
)
# Build the iol_bridge marker filter command:
# iol_bridge add_packet_filter {br} {bay} {unit} {name} mark "{bpf}" [tag {id}] pcap "{path}"
enabled = spec.get("enabled", True)
if (name, link_id) in self._marker_filter_bridges:
installed_spec = self._marker_specs.get((name, link_id))
if installed_spec is None:
continue # installed (legacy, no spec) — skip to avoid dup
if any(installed_spec.get(f) != spec.get(f) for f in rebuild_fields):
installed_location = self._marker_filter_bridges.get((name, link_id))
await self._ubridge_delete_marker_filter(installed_location, name)
elif installed_spec.get("enabled", True) != enabled:
await self._ubridge_set_marker_filter_state(name, enabled)
self._marker_specs[(name, link_id)] = spec
continue
else:
continue
pcap_path = os.path.join(markers_dir, f"{self._id}_{link_id}_{name}.pcap")
# iol_bridge add_packet_filter {br} {bay} {unit} {name} mark "{bpf}" [tag {id}] pcap "{path}"
cmd = 'iol_bridge add_packet_filter {loc} {name} mark "{bpf}"'.format(
loc=location, name=name, bpf=bpf
)
if tag is not None:
cmd += f" tag {tag}"
# IOU uses one per-node IOL-BRIDGE for every link, so bridge+filter
# are identical across this node's links — `link` is the only way the
# controller can tell their signals apart (contract §3.2).
if link_id:
cmd += f" link {link_id}"
direction = spec.get("direction")
@ -1333,16 +1354,14 @@ class IOUVM(BaseNode):
self.project.emit("log.warning", {"message": message})
continue
raise
if not spec.get("enabled", True):
if not enabled:
try:
await self._ubridge_send(f"iol_bridge enable_packet_filter {location} {name} off")
except UbridgeError as e:
log.warning(f"Could not turn marker '{name}' off on {location}: {e}")
manager.register(
str(self.project.id), self._id, name, link_id, tag
)
# Record name -> location (bridge bay unit) for instant toggle.
manager.register(str(self.project.id), self._id, name, link_id, tag)
self._marker_filter_bridges[name, link_id] = location
self._marker_specs[name, link_id] = spec
async def _ubridge_set_marker_filter_state(self, name, enabled):
"""IOU override: toggle every (name, link_id) entry via ``iol_bridge``."""

8
gns3server/controller/compute.py
View File

@ -106,13 +106,7 @@ class Compute:
def _session(self):
if self._http_session is None or self._http_session.closed is True:
# Reuse TCP keep-alive for local compute (loopback) to avoid paying
# a TCP handshake on every HTTP request; force-close for remote
# computes in case intermediate firewalls/NATs drop idle connections.
_local = self._host in ("127.0.0.1", "::1", "localhost")
connector = aiohttp.TCPConnector(
force_close=not _local, ssl_context=self._ssl_context
)
connector = aiohttp.TCPConnector(force_close=True, ssl_context=self._ssl_context)
self._http_session = aiohttp.ClientSession(connector=connector)
return self._http_session

73
tests/compute/test_base_node.py
View File

@ -367,6 +367,79 @@ async def test_apply_markers_skips_already_installed(compute_project, manager):
assert not any("add_packet_filter" in c for c in cmds) # skipped, not re-added
@pytest.mark.asyncio
async def test_apply_markers_deletes_removed(compute_project, manager):
# Reconcile: a marker no longer in nio.markers is deleted from uBridge
# (filter + registry), not left as an orphan still matching packets.
node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager)
node._ubridge_send = AsyncioMagicMock()
node._ubridge_hypervisor = MagicMock()
node._ubridge_hypervisor.is_running.return_value = True
node._marker_filter_bridges["m", "L1"] = "VPCS-10"
node._marker_specs["m", "L1"] = {"bpf": "icmp", "enabled": True}
nio = NIOUDP(1234, "127.0.0.1", 4321)
nio.markers = {} # marker removed
with patch("gns3server.compute.marker.marker_manager.MarkerManager") as mm:
mm.instance.return_value.unregister = MagicMock()
await node._ubridge_apply_markers("VPCS-10", nio)
cmds = [c.args[0] for c in node._ubridge_send.call_args_list]
assert any("delete_packet_filter VPCS-10 m" in c for c in cmds)
assert ("m", "L1") not in node._marker_filter_bridges
assert ("m", "L1") not in node._marker_specs
@pytest.mark.asyncio
async def test_apply_markers_rebuilds_changed_bpf(compute_project, manager):
# Reconcile: a marker whose bpf changed is rebuilt (delete + re-add), so
# uBridge ends up with the new expression — not the stale original.
node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager)
node._ubridge_send = AsyncioMagicMock()
node._ubridge_hypervisor = MagicMock()
node._ubridge_hypervisor.is_running.return_value = True
node._marker_filter_bridges["m", "L1"] = "VPCS-10"
node._marker_specs["m", "L1"] = {"bpf": "icmp", "tag": None, "direction": None,
"data_link_type": None, "enabled": True}
nio = NIOUDP(1234, "127.0.0.1", 4321)
nio.markers = {"m": {"bpf": "tcp", "tag": None, "link_id": "L1",
"direction": None, "enabled": True}}
with patch("gns3server.compute.marker.marker_manager.MarkerManager") as mm:
mm.instance.return_value.register = MagicMock()
await node._ubridge_apply_markers("VPCS-10", nio)
cmds = [c.args[0] for c in node._ubridge_send.call_args_list]
assert any("delete_packet_filter VPCS-10 m" in c for c in cmds) # old removed
assert any("add_packet_filter VPCS-10 m mark" in c and "tcp" in c for c in cmds) # new added
@pytest.mark.asyncio
async def test_apply_markers_preserves_markers_on_other_bridges(compute_project, manager):
# Regression: reconciling one NIO must not delete markers installed on this
# node's OTHER bridges/NIOs. _marker_filter_bridges is node-wide, but
# `desired` only carries the current NIO's markers — the delete pass must be
# scoped to the current bridge, or updating one link wipes every other link's
# markers + pcaps.
node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager)
node._ubridge_send = AsyncioMagicMock()
node._ubridge_hypervisor = MagicMock()
node._ubridge_hypervisor.is_running.return_value = True
# Two links, each with a marker on its own bridge:
node._marker_filter_bridges["m", "L1"] = "VPCS-10"
node._marker_specs["m", "L1"] = {"bpf": "icmp", "enabled": True}
node._marker_filter_bridges["m", "L2"] = "VPCS-20"
node._marker_specs["m", "L2"] = {"bpf": "tcp", "enabled": True}
# Update only the VPCS-10 NIO; "m" is gone from this link — but the marker
# on VPCS-20 (L2) must survive untouched.
nio = NIOUDP(1234, "127.0.0.1", 4321)
nio.markers = {}
with patch("gns3server.compute.marker.marker_manager.MarkerManager") as mm:
mm.instance.return_value.unregister = MagicMock()
await node._ubridge_apply_markers("VPCS-10", nio)
cmds = [c.args[0] for c in node._ubridge_send.call_args_list]
assert any("delete_packet_filter VPCS-10 m" in c for c in cmds) # L1 removed on its bridge
assert not any("VPCS-20" in c for c in cmds) # other bridge untouched
assert ("m", "L1") not in node._marker_filter_bridges
assert ("m", "L2") in node._marker_filter_bridges # L2 preserved
@pytest.mark.asyncio
async def test_stop_ubridge_clears_marker_bridges(compute_project, manager):
# uBridge stopping drops every marker filter — the map must clear so the next