From 2f70bd6daa4f30c1bd5057c2fd10bb526dddfa10 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Tue, 11 Aug 2026 01:16:13 +0800 Subject: [PATCH] revert: drop _connect_nio thread-pool executor, restore async _ubridge_send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _connect_nio thread-pool optimisation (send_batch_sync) targeted node-start performance, but start_all already runs at concurrency=3 (by design, to avoid overwhelming the host). It also introduced a Python 3.13 incompatibility (trsock.setblocking forbidden) that prevented docker nodes from starting. Since node-start is not the target of this branch (project-open link creation is), revert to the simple per-command async _ubridge_send. The project-open batch NIO dispatch (create_batch_nios) is unaffected — it never called _connect_nio (nodes aren't started during open). --- gns3server/compute/docker/docker_vm.py | 46 +++------- .../compute/ubridge/ubridge_hypervisor.py | 84 ------------------- tests/compute/docker/test_docker_vm.py | 9 +- 3 files changed, 15 insertions(+), 124 deletions(-) diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index afc0fb665..ef4210668 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -38,7 +38,6 @@ from gns3server.utils.hostname import is_rfc1123_hostname_valid from gns3server.utils import macaddress_to_int, int_to_macaddress from gns3server.compute.ubridge.ubridge_error import UbridgeError, UbridgeNamespaceError -from gns3server.compute.ubridge.ubridge_hypervisor import _ubridge_sync_pool from ..base_node import BaseNode from ..adapters.ethernet_adapter import EthernetAdapter @@ -1218,40 +1217,19 @@ class DockerVM(BaseNode): async def _connect_nio(self, adapter_number, nio): bridge_name = f"bridge{adapter_number}" - - # Build the command batch for this NIO. We send everything in one - # executor call so that a single async-lock acquisition covers the - # whole batch, and different nodes' batches can overlap in the - # thread pool via blocking socket I/O. - commands = [ - f"bridge add_nio_udp {bridge_name} {nio.lport} {nio.rhost} {nio.rport}", - ] - if nio.capturing: - commands.append(f'bridge start_capture {bridge_name} "{nio.pcap_output_file}"') - commands.append(f"bridge start {bridge_name}") - commands.append(f"bridge reset_packet_filters {bridge_name}") - for packet_filter in self._build_filter_list(nio.filters): - commands.append(f"bridge add_packet_filter {bridge_name} {packet_filter}") - - # Hold the per-node ubridge lock across the entire executor batch so - # that no async _ubridge_send for this node can interleave with the - # sync socket writes. The lock is created lazily (mirrors the - # @locking decorator on _ubridge_send). - lock_name = "___ubridge_send_lock" - if not hasattr(self, lock_name): - setattr(self, lock_name, asyncio.Lock()) - async with getattr(self, lock_name): - loop = asyncio.get_running_loop() - await loop.run_in_executor( - _ubridge_sync_pool, # dedicated 500-worker thread pool - self._ubridge_hypervisor.send_batch_sync, - commands, + await self._ubridge_send( + "bridge add_nio_udp {bridge_name} {lport} {rhost} {rport}".format( + bridge_name=bridge_name, lport=nio.lport, rhost=nio.rhost, rport=nio.rport ) - - # Traffic-insight markers are rare — keep them on the async path so - # they benefit from the existing marker-management logic. The - # per-node lock is already released at this point, but markers - # serialise themselves via _ubridge_send's own @locking. + ) + if nio.capturing: + await self._ubridge_send( + 'bridge start_capture {bridge_name} "{pcap_file}"'.format( + bridge_name=bridge_name, pcap_file=nio.pcap_output_file + ) + ) + await self._ubridge_send(f"bridge start {bridge_name}") + await self._ubridge_apply_filters(bridge_name, nio.filters) await self._ubridge_apply_markers(bridge_name, nio) async def adapter_add_nio_binding(self, adapter_number, nio): diff --git a/gns3server/compute/ubridge/ubridge_hypervisor.py b/gns3server/compute/ubridge/ubridge_hypervisor.py index 009416d52..c010e1e2d 100644 --- a/gns3server/compute/ubridge/ubridge_hypervisor.py +++ b/gns3server/compute/ubridge/ubridge_hypervisor.py @@ -18,25 +18,12 @@ import re import time import logging import asyncio -import threading -import concurrent.futures -import socket as _socket from gns3server.utils.asyncio import locking from .ubridge_error import UbridgeError log = logging.getLogger(__name__) -# Dedicated thread pool for blocking ubridge socket I/O. Every node gets -# its own ubridge process + socket, so N nodes can send commands in true -# OS-thread parallelism. The default asyncio executor caps at ~32 threads; -# sizing for the large-topology case (2500+ links → thousands of NIO add -# calls across hundreds of nodes). -_ubridge_sync_pool = concurrent.futures.ThreadPoolExecutor( - max_workers=500, - thread_name_prefix="ubridge-sync", -) - class UBridgeHypervisor: @@ -70,8 +57,6 @@ class UBridgeHypervisor: self._timeout = timeout self._reader = None self._writer = None - self._recv_buf = b"" # leftover bytes from last sync recv - self._send_lock = threading.Lock() async def connect(self, timeout=10): """ @@ -281,72 +266,3 @@ class UBridgeHypervisor: log.debug(f"returned result {data}") return data - - def send_batch_sync(self, commands): - """ - Send multiple commands to uBridge using blocking socket I/O. Designed - to run inside ``loop.run_in_executor`` so that a single per-node batch - doesn't bounce through the event loop between every command, and - batches for *different* nodes run in parallel across the thread pool. - - :param commands: iterable of command strings - :raises UbridgeError: if any command fails - """ - if self._writer is None: - raise UbridgeError("Not connected") - transport = self._writer.transport - if transport is None or transport.is_closing(): - raise UbridgeError("Transport closed") - tr_sock = transport.get_extra_info("socket") - if tr_sock is None: - raise UbridgeError("No underlying socket for sync send_batch") - - # Python 3.13's transport socket wrapper (trsock) rejects - # setblocking(), so dup the underlying fd into a plain socket - # that the executor thread can drive in blocking mode. - sock = _socket.fromfd(tr_sock.fileno(), tr_sock.family, _socket.SOCK_STREAM) - - # Serialise access to this hypervisor's socket — only one batch (sync - # or async) talks to uBridge at a time. The node-level async lock - # (:func:`_ubridge_send`) is held for the entire executor call, so no - # async ``send()`` can interleave. - with self._send_lock: - sock.setblocking(True) - try: - for command in commands: - cmd = (command.strip() + "\n").encode() - sock.sendall(cmd) - - # Read until the terminating line (100-… or 2xx-…) - buf = self._recv_buf - while True: - try: - chunk = sock.recv(4096) - except BlockingIOError: - continue - if not chunk: - raise UbridgeError( - f"uBridge closed connection during '{command}'" - ) - buf += chunk - decoded = buf.decode("utf-8", errors="replace") - # Last complete line determines termination - tail = decoded.rsplit("\r\n", 1)[-1] - if tail and tail[0] in "12" and tail[1:3].isdigit() and len(tail) >= 4 and tail[3] == "-": - break - - # Check for error codes (2xx-…) - last_line = decoded.strip().split("\r\n")[-1] - if self.error_re.match(last_line): - raise UbridgeError(last_line[4:]) - - # Keep any leftover bytes (after the trailing \r\n) for the - # next read in the batch - trailer_start = decoded.rfind("\r\n") - if trailer_start >= 0: - self._recv_buf = buf[trailer_start + 2:] - else: - self._recv_buf = b"" - finally: - sock.setblocking(False) - sock.detach() # release the dup'd fd, don't close transport diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index ae40d91d9..c9e701306 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -1440,12 +1440,9 @@ async def test_add_ubridge_connection(vm): call.send('bridge create bridge0'), call.send("bridge add_nio_tap bridge0 tap-gns3-e0"), call.send('docker move_to_ns tap-gns3-e0 42 eth0'), - call.send_batch_sync([ - 'bridge add_nio_udp bridge0 4242 127.0.0.1 4343', - 'bridge start_capture bridge0 "/tmp/capture.pcap"', - 'bridge start bridge0', - 'bridge reset_packet_filters bridge0', - ]), + call.send('bridge add_nio_udp bridge0 4242 127.0.0.1 4343'), + call.send('bridge start_capture bridge0 "/tmp/capture.pcap"'), + call.send('bridge start bridge0'), ] assert 'bridge0' in vm._bridges # We need to check any_order otherwise mock is confused by asyncio