From 61020d3bbc01043ea1e24c53feb3eb6f8df8e9d6 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Mon, 28 Sep 2026 10:04:00 +0800 Subject: [PATCH] feat(docker): traffic-insight markers on kernel-datapath links via AF_PACKET Markers no longer disqualify the kernel datapath. They ride the NIO like on the relay path and attach to the veth host end through uBridge's marker add_kernel/delete_kernel/enable_kernel commands (AF_PACKET taps, same BPF/direction/signal semantics as the relay mark filter). - base_node: marker anchors are now polymorphic (relay bridge name or kernel veth host interface); the reconcile loop is datapath-agnostic and DockerVM overrides the add/delete/enable primitives to translate the commands. Marker-name validation extracted for both add paths. - docker_vm: kernel NIO connect/update apply markers on the veth anchor; link deletion tears markers down explicitly (the veth survives the link, unlike a relay bridge whose death drops its filters). - controller: eligibility and start_marker no longer exclude markers or project-level marker definitions; the kernel NIO carries the capture node's marker specs. - NIO creation accepts markers on kernel NIOs (filters still rejected). --- gns3server/compute/base_manager.py | 7 +- gns3server/compute/base_node.py | 78 ++++++++--- gns3server/compute/docker/docker_vm.py | 117 +++++++++++++++- gns3server/compute/nios/nio_bridge.py | 8 +- gns3server/controller/udp_link.py | 26 ++-- gns3server/schemas/compute/nios.py | 9 +- .../docker/test_docker_kernel_datapath.py | 131 +++++++++++++++++- tests/controller/test_kernel_datapath_link.py | 71 ++++++++-- 8 files changed, 378 insertions(+), 69 deletions(-) diff --git a/gns3server/compute/base_manager.py b/gns3server/compute/base_manager.py index e7cd790fb..7c4887727 100644 --- a/gns3server/compute/base_manager.py +++ b/gns3server/compute/base_manager.py @@ -361,8 +361,11 @@ class BaseManager: nio.suspend = nio_settings.get("suspend", False) elif nio_settings["type"] == "nio_bridge": nio = NIOBridge(nio_settings["bridge"]) - if nio_settings.get("filters") or nio_settings.get("markers"): - raise ComputeError("Packet filters and markers are not supported on kernel-datapath links") + if nio_settings.get("filters"): + raise ComputeError("Packet filters are not supported on kernel-datapath links") + # Markers ride the NIO and attach to the veth host interface via + # uBridge's AF_PACKET marker module (marker add_kernel). + nio.markers = nio_settings.get("markers", {}) nio.suspend = nio_settings.get("suspend", False) elif nio_settings["type"] == "nio_tap": tap_device = nio_settings["tap_device"] diff --git a/gns3server/compute/base_node.py b/gns3server/compute/base_node.py index 033ef96d9..74f6a028c 100644 --- a/gns3server/compute/base_node.py +++ b/gns3server/compute/base_node.py @@ -1172,6 +1172,24 @@ class BaseNode: dlt = dlt[4:] return None if dlt == "EN10MB" else dlt + @staticmethod + def _validate_marker_name(name): + """ + Defense-in-depth marker-name check shared by every add path (relay + ``mark`` filter and kernel ``marker add_kernel``). The name travels + from the controller REST layer (MarkerCreate schema) but is validated + here too against hand-edited topology files. Note: "global-*" names are + legitimate — they come from project-level marker definitions + (inherit_marker). The prefix is only forbidden at the user-facing + schema layer, not at the uBridge boundary. The user-facing name is + capped at 32 by the schema; inherited copies carry a ``global-`` + prefix (≤ 39), so allow up to 48 here. + """ + + _MARKER_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*$") + if not _MARKER_NAME_RE.match(name) or len(name) > 48: + raise UbridgeError(f"Invalid marker name: {name!r}") + async def _ubridge_add_marker_filter( self, bridge_name, name, bpf, pcap_path, tag=None, link_id=None, direction=None, data_link_type=None ): @@ -1184,7 +1202,11 @@ class BaseNode: and it is added/removed on its own (not via reset_packet_filters) so the pcap is not closed/reopened on unrelated filter changes. - :param bridge_name: uBridge bridge carrying the link's traffic + ``bridge_name`` is the anchor the marker attaches to: the uBridge relay + bridge here, or the kernel veth host interface on the kernel datapath + (DockerVM overrides this to emit ``marker add_kernel``). + + :param bridge_name: anchor carrying the link's traffic :param name: stable, gns3server-chosen filter name (pcap identity + echoed in signals) :param bpf: libpcap BPF expression :param pcap_path: absolute path ubridge appends matched packets to @@ -1192,18 +1214,8 @@ class BaseNode: """ # mark [tag ] [pcap ] — tag/pcap keyword pairs, any order. - # name travels from the controller REST layer (MarkerCreate schema) but is - # validated here too as defense-in-depth against hand-edited topology files. - # Note: "global-*" names are legitimate here — they come from project-level - # marker definitions (inherit_marker). The prefix is only forbidden at the - # user-facing schema layer, not at the uBridge boundary. - _MARKER_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*$") - # Defense-in-depth vs hand-edited topology: the user-facing name is capped - # at 32 by the schema; inherited copies carry a ``global-`` prefix (≤ 39), - # so allow up to 48 here. - if not _MARKER_NAME_RE.match(name) or len(name) > 48: - raise UbridgeError(f"Invalid marker name: {name!r}") - cmd = f'bridge add_packet_filter {bridge_name} {name} mark "{bpf}"' + self._validate_marker_name(name) + cmd = 'bridge add_packet_filter {bridge} {name} mark "{bpf}"'.format(bridge=bridge_name, name=name, bpf=bpf) if tag is not None: cmd += f" tag {tag}" # Per-link attribution (contract §3.2): when one ubridge bridge serves @@ -1290,6 +1302,12 @@ class BaseNode: Reconcile the traffic-insight markers carried by *nio* onto bridge *bridge_name* with what is already installed there. + ``bridge_name`` is the marker anchor: the uBridge relay bridge on the + relay datapath, or the kernel veth host interface on the kernel + datapath (DockerVM passes the veth name; its add/delete/enable + primitive overrides translate the commands to ``marker *_kernel``). + The reconcile itself is datapath-agnostic. + 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``: @@ -1303,8 +1321,9 @@ class BaseNode: * 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). + installs all), ``update_ubridge_udp_connection`` / the batch NIO + update path (incremental reconcile), and DockerVM's kernel-datapath + connection paths (veth anchor). """ from gns3server.compute.marker.marker_manager import MarkerManager @@ -1381,12 +1400,13 @@ class BaseNode: raise # 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). + # enable_packet_filter / marker enable_kernel, no NIO rebuild + # (ubridge contract §3.2). if not enabled: try: - await self._ubridge_send(f"bridge enable_packet_filter {bridge_name} {name} off") + await self._ubridge_enable_marker_filter(bridge_name, name, "off") except UbridgeError as e: - # Old ubridge without enable_packet_filter: leave it installed + # Old ubridge without the enable command: 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) @@ -1396,12 +1416,26 @@ class BaseNode: self._marker_filter_bridges[name, link_id] = bridge_name self._marker_specs[name, link_id] = spec + async def _ubridge_enable_marker_filter(self, anchor, name, state): + """ + Flip one installed marker's on/off state with a single uBridge command + (``bridge enable_packet_filter … on|off``). ``anchor`` is the uBridge + relay bridge, or the kernel veth host interface on the kernel datapath + (DockerVM overrides this to emit ``marker enable_kernel``). + + :param anchor: relay bridge name or kernel veth host interface + :param name: marker filter name + :param state: "on" or "off" + """ + + await self._ubridge_send(f"bridge enable_packet_filter {anchor} {name} {state}") + async def _ubridge_set_marker_filter_state(self, name, enabled): """ Toggle an installed marker filter on/off with a single uBridge command - (``bridge enable_packet_filter … on|off``) — no NIO reset/reapply, so the - pcap identity and emitted counter are preserved (ubridge contract §3.2). - The bridge is resolved from the (name, link_id)→bridge map populated at + — no NIO reset/reapply, so the pcap identity and emitted counter are + preserved (ubridge contract §3.2). + The anchor is resolved from the (name, link_id)→bridge map populated at apply time; entries are iterated so a node that hosts the same marker name on several links (e.g. IOU with one IOL-BRIDGE per node) toggles every copy. IOU overrides this for its ``iol_bridge`` command shape. @@ -1413,7 +1447,7 @@ class BaseNode: state = "on" if enabled else "off" for (n, lid), bridge_name in list(self._marker_filter_bridges.items()): if n == name: - await self._ubridge_send(f"bridge enable_packet_filter {bridge_name} {name} {state}") + await self._ubridge_enable_marker_filter(bridge_name, name, state) async def _ubridge_marker_pause(self): """ diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index a3f98f96f..f70e37edd 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -1823,9 +1823,9 @@ class DockerVM(BaseNode): whose host end will be enslaved to the per-link kernel bridge, and whose guest end becomes the container interface (the role the TAP plays on the relay path). No uBridge relay bridge is created — frames - flow entirely in the kernel. Filters and markers are unavailable on - this datapath (they live in the uBridge relay); packet capture is - provided by uBridge's AF_PACKET module on the veth host end. + flow entirely in the kernel. Filters are unavailable on this datapath + (they live in the uBridge relay); packet capture and markers are + provided by uBridge's AF_PACKET modules on the veth host end. """ try: @@ -1947,6 +1947,9 @@ class DockerVM(BaseNode): # Restore a capture that was active before a node restart # (mirrors the relay path's start_capture in _connect_nio). await self._ubridge_send(f'capture start_kernel {host_ifc} "{nio.pcap_output_file}"') + # Markers carried by the NIO attach to the veth host end (AF_PACKET + # taps) — the anchor is the interface, not a relay bridge. + await self._ubridge_apply_markers(host_ifc, nio) return bridge_name = self._bridge_name(adapter_number, port_number) @@ -2009,12 +2012,17 @@ class DockerVM(BaseNode): if self.ubridge: if isinstance(nio, NIOBridge): - if nio.filters or nio.markers: + if nio.filters: raise DockerError( - "Packet filters and markers are not supported on kernel-datapath links; " + "Packet filters are not supported on kernel-datapath links; " "delete and recreate the link to use them" ) if self.status == "started": + host_ifc = self._kernel_veths.get((adapter_number, port_number)) + if host_ifc is not None: + # Incremental marker reconcile on the veth anchor — + # the relay-datapath equivalent of the branch below. + await self._ubridge_apply_markers(host_ifc, nio) await self._set_adapter_carrier(adapter_number, not nio.suspend, port_number) return @@ -2060,6 +2068,100 @@ class DockerVM(BaseNode): log.debug(f"Docker VM '{self.name}' [{self.id}]: {adapter.host_ifc} removed from adapter {adapter_number}") + def _kernel_marker_anchor(self, anchor): + """ + Whether *anchor* names one of this container's kernel-datapath veth + host ends. Marker anchors are polymorphic: a relay bridge name on the + relay datapath, the veth host interface on the kernel datapath. A veth + name can never collide with a uBridge bridge name (relay bridges are + ``bridge{adapter}``, veths are ``gv…e…p…``). + """ + + return anchor in self._kernel_veths.values() + + async def _ubridge_add_marker_filter( + self, bridge_name, name, bpf, pcap_path, tag=None, link_id=None, direction=None, data_link_type=None + ): + """ + Kernel-datapath variant of the `mark` filter attach: ``marker + add_kernel`` binds an AF_PACKET socket to the veth host end. Keyword + pairs and semantics (match → MARK signal + pcap append) are identical + to the relay ``bridge add_packet_filter … mark`` command. + """ + + if self._kernel_marker_anchor(bridge_name): + self._validate_marker_name(name) + cmd = 'marker add_kernel {name} {ifc} "{bpf}"'.format(name=name, ifc=bridge_name, bpf=bpf) + if tag is not None: + cmd += f" tag {tag}" + if link_id: + cmd += f" link {link_id}" + if direction is not None: + cmd += f" dir {direction}" + linktype = self._marker_linktype(data_link_type) + if linktype is not None: + cmd += f" linktype {linktype}" + cmd += ' pcap "{path}"'.format(path=pcap_path) + await self._ubridge_send(cmd) + return + await super()._ubridge_add_marker_filter( + bridge_name, name, bpf, pcap_path, tag=tag, link_id=link_id, direction=direction, data_link_type=data_link_type + ) + + async def _ubridge_delete_marker_filter(self, bridge_name, name): + """ + Kernel-datapath variant of the marker removal: ``marker delete_kernel`` + (idempotent in uBridge — a no-op when the marker is already gone). + """ + + if self._kernel_marker_anchor(bridge_name): + if not (self._ubridge_hypervisor and self._ubridge_hypervisor.is_running()): + return + try: + await self._ubridge_send(f"marker delete_kernel {bridge_name} {name}") + except UbridgeError as e: + log.warning("Could not remove kernel marker '%s' from %s: %s", name, bridge_name, e) + return + await super()._ubridge_delete_marker_filter(bridge_name, name) + + async def _ubridge_enable_marker_filter(self, anchor, name, state): + """ + Kernel-datapath variant of the marker on/off toggle: ``marker + enable_kernel`` — installed but silent when off, pcap preserved. + """ + + if self._kernel_marker_anchor(anchor): + await self._ubridge_send(f"marker enable_kernel {anchor} {name} {state}") + return + await super()._ubridge_enable_marker_filter(anchor, name, state) + + async def _remove_kernel_markers(self, host_ifc): + """ + Tear down every kernel marker anchored to *host_ifc*: the veth survives + link deletion (it is the adapter interface, not the link), so the + AF_PACKET sockets must be closed explicitly — otherwise a deleted + link's markers would keep sniffing and signaling. Mirrors the relay + datapath where deleting the uBridge bridge silently drops its filters. + """ + + from gns3server.compute.marker.marker_manager import MarkerManager + + manager = MarkerManager.instance() + markers_dir = self.project.markers_working_directory() + for (name, link_id), anchor in list(self._marker_filter_bridges.items()): + if anchor != host_ifc: + continue + self._marker_filter_bridges.pop((name, link_id)) + self._marker_specs.pop((name, link_id), None) + await self._ubridge_delete_marker_filter(host_ifc, name) + try: + os.remove(os.path.join(markers_dir, f"{self._id}_{link_id}_{name}.pcap")) + except FileNotFoundError: + pass + except OSError as e: + log.warning("Could not remove marker pcap for '%s' on link %s: %s", name, link_id, e) + manager.unregister(self._id, name) + async def _remove_kernel_nio(self, nio, adapter_number, port_number=0): """ Detach a kernel-datapath adapter port from its per-link kernel bridge. @@ -2070,9 +2172,10 @@ class DockerVM(BaseNode): """ host_ifc = self._kernel_veths.get((adapter_number, port_number)) - if self.status == "started" and host_ifc is not None: - await self._set_adapter_carrier(adapter_number, False, port_number) if host_ifc is not None: + await self._remove_kernel_markers(host_ifc) + if self.status == "started": + await self._set_adapter_carrier(adapter_number, False, port_number) with contextlib.suppress(UbridgeError): await self._ubridge_send(f'brctl delif "{nio.bridge}" "{host_ifc}"') with contextlib.suppress(UbridgeError): diff --git a/gns3server/compute/nios/nio_bridge.py b/gns3server/compute/nios/nio_bridge.py index 6692752d1..785ac61e3 100644 --- a/gns3server/compute/nios/nio_bridge.py +++ b/gns3server/compute/nios/nio_bridge.py @@ -20,10 +20,10 @@ Interface for kernel-datapath bridge NIOs. A NIO of this type instructs the node to enslave its adapter's veth host end into the named Linux kernel bridge instead of wiring a uBridge UDP relay. -Frames then flow entirely in the kernel (veth -> bridge -> veth). Filters, -markers and packet capture are not available on this datapath (they live in -the uBridge relay); the controller must only emit this NIO type for links -where those features are not in use. +Frames then flow entirely in the kernel (veth -> bridge -> veth). Packet +filters are not available on this datapath (they live in the uBridge relay); +markers and packet capture are served by uBridge's AF_PACKET modules on the +veth host end (marker add_kernel / capture start_kernel). """ from .nio import NIO diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 27b2923a1..ea759a5a8 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -135,9 +135,9 @@ class UDPLink(Link): """ Whether this link can be wired on the kernel datapath (veth pairs enslaved into a per-link Linux bridge) instead of the uBridge UDP - relay. The kernel path has no userspace relay, so filters and - markers — which live in the relay — disqualify it (capture is - served separately by uBridge's AF_PACKET module). + relay. The kernel path has no userspace relay, so filters — which + live in the relay — disqualify it. Markers and capture are served + by uBridge's AF_PACKET modules on the veth host end. It is host-local (same compute) and requires the adapter interfaces to be created as veths at container start, hence stopped nodes only. """ @@ -150,12 +150,6 @@ class UDPLink(Link): return False if self.get_active_filters(): return False - if self._markers: - return False - # Project-level marker definitions are inherited by every link right - # after creation (apply_defs_to_new_link) and need the relay datapath. - if getattr(self._project, "_marker_definitions", None): - return False # Running nodes already have relay TAPs wired into uBridge; the # kernel path needs the veth variant created at container start. if node1.status != "stopped" or node2.status != "stopped": @@ -195,10 +189,14 @@ class UDPLink(Link): # both ends compute it independently (no coordinator) and crash # leftovers sweep deterministically; 11 hex chars fill IFNAMSIZ # (15), pushing name-collision probability to irrelevance. + # Markers ride the NIO like on the relay datapath, routed by + # capture node; they attach to the veth host end via uBridge's + # AF_PACKET marker module instead of a relay `mark` filter. bridge_name = "gns3" + self._id.replace("-", "")[:11] + node1_markers, node2_markers = self._get_node_markers(node1, node2) self._link_data = [ - {"type": "nio_bridge", "bridge": bridge_name, "filters": {}, "markers": {}, "suspend": self._suspended}, - {"type": "nio_bridge", "bridge": bridge_name, "filters": {}, "markers": {}, "suspend": self._suspended}, + {"type": "nio_bridge", "bridge": bridge_name, "filters": {}, "markers": node1_markers, "suspend": self._suspended}, + {"type": "nio_bridge", "bridge": bridge_name, "filters": {}, "markers": node2_markers, "suspend": self._suspended}, ] return [ (node1, adapter_number1, port_number1, self._link_data[0]), @@ -542,12 +540,6 @@ class UDPLink(Link): if name in self._markers: raise ControllerError(f"Marker '{name}' already exists on link {self._id}") - if self.kernel_datapath: - raise ControllerError( - "Markers are not supported on kernel-datapath links (no uBridge relay in the " - "forwarding path); delete and recreate the link to use markers" - ) - # Validate the BPF only for private per-link markers. An inherited copy # (``inherited_from`` set) fans out from a definition whose BPF was # already validated once at create/update (and on project load), so diff --git a/gns3server/schemas/compute/nios.py b/gns3server/schemas/compute/nios.py index 4ae5a6475..6ee90c2a3 100644 --- a/gns3server/schemas/compute/nios.py +++ b/gns3server/schemas/compute/nios.py @@ -74,17 +74,16 @@ class BridgeNIO(BaseModel): """ Kernel-datapath bridge Network Input/Output properties. The NIO tells the node to enslave its veth host end into the named kernel bridge instead of - wiring a uBridge UDP relay. ``filters`` and ``markers`` must stay empty on - this datapath (rejected server-side); packet capture is supported via - uBridge's AF_PACKET module. Both are accepted as fields only because - link update payloads always carry the full NIO dictionary. + wiring a uBridge UDP relay. ``filters`` must stay empty on this datapath + (rejected server-side); markers and packet capture are served by uBridge's + AF_PACKET modules on the veth host end. """ type: BridgeNIOType bridge: str = Field(..., description="Kernel bridge name e.g. gns3a1b2c3d4e5") suspend: Optional[bool] = Field(None, description="Suspend the NIO") filters: Optional[dict] = Field(None, description="Packet filters (unsupported, must be empty)") - markers: Optional[dict] = Field(None, description="Traffic-insight markers (unsupported, must be empty)") + markers: Optional[dict] = Field(None, description="Traffic-insight markers (attached to the veth host end)") class MarkerToggle(BaseModel): diff --git a/tests/compute/docker/test_docker_kernel_datapath.py b/tests/compute/docker/test_docker_kernel_datapath.py index e9437861c..e200590da 100644 --- a/tests/compute/docker/test_docker_kernel_datapath.py +++ b/tests/compute/docker/test_docker_kernel_datapath.py @@ -21,6 +21,7 @@ creation, per-link kernel bridge enslavement, carrier semantics, relay fallback and cleanup. """ +import os import uuid from unittest.mock import MagicMock, call @@ -78,10 +79,15 @@ def test_create_nio_bridge_rejects_filters(vm): vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "filters": {"loss": [10]}}) -def test_create_nio_bridge_rejects_markers(vm): +def test_create_nio_bridge_accepts_markers(vm): + """ + Markers ride the kernel NIO (attached to the veth host end via uBridge's + AF_PACKET marker module); only filters are rejected. + """ - with pytest.raises(ComputeError): - vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "markers": {"m": {"bpf": "icmp"}}}) + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "markers": {"m": {"bpf": "icmp"}}}) + assert isinstance(nio, NIOBridge) + assert nio.markers == {"m": {"bpf": "icmp"}} # --------------------------------------------------------------------------- @@ -409,6 +415,125 @@ async def test_connect_nio_kernel_restarts_capture(vm): vm._ubridge_send.assert_any_call(f'capture start_kernel {host_ifc} "/tmp/capture.pcap"') +# --------------------------------------------------------------------------- +# Markers (AF_PACKET taps on the veth host end) +# --------------------------------------------------------------------------- + +def _marker_spec(bpf="icmp", tag=1, enabled=True, direction=None, link_id="l1"): + return {"bpf": bpf, "tag": tag, "enabled": enabled, "direction": direction, + "data_link_type": "DLT_EN10MB", "link_id": link_id} + + +@pytest.mark.asyncio +async def test_connect_nio_kernel_applies_markers(vm): + + vm._ubridge_send = AsyncioMagicMock() + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE, "markers": {"icmp-m": _marker_spec()}}) + await vm._connect_nio(0, nio) + + pcap = os.path.join(vm.project.markers_working_directory(), f"{vm.id}_l1_icmp-m.pcap") + vm._ubridge_send.assert_any_call(f'marker add_kernel icmp-m {host_ifc} "icmp" tag 1 link l1 pcap "{pcap}"') + assert vm._marker_filter_bridges[("icmp-m", "l1")] == host_ifc + + +@pytest.mark.asyncio +async def test_connect_nio_kernel_marker_disabled_at_apply(vm): + """ + A disabled marker is installed but silenced (marker enable_kernel off) so + the UI can flip it back on instantly — same contract as the relay path. + """ + + vm._ubridge_send = AsyncioMagicMock() + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + + nio = vm.manager.create_nio( + {"type": "nio_bridge", "bridge": BRIDGE, "markers": {"icmp-m": _marker_spec(enabled=False)}} + ) + await vm._connect_nio(0, nio) + + vm._ubridge_send.assert_any_call(f"marker enable_kernel {host_ifc} icmp-m off") + + +@pytest.mark.asyncio +async def test_update_nio_kernel_applies_markers(vm): + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_send = AsyncioMagicMock() + vm.status = "started" + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + vm._ethernet_adapters[0].add_nio(0, nio) + + nio.markers = {"tcp-m": _marker_spec(bpf="tcp", tag=2, link_id="l2")} + await vm.adapter_update_nio_binding(0, nio) + + pcap = os.path.join(vm.project.markers_working_directory(), f"{vm.id}_l2_tcp-m.pcap") + vm._ubridge_send.assert_any_call(f'marker add_kernel tcp-m {host_ifc} "tcp" tag 2 link l2 pcap "{pcap}"') + + +@pytest.mark.asyncio +async def test_marker_toggle_kernel_uses_enable_kernel(vm): + + vm._ubridge_send = AsyncioMagicMock() + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + vm._marker_filter_bridges[("icmp-m", "l1")] = host_ifc + + await vm._ubridge_set_marker_filter_state("icmp-m", False) + vm._ubridge_send.assert_any_call(f"marker enable_kernel {host_ifc} icmp-m off") + await vm._ubridge_set_marker_filter_state("icmp-m", True) + vm._ubridge_send.assert_any_call(f"marker enable_kernel {host_ifc} icmp-m on") + + +@pytest.mark.asyncio +async def test_remove_kernel_nio_deletes_markers(vm): + """ + The veth survives link deletion (it is the adapter interface), so kernel + markers must be torn down explicitly — otherwise a deleted link's markers + would keep sniffing and signaling. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_send = AsyncioMagicMock() + vm.status = "started" + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + vm._marker_filter_bridges[("icmp-m", "l1")] = host_ifc + vm._marker_specs[("icmp-m", "l1")] = _marker_spec() + vm._ethernet_adapters[0].add_nio(0, vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE})) + + await vm.adapter_remove_nio_binding(0) + + vm._ubridge_send.assert_any_call(f"marker delete_kernel {host_ifc} icmp-m") + assert ("icmp-m", "l1") not in vm._marker_filter_bridges + assert ("icmp-m", "l1") not in vm._marker_specs + + +@pytest.mark.asyncio +async def test_rebuild_marker_filter_kernel(vm): + """ + The fine-grained rebuild path (bpf/tag/direction change) goes through the + overridden primitives: delete_kernel + add_kernel with the new expression. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_send = AsyncioMagicMock() + host_ifc, _ = vm._veth_names(0, 0) + vm._kernel_veths[(0, 0)] = host_ifc + vm._marker_filter_bridges[("icmp-m", "l1")] = host_ifc + + await vm.rebuild_marker_filter("icmp-m", "l1", "tcp", tag=9) + + pcap = os.path.join(vm.project.markers_working_directory(), f"{vm.id}_l1_icmp-m.pcap") + vm._ubridge_send.assert_any_call(f"marker delete_kernel {host_ifc} icmp-m") + vm._ubridge_send.assert_any_call(f'marker add_kernel icmp-m {host_ifc} "tcp" tag 9 link l1 pcap "{pcap}"') + + @pytest.mark.asyncio async def test_remove_kernel_veths(vm): diff --git a/tests/controller/test_kernel_datapath_link.py b/tests/controller/test_kernel_datapath_link.py index e3502cdca..755ad3bfa 100644 --- a/tests/controller/test_kernel_datapath_link.py +++ b/tests/controller/test_kernel_datapath_link.py @@ -21,7 +21,7 @@ Docker links, NIO emission and the filters/markers guards. """ import pytest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from gns3server.config import Config from gns3server.controller.udp_link import UDPLink @@ -96,20 +96,28 @@ async def test_kernel_datapath_not_eligible_with_filters(project): @pytest.mark.asyncio -async def test_kernel_datapath_not_eligible_with_markers(project): +async def test_kernel_datapath_eligible_with_markers(project): + """ + Markers no longer disqualify the kernel datapath: they attach to the veth + host end via uBridge's AF_PACKET marker module (marker add_kernel). + """ link, node1, node2 = await _kernel_link(project) link._markers = {"m": {"bpf": "icmp"}} - assert link._kernel_datapath_eligible(node1, node2) is False + assert link._kernel_datapath_eligible(node1, node2) is True @pytest.mark.asyncio -async def test_kernel_datapath_not_eligible_with_project_marker_definitions(project): +async def test_kernel_datapath_eligible_with_project_marker_definitions(project): + """ + Project-level marker definitions are inherited by every new link and ride + the kernel datapath like private markers. + """ link, node1, node2 = await _kernel_link(project) project._marker_definitions = {"global-m": {"bpf": "icmp"}} try: - assert link._kernel_datapath_eligible(node1, node2) is False + assert link._kernel_datapath_eligible(node1, node2) is True finally: project._marker_definitions = {} @@ -209,13 +217,58 @@ async def test_update_filters_rejected_on_kernel_link(project): @pytest.mark.asyncio -async def test_start_marker_rejected_on_kernel_link(project): +async def test_prepare_kernel_link_carries_markers(project): + """ + Markers on a kernel link ride the capture node's NIO (routed by + capture_node_id) exactly like on the relay datapath. + """ - link, _n1, _n2 = await _kernel_link(project) + link, node1, node2 = await _kernel_link(project) + link._markers = {"icmp": {"bpf": "icmp", "tag": 1, "capture_node_id": node1.id, "direction": None}} + + entries = await link._prepare() + + by_node = {entry[0].id: entry[3] for entry in entries} + assert by_node[node1.id]["type"] == "nio_bridge" + assert by_node[node1.id]["markers"]["icmp"]["bpf"] == "icmp" + assert by_node[node1.id]["markers"]["icmp"]["tag"] == 1 + assert by_node[node2.id]["markers"] == {} + + +@pytest.mark.asyncio +async def test_start_marker_on_kernel_link_pushes_nio(project): + """ + start_marker works on kernel links: the marker is stored and pushed via + the NIO update like on the relay datapath (no kernel guard anymore). + """ + + link, node1, node2 = await _kernel_link(project) await link._prepare() + link._created = True - with pytest.raises(ControllerError, match="kernel-datapath"): - await link.start_marker("m", "icmp") + bodies = [] + + async def put1(path, data=None, **kwargs): + bodies.append((node1, data)) + + async def put2(path, data=None, **kwargs): + bodies.append((node2, data)) + + node1.put = put1 + node2.put = put2 + + with patch("gns3server.controller.udp_link.validate_bpf_syntax", return_value={"valid": True, "error": None}): + await link.start_marker("icmp", "icmp", tag=3) + + entry = link._markers["icmp"] + assert entry["bpf"] == "icmp" + assert entry["capture_node_id"] == node1.id # auto-picked: first marker-capable node + n1_data = next(data for node, data in bodies if node is node1) + n2_data = next(data for node, data in bodies if node is node2) + assert n1_data["type"] == "nio_bridge" + assert n1_data["filters"] == {} + assert n1_data["markers"]["icmp"]["tag"] == 3 + assert n2_data["markers"] == {} @pytest.mark.asyncio