mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-08-28 04:50:16 +03:00
revert: drop _connect_nio thread-pool executor, restore async _ubridge_send
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).
This commit is contained in:
YueGuobin
parent
46bad0b639
commit
2f70bd6daa
@ -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):
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user