mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-11-03 05:51:15 +02:00
fix: resolve the code-review findings on the kernel datapath
Compute: - Track the anchor that owns uBridge's single AF_PACKET capture slot: a second kernel port's capture is refused with a clear error (its flag rolled back), a failed start no longer wedges the port, and only the owning port's stop issues the process-wide `capture stop_kernel` (docker/qemu/iou/dynamips/switch, via the mixin's slot helpers). - Unwind failed starts instead of leaking uBridge and the anchor TAPs: QEMU's stop body runs under try/finally, QEMU and IOU start the datapath inside the launch try, Dynamips undoes a half-attached port (the retry used to be short-circuited into a silent black hole), and Docker deletes the veth pair when the namespace move fails. - Restore the relay filter guard on IOU (kernel-only filters raise NodeError, an uncompilable BPF line degrades to a warning), validate the eBPF/quota/window/bpf filter shapes before touching the bridge (short or non-numeric values surfaced as 500s after partial state), and refuse a second NIO on an occupied Docker/QEMU port (the guard IOU and Dynamips already had). - Probe fixes: `_probe_bridge_tap` survives a failing uBridge start, and the probe caches key on ctime too, so a chmod/setcap remedy re-probes. Controller: - Parse GNS3_UNIX_SOCKET_NIO like the compute does (line-anchored, truthy value) and mirror the compute's class selection in kernel_anchor_type. - Read partial capability reports defensively; make BridgeNIO.bridge required-but-nullable so an omitted key is a 422 rather than a KeyError. - Strip the packet_loss correlation slot even when it is zero: the relay filter takes exactly one argument and rejected `packet_loss 10 0`. Ethernet switch: skip ports whose absorbed anchor does not exist yet (the join is deferred by design) and roll the ports_mapping back when the VLAN reconcile fails, so a retry still diffs against the old mapping. Agent: reject device config entries without a device_name instead of silently dropping them (a malformed batch used to execute partially while reporting success). Also split the netem builder to clear C901 and drop the two per-file ruff ignores the PR had added, keeping the remove-only policy.
This commit is contained in:
21 files changed
+654
-274
No files matched your search
@@ -1486,11 +1486,23 @@ class Router(KernelDatapathMixin, BaseNode):
|
||||
if isinstance(nio, NIOBridge):
|
||||
# Kernel link: capture on the port's TAP anchor (AF_PACKET),
|
||||
# not in the Dynamips NIO (this link has no Dynamips NIO).
|
||||
if nio.capturing:
|
||||
raise DynamipsError(f"Packet capture is already activated on port {slot_number}/{port_number}")
|
||||
nio.start_packet_capture(output_file, data_link_type)
|
||||
if self.ubridge:
|
||||
anchor = self._kernel_host_ifc(slot_number, port_number)
|
||||
if anchor is not None:
|
||||
await self._ubridge_send(f'capture start_kernel {anchor} "{output_file}"')
|
||||
# One capture per uBridge process — see
|
||||
# _reserve_kernel_capture; a failed start rolls the
|
||||
# port's flag back so it cannot later stop the winner's
|
||||
# capture.
|
||||
try:
|
||||
self._reserve_kernel_capture(anchor)
|
||||
await self._ubridge_send(f'capture start_kernel {anchor} "{output_file}"')
|
||||
except Exception:
|
||||
self._release_kernel_capture(anchor)
|
||||
nio.stop_packet_capture()
|
||||
raise
|
||||
log.debug(
|
||||
f'Router "{self._name}" [{self._id}]: starting packet capture on port {slot_number}/{port_number}'
|
||||
)
|
||||
@@ -1526,8 +1538,12 @@ class Router(KernelDatapathMixin, BaseNode):
|
||||
|
||||
if isinstance(nio, NIOBridge):
|
||||
nio.stop_packet_capture()
|
||||
if self.ubridge and self._kernel_host_ifc(slot_number, port_number) is not None:
|
||||
anchor = self._kernel_host_ifc(slot_number, port_number)
|
||||
if self.ubridge and anchor is not None and self._kernel_capture_owned_by(anchor):
|
||||
# Process-wide and argument-less: only the port owning the
|
||||
# slot may stop it.
|
||||
await self._ubridge_send("capture stop_kernel")
|
||||
self._release_kernel_capture(anchor)
|
||||
return
|
||||
await nio.stop_packet_capture()
|
||||
|
||||
@@ -1720,8 +1736,21 @@ class Router(KernelDatapathMixin, BaseNode):
|
||||
raise
|
||||
await self.slot_enable_nio(slot_number, port_number)
|
||||
self._tap_nios[(slot_number, port_number)] = tap_nio
|
||||
await self._kernel_attach(anchor, nio)
|
||||
await self._set_adapter_carrier(slot_number, not nio.suspend, port_number)
|
||||
try:
|
||||
await self._kernel_attach(anchor, nio)
|
||||
await self._set_adapter_carrier(slot_number, not nio.suspend, port_number)
|
||||
except Exception:
|
||||
# Undo the half-attach: while the _tap_nios entry exists the port
|
||||
# guard above short-circuits every retry, and a link delete reads
|
||||
# the still-unbound adapter as "nothing to do" — the port would
|
||||
# stay a silent black hole until the router restarts. The undo
|
||||
# tolerates the partial state (its marker/bridge steps are
|
||||
# best-effort) and takes the hypervisor binding and the TAP with
|
||||
# it, so a retry starts from scratch. It is best-effort here as
|
||||
# well: the original failure is what must surface.
|
||||
with contextlib.suppress(Exception):
|
||||
await self._remove_kernel_nio_binding(slot_number, port_number, nio)
|
||||
raise
|
||||
|
||||
async def _remove_kernel_nio_binding(self, slot_number, port_number, nio):
|
||||
"""
|
||||
|
||||
Reference in new issue
Block a user