diff --git a/docs/features/marker-traffic-insight.md b/docs/features/marker-traffic-insight.md index 84ded58e1..b275c02b7 100644 --- a/docs/features/marker-traffic-insight.md +++ b/docs/features/marker-traffic-insight.md @@ -100,6 +100,96 @@ pcap file, and its own `link=`. The shared bridge name is irrelevant to attribut capable node types (`qemu`, `docker`, `vpcs`, `cloud`) already use one bridge per link; `link` applies uniformly to all of them. +## Direction + +A `MARK` signal optionally carries `dir=` — the matched packet's travel direction +**relative to the capture node** (the `node=` in the same signal, i.e. the node whose +uBridge hosts the marker): + +| `dir` | Ingress NIO | Meaning | +|-------|-------------|---------| +| `tx` | device side (`source_nio` on a generic bridge; the IOL instance on an IOU `IOL-BRIDGE`) | capture node is **sending** | +| `rx` | link side (`destination_nio` on a generic bridge; the NIO side on an IOU `IOL-BRIDGE`) | capture node is **receiving** | + +A marker is single-sided: only the chosen capture node's uBridge installs the `mark` filter, +yet both directions of the link transit that one bridge (it carries exactly two NIOs — the +device side and the link side), so that single uBridge observes and classifies both +directions. The `marker.match` event forwards `dir` through unchanged; the Web UI combines it +with the link's two endpoints and the capture `node_id` to draw an arrow: + +- `dir=tx` → `capture_node → far_node` +- `dir=rx` → `far_node → capture_node` +- `dir` absent (older uBridge) → undirected highlight (current behaviour) + +Because the listener ignores unknown keys, `dir` is **additive**: an older server silently +drops it and an older uBridge simply omits it — either way the system falls back to +undirected rendering with no error. + +### Choosing the capture node + +Since `dir` is relative to the capture node, *which* endpoint is the observer decides what +`tx`/`rx` mean. By default the server auto-picks (first started marker-capable endpoint, in +link-endpoint order). To pin it — e.g. so `dir=tx` unambiguously means "vpcs1 is sending" — +pass `capture_node_id` on marker **create**: + +```json +{ "bpf": "icmp", "direction": "tx", "capture_node_id": "" } +``` + +The value must be one of the link's two endpoints and a marker-capable type (`vpcs`, `qemu`, +`docker`, `iou`, `dynamips`, `cloud`); any other id is rejected with `409`. Omit it to keep +the auto-pick. The chosen id is echoed back as `capture_node_id` in the marker entry and in +each `MARK` signal's `node=`, so the Web UI always knows the observer regardless of who +picked it. + +`capture_node_id` is **create-only**: it is fixed once the marker exists (changing the +observer would silently flip the meaning of stored `direction`, so recreate the marker +instead). It is not accepted on project-level definitions — a definition is link-agnostic and +has no endpoints to choose from, so inherited markers always auto-pick per link. + +For the same reason, a definition **rejects `direction: tx|rx`** (HTTP 409): each inherited +copy auto-picks its capture node, so a fixed tx/rx would denote different session directions +on different links. A definition is `both` only; encode the direction you want in the BPF +instead — e.g. `icmp and icmp[icmptype]==8` for echo requests, a packet-intrinsic property +that is consistent on every link regardless of capture node. tx/rx remains available on +per-link markers, where the capture node is fixed. + +## Pause & resume + +Two levels of silencing, both instant (no NIO rebuild, no pcap flush): + +- **Per-marker (private)** — `PUT /v3/projects/{pid}/links/{lid}/markers/{name}` + with `{"enabled": false}` flips that one filter off in place (uBridge + `enable_packet_filter … off`): no signal, no pcap, but traffic still relays — + a paused `mark` is a no-op tap, not a drop. `{"enabled": true}` flips it back. + A change to `enabled` alone is a single command (the pcap identity and emitted + counter are preserved). Changing `bpf`, `tag`, or `direction` rebuilds just that + one filter (`delete_packet_filter` + add) — only that marker's own pcap reopens + (a new capture session for the new BPF); changing `color`/`highlight_duration` + is UI-only, nothing is pushed to uBridge. +- **Per-definition (inherited)** — `POST /v3/projects/{pid}/marker-definitions/{name}/pause` + and `/resume` toggle **every** inherited `global-{name}` copy across all links + at once (same `enable_packet_filter on|off`, fanned out per copy). Use to + pause or resume a whole rule independently of the others. The definition's + `paused` flag is persisted to the `.gns3` and echoed on the definition object, + so links created later inherit it already paused, and the Web UI renders the + per-rule button from server truth. + +| Action | signal | pcap | sink | +|--------|--------|------|------| +| per-marker `enabled: false` | stop | stop | n/a | +| per-def `pause` (all `global-{name}` copies) | stop | stop | n/a | +| per-def `resume` | resume | resume | n/a | + +## Capture files + +Each marker appends matches to `/project-files/markers/__.pcap`. +Removing a marker — per-link `DELETE .../markers/{name}` or deleting a definition (which +removes every inherited copy) — deletes that marker's pcap too, even with the capture node +stopped (the filter is removed with `delete_packet_filter`, the file is unlinked). uBridge's +`reset_packet_filters` (run on NIO/filter changes) preserves mark filters, so unrelated +changes no longer close/reopen any marker's pcap. + ## API Endpoints All endpoints require a JWT bearer token (`POST /v3/access/users/authenticate`). The @@ -122,6 +212,8 @@ All endpoints require a JWT bearer token (`POST /v3/access/users/authenticate`). | POST | `/v3/projects/{pid}/marker-definitions` | Create definition (fans out to every link) | Project.Modify | | PUT | `/v3/projects/{pid}/marker-definitions/{name}` | Update definition (syncs all copies) | Project.Modify | | DELETE | `/v3/projects/{pid}/marker-definitions/{name}` | Delete definition (clears all copies) | Project.Modify | +| POST | `/v3/projects/{pid}/marker-definitions/{name}/pause` | Pause every inherited copy (instant, persisted) | Project.Modify | +| POST | `/v3/projects/{pid}/marker-definitions/{name}/resume` | Resume every inherited copy | Project.Modify | ### Aggregation @@ -142,12 +234,17 @@ extra request. "name": "icmp", "bpf": "icmp", "tag": 1, + "direction": "tx", + "capture_node_id": "a37e2235-e21f-46c9-a2ab-ba0f8c5465e6", "color": "#ff5722", "highlight_duration": 800, "enabled": true } ``` +`direction` and `capture_node_id` are both optional and create-only (see +[Direction](#direction)). + **Definition create body** (`MarkerDefinitionCreate`, shared by POST and PUT): ```json @@ -183,6 +280,8 @@ extra request. "tag": 5, "color": null, "highlight_duration": 1200, + "direction": null, + "paused": false, "link_ids": ["656ed826-...", "6bd9d156-..."] } } @@ -196,10 +295,11 @@ extra request. |-------|------|-------------| | `bpf` | string | libpcap BPF expression (required) | | `tag` | int \| null | Correlation id echoed in `MARK` signals | -| `enabled` | bool | Whether the marker is active | +| `enabled` | bool | Whether the marker is active. Toggle is instant: `false` flips the uBridge filter off in place (no signal/pcap), `true` back on — no NIO rebuild (see [Pause & resume](#pause--resume)) | | `color` | string \| null | Hex color render hint, e.g. `#ff5722` | | `highlight_duration` | int \| null | UI highlight duration in ms after a match; `null` = UI default | -| `capture_node_id` | string | Server-chosen node whose uBridge hosts the marker | +| `direction` | string \| null | `tx` / `rx` filter relative to the capture node; `null` = both | +| `capture_node_id` | string | Node whose uBridge hosts the marker — caller-set on create, else auto-picked | | `inherited_from` | string | Source definition name — present on inherited markers only | ### Definition @@ -210,6 +310,8 @@ extra request. | `tag` | int \| null | Correlation id | | `color` | string \| null | Hex color render hint | | `highlight_duration` | int \| null | UI highlight duration in ms; `null` = UI default | +| `direction` | string \| null | `tx` / `rx` filter relative to the capture node; `null` = both | +| `paused` | bool | Per-definition mute flag — `true` mutes every inherited copy (persisted) | | `link_ids` | string[] | Links currently carrying an inherited copy (GET only) | ### Notifications @@ -217,10 +319,11 @@ extra request. | Event | Payload | Delivered to | |-------|---------|--------------| | `link.updated` | Link object (its `markers` field is the source of truth) | Project notification ws | -| `marker.match` | `project_id`, `node_id`, `link_id`, `filter`, `tag`, `ts`, `len` | Project notification ws only | +| `marker.match` | `project_id`, `node_id`, `link_id`, `filter`, `tag`, `ts`, `len`, `dir` | Project notification ws only | The `marker.match` `link_id` is taken from the signal's `link=` field (authoritative); see -[Per-link attribution](#per-link-attribution). +[Per-link attribution](#per-link-attribution). The `dir` field is the matched packet's travel +direction relative to the capture node; see [Direction](#direction). ## Error Responses @@ -237,6 +340,8 @@ The `marker.match` `link_id` is taken from the signal's `link=` field (authorita filter, the pcap filename, and `MARK` signal routing — so rename is a delete + recreate, not a field update. PUT ignores the body `name`; the `{name}` path parameter identifies the target, and only `bpf / tag / color / enabled / highlight_duration` are changeable. + Names are 1–32 chars (`[A-Za-z0-9][A-Za-z0-9_.-]*`); inherited copies carry a `global-` + prefix, so their filter names reach ~39. - **`global` prefix reserved.** User-chosen names may not start with `global`; inherited markers are stored as `global-{definition_name}` so the two namespaces cannot collide. Omitting `name` on create yields an auto-generated, prefix-free name. @@ -245,6 +350,12 @@ The `marker.match` `link_id` is taken from the signal's `link=` field (authorita - **Render hints are not enforced.** `color` and `highlight_duration` (milliseconds, `>= 1`) are stored on the link and never sent to uBridge; `null` lets the UI apply its own default. A partial PUT (e.g. changing only `bpf`) leaves them untouched. +- **BPF is validated once per source.** A private per-link marker validates its BPF inline + on create/update. A definition validates its BPF once at create/update (and once per + definition on project load, dropping any whose BPF has gone invalid); the inherited + fan-out to every link then skips re-validation, so creating a definition over *N* links + runs one `tcpdump -d` rather than *N*. (uBridge still runs `pcap_compile` itself at + install time, so an invalid expression can never slip through.) - **Supported node types.** A marker needs a uBridge bridge: `vpcs`, `qemu`, `docker`, `iou`, `dynamips`, `cloud` (one capable endpoint suffices). Types without a uBridge are silently skipped by the inheritance fan-out. IOU uses one shared `IOL-BRIDGE` per node but @@ -256,3 +367,12 @@ The `marker.match` `link_id` is taken from the signal's `link=` field (authorita - **Persistence.** Definitions and private markers persist in the topology; inherited markers are re-created from definitions on project load, so reopening a project restores the same configuration and stale inherited copies cannot survive on disk. +- **Log interpretation across node types.** Each node type logs its startup and link + operations differently — do not mistake sparse logs from one type for inactivity. + QEMU prints `set_link gns3- on` via its QEMU monitor, which is the most visible + startup log among all types. VPCS, Docker, IOU, Dynamips, and Cloud each have their own + startup paths (fork + ubridge, container veth, iouyap, Dynamips hypervisor, and TAP + device respectively) and none of them emit QEMU-monitor-style logs. To verify marker + operations (toggle, pause, resume) on non-QEMU types, either inspect uBridge's + own log for `enable_packet_filter` / `marker pause` / `marker resume` commands, or + watch the gns3server log for the corresponding compute-route calls at INFO level. diff --git a/gns3server/api/routes/compute/cloud_nodes.py b/gns3server/api/routes/compute/cloud_nodes.py index 42e691235..a5f2bb6a9 100644 --- a/gns3server/api/routes/compute/cloud_nodes.py +++ b/gns3server/api/routes/compute/cloud_nodes.py @@ -255,3 +255,83 @@ async def stream_pcap_file( nio = node.get_nio(port_number) stream = Builtin.instance().stream_pcap_file(nio, node.project.id) return StreamingResponse(stream, media_type="application/vnd.tcpdump.pcap") + + +@router.put( + "/{node_id}/markers/{marker_name}" +) +async def toggle_cloud_marker( + marker_name: str, + toggle_data: schemas.MarkerToggle, + node: Cloud = Depends(dep_node) +) -> dict: + """ + Toggle a marker filter on/off without an NIO rebuild (ubridge contract §3.2). + """ + + if not any(n == marker_name for (n, lid) in node._marker_filter_bridges): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Marker '{marker_name}' is not installed on this node", + ) + await node._ubridge_set_marker_filter_state(marker_name, toggle_data.enabled) + return {"marker_name": marker_name, "enabled": toggle_data.enabled} + + +@router.post( + "/{node_id}/markers/pause", + status_code=status.HTTP_204_NO_CONTENT +) +async def pause_cloud_markers(node: Cloud = Depends(dep_node)) -> None: + + await node._ubridge_marker_pause() + + +@router.post( + "/{node_id}/markers/resume", + status_code=status.HTTP_204_NO_CONTENT +) +async def resume_cloud_markers(node: Cloud = Depends(dep_node)) -> None: + + await node._ubridge_marker_resume() + + +@router.delete( + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", + status_code=status.HTTP_204_NO_CONTENT +) +async def delete_cloud_marker_capture( + *, + marker_name: str, + adapter_number: int = Path(..., ge=0, le=0), + port_number: int, + link_id: str = "", + node: Cloud = Depends(dep_node) +) -> None: + """ + Delete a marker's capture pcap (called by the controller when the marker is + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). + """ + + nio = node.get_nio(port_number) + await node.delete_marker_capture(marker_name, link_id, nio) + + +@router.put("/{node_id}/markers/{marker_name}/rebuild") +async def rebuild_cloud_marker( + marker_name: str, + rebuild_data: schemas.MarkerRebuild, + node: Cloud = Depends(dep_node) +) -> dict: + """ + Re-install a single marker filter with new BPF/tag/direction (delete + add, + no bridge reset) so sibling markers' pcaps stay open. + """ + + await node.rebuild_marker_filter( + marker_name, rebuild_data.link_id, rebuild_data.bpf, + rebuild_data.tag, rebuild_data.direction, rebuild_data.enabled, + ) + return {"marker_name": marker_name} diff --git a/gns3server/api/routes/compute/docker_nodes.py b/gns3server/api/routes/compute/docker_nodes.py index f08f9a497..86ff4d84d 100644 --- a/gns3server/api/routes/compute/docker_nodes.py +++ b/gns3server/api/routes/compute/docker_nodes.py @@ -20,7 +20,7 @@ API routes for Docker nodes. import os -from fastapi import APIRouter, WebSocket, Depends, Body, status +from fastapi import APIRouter, WebSocket, Depends, Body, status, HTTPException from fastapi.encoders import jsonable_encoder from fastapi.responses import StreamingResponse from uuid import UUID @@ -408,3 +408,89 @@ async def vnc_console_ws( async def reset_console(node: DockerVM = Depends(dep_node)) -> None: await node.reset_console() + + +@router.put( + "/{node_id}/markers/{marker_name}", + dependencies=[Depends(compute_authentication)] +) +async def toggle_docker_marker( + marker_name: str, + toggle_data: schemas.MarkerToggle, + node: DockerVM = Depends(dep_node) +) -> dict: + """ + Toggle a marker filter on/off without an NIO rebuild (ubridge contract §3.2). + """ + + if not any(n == marker_name for (n, lid) in node._marker_filter_bridges): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Marker '{marker_name}' is not installed on this node", + ) + await node._ubridge_set_marker_filter_state(marker_name, toggle_data.enabled) + return {"marker_name": marker_name, "enabled": toggle_data.enabled} + + +@router.post( + "/{node_id}/markers/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def pause_docker_markers(node: DockerVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_pause() + + +@router.post( + "/{node_id}/markers/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def resume_docker_markers(node: DockerVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_resume() + + +@router.delete( + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def delete_docker_marker_capture( + marker_name: str, + adapter_number: int, + port_number: int, + link_id: str = "", + node: DockerVM = Depends(dep_node) +) -> None: + """ + Delete a marker's capture pcap (called by the controller when the marker is + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). + """ + + nio = node.get_nio(adapter_number) + await node.delete_marker_capture(marker_name, link_id, nio) + + +@router.put( + "/{node_id}/markers/{marker_name}/rebuild", + dependencies=[Depends(compute_authentication)] +) +async def rebuild_docker_marker( + marker_name: str, + rebuild_data: schemas.MarkerRebuild, + node: DockerVM = Depends(dep_node) +) -> dict: + """ + Re-install a single marker filter with new BPF/tag/direction (delete + add, + no bridge reset) so sibling markers' pcaps stay open. + """ + + await node.rebuild_marker_filter( + marker_name, rebuild_data.link_id, rebuild_data.bpf, + rebuild_data.tag, rebuild_data.direction, rebuild_data.enabled, + ) + return {"marker_name": marker_name} diff --git a/gns3server/api/routes/compute/dynamips_nodes.py b/gns3server/api/routes/compute/dynamips_nodes.py index 70d2b10c6..744dfa3ba 100644 --- a/gns3server/api/routes/compute/dynamips_nodes.py +++ b/gns3server/api/routes/compute/dynamips_nodes.py @@ -20,7 +20,7 @@ API routes for Dynamips nodes. import os -from fastapi import APIRouter, WebSocket, Body, Depends, status +from fastapi import APIRouter, WebSocket, Body, Depends, status, HTTPException from fastapi.encoders import jsonable_encoder from fastapi.responses import StreamingResponse from typing import List, Union @@ -367,3 +367,89 @@ async def console_ws( async def reset_console(node: Router = Depends(dep_node)) -> None: await node.reset_console() + + +@router.put( + "/{node_id}/markers/{marker_name}", + dependencies=[Depends(compute_authentication)] +) +async def toggle_dynamips_marker( + marker_name: str, + toggle_data: schemas.MarkerToggle, + node: Router = Depends(dep_node) +) -> dict: + """ + Toggle a marker filter on/off without an NIO rebuild (ubridge contract §3.2). + """ + + if not any(n == marker_name for (n, lid) in node._marker_filter_bridges): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Marker '{marker_name}' is not installed on this node", + ) + await node._ubridge_set_marker_filter_state(marker_name, toggle_data.enabled) + return {"marker_name": marker_name, "enabled": toggle_data.enabled} + + +@router.post( + "/{node_id}/markers/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def pause_dynamips_markers(node: Router = Depends(dep_node)) -> None: + + await node._ubridge_marker_pause() + + +@router.post( + "/{node_id}/markers/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def resume_dynamips_markers(node: Router = Depends(dep_node)) -> None: + + await node._ubridge_marker_resume() + + +@router.delete( + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def delete_dynamips_marker_capture( + marker_name: str, + adapter_number: int, + port_number: int, + link_id: str = "", + node: Router = Depends(dep_node) +) -> None: + """ + Delete a marker's capture pcap (called by the controller when the marker is + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). + """ + + nio = node.get_nio(adapter_number, port_number) + await node.delete_marker_capture(marker_name, link_id, nio) + + +@router.put( + "/{node_id}/markers/{marker_name}/rebuild", + dependencies=[Depends(compute_authentication)] +) +async def rebuild_dynamips_marker( + marker_name: str, + rebuild_data: schemas.MarkerRebuild, + node: Router = Depends(dep_node) +) -> dict: + """ + Re-install a single marker filter with new BPF/tag/direction (delete + add, + no bridge reset) so sibling markers' pcaps stay open. + """ + + await node.rebuild_marker_filter( + marker_name, rebuild_data.link_id, rebuild_data.bpf, + rebuild_data.tag, rebuild_data.direction, rebuild_data.enabled, + ) + return {"marker_name": marker_name} diff --git a/gns3server/api/routes/compute/iou_nodes.py b/gns3server/api/routes/compute/iou_nodes.py index 443f625cb..b826b21d9 100644 --- a/gns3server/api/routes/compute/iou_nodes.py +++ b/gns3server/api/routes/compute/iou_nodes.py @@ -346,3 +346,89 @@ async def console_ws( async def reset_console(node: IOUVM = Depends(dep_node)) -> None: await node.reset_console() + + +@router.put( + "/{node_id}/markers/{marker_name}", + dependencies=[Depends(compute_authentication)] +) +async def toggle_iou_marker( + marker_name: str, + toggle_data: schemas.MarkerToggle, + node: IOUVM = Depends(dep_node) +) -> dict: + """ + Toggle a marker filter on/off without an NIO rebuild (ubridge contract §3.2). + """ + + if not any(n == marker_name for (n, lid) in node._marker_filter_bridges): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Marker '{marker_name}' is not installed on this node", + ) + await node._ubridge_set_marker_filter_state(marker_name, toggle_data.enabled) + return {"marker_name": marker_name, "enabled": toggle_data.enabled} + + +@router.post( + "/{node_id}/markers/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def pause_iou_markers(node: IOUVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_pause() + + +@router.post( + "/{node_id}/markers/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def resume_iou_markers(node: IOUVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_resume() + + +@router.delete( + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def delete_iou_marker_capture( + marker_name: str, + adapter_number: int, + port_number: int, + link_id: str = "", + node: IOUVM = Depends(dep_node) +) -> None: + """ + Delete a marker's capture pcap (called by the controller when the marker is + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). + """ + + nio = node.get_nio(adapter_number, port_number) + await node.delete_marker_capture(marker_name, link_id, nio) + + +@router.put( + "/{node_id}/markers/{marker_name}/rebuild", + dependencies=[Depends(compute_authentication)] +) +async def rebuild_iou_marker( + marker_name: str, + rebuild_data: schemas.MarkerRebuild, + node: IOUVM = Depends(dep_node) +) -> dict: + """ + Re-install a single marker filter with new BPF/tag/direction (delete + add, + no bridge reset) so sibling markers' pcaps stay open. + """ + + await node.rebuild_marker_filter( + marker_name, rebuild_data.link_id, rebuild_data.bpf, + rebuild_data.tag, rebuild_data.direction, rebuild_data.enabled, + ) + return {"marker_name": marker_name} diff --git a/gns3server/api/routes/compute/qemu_nodes.py b/gns3server/api/routes/compute/qemu_nodes.py index 6582dc731..51f8f60d8 100644 --- a/gns3server/api/routes/compute/qemu_nodes.py +++ b/gns3server/api/routes/compute/qemu_nodes.py @@ -20,7 +20,7 @@ API routes for Qemu nodes. import os -from fastapi import APIRouter, WebSocket, Depends, Body, Path, status +from fastapi import APIRouter, WebSocket, Depends, Body, Path, status, HTTPException from fastapi.encoders import jsonable_encoder from fastapi.responses import StreamingResponse from typing import Union @@ -438,3 +438,89 @@ async def vnc_console_ws( async def reset_console(node: QemuVM = Depends(dep_node)) -> None: await node.reset_console() + + +@router.put( + "/{node_id}/markers/{marker_name}", + dependencies=[Depends(compute_authentication)] +) +async def toggle_qemu_marker( + marker_name: str, + toggle_data: schemas.MarkerToggle, + node: QemuVM = Depends(dep_node) +) -> dict: + """ + Toggle a marker filter on/off without an NIO rebuild (ubridge contract §3.2). + """ + + if not any(n == marker_name for (n, lid) in node._marker_filter_bridges): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Marker '{marker_name}' is not installed on this node", + ) + await node._ubridge_set_marker_filter_state(marker_name, toggle_data.enabled) + return {"marker_name": marker_name, "enabled": toggle_data.enabled} + + +@router.post( + "/{node_id}/markers/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def pause_qemu_markers(node: QemuVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_pause() + + +@router.post( + "/{node_id}/markers/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def resume_qemu_markers(node: QemuVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_resume() + + +@router.delete( + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def delete_qemu_marker_capture( + marker_name: str, + adapter_number: int, + port_number: int = Path(..., ge=0, le=0), + link_id: str = "", + node: QemuVM = Depends(dep_node) +) -> None: + """ + Delete a marker's capture pcap (called by the controller when the marker is + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). + """ + + nio = node.get_nio(adapter_number) + await node.delete_marker_capture(marker_name, link_id, nio) + + +@router.put( + "/{node_id}/markers/{marker_name}/rebuild", + dependencies=[Depends(compute_authentication)] +) +async def rebuild_qemu_marker( + marker_name: str, + rebuild_data: schemas.MarkerRebuild, + node: QemuVM = Depends(dep_node) +) -> dict: + """ + Re-install a single marker filter with new BPF/tag/direction (delete + add, + no bridge reset) so sibling markers' pcaps stay open. + """ + + await node.rebuild_marker_filter( + marker_name, rebuild_data.link_id, rebuild_data.bpf, + rebuild_data.tag, rebuild_data.direction, rebuild_data.enabled, + ) + return {"marker_name": marker_name} diff --git a/gns3server/api/routes/compute/vpcs_nodes.py b/gns3server/api/routes/compute/vpcs_nodes.py index 07c439972..546fabdb0 100644 --- a/gns3server/api/routes/compute/vpcs_nodes.py +++ b/gns3server/api/routes/compute/vpcs_nodes.py @@ -345,3 +345,90 @@ async def console_ws( async def reset_console(node: VPCSVM = Depends(dep_node)) -> None: await node.reset_console() + + +@router.put( + "/{node_id}/markers/{marker_name}", + dependencies=[Depends(compute_authentication)] +) +async def toggle_vpcs_marker( + marker_name: str, + toggle_data: schemas.MarkerToggle, + node: VPCSVM = Depends(dep_node) +) -> dict: + """ + Toggle a marker filter on/off without an NIO rebuild (ubridge contract §3.2). + """ + + if not any(n == marker_name for (n, lid) in node._marker_filter_bridges): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Marker '{marker_name}' is not installed on this node", + ) + await node._ubridge_set_marker_filter_state(marker_name, toggle_data.enabled) + return {"marker_name": marker_name, "enabled": toggle_data.enabled} + + +@router.post( + "/{node_id}/markers/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def pause_vpcs_markers(node: VPCSVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_pause() + + +@router.post( + "/{node_id}/markers/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def resume_vpcs_markers(node: VPCSVM = Depends(dep_node)) -> None: + + await node._ubridge_marker_resume() + + +@router.delete( + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(compute_authentication)] +) +async def delete_vpcs_marker_capture( + *, + marker_name: str, + adapter_number: int = Path(..., ge=0, le=0), + port_number: int, + link_id: str = "", + node: VPCSVM = Depends(dep_node) +) -> None: + """ + Delete a marker's capture pcap (called by the controller when the marker is + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). + """ + + nio = node.get_nio(port_number) + await node.delete_marker_capture(marker_name, link_id, nio) + + +@router.put( + "/{node_id}/markers/{marker_name}/rebuild", + dependencies=[Depends(compute_authentication)] +) +async def rebuild_vpcs_marker( + marker_name: str, + rebuild_data: schemas.MarkerRebuild, + node: VPCSVM = Depends(dep_node) +) -> dict: + """ + Re-install a single marker filter with new BPF/tag/direction (delete + add, + no bridge reset) so sibling markers' pcaps stay open. + """ + + await node.rebuild_marker_filter( + marker_name, rebuild_data.link_id, rebuild_data.bpf, + rebuild_data.tag, rebuild_data.direction, rebuild_data.enabled, + ) + return {"marker_name": marker_name} diff --git a/gns3server/api/routes/controller/links.py b/gns3server/api/routes/controller/links.py index 9e33b7ba3..bf6c5467f 100644 --- a/gns3server/api/routes/controller/links.py +++ b/gns3server/api/routes/controller/links.py @@ -32,7 +32,7 @@ from uuid import UUID, uuid4 from gns3server.controller import Controller from gns3server.controller.controller_error import ControllerError from gns3server.db.repositories.rbac import RbacRepository -from gns3server.controller.link import Link +from gns3server.controller.link import Link, _UNSET from gns3server.utils.http_client import HTTPClient from gns3server.utils.port_allocator import link_id_to_port from gns3server.utils.websocket_to_websocket import websocket_proxy @@ -465,8 +465,11 @@ async def create_marker( name=name, bpf=marker_data.bpf, tag=marker_data.tag, + direction=marker_data.direction, + capture_node_id=marker_data.capture_node_id, color=marker_data.color, highlight_duration=marker_data.highlight_duration, + data_link_type=marker_data.data_link_type, ) return link.markers.get(name, {}) @@ -495,7 +498,7 @@ async def delete_marker( ) async def update_marker( marker_name: str, - marker_data: schemas.MarkerCreate, + marker_data: schemas.MarkerUpdate, link: Link = Depends(dep_link) ) -> dict: """ @@ -508,6 +511,7 @@ async def update_marker( name=marker_name, bpf=marker_data.bpf if marker_data.bpf else None, tag=marker_data.tag, + direction=marker_data.direction if "direction" in marker_data.model_fields_set else _UNSET, color=marker_data.color, enabled=marker_data.enabled, highlight_duration=marker_data.highlight_duration, diff --git a/gns3server/api/routes/controller/projects.py b/gns3server/api/routes/controller/projects.py index 5e6f1c6de..f9c838a34 100644 --- a/gns3server/api/routes/controller/projects.py +++ b/gns3server/api/routes/controller/projects.py @@ -40,6 +40,7 @@ from uuid import UUID from gns3server import schemas from gns3server.controller import Controller from gns3server.controller.project import Project +from gns3server.controller.link import _UNSET from gns3server.controller.controller_error import ControllerError, ControllerBadRequestError from gns3server.controller.import_project import import_project as import_controller_project from gns3server.controller.export_project import export_project as export_controller_project @@ -267,8 +268,10 @@ async def create_marker_definition( name=name, bpf=def_data.bpf, tag=def_data.tag, + direction=def_data.direction, color=def_data.color, highlight_duration=def_data.highlight_duration, + data_link_type=def_data.data_link_type, ) return project.marker_definitions.get(name, {}) @@ -292,12 +295,52 @@ async def update_marker_definition( name=def_name, bpf=def_data.bpf if def_data.bpf else None, tag=def_data.tag, + direction=def_data.direction if "direction" in def_data.model_fields_set else _UNSET, color=def_data.color, highlight_duration=def_data.highlight_duration, + data_link_type=def_data.data_link_type if "data_link_type" in def_data.model_fields_set else _UNSET, ) return project.marker_definitions.get(def_name, {}) +@router.post( + "/{project_id}/marker-definitions/{def_name}/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(has_privilege("Project.Modify"))] +) +async def pause_marker_definition( + def_name: str, + project: Project = Depends(dep_project) +) -> None: + """ + Pause a definition: toggle off every inherited ``global-{def_name}`` copy + on every link (uBridge ``enable_packet_filter off``, instant — no NIO + rebuild). The definition's ``paused`` flag is persisted, so links created + later inherit it already paused. + + Required privilege: Project.Modify + """ + + await project.pause_marker_definition(def_name) + + +@router.post( + "/{project_id}/marker-definitions/{def_name}/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(has_privilege("Project.Modify"))] +) +async def resume_marker_definition( + def_name: str, + project: Project = Depends(dep_project) +) -> None: + """Resume a paused definition (toggle on every inherited copy). + + Required privilege: Project.Modify + """ + + await project.resume_marker_definition(def_name) + + @router.delete( "/{project_id}/marker-definitions/{def_name}", status_code=status.HTTP_204_NO_CONTENT, diff --git a/gns3server/api/routes/mcp/__init__.py b/gns3server/api/routes/mcp/__init__.py index f7d57d6e2..021f66b99 100644 --- a/gns3server/api/routes/mcp/__init__.py +++ b/gns3server/api/routes/mcp/__init__.py @@ -1015,6 +1015,8 @@ async def link_marker( name: Annotated[str | None, Field(description="Custom marker name for create action (auto-generated if omitted)")] = None, tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None, enabled: Annotated[bool | None, Field(description="Enable or disable the marker (for update action)")] = None, + direction: Annotated[str | None, Field(description="Direction filter: 'tx' (capture node sending only), 'rx' (receiving only), or 'both' (no filter — on update this clears a previously set direction). Omit to leave unchanged on update.")] = None, + capture_node_id: Annotated[str | None, Field(description="UUID of the endpoint whose uBridge hosts the marker (the observer; tx/rx are from its perspective). Must be a link endpoint and marker-capable. Omit to auto-pick.")] = None, color: Annotated[str | None, Field(description="Hex color for UI highlight, e.g. '#ff5722'")] = None, highlight_duration: Annotated[int | None, Field(description="UI highlight duration in milliseconds")] = None, ) -> list[dict[str, Any]]: @@ -1024,7 +1026,7 @@ async def link_marker( Set action='create' to add a marker, 'update' to modify it, 'delete' to remove. Create requires: project_id, link_id, action='create', bpf - Update requires: project_id, link_id, action='update', marker_name, and at least one of (bpf, tag, enabled, color, highlight_duration) + Update requires: project_id, link_id, action='update', marker_name, and at least one of (bpf, tag, enabled, direction, color, highlight_duration) Delete requires: project_id, link_id, action='delete', marker_name To read current markers, use link_get — the response includes a 'markers' dict. @@ -1033,7 +1035,7 @@ async def link_marker( and cannot be modified or deleted via this tool. """ params = {"project_id": project_id, "link_id": link_id, "action": action} - for opt in ("bpf", "marker_name", "name", "tag", "enabled", "color", "highlight_duration"): + for opt in ("bpf", "marker_name", "name", "tag", "enabled", "direction", "capture_node_id", "color", "highlight_duration"): val = locals().get(opt) if val is not None: params[opt] = val @@ -1050,6 +1052,7 @@ async def marker_definition( tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None, color: Annotated[str | None, Field(description="Hex color for UI highlight, e.g. '#ff5722'")] = None, highlight_duration: Annotated[int | None, Field(description="UI highlight duration in milliseconds")] = None, + data_link_type: Annotated[str | None, Field(description="pcap link-layer type for serial links (DLT_C_HDLC / DLT_PPP_SERIAL / DLT_FRELAY / DLT_ATM_RFC1483). Omit = Ethernet-only (serial links skipped); setting it also covers serial links with that encapsulation")] = None, ) -> list[dict[str, Any]]: """Manage project-level marker definitions — traffic-insight rules that apply to ALL links. @@ -1058,14 +1061,20 @@ async def marker_definition( On delete, 'global-{name}' is removed from every link. Create requires: project_id, action='create', bpf - Update requires: project_id, action='update', def_name, and at least one of (bpf, tag, color, highlight_duration) + Update requires: project_id, action='update', def_name, and at least one of (bpf, tag, color, highlight_duration, data_link_type) Delete requires: project_id, action='delete', def_name List requires: project_id, action='list' + A definition has NO direction (tx/rx): it fans out to every link and auto-selects + its capture node on each, so a fixed direction has no consistent meaning. Encode + the direction you want in the BPF instead (e.g. 'icmp and icmp[icmptype]==8' for + echo requests only). For a capture-node-relative direction on a single link, use + the per-link `link_marker` tool. + Common BPF examples: 'arp', 'icmp', 'ospf', 'tcp port 22', 'udp port 53' """ params = {"project_id": project_id, "action": action} - for opt in ("bpf", "def_name", "name", "tag", "color", "highlight_duration"): + for opt in ("bpf", "def_name", "name", "tag", "color", "highlight_duration", "data_link_type"): val = locals().get(opt) if val is not None: params[opt] = val diff --git a/gns3server/api/routes/mcp/links.py b/gns3server/api/routes/mcp/links.py index b2a0352c3..314f8b8c9 100644 --- a/gns3server/api/routes/mcp/links.py +++ b/gns3server/api/routes/mcp/links.py @@ -395,9 +395,12 @@ def link_marker_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic if not bpf: return {"error": "bpf is required for create action"} body: dict[str, Any] = {"bpf": bpf} - for opt in ("name", "tag", "color", "highlight_duration"): + for opt in ("name", "tag", "capture_node_id", "color", "highlight_duration"): if params.get(opt) is not None: body[opt] = params[opt] + # direction: "tx"/"rx" set a one-way filter; "both"/omitted = no filter. + if params.get("direction") in ("tx", "rx"): + body["direction"] = params["direction"] return conn.http_call("post", base, json_data=body).json() marker_name = params.get("marker_name") @@ -411,8 +414,14 @@ def link_marker_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic for opt in ("bpf", "tag", "enabled", "color", "highlight_duration"): if params.get(opt) is not None: body[opt] = params[opt] + # direction tri-state: omitted=preserve, "tx"/"rx"=set, "both"=clear (→ null). + direction = params.get("direction") + if direction == "both": + body["direction"] = None + elif direction in ("tx", "rx"): + body["direction"] = direction if not body: - return {"error": "At least one update field is required (bpf, tag, enabled, color, highlight_duration)"} + return {"error": "At least one update field is required (bpf, tag, enabled, direction, color, highlight_duration)"} return conn.http_call("put", url, json_data=body).json() # action == "delete" @@ -448,9 +457,12 @@ def marker_definition_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) if not bpf: return {"error": "bpf is required for create action"} body: dict[str, Any] = {"bpf": bpf} - for opt in ("name", "tag", "color", "highlight_duration"): + for opt in ("name", "tag", "color", "highlight_duration", "data_link_type"): if params.get(opt) is not None: body[opt] = params[opt] + # No direction: a definition fans out to every link and auto-selects its + # capture node on each, so tx/rx (which is relative to that node) has no + # consistent meaning. Encode direction in the BPF instead. return conn.http_call("post", base, json_data=body).json() def_name = params.get("def_name") @@ -461,11 +473,11 @@ def marker_definition_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) if action == "update": body = {} - for opt in ("bpf", "tag", "color", "highlight_duration"): + for opt in ("bpf", "tag", "color", "highlight_duration", "data_link_type"): if params.get(opt) is not None: body[opt] = params[opt] if not body: - return {"error": "At least one update field is required (bpf, tag, color, highlight_duration)"} + return {"error": "At least one update field is required (bpf, tag, color, highlight_duration, data_link_type)"} return conn.http_call("put", url, json_data=body).json() # action == "delete" diff --git a/gns3server/compute/base_node.py b/gns3server/compute/base_node.py index df92de0aa..fc048892a 100644 --- a/gns3server/compute/base_node.py +++ b/gns3server/compute/base_node.py @@ -100,6 +100,9 @@ class BaseNode: self._internal_aux_port = None self._custom_adapters = [] self._ubridge_require_privileged_access = False + # 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 = {} if self._console is not None: # use a previously allocated console port @@ -926,13 +929,16 @@ class BaseNode: raise NodeError("uBridge requires root access or the capability to interact with network adapters") server_host = self._manager.config.settings.Server.host + transport = self._manager.config.settings.Server.ubridge_control_transport if not self.ubridge: - self._ubridge_hypervisor = Hypervisor(self._project, self.ubridge_path, self.working_dir, server_host) - log.info(f"Starting new uBridge hypervisor {self._ubridge_hypervisor.host}:{self._ubridge_hypervisor.port}") + self._ubridge_hypervisor = Hypervisor( + self._project, self.ubridge_path, self.working_dir, transport, server_host, self.id + ) + log.info(f"Starting new uBridge hypervisor at {self._ubridge_hypervisor.endpoint}") await self._ubridge_hypervisor.start() if self._ubridge_hypervisor: log.info( - f"Hypervisor {self._ubridge_hypervisor.host}:{self._ubridge_hypervisor.port} has successfully started" + f"Hypervisor at {self._ubridge_hypervisor.endpoint} has successfully started" ) await self._ubridge_hypervisor.connect() # Tell this uBridge where to send MARK signals and which node id to @@ -981,9 +987,13 @@ class BaseNode: """ if self._ubridge_hypervisor and self._ubridge_hypervisor.is_running(): - log.info(f"Stopping uBridge hypervisor {self._ubridge_hypervisor.host}:{self._ubridge_hypervisor.port}") + log.info(f"Stopping uBridge hypervisor at {self._ubridge_hypervisor.endpoint}") await self._ubridge_hypervisor.stop() self._ubridge_hypervisor = None + # uBridge is gone, so every marker filter (and its in-bridge state) is + # 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() async def add_ubridge_udp_connection(self, bridge_name, source_nio, destination_nio): """ @@ -1081,7 +1091,25 @@ class BaseNode: ) i += 1 - async def _ubridge_add_marker_filter(self, bridge_name, name, bpf, pcap_path, tag=None, link_id=None): + @staticmethod + def _marker_linktype(data_link_type): + """ + Normalize a GNS3 pcap data-link type (e.g. ``DLT_C_HDLC``) to the bare + uBridge ``linktype`` token (``C_HDLC``) by stripping the ``DLT_`` prefix. + Returns ``None`` for Ethernet (``DLT_EN10MB`` / unset) so the ``linktype`` + keyword is omitted and uBridge defaults to EN10MB. Values come straight + from ``SerialPort.data_link_types`` (the single source of truth); uBridge + resolves them with ``pcap_datalink_name_to_val``, which is case-sensitive + and expects the canonical uppercase form. + """ + if not data_link_type: + return None + dlt = data_link_type.upper() + if dlt.startswith("DLT_"): + dlt = dlt[4:] + return None if dlt == "EN10MB" else dlt + + async def _ubridge_add_marker_filter(self, bridge_name, name, bpf, pcap_path, tag=None, link_id=None, direction=None, data_link_type=None): """ Attach a `mark` packet filter to a uBridge bridge for traffic insight. @@ -1105,7 +1133,10 @@ class BaseNode: # 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_.-]*$") - if not _MARKER_NAME_RE.match(name): + # 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 = 'bridge add_packet_filter {bridge} {name} mark "{bpf}"'.format( bridge=bridge_name, name=name, bpf=bpf @@ -1117,18 +1148,88 @@ class BaseNode: # so the link id is the only way to tell signals — and pcap files — apart. 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) # Let BPF compile errors propagate — the marker is the user's intent, so a # bad expression must surface instead of being silently dropped. await self._ubridge_send(cmd) + async def delete_marker_capture(self, name, link_id, nio=None): + """ + Remove a marker from uBridge (fine-grained ``delete_packet_filter`` — NOT + reset_packet_filters, so sibling markers' pcaps aren't closed/reopened) + and delete its capture pcap. Called by the controller when a marker is + removed; safe with the node stopped (filter removal is skipped, the file + is still unlinked). IOU overrides ``_ubridge_delete_marker_filter`` for + its ``iol_bridge`` command shape. + + ``nio`` is the port NIO whose cached ``nio.markers`` carries this marker + spec; it is dropped here so a later node start / NIO reapply + (``_ubridge_apply_markers``) does not reinstall the marker. Without this, + deleting a marker while the node is stopped left the spec in + ``nio.markers``, and starting the node recreated an empty pcap. + """ + 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) + if bridge_name is not None: + await self._ubridge_delete_marker_filter(bridge_name, name) + try: + markers_dir = self.project.markers_working_directory() + pcap_path = os.path.join(markers_dir, f"{self._id}_{link_id}_{name}.pcap") + os.remove(pcap_path) + except FileNotFoundError: + pass + except OSError as e: + log.warning("Could not remove marker pcap for '%s' on link %s: %s", name, link_id, e) + + async def _ubridge_delete_marker_filter(self, bridge_name, name): + """ + Remove a single marker filter from uBridge with ``delete_packet_filter`` + (not a bridge-wide reset) so other markers keep their pcaps open. A no-op + when uBridge isn't running — the pcap cleanup in the caller still proceeds. + """ + if not (self._ubridge_hypervisor and self._ubridge_hypervisor.is_running()): + return + try: + await self._ubridge_send(f"bridge delete_packet_filter {bridge_name} {name}") + except UbridgeError as e: + log.warning("Could not remove marker filter '%s' from %s: %s", name, bridge_name, e) + + async def rebuild_marker_filter(self, name, link_id, bpf, tag=None, direction=None, enabled=True): + """ + Re-install a single marker filter with new params (delete + add), without + a bridge-wide reset — so sibling markers keep their pcaps open. uBridge + reopens the marker's own pcap on re-add (a new capture session for the + new BPF), which is expected. No-op if the marker isn't installed (node + stopped) — the next NIO reapply picks up the updated ``_markers``. + + IOU needs no override: this calls ``_ubridge_delete_marker_filter`` / + ``_ubridge_add_marker_filter`` / ``_ubridge_set_marker_filter_state``, + all of which IOU already overrides for ``iol_bridge``. + """ + bridge_name = self._marker_filter_bridges.get((name, link_id)) + if bridge_name is None: + return + await self._ubridge_delete_marker_filter(bridge_name, name) + pcap_path = os.path.join(self.project.markers_working_directory(), f"{self._id}_{link_id}_{name}.pcap") + await self._ubridge_add_marker_filter(bridge_name, name, bpf, pcap_path, tag, link_id, direction=direction) + if not enabled: + await self._ubridge_set_marker_filter_state(name, enabled=False) + async def _ubridge_apply_markers(self, bridge_name, nio): """ - (Re-)apply every traffic-insight marker carried by *nio* to the uBridge - bridge *bridge_name*. Called from ``add_ubridge_udp_connection`` (bridge - creation / node restart) and ``update_ubridge_udp_connection`` (NIO update - — the preceding ``_ubridge_apply_filters`` has already issued - ``reset_packet_filters``, so we must re-add markers to survive the reset). + 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). """ from gns3server.compute.marker.marker_manager import MarkerManager @@ -1139,14 +1240,21 @@ class BaseNode: 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: + continue bpf = spec.get("bpf", "") tag = spec.get("tag") - link_id = spec.get("link_id", "") 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) + await self._ubridge_add_marker_filter(bridge_name, name, bpf, pcap_path, tag, link_id, + direction=spec.get("direction"), + data_link_type=spec.get("data_link_type")) except UbridgeError as e: # Swallow BPF compile errors (warn + skip) so a single bad # expression can't break link creation / node restart — mirrors @@ -1157,9 +1265,67 @@ class BaseNode: self.project.emit("log.warning", {"message": message}) continue 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). + if not spec.get("enabled", True): + 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 ) + # 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. + self._marker_filter_bridges[name, link_id] = bridge_name + + 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 + 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. + + :param name: marker filter name + :param enabled: True = on (signal+pcap), False = off (paused tap) + """ + + 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}") + + async def _ubridge_marker_pause(self): + """ + Pause all marker signal+pcap emission on this node's uBridge + (``marker pause``). Keeps the sink open so ``resume`` is instant. Safe + on old ubridge builds (the error is downgraded to a warning). Called by + the project-level pause fan-out. + """ + + if self._ubridge_hypervisor: + try: + await self._ubridge_hypervisor.send("marker pause") + except UbridgeError as e: + log.warning(f"Could not pause markers on node {self._id}: {e}") + + async def _ubridge_marker_resume(self): + """Resume marker signal+pcap emission (``marker resume``).""" + + if self._ubridge_hypervisor: + try: + await self._ubridge_hypervisor.send("marker resume") + except UbridgeError as e: + log.warning(f"Could not resume markers on node {self._id}: {e}") async def _add_ubridge_ethernet_connection(self, bridge_name, ethernet_interface, block_host_traffic=False): """ diff --git a/gns3server/compute/iou/iou_vm.py b/gns3server/compute/iou/iou_vm.py index 3697fc327..edba56876 100644 --- a/gns3server/compute/iou/iou_vm.py +++ b/gns3server/compute/iou/iou_vm.py @@ -1290,9 +1290,18 @@ class IOUVM(BaseNode): 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: + continue bpf = spec.get("bpf", "") tag = spec.get("tag") - link_id = spec.get("link_id", "") pcap_path = os.path.join( markers_dir, f"{self._id}_{link_id}_{name}.pcap" ) @@ -1308,6 +1317,12 @@ class IOUVM(BaseNode): # controller can tell their signals apart (contract §3.2). if link_id: cmd += f" link {link_id}" + direction = spec.get("direction") + if direction is not None: + cmd += f" dir {direction}" + linktype = self._marker_linktype(spec.get("data_link_type")) + if linktype is not None: + cmd += f" linktype {linktype}" cmd += ' pcap "{path}"'.format(path=pcap_path) try: await self._ubridge_send(cmd) @@ -1318,9 +1333,35 @@ class IOUVM(BaseNode): self.project.emit("log.warning", {"message": message}) continue raise + if not spec.get("enabled", True): + 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. + self._marker_filter_bridges[name, link_id] = location + + async def _ubridge_set_marker_filter_state(self, name, enabled): + """IOU override: toggle every (name, link_id) entry via ``iol_bridge``.""" + + state = "on" if enabled else "off" + for (n, lid), location in list(self._marker_filter_bridges.items()): + if n == name: + await self._ubridge_send(f"iol_bridge enable_packet_filter {location} {name} {state}") + + async def _ubridge_delete_marker_filter(self, location, name): + """IOU override: remove a single marker filter via ``iol_bridge`` + (location = ``{bridge} {bay} {unit}``), not a bridge-wide reset.""" + + if not (self._ubridge_hypervisor and self._ubridge_hypervisor.is_running()): + return + try: + await self._ubridge_send(f"iol_bridge delete_packet_filter {location} {name}") + except UbridgeError as e: + log.warning("Could not remove marker filter '%s' from %s: %s", name, location, e) async def adapter_remove_nio_binding(self, adapter_number, port_number): """ diff --git a/gns3server/compute/marker/marker_listener.py b/gns3server/compute/marker/marker_listener.py index f22f59bff..2b5e5d47f 100644 --- a/gns3server/compute/marker/marker_listener.py +++ b/gns3server/compute/marker/marker_listener.py @@ -28,9 +28,18 @@ class MarkerListener(asyncio.DatagramProtocol): Signal format (one datagram per match, ASCII):: - MARK node= filter= tag= len=\\n + MARK node= filter= tag= len= [dir=]\\n - The signal carries metadata only (no packet bytes). The compute-side + The signal carries metadata only (no packet bytes). ``dir`` is optional and + additive: uBridge stamps it from the ingress NIO of the matched packet to + indicate travel direction relative to the capture node (the ``node=`` + above) — ``tx`` = the capture node is sending (ingressed on the device-side + NIO), ``rx`` = it is receiving (ingressed on the link-side NIO). Older + uBridge builds omit it, so the listener leaves ``dir`` unset and consumers + fall back to undirected rendering. Unknown keys are always ignored, so the + field ships safely with no version coupling. + + The compute-side :class:`~gns3server.compute.marker.marker_manager.MarkerManager` registry resolves ``(node_id, filter_name)`` to ``(project_id, link_id, tag)`` so the event can be emitted on the right project-scoped notification stream. @@ -82,6 +91,11 @@ class MarkerListener(asyncio.DatagramProtocol): link = kv.get("link") tag = kv.get("tag") length = kv.get("len") + # Travel direction relative to the capture node (the node= above): + # "tx" = capture node is sending (matched packet ingressed on the + # device-side NIO), "rx" = it is receiving (link-side NIO). Older + # uBridge builds omit dir; None here lets consumers render undirected. + direction = kv.get("dir") project_id, link_id, registered_tag = self._manager.lookup(node_id, filter_name) if project_id is None: @@ -105,5 +119,8 @@ class MarkerListener(asyncio.DatagramProtocol): "tag": tag if tag and tag != "-" else registered_tag, "ts": ts, "len": int(length) if length and length.isdigit() else 0, + # Travel direction relative to the capture node (node_id above); + # None when the signal carries none (older uBridge) — undirected. + "dir": direction, } self._manager.emit_match(project_id, event) diff --git a/gns3server/compute/qemu/qemu_vm.py b/gns3server/compute/qemu/qemu_vm.py index 92884e0c7..c4866fbb0 100644 --- a/gns3server/compute/qemu/qemu_vm.py +++ b/gns3server/compute/qemu/qemu_vm.py @@ -1338,7 +1338,7 @@ class QemuVM(BaseNode): ) ) else: - log.info( + log.debug( f"Connected to QEMU monitor on {self._monitor_host}:{self._monitor} after {time.time() - begin:.4f} seconds" ) return reader, writer @@ -1355,7 +1355,7 @@ class QemuVM(BaseNode): result = None if self.is_running() and self._monitor: - log.info(f"Execute QEMU monitor command: {command}") + log.debug(f"Execute QEMU monitor command: {command}") reader, writer = await self._open_qemu_monitor_connection_vm() if reader is None and writer is None: return result @@ -1405,7 +1405,7 @@ class QemuVM(BaseNode): return for command in commands: - log.info(f"Execute QEMU monitor command: {command}") + log.debug(f"Execute QEMU monitor command: {command}") try: cmd_byte = command.encode("ascii") writer.write(cmd_byte + b"\n") diff --git a/gns3server/compute/ubridge/hypervisor.py b/gns3server/compute/ubridge/hypervisor.py index a702adb34..89d815100 100644 --- a/gns3server/compute/ubridge/hypervisor.py +++ b/gns3server/compute/ubridge/hypervisor.py @@ -18,11 +18,11 @@ Represents a uBridge hypervisor and starts/stops the associated uBridge process. """ -import sys import os +import socket import subprocess import asyncio -import socket +import tempfile import re from gns3server.utils import parse_version @@ -44,17 +44,42 @@ class Hypervisor(UBridgeHypervisor): :param project: Project instance :param path: path to uBridge executable :param working_dir: working directory - :param host: host/address for this hypervisor - :param port: port for this hypervisor + :param transport: control channel transport — "unix" (-U) or "tcp" (-H) + :param host: host/address for the TCP transport (unused for "unix") + :param node_id: node id used to name the AF_UNIX socket (unix transport) """ - _instance_count = 1 + _instance_count = 0 - def __init__(self, project, path, working_dir, host, port=None): + def __init__(self, project, path, working_dir, transport, host=None, node_id=None): - if port is None: + self._project = project + self._path = path + self._working_dir = working_dir + + if transport == "unix": + # AF_UNIX control socket (-U). Name it after the node so the socket + # is self-describing (one ubridge per node => node_id is unique). + # sun_path is capped at 107 bytes; a single UUID fits comfortably + # (~69 bytes with this prefix), so no project_id is needed. + if node_id: + socket_name = f"ubridge-{node_id}.sock" + else: + Hypervisor._instance_count += 1 + socket_name = f"ubridge-{Hypervisor._instance_count}.sock" + runtime_dir = os.environ.get("XDG_RUNTIME_DIR") or tempfile.gettempdir() + socket_dir = os.path.join(runtime_dir, "gns3") + try: + os.makedirs(socket_dir, mode=0o700, exist_ok=True) + os.chmod(socket_dir, 0o700) + except OSError as e: + raise UbridgeError(f"Could not create uBridge socket directory {socket_dir}: {e}") + socket_path = os.path.join(socket_dir, socket_name) + super().__init__(socket_path=socket_path) + else: + # TCP control channel (-H): let the OS find an unused local port. + port = None try: - port = None info = socket.getaddrinfo(host, 0, socket.AF_UNSPEC, socket.SOCK_STREAM, 0, socket.AI_PASSIVE) if not info: raise UbridgeError(f"getaddrinfo returns an empty list on {host}") @@ -68,11 +93,8 @@ class Hypervisor(UBridgeHypervisor): break except OSError as e: raise UbridgeError(f"Could not find free port for the uBridge hypervisor: {e}") + super().__init__(host=host, port=port) - super().__init__(host, port) - self._project = project - self._path = path - self._working_dir = working_dir self._command = [] self._process = None self._stdout_file = "" @@ -131,19 +153,17 @@ class Hypervisor(UBridgeHypervisor): async def _check_ubridge_version(self, env=None): """ - Checks if the ubridge executable version + Checks if the ubridge executable version meets the minimum required. """ try: output = await subprocess_check_output(self._path, "-v", cwd=self._working_dir, env=env) match = re.search(r"ubridge version ([0-9a-z\.]+)", output) if match: self._version = match.group(1) - if sys.platform.startswith("darwin"): - minimum_required_version = "0.9.12" - else: - # uBridge version 0.9.14 is required for packet filters - # to work for IOU nodes. - minimum_required_version = "0.9.14" + # uBridge >= 1.2.0 is required for features this server now + # relies on: the AF_UNIX control channel (-U), the marker + # (mark) filter, and the brctl-backed builtin Ethernet Switch. + minimum_required_version = "1.2.0" if parse_version(self._version) < parse_version(minimum_required_version): raise UbridgeError(f"uBridge executable version must be >= {minimum_required_version}") else: @@ -169,6 +189,17 @@ class Hypervisor(UBridgeHypervisor): ) log.info(f"ubridge started PID={self._process.pid}") + # An unsupported flag (e.g. -U on an old ubridge build) makes ubridge exit + # immediately with a non-zero code. Detect that here and surface the real + # reason from ubridge.log instead of waiting for connect() to time out with + # a confusing "couldn't connect" error. + await asyncio.sleep(0.3) + if self._process.returncode is not None: + raise UbridgeError( + f"uBridge exited immediately (code {self._process.returncode}); if " + f"ubridge_control_transport is 'unix', the installed ubridge may not " + f"support -U.\n{self.read_stdout()}" + ) # recv: Bad address is received by uBridge when a docker image stops by itself # see https://github.com/GNS3/gns3-gui/issues/2957 # monitor_process(self._process, self._termination_callback) @@ -214,6 +245,16 @@ class Hypervisor(UBridgeHypervisor): os.remove(self._stdout_file) except OSError as e: log.warning(f"could not delete temporary uBridge log file: {e}") + + # ubridge unlinks its AF_UNIX control socket on a clean exit; for the + # unix transport remove it here too so a killed process leaves no stale + # socket behind. The TCP transport has no socket_path. + if self._socket_path: + try: + os.unlink(self._socket_path) + except OSError: + pass + self._process = None self._started = False @@ -250,7 +291,10 @@ class Hypervisor(UBridgeHypervisor): """ command = [self._path] - command.extend(["-H", f"{self._host}:{self._port}"]) + if self._socket_path: + command.extend(["-U", self._socket_path]) + else: + command.extend(["-H", f"{self._host}:{self._port}"]) if log.getEffectiveLevel() == logging.DEBUG: command.extend(["-d", "1"]) return command diff --git a/gns3server/compute/ubridge/ubridge_hypervisor.py b/gns3server/compute/ubridge/ubridge_hypervisor.py index a44bf3834..83f765d16 100644 --- a/gns3server/compute/ubridge/ubridge_hypervisor.py +++ b/gns3server/compute/ubridge/ubridge_hypervisor.py @@ -28,20 +28,29 @@ log = logging.getLogger(__name__) class UBridgeHypervisor: """ - Creates a new connection to uBridge hypervisor. + Creates a new connection to a uBridge hypervisor control channel. - :param host: the hostname or ip address string of the uBridge hypervisor - :param port: the tcp port integer + Two transports, selected by which argument is set: + * ``socket_path`` -> AF_UNIX (``-U``), authenticated in-kernel via + SO_PEERCRED (ubridge accepts only its own UID; the compute process that + spawned it shares that UID). Recommended on Linux. + * ``host``/``port`` -> TCP (``-H``), retained for backward compatibility. + + :param socket_path: path to the uBridge AF_UNIX control socket (None for TCP) + :param host: TCP hostname/IP (None for AF_UNIX) + :param port: TCP port :param timeout: timeout integer for how long to wait for a response to commands sent to the - hypervisor (defaults to 30 seconds) + hypervisor (defaults to 30 seconds) """ # Used to parse Ubridge response codes error_re = re.compile(r"""^2[0-9]{2}-""") success_re = re.compile(r"""^1[0-9]{2}\s{1}""") - def __init__(self, host, port, timeout=30.0): + def __init__(self, socket_path=None, host=None, port=None, timeout=30.0): + # Exactly one transport is active: socket_path (AF_UNIX) or host/port (TCP). + self._socket_path = socket_path self._host = host self._port = port self._version = "N/A" @@ -54,22 +63,23 @@ class UBridgeHypervisor: Connects to the hypervisor. """ - # connect to a local address by default - # if listening to all addresses (IPv4 or IPv6) - if self._host == "0.0.0.0": - host = "127.0.0.1" - elif self._host == "::": - host = "::1" - else: - host = self._host - begin = time.time() connection_success = False last_exception = None while time.time() - begin < timeout: await asyncio.sleep(0.1) try: - self._reader, self._writer = await asyncio.open_connection(host, self._port) + if self._socket_path: + self._reader, self._writer = await asyncio.open_unix_connection(self._socket_path) + else: + # connect to a local address by default if listening on all addresses + if self._host == "0.0.0.0": + host = "127.0.0.1" + elif self._host == "::": + host = "::1" + else: + host = self._host + self._reader, self._writer = await asyncio.open_connection(host, self._port) except OSError as e: last_exception = e continue @@ -77,9 +87,9 @@ class UBridgeHypervisor: break if not connection_success: - raise UbridgeError(f"Couldn't connect to hypervisor on {host}:{self._port} :{last_exception}") + raise UbridgeError(f"Couldn't connect to hypervisor on {self.endpoint} :{last_exception}") else: - log.info(f"Connected to uBridge hypervisor on {host}:{self._port} after {time.time() - begin:.4f} seconds") + log.info(f"Connected to uBridge hypervisor on {self.endpoint} after {time.time() - begin:.4f} seconds") try: await asyncio.sleep(0.1) @@ -122,7 +132,7 @@ class UBridgeHypervisor: await self._writer.drain() self._writer.close() except OSError as e: - log.debug(f"Stopping hypervisor {self._host}:{self._port} {e}") + log.debug(f"Stopping hypervisor {self.endpoint} {e}") self._reader = self._writer = None async def reset(self): @@ -133,44 +143,17 @@ class UBridgeHypervisor: await self.send("hypervisor reset") @property - def port(self): + def endpoint(self): """ - Returns the port used to start the hypervisor. + Returns a human-readable control endpoint: the AF_UNIX socket path when + using -U, or host:port when using -H. Used for logging and errors. - :returns: port number (integer) + :returns: endpoint (string) """ - return self._port - - @port.setter - def port(self, port): - """ - Sets the port used to start the hypervisor. - - :param port: port number (integer) - """ - - self._port = port - - @property - def host(self): - """ - Returns the host (binding) used to start the hypervisor. - - :returns: host/address (string) - """ - - return self._host - - @host.setter - def host(self, host): - """ - Sets the host (binding) used to start the hypervisor. - - :param host: host/address (string) - """ - - self._host = host + if self._socket_path: + return self._socket_path + return f"{self._host}:{self._port}" @locking async def send(self, command): @@ -205,8 +188,8 @@ class UBridgeHypervisor: await self._writer.drain() except OSError as e: raise UbridgeError( - "Lost communication with {host}:{port} when sending command '{command}': {error}, uBridge process running: {run}".format( - host=self._host, port=self._port, command=command, error=e, run=self.is_running() + "Lost communication with {endpoint} when sending command '{command}': {error}, uBridge process running: {run}".format( + endpoint=self.endpoint, command=command, error=e, run=self.is_running() ) ) @@ -232,8 +215,8 @@ class UBridgeHypervisor: if not chunk: if retries > max_retries: raise UbridgeError( - "No data returned from {host}:{port} after sending command '{command}', uBridge process running: {run}".format( - host=self._host, port=self._port, command=command, run=self.is_running() + "No data returned from {endpoint} after sending command '{command}', uBridge process running: {run}".format( + endpoint=self.endpoint, command=command, run=self.is_running() ) ) else: @@ -244,8 +227,8 @@ class UBridgeHypervisor: buf += chunk.decode("utf-8") except OSError as e: raise UbridgeError( - "Lost communication with {host}:{port} after sending command '{command}': {error}, uBridge process running: {run}".format( - host=self._host, port=self._port, command=command, error=e, run=self.is_running() + "Lost communication with {endpoint} after sending command '{command}': {error}, uBridge process running: {run}".format( + endpoint=self.endpoint, command=command, error=e, run=self.is_running() ) ) @@ -255,8 +238,8 @@ class UBridgeHypervisor: continue except IndexError: raise UbridgeError( - "Could not communicate with {host}:{port} after sending command '{command}', uBridge process running: {run}".format( - host=self._host, port=self._port, command=command, run=self.is_running() + "Could not communicate with {endpoint} after sending command '{command}', uBridge process running: {run}".format( + endpoint=self.endpoint, command=command, run=self.is_running() ) ) diff --git a/gns3server/config_samples/gns3_server.conf b/gns3server/config_samples/gns3_server.conf index a8db5004c..50a6ca01a 100644 --- a/gns3server/config_samples/gns3_server.conf +++ b/gns3server/config_samples/gns3_server.conf @@ -92,6 +92,12 @@ udp_end_port_range = 30000 ; uBridge executable location, default: search in PATH ;ubridge_path = ubridge +; uBridge control channel transport: "unix" (-U socket_path; AF_UNIX + +; SO_PEERCRED, default — recommended on Linux for kernel-level peer +; authentication) or "tcp" (-H host:port; retained for backward compatibility, +; binds loopback). +;ubridge_control_transport = unix + ; Marker (traffic-insight) UDP sink: one listener per compute process that ; receives uBridge MARK signals from every uBridge on this host. ; marker_listen_host defaults to 127.0.0.1 because uBridge runs locally. diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index 008deda4d..e716bfd03 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -30,6 +30,13 @@ import logging log = logging.getLogger(__name__) +# Sentinel for "argument not passed". Distinct from None so marker/definition +# updaters can tell "caller omitted direction" (keep current value) from +# "caller passed direction=None" (clear it back to both directions). See +# UDPLink.update_marker and Project.update_marker_definition. +_UNSET = object() + + FILTERS = [ { "type": "frequency_drop", @@ -107,7 +114,7 @@ class Link: """ return self._markers - async def inherit_marker(self, def_name, marker_def): + async def inherit_marker(self, def_name, marker_def, dump=True): """ Apply a project-level marker definition to this link. @@ -115,15 +122,32 @@ class Link: with a per-link private marker of the same name. It carries an ``inherited_from`` back-reference that (a) guards against per-link edits and (b) lets the project sync changes to every copy at once. + + The pcap link-layer follows the link type: Ethernet is always EN10MB. + A serial link needs the definition's WAN encapsulation (HDLC / PPP / + Frame Relay); if none was chosen the serial link is skipped — an EN10MB + pcap on a serial link is undecodable. """ + def_data_link_type = marker_def.get("data_link_type", "DLT_EN10MB") + if self._link_type == "serial": + if def_data_link_type.upper() == "DLT_EN10MB": + return # definition is Ethernet-only; skip this serial link + data_link_type = def_data_link_type + else: + data_link_type = "DLT_EN10MB" + await self.start_marker( name=f"global-{def_name}", bpf=marker_def["bpf"], tag=marker_def.get("tag"), + direction=marker_def.get("direction"), + data_link_type=data_link_type, color=marker_def.get("color"), highlight_duration=marker_def.get("highlight_duration"), + enabled=not marker_def.get("paused", False), inherited_from=def_name, + dump=dump, ) def _persist_markers(self): @@ -333,7 +357,7 @@ class Link: raise NotImplementedError - async def start_marker(self, name, bpf, tag=None): + async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None, enabled=True): """ Attach a traffic-insight marker to this link (base — UDPLink overrides). """ @@ -345,7 +369,7 @@ class Link: """ raise NotImplementedError - async def update_marker(self, name, bpf=None, tag=None, enabled=None): + async def update_marker(self, name, bpf=None, tag=None, enabled=None, direction=_UNSET): """ Update an existing marker's BPF, tag, or enabled flag. diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 9ada6506a..e6a0ffd6a 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -37,6 +37,7 @@ from .snapshot import Snapshot from .drawing import Drawing from .topology import project_to_topology, load_topology from .udp_link import UDPLink +from .link import _UNSET from ..config import Config from ..utils.path import check_path_allowed, get_default_project_directory from ..utils.application_id import get_next_application_id @@ -790,7 +791,9 @@ class Project: "tag": marker.get("tag"), "enabled": marker.get("enabled", True), "color": marker.get("color"), + "highlight_duration": marker.get("highlight_duration"), "capture_node_id": marker.get("capture_node_id"), + "direction": marker.get("direction"), } if "link_style" in link_data: await link.update_link_style(link_data["link_style"]) @@ -923,6 +926,50 @@ class Project: } return result + async def pause_marker_definition(self, name): + """ + Pause every inherited copy of a definition (``global-{name}``) on every + link: toggle each filter off in place via ``update_marker(enabled=False)`` + — uBridge ``enable_packet_filter off``, no NIO rebuild, pcap/emitted + preserved. The definition's ``paused`` flag is persisted, so links + created later inherit the marker already paused. + """ + + if name not in self._marker_definitions: + raise ControllerError(f"Marker definition '{name}' not found") + self._marker_definitions[name]["paused"] = True + marker_name = f"global-{name}" + affected = [ + link for link in self._links.values() + if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name + ] + await self._marker_apply_concurrently( + affected, + lambda link: link.update_marker(marker_name, enabled=False, inherited=True, dump=False), + lambda link, e: f"Failed to pause marker {marker_name} on link {link.id}: {e}", + ) + self.dump() + self.emit_notification("project.updated", self.asdict()) + + async def resume_marker_definition(self, name): + """Resume every inherited copy of a definition (toggle on).""" + + if name not in self._marker_definitions: + raise ControllerError(f"Marker definition '{name}' not found") + self._marker_definitions[name]["paused"] = False + marker_name = f"global-{name}" + affected = [ + link for link in self._links.values() + if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name + ] + await self._marker_apply_concurrently( + affected, + lambda link: link.update_marker(marker_name, enabled=True, inherited=True, dump=False), + lambda link, e: f"Failed to resume marker {marker_name} on link {link.id}: {e}", + ) + self.dump() + self.emit_notification("project.updated", self.asdict()) + @property def marker_definitions(self): """ @@ -930,7 +977,42 @@ class Project: """ return self._marker_definitions - async def create_marker_definition(self, name, bpf, tag=None, color=None, highlight_duration=None): + def _validate_marker_definition_bpf(self, name, bpf): + """ + Validate a marker definition's BPF once, here, so the fan-out to every + link (``_apply_def_to_all_links`` → ``inherit_marker`` → ``start_marker``) + and the per-link sync (``update_marker_definition`` → ``update_marker``) + can skip re-validation for the inherited copies — otherwise one + ``tcpdump -d`` subprocess runs per link for the same expression. A + private per-link marker still validates in ``start_marker``/``update_marker``. + """ + result = validate_bpf_syntax(bpf) + if not result.get("valid"): + raise ControllerError( + f"Marker definition '{name}': invalid BPF — {result.get('error', 'unknown error')}" + ) + + def _validate_marker_definition_direction(self, name, direction): + """ + Reject tx/rx on a marker definition: a definition fans out to every link + and auto-selects its capture node on each (``_choose_marker_side``), + while tx/rx is relative to that node, so a fixed direction has no + consistent meaning across links. Only 'both' (the default, = ``None``) + is allowed — encode the direction in the BPF instead (e.g. + ``icmp[icmptype]==8`` for echo requests), or use a per-link marker whose + capture node is pinned. + """ + if direction in ("tx", "rx"): + raise ControllerError( + f"Marker definition '{name}': direction '{direction}' is not allowed. " + "A definition fans out to every link and auto-selects its capture node on each, " + "but tx/rx is relative to that node, so a fixed direction has no consistent " + "meaning across links. Keep 'both' (the default) and encode the direction in " + "the BPF instead, e.g. 'icmp and icmp[icmptype]==8' for echo requests only. " + "For a capture-node-relative direction on a single link, use a per-link marker." + ) + + async def create_marker_definition(self, name, bpf, tag=None, direction=None, color=None, highlight_duration=None, data_link_type="DLT_EN10MB"): """ Create a project-level marker definition and fan out to every existing link that has a capable node. Links without a capable node are silently @@ -942,12 +1024,14 @@ class Project: f"Marker definition '{name}' already exists in this project" ) - self._marker_definitions[name] = {"bpf": bpf, "tag": tag, "color": color, "highlight_duration": highlight_duration} + self._validate_marker_definition_bpf(name, bpf) + self._validate_marker_definition_direction(name, direction) + self._marker_definitions[name] = {"bpf": bpf, "tag": tag, "direction": direction, "color": color, "highlight_duration": highlight_duration, "data_link_type": data_link_type, "paused": False} await self._apply_def_to_all_links(name) self.dump() self.emit_notification("project.updated", self.asdict()) - async def update_marker_definition(self, name, bpf=None, tag=None, color=None, highlight_duration=None): + async def update_marker_definition(self, name, bpf=None, tag=None, direction=_UNSET, color=None, highlight_duration=None, data_link_type=_UNSET): """ Update a marker definition and sync every inherited copy on every link. """ @@ -959,6 +1043,7 @@ class Project: d = self._marker_definitions[name] if bpf is not None: + self._validate_marker_definition_bpf(name, bpf) d["bpf"] = bpf if tag is not None: d["tag"] = tag @@ -966,15 +1051,40 @@ class Project: d["color"] = color if highlight_duration is not None: d["highlight_duration"] = highlight_duration + if direction is not _UNSET: + self._validate_marker_definition_direction(name, direction) + d["direction"] = direction # None = clear back to both directions + if data_link_type is not _UNSET: + d["data_link_type"] = data_link_type - # Sync: update every inherited copy across all links. - for link in list(self._links.values()): - marker_name = f"global-{name}" - if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name: - await link.update_marker( - marker_name, bpf=d["bpf"], tag=d.get("tag"), color=d.get("color"), - highlight_duration=d.get("highlight_duration"), inherited=True - ) + # Links that currently carry an inherited copy of this definition. + affected = [ + link for link in self._links.values() + if f"global-{name}" in link.markers + and link.markers[f"global-{name}"].get("inherited_from") == name + ] + + if data_link_type is not _UNSET: + # data_link_type decides which links host an inherited copy (serial + # links are skipped unless a WAN encapsulation is chosen), so a change + # needs a full re-fan-out: drop every copy, then re-apply. + await self._marker_apply_concurrently( + affected, + lambda link: link.stop_marker(f"global-{name}", inherited=True, dump=False), + lambda link, e: f"Failed to remove inherited marker global-{name} from link {link.id}: {e}", + ) + await self._apply_def_to_all_links(name) + else: + # Sync: update every inherited copy across all links. + await self._marker_apply_concurrently( + affected, + lambda link: link.update_marker( + f"global-{name}", bpf=d["bpf"], tag=d.get("tag"), direction=d.get("direction"), + color=d.get("color"), highlight_duration=d.get("highlight_duration"), inherited=True, + dump=False + ), + lambda link, e: f"Failed to sync marker global-{name} on link {link.id}: {e}", + ) self.dump() self.emit_notification("project.updated", self.asdict()) @@ -990,17 +1100,17 @@ class Project: del self._marker_definitions[name] - for link in list(self._links.values()): - marker_name = f"global-{name}" - if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name: - try: - await link.stop_marker(marker_name, inherited=True) - except ControllerError: - # A missing compute or broken link shouldn't block the delete. - log.warning( - "Failed to remove inherited marker %s from link %s", - marker_name, link.id - ) + affected = [ + link for link in self._links.values() + if f"global-{name}" in link.markers + and link.markers[f"global-{name}"].get("inherited_from") == name + ] + await self._marker_apply_concurrently( + affected, + lambda link: link.stop_marker(f"global-{name}", inherited=True), + # A missing compute or broken link shouldn't block the delete. + lambda link, e: f"Failed to remove inherited marker global-{name} from link {link.id}: {e}", + ) self.dump() self.emit_notification("project.updated", self.asdict()) @@ -1013,32 +1123,64 @@ class Project: """ d = self._marker_definitions[def_name] - for link in list(self._links.values()): - try: - await link.inherit_marker(def_name, d) - except ControllerError as e: - # Per-link failures (e.g. no capable node) shouldn't block the - # definition from serving the rest. - log.warning( - "Marker definition '%s' could not be applied to link %s: %s", - def_name, link.id, e - ) + # dump=False: per-link topology writes are the dominant cost on large + # projects — the callers (create/update_marker_definition) dump once + # after the fan-out. + await self._marker_apply_concurrently( + list(self._links.values()), + lambda link: link.inherit_marker(def_name, d, dump=False), + lambda link, e: f"Marker definition '{def_name}' could not be applied to link {link.id}: {e}", + ) async def apply_defs_to_new_link(self, link): """ Apply every active marker definition to a newly created link so it inherits project-level rules automatically. + + Deliberately serial: all definitions share the same link, and each + ``inherit_marker`` pushes the link's full marker set — concurrent + pushes would race (a later push overwriting an earlier one's spec and + losing markers). """ for def_name, d in self._marker_definitions.items(): try: - await link.inherit_marker(def_name, d) + # dump=False: the caller (link create / project open) dumps once + # after; per-def dumps here would be N full topology writes. + await link.inherit_marker(def_name, d, dump=False) except ControllerError as e: log.warning( "Marker definition '%s' could not be applied to new link %s: %s", def_name, link.id, e ) + async def _marker_apply_concurrently(self, links, operation, fail_msg): + """ + Run an async per-link marker operation across *links* with bounded + concurrency. A serial loop takes N sequential compute round-trips — a + definition over 1000 links would take minutes on remote computes — so + fan out in parallel batches. Links are independent (own ``_markers`` / + ``_link_data``), so this is race-free; per-link ``ControllerError`` is + logged and skipped, preserving the serial loop's isolation semantics. + ``Project.dump`` is synchronous and writes atomically (tmp + rename), + so concurrent dumps from the fan-out cannot corrupt the topology file. + + :param links: iterable of links to operate on + :param operation: async callable ``(link) -> coroutine`` + :param fail_msg: callable ``(link, error) -> log message`` + """ + + sem = asyncio.Semaphore(32) + + async def guarded(link): + async with sem: + try: + await operation(link) + except ControllerError as e: + log.warning(fail_msg(link, e)) + + await asyncio.gather(*(guarded(link) for link in links)) + @property def snapshots(self): """ @@ -1430,10 +1572,27 @@ class Project: setattr(self, key, val) # marker_definitions is loaded separately (it is not a __init__ kwarg - # nor a simple attribute — it backs a read-only property). + # nor a simple attribute — it backs a read-only property). Each BPF + # is validated once here so the inherited fan-out (start_marker) can + # skip re-validation; an invalid definition is dropped with a warning + # rather than failing the open — it could not fan out anyway. defs = project_data.get("marker_definitions") if isinstance(defs, dict): - self._marker_definitions = defs + clean_defs = {} + for def_name, d in defs.items(): + bpf = d.get("bpf") + if not bpf: + log.warning("Dropping marker definition '%s' on load: missing bpf", def_name) + continue + result = validate_bpf_syntax(bpf) + if not result.get("valid"): + log.warning( + "Dropping marker definition '%s' on load: invalid BPF (%s)", + def_name, result.get("error") + ) + continue + clean_defs[def_name] = d + self._marker_definitions = clean_defs topology = project_data["topology"] for compute in topology.get("computes", []): diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 609c17495..b25e1126b 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -17,7 +17,7 @@ from .controller_error import ControllerError, ControllerNotFoundError -from .link import Link +from .link import Link, _UNSET from .node_types import BUILTIN_NODE_TYPES from gns3server.utils.packet_filter_validation import validate_bpf_syntax, FilterValidationError @@ -57,14 +57,20 @@ class UDPLink(Link): def _markers_for_node(self, node): """ - Marker specs (name -> {bpf, tag, link_id}) for the markers whose capture - side is ``node`` and that are enabled. Routed by capture_node_id so a - marker only rides the NIO of the node whose uBridge will host it. + Marker specs (name -> {bpf, tag, link_id, direction, data_link_type, + enabled}) for the markers whose capture side is ``node``. Routed by + capture_node_id so a marker only rides the NIO of the node whose uBridge + will host it. A disabled marker is included (installed then turned + ``off`` at uBridge, not dropped) so the UI can toggle it instantly + without an NIO rebuild. """ return { - name: {"bpf": m["bpf"], "tag": m.get("tag"), "link_id": self._id} + name: {"bpf": m["bpf"], "tag": m.get("tag"), "link_id": self._id, + "direction": m.get("direction"), + "data_link_type": m.get("data_link_type", "DLT_EN10MB"), + "enabled": m.get("enabled", True)} for name, m in self._markers.items() - if m.get("enabled", True) and m.get("capture_node_id") == node.id + if m.get("capture_node_id") == node.id } def _get_node_markers(self, node1, node2): @@ -310,6 +316,31 @@ class UDPLink(Link): "traffic insight" ) + def _node_by_id(self, node_id): + """ + Resolve a caller-chosen capture node by id, validating it is an + endpoint of this link and marker-capable. Used when the caller + (REST/MCP) explicitly pins the observer side instead of letting + ``_choose_marker_side`` auto-pick. + + :param node_id: node id (UUID or str) the caller requested + :returns: a ``self._nodes`` entry (node/adapter_number/port_number) + """ + + target = str(node_id) + for node in self._nodes: + if str(node["node"].id) != target: + continue + if node["node"].node_type not in _MARKER_CAPABLE_TYPES: + raise ControllerError( + f"Node {node_id} ({node['node'].node_type}) cannot host a " + f"marker — no uBridge bridge to attach the filter to" + ) + return node + raise ControllerNotFoundError( + f"Node {node_id} is not an endpoint of link {self._id}" + ) + async def node_updated(self, node): """ Called when a node member of the link is updated @@ -322,7 +353,7 @@ class UDPLink(Link): # explicitly deletes a marker via the REST API, and a marker is torn # down automatically only when its link is deleted. - async def start_marker(self, name, bpf, tag=None, color=None, highlight_duration=None, inherited_from=None): + async def start_marker(self, name, bpf, tag=None, direction=None, data_link_type="DLT_EN10MB", capture_node_id=None, color=None, highlight_duration=None, enabled=True, inherited_from=None, dump=True): """ Attach a traffic-insight marker to this link. @@ -335,6 +366,11 @@ class UDPLink(Link): :param name: stable filter name — echoed in MARK signals + pcap identity :param bpf: libpcap BPF expression :param tag: optional correlation id + :param capture_node_id: optional explicit observer node. When set the + marker is pinned to that endpoint's uBridge (and ``direction`` is + interpreted from its perspective); validated by ``_node_by_id``. + Omitted = auto-pick via ``_choose_marker_side``. Ignored for + inherited markers (project defs are link-agnostic → always auto). :param color: optional hex color for the Web UI (e.g. '#ff5722'); stored with the link and persisted in the topology, never sent to uBridge :param highlight_duration: optional UI-only hint (milliseconds) for how @@ -346,18 +382,29 @@ class UDPLink(Link): if name in self._markers: raise ControllerError(f"Marker '{name}' already exists on link {self._id}") - result = validate_bpf_syntax(bpf) - if not result.get("valid"): - raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}") + # 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 + # re-validating per link would spawn one ``tcpdump -d`` per link for the + # same expression. + if not inherited_from: + result = validate_bpf_syntax(bpf) + if not result.get("valid"): + raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}") - marker_side = self._choose_marker_side() + if capture_node_id and not inherited_from: + marker_side = self._node_by_id(capture_node_id) + else: + marker_side = self._choose_marker_side() marker_entry = { "bpf": bpf, "tag": tag, - "enabled": True, + "enabled": enabled, "color": color, "highlight_duration": highlight_duration, "capture_node_id": marker_side["node"].id, + "direction": direction, + "data_link_type": data_link_type, } if inherited_from: marker_entry["inherited_from"] = inherited_from @@ -365,9 +412,12 @@ class UDPLink(Link): if self._created: await self.update() self._project.emit_notification("link.updated", self.asdict()) - self._project.dump() + # Bulk fan-out passes dump=False: N per-link topology writes on a + # 500-link project are the dominant cost — the caller dumps once after. + if dump: + self._project.dump() - async def stop_marker(self, name, inherited=False): + async def stop_marker(self, name, inherited=False, dump=True): """ Remove a traffic-insight marker from this link. @@ -390,17 +440,33 @@ class UDPLink(Link): "Delete or update it via the marker-definitions API instead." ) + capture_node_id = self._markers[name].get("capture_node_id") del self._markers[name] - if self._created: - await self.update() + # Remove the marker filter + its pcap on the capture node directly — NOT a + # full NIO reapply (which would reset_packet_filters and close/reopen every + # sibling marker's pcap). delete_packet_filter removes just this filter; + # the marker is already gone from _markers, so any later reapply (filter + # change, node restart) won't re-add it either. + if capture_node_id is not None: + side = next((s for s in self._nodes if str(s["node"].id) == str(capture_node_id)), None) + if side is not None: + try: + await side["node"].delete( + f"/adapters/{side['adapter_number']}/ports/{side['port_number']}/markers/{name}", + params={"link_id": self._id}, + ) + except Exception: + pass # best-effort: old compute without the route leaves the file self._project.emit_notification("link.updated", self.asdict()) - self._project.dump() + if dump: + self._project.dump() - async def update_marker(self, name, bpf=None, tag=None, enabled=None, color=None, highlight_duration=None, inherited=False): + async def update_marker(self, name, bpf=None, tag=None, enabled=None, direction=_UNSET, color=None, highlight_duration=None, inherited=False, dump=True): """ - Update an existing marker's BPF/tag/enabled/color. Any change pushes via - ``self.update()``; uBridge picks up the new params on the next NIO - reset+reapply (same as packet filters). + Update an existing marker's fields and push to uBridge fine-grained — no + full NIO reapply, so sibling markers' pcaps stay open. bpf/tag/direction + rebuild just this filter (delete + add); enabled is an instant toggle; + color/highlight_duration are UI-only (stored, never pushed). :param name: filter name to update :param bpf: new BPF expression (None = keep existing) @@ -423,10 +489,15 @@ class UDPLink(Link): "Update it via the marker-definitions API instead." ) + # Merge every changed field into the marker state first. if bpf is not None and bpf != marker_info["bpf"]: - result = validate_bpf_syntax(bpf) - if not result.get("valid"): - raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}") + # An inherited marker is synced from a definition whose BPF was + # already validated at create/update (or load); re-validating per + # link is redundant. Private markers validate here as before. + if not inherited: + result = validate_bpf_syntax(bpf) + if not result.get("valid"): + raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}") marker_info["bpf"] = bpf if tag is not None: marker_info["tag"] = tag @@ -436,8 +507,38 @@ class UDPLink(Link): marker_info["color"] = color if highlight_duration is not None: marker_info["highlight_duration"] = highlight_duration + if direction is not _UNSET: + marker_info["direction"] = direction # None = clear back to both directions + # Push to uBridge fine-grained — NO full NIO reapply (which would + # reset_packet_filters and close/reopen every sibling marker's pcap): + # * bpf/tag/direction changed → rebuild just this filter (delete + add), + # reopening only this marker's pcap (expected, new BPF) + # * only enabled changed → instant toggle (enable_packet_filter) + # * only UI fields changed → nothing to push to uBridge if self._created: - await self.update() + ubridge_rebuild = (bpf is not None) or (tag is not None) or (direction is not _UNSET) + capture_node_id = marker_info.get("capture_node_id") + side = next((s for s in self._nodes if str(s["node"].id) == str(capture_node_id)), None) + if side is not None: + try: + if ubridge_rebuild: + await side["node"].put( + f"/markers/{name}/rebuild", + data={ + "bpf": marker_info["bpf"], + "tag": marker_info.get("tag"), + "direction": marker_info.get("direction"), + "enabled": marker_info.get("enabled", True), + "link_id": self._id, + }, + ) + elif enabled is not None: + await side["node"].put(f"/markers/{name}", data={"enabled": enabled}) + except Exception: + # Old compute without the route / node down: state is already + # correct in _markers; the next NIO reapply converges uBridge. + pass self._project.emit_notification("link.updated", self.asdict()) - self._project.dump() + if dump: + self._project.dump() diff --git a/gns3server/main.py b/gns3server/main.py index bb30a2d81..4069f5e3a 100644 --- a/gns3server/main.py +++ b/gns3server/main.py @@ -31,6 +31,8 @@ import os import sys import asyncio import argparse +import logging +import resource def daemonize(): @@ -97,6 +99,34 @@ def parse_arguments(argv): return parser, args +log = logging.getLogger(__name__) + + +def _raise_open_files_limit(target=65535): + """ + Raise RLIMIT_NOFILE at startup so large topologies don't hit EMFILE. + Every started node holds ~3 file descriptors in the server's table + (pidfd + stdout/stderr pipes per child process), so a few hundred nodes + exhaust the default 1024 limit. Best-effort: the hard limit caps what we + can request; failures are logged but never fatal. Runs before daemonize() + so the daemon inherits the raised limit. + """ + try: + soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE) + if soft >= target: + return + new_soft = min(target, hard) + resource.setrlimit(resource.RLIMIT_NOFILE, (new_soft, hard)) + if new_soft < target: + log.warning( + f"Open-files limit raised to {new_soft} (hard limit), below the requested {target}" + ) + else: + log.info(f"Open-files limit raised from {soft} to {new_soft}") + except (OSError, ValueError) as e: + log.warning(f"Could not raise the open-files limit: {e}") + + def main(): """ Entry point for GNS3 server @@ -104,6 +134,7 @@ def main(): if sys.platform.startswith("win"): raise SystemExit("Windows is not a supported platform to run the GNS3 server") + _raise_open_files_limit() if "--daemon" in sys.argv: daemonize() diff --git a/gns3server/schemas/__init__.py b/gns3server/schemas/__init__.py index 7519639bc..af026c241 100644 --- a/gns3server/schemas/__init__.py +++ b/gns3server/schemas/__init__.py @@ -20,7 +20,7 @@ from .common import ErrorMessage from .version import Version # Controller schemas -from .controller.links import LinkCreate, LinkUpdate, Link, UDPPortInfo, EthernetPortInfo, LinkCapture, MarkerCreate, MarkerDefinitionCreate +from .controller.links import LinkCreate, LinkUpdate, Link, UDPPortInfo, EthernetPortInfo, LinkCapture, MarkerCreate, MarkerUpdate, MarkerDefinitionCreate from .controller.computes import ComputeCreate, ComputeUpdate, ComputeVirtualBoxVM, ComputeVMwareVM, ComputeDockerImage, AutoIdlePC, Compute from .controller.templates import TemplateCreate, TemplateUpdate, TemplateUsage, Template from .controller.images import Image, ImageType @@ -91,7 +91,7 @@ from .controller.templates.dynamips_templates import ( ) # Compute schemas -from .compute.nios import UDPNIO, TAPNIO, EthernetNIO +from .compute.nios import UDPNIO, TAPNIO, EthernetNIO, MarkerToggle, MarkerRebuild from .compute.atm_switch_nodes import ATMSwitchCreate, ATMSwitchUpdate, ATMSwitch from .compute.cloud_nodes import CloudCreate, CloudUpdate, Cloud from .compute.docker_nodes import DockerCreate, DockerUpdate, Docker diff --git a/gns3server/schemas/compute/nios.py b/gns3server/schemas/compute/nios.py index f5cf5f073..5830847c5 100644 --- a/gns3server/schemas/compute/nios.py +++ b/gns3server/schemas/compute/nios.py @@ -65,3 +65,29 @@ class TAPNIO(BaseModel): type: TAPNIOType tap_device: str = Field(..., description="TAP device name e.g. tap0") + + +class MarkerToggle(BaseModel): + """ + Body for the per-marker enable/disable toggle endpoint: flips a running + uBridge marker filter with ``enable_packet_filter on|off`` (no NIO rebuild, + so the pcap identity and emitted counter are preserved). + """ + + enabled: bool + + +class MarkerRebuild(BaseModel): + """ + Body for the per-marker rebuild endpoint: re-install a single uBridge marker + filter with new BPF/tag/direction via ``delete_packet_filter`` + add (NOT a + bridge-wide reset), so sibling markers keep their pcaps open. The marker's + own pcap is reopened by uBridge on re-add (new capture session for the new + BPF), which is expected. + """ + + bpf: str + tag: Optional[int] = None + direction: Optional[str] = None + enabled: bool = True + link_id: str = "" diff --git a/gns3server/schemas/config.py b/gns3server/schemas/config.py index 851f0d4e3..0c712b351 100644 --- a/gns3server/schemas/config.py +++ b/gns3server/schemas/config.py @@ -113,6 +113,16 @@ class ServerProtocol(str, Enum): https = "https" +class UbridgeControlTransport(str, Enum): + + # TCP control channel: -H host:port. ubridge now binds loopback by default, + # so this is reachable only locally. Retained for backward compatibility. + tcp = "tcp" + # AF_UNIX control channel: -U socket_path, authenticated in-kernel via + # SO_PEERCRED (ubridge accepts only its own UID). Recommended on Linux. + unix = "unix" + + class BuiltinSymbolTheme(str, Enum): classic = "Classic" @@ -154,6 +164,11 @@ class ServerSettings(BaseModel): udp_start_port_range: int = Field(10000, gt=0, le=65535) udp_end_port_range: int = Field(30000, gt=0, le=65535) ubridge_path: str = "ubridge" + # Transport for the uBridge hypervisor control channel. "unix" (-U, + # AF_UNIX + SO_PEERCRED) is the default — recommended on Linux for + # kernel-level peer authentication. "tcp" (-H) is retained for backward + # compatibility. + ubridge_control_transport: UbridgeControlTransport = UbridgeControlTransport.unix # Marker (traffic-insight) UDP sink: one listener per compute process that # receives ubridge MARK signals from every ubridge on this host. The host # defaults to loopback because ubridge runs on the same host as the compute. diff --git a/gns3server/schemas/controller/links.py b/gns3server/schemas/controller/links.py index 23a0cd18a..fef63f8b1 100644 --- a/gns3server/schemas/controller/links.py +++ b/gns3server/schemas/controller/links.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator from typing import List, Optional, Tuple from enum import Enum from uuid import UUID, uuid4 @@ -152,7 +152,7 @@ class MarkerCreate(BaseModel): name: Optional[str] = Field( None, pattern=r"^[A-Za-z0-9][A-Za-z0-9_.-]*$", - max_length=128, + max_length=32, description='Unique marker name on the link. Auto-generated when absent.', ) bpf: str @@ -175,6 +175,66 @@ class MarkerCreate(BaseModel): None, description="Whether the marker is active. Defaults to true on creation.", ) + direction: Optional[str] = Field( + None, + pattern=r"^(tx|rx|both)$", + description="Direction filter: 'tx' = capture node sending only, 'rx' = capture node receiving only, 'both' or null = both directions.", + ) + capture_node_id: Optional[UUID] = Field( + None, + description=( + "Which endpoint's uBridge hosts this marker (the 'observer'). " + "tx/rx in `direction` are interpreted from this node's perspective. " + "Must be one of the link's two endpoints and a marker-capable type. " + "Omitted = server auto-picks (first started marker-capable endpoint)." + ), + ) + data_link_type: str = Field( + "DLT_EN10MB", + description=( + "pcap link-layer type the marker's BPF compiles against and its " + "capture file is written with (a uBridge `linktype` token). Defaults " + "to DLT_EN10MB (Ethernet), which is omitted from the uBridge command. " + "Only meaningful for serial links: set it to the matching serial DLT " + "from the port's data_link_types — DLT_C_HDLC / DLT_PPP_SERIAL / " + "DLT_FRELAY / DLT_ATM_RFC1483 — so the BPF offsets and pcap decode " + "match the encapsulation configured in IOS. Create-only (changing it " + "would invalidate the pcap)." + ), + ) + + @field_validator("direction", mode="before") + @classmethod + def _both_to_none(cls, v): + return None if v == "both" else v + + +class MarkerUpdate(BaseModel): + """ + Body for updating a marker — partial update, every field optional. + + ``bpf`` is optional here (it is required on create). ``capture_node_id`` and + ``name`` are create-only / path-driven and intentionally absent; an explicit + ``direction: null`` clears the direction back to both (omitting keeps it). + """ + + bpf: Optional[str] = None + tag: Optional[int] = None + direction: Optional[str] = Field( + None, + pattern=r"^(tx|rx|both)$", + description="Direction filter; 'both' or an explicit null clears it to both. Omit to keep.", + ) + color: Optional[str] = Field(None, description="Hex color render hint, e.g. '#ff5722'") + highlight_duration: Optional[int] = Field( + None, ge=1, description="UI highlight duration in ms; null = UI default" + ) + enabled: Optional[bool] = Field(None, description="Toggle the marker on/off (instant).") + + @field_validator("direction", mode="before") + @classmethod + def _both_to_none(cls, v): + return None if v == "both" else v class MarkerDefinitionCreate(BaseModel): @@ -189,7 +249,7 @@ class MarkerDefinitionCreate(BaseModel): name: Optional[str] = Field( None, pattern=r"^[A-Za-z0-9][A-Za-z0-9_.-]*$", - max_length=128, + max_length=32, description="Unique definition name. Auto-generated when absent.", ) bpf: str @@ -207,5 +267,26 @@ class MarkerDefinitionCreate(BaseModel): "stored with the definition, never sent to uBridge." ), ) + direction: Optional[str] = Field( + None, + pattern=r"^(tx|rx|both)$", + description="Direction filter: 'tx' = capture node sending only, 'rx' = capture node receiving only, 'both' or null = both directions.", + ) + data_link_type: str = Field( + "DLT_EN10MB", + description=( + "pcap link-layer type for inherited markers on serial links (uBridge " + "`linktype`). Defaults to DLT_EN10MB (Ethernet): the definition then " + "applies only to Ethernet links and serial links are skipped. Set a " + "serial DLT — DLT_C_HDLC / DLT_PPP_SERIAL / DLT_FRELAY / " + "DLT_ATM_RFC1483 — to also cover serial links with that encapsulation; " + "Ethernet links stay EN10MB regardless. Changing it re-fans-out." + ), + ) + + @field_validator("direction", mode="before") + @classmethod + def _both_to_none(cls, v): + return None if v == "both" else v diff --git a/tests/api/routes/controller/test_markers.py b/tests/api/routes/controller/test_markers.py index d9fc27b3b..53e57e7d9 100644 --- a/tests/api/routes/controller/test_markers.py +++ b/tests/api/routes/controller/test_markers.py @@ -105,6 +105,17 @@ class TestMarkerRoutes: ) assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + async def test_create_marker_name_too_long_rejected(self, app: FastAPI, client: AsyncClient, project: Project) -> None: + + link = UDPLink(project) + project._links = {link.id: link} + + response = await client.post( + app.url_path_for("create_marker", project_id=project.id, link_id=link.id), + json={"name": "x" * 33, "bpf": "icmp"}, # max_length is 32 + ) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + async def test_get_markers(self, app: FastAPI, client: AsyncClient, project: Project) -> None: link = UDPLink(project) diff --git a/tests/api/routes/mcp/test_handlers.py b/tests/api/routes/mcp/test_handlers.py index c181c1c87..fc8cd00d2 100644 --- a/tests/api/routes/mcp/test_handlers.py +++ b/tests/api/routes/mcp/test_handlers.py @@ -359,3 +359,157 @@ class TestTemplate: m.return_value = _mock_conn({}) result = delete_template_handler({"template_id": "t1"}, ctx) assert "deleted" in str(result).lower() + + +# ── Marker (traffic-insight) ──────────────────────────────────────────── + + +class TestLinkMarker: + """link_marker_handler direction tri-state: omit=preserve, tx/rx=set, both=clear (→ null).""" + + mod = "links" + + def test_update_direction_both_clears(self, ctx): + from gns3server.api.routes.mcp.links import link_marker_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "icmp"}) + m.return_value = conn + link_marker_handler( + {"project_id": "p", "link_id": "l", "action": "update", + "marker_name": "icmp", "direction": "both"}, ctx, + ) + conn.http_call.assert_called_with( + "put", "http://192.168.1.3:3080/v3/projects/p/links/l/markers/icmp", + json_data={"direction": None}, + ) + + def test_update_direction_tx_sets(self, ctx): + from gns3server.api.routes.mcp.links import link_marker_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "icmp"}) + m.return_value = conn + link_marker_handler( + {"project_id": "p", "link_id": "l", "action": "update", + "marker_name": "icmp", "direction": "tx"}, ctx, + ) + conn.http_call.assert_called_with( + "put", "http://192.168.1.3:3080/v3/projects/p/links/l/markers/icmp", + json_data={"direction": "tx"}, + ) + + def test_update_direction_omitted_preserved(self, ctx): + from gns3server.api.routes.mcp.links import link_marker_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "icmp"}) + m.return_value = conn + link_marker_handler( + {"project_id": "p", "link_id": "l", "action": "update", + "marker_name": "icmp", "tag": 1}, ctx, + ) + conn.http_call.assert_called_with( + "put", "http://192.168.1.3:3080/v3/projects/p/links/l/markers/icmp", + json_data={"tag": 1}, + ) + + def test_create_direction_both_omitted(self, ctx): + from gns3server.api.routes.mcp.links import link_marker_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "icmp"}) + m.return_value = conn + link_marker_handler( + {"project_id": "p", "link_id": "l", "action": "create", + "bpf": "icmp", "direction": "both"}, ctx, + ) + conn.http_call.assert_called_with( + "post", "http://192.168.1.3:3080/v3/projects/p/links/l/markers", + json_data={"bpf": "icmp"}, + ) + + def test_create_direction_tx(self, ctx): + from gns3server.api.routes.mcp.links import link_marker_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "icmp"}) + m.return_value = conn + link_marker_handler( + {"project_id": "p", "link_id": "l", "action": "create", + "bpf": "icmp", "direction": "tx"}, ctx, + ) + conn.http_call.assert_called_with( + "post", "http://192.168.1.3:3080/v3/projects/p/links/l/markers", + json_data={"bpf": "icmp", "direction": "tx"}, + ) + + +class TestMarkerDefinition: + """marker_definition_handler build create/update bodies. + + A definition has NO direction: it fans out to every link and auto-selects its + capture node on each, so tx/rx (relative to that node) has no consistent + meaning — any direction passed is ignored, never reaching the request body. + """ + + mod = "links" + + def test_create_builds_body(self, ctx): + from gns3server.api.routes.mcp.links import marker_definition_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "arp"}) + m.return_value = conn + marker_definition_handler( + {"project_id": "p", "action": "create", + "bpf": "arp", "tag": 1, "color": "#fff"}, ctx, + ) + conn.http_call.assert_called_with( + "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions", + json_data={"bpf": "arp", "tag": 1, "color": "#fff"}, + ) + + def test_create_ignores_direction(self, ctx): + from gns3server.api.routes.mcp.links import marker_definition_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "arp"}) + m.return_value = conn + marker_definition_handler( + {"project_id": "p", "action": "create", + "bpf": "arp", "direction": "tx"}, ctx, + ) + conn.http_call.assert_called_with( + "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions", + json_data={"bpf": "arp"}, + ) + + def test_update_builds_body(self, ctx): + from gns3server.api.routes.mcp.links import marker_definition_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "arp"}) + m.return_value = conn + marker_definition_handler( + {"project_id": "p", "action": "update", + "def_name": "arp", "tag": 1}, ctx, + ) + conn.http_call.assert_called_with( + "put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp", + json_data={"tag": 1}, + ) + + def test_update_ignores_direction(self, ctx): + from gns3server.api.routes.mcp.links import marker_definition_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn({"name": "arp"}) + m.return_value = conn + marker_definition_handler( + {"project_id": "p", "action": "update", + "def_name": "arp", "tag": 1, "direction": "rx"}, ctx, + ) + conn.http_call.assert_called_with( + "put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp", + json_data={"tag": 1}, + ) + + def test_update_requires_a_field(self, ctx): + from gns3server.api.routes.mcp.links import marker_definition_handler + with patch(f"{BASE}.{self.mod}._get_connector"): + result = marker_definition_handler( + {"project_id": "p", "action": "update", "def_name": "arp"}, ctx, + ) + assert "error" in result diff --git a/tests/compute/marker/test_marker_manager.py b/tests/compute/marker/test_marker_manager.py index 4a2796002..da495ae92 100644 --- a/tests/compute/marker/test_marker_manager.py +++ b/tests/compute/marker/test_marker_manager.py @@ -124,6 +124,8 @@ class TestMarkerListener: assert ev["tag"] == "7" assert ev["ts"] == pytest.approx(1700000000.123456) assert ev["len"] == 98 + # No dir= in the signal (legacy uBridge) → undirected. + assert ev["dir"] is None def test_unknown_node_dropped(self): fmgr = FakeMarkerManager() @@ -183,6 +185,34 @@ class TestMarkerListener: lis.datagram_received(b"MARK 3.0 node=n filter=f link=- tag=1 len=42\n", None) assert fmgr.events[0][1]["link_id"] == "registry-link" + def test_dir_tx_passthrough(self): + # dir=tx = capture node sending (matched packet ingressed the device-side NIO). + fmgr = FakeMarkerManager() + fmgr.register("p", "n", "f", "l", tag=1) + lis = MarkerListener(fmgr) + lis.connection_made(None) + lis.datagram_received(b"MARK 1.0 node=n filter=f tag=1 len=10 dir=tx\n", None) + assert fmgr.events[0][1]["dir"] == "tx" + + def test_dir_rx_passthrough(self): + # dir=rx = capture node receiving (matched packet ingressed the link-side NIO). + fmgr = FakeMarkerManager() + fmgr.register("p", "n", "f", "l", tag=1) + lis = MarkerListener(fmgr) + lis.connection_made(None) + lis.datagram_received(b"MARK 1.0 node=n filter=f tag=1 len=10 dir=rx\n", None) + assert fmgr.events[0][1]["dir"] == "rx" + + def test_dir_absent_is_none(self): + # Older uBridge builds omit dir; the event then carries None so the UI + # falls back to undirected rendering. + fmgr = FakeMarkerManager() + fmgr.register("p", "n", "f", "l", tag=1) + lis = MarkerListener(fmgr) + lis.connection_made(None) + lis.datagram_received(b"MARK 1.0 node=n filter=f tag=1 len=10\n", None) + assert fmgr.events[0][1]["dir"] is None + def test_exception_does_not_kill_listener(self): fmgr = FakeMarkerManager() lis = MarkerListener(fmgr) diff --git a/tests/compute/test_base_node.py b/tests/compute/test_base_node.py index 58c06ff5c..38371538d 100644 --- a/tests/compute/test_base_node.py +++ b/tests/compute/test_base_node.py @@ -15,12 +15,14 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import os from collections import OrderedDict import pytest import pytest_asyncio from tests.utils import asyncio_patch, AsyncioMagicMock +from unittest.mock import patch, MagicMock from gns3server.compute.vpcs.vpcs_vm import VPCSVM from gns3server.compute.docker.docker_vm import DockerVM @@ -172,3 +174,204 @@ async def test_ubridge_apply_bpf_filters(node): node._ubridge_send.assert_any_call("bridge reset_packet_filters VPCS-10") node._ubridge_send.assert_any_call("bridge add_packet_filter VPCS-10 filter0 bpf \"icmp[icmptype] == 8\"") node._ubridge_send.assert_any_call("bridge add_packet_filter VPCS-10 filter1 bpf \"tcp src port 53\"") + + +@pytest.mark.asyncio +async def test_set_marker_filter_state_off(compute_project, manager): + + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_send = AsyncioMagicMock() + node._marker_filter_bridges["m", "L"] = "VPCS-10" + await node._ubridge_set_marker_filter_state("m", False) + node._ubridge_send.assert_called_with("bridge enable_packet_filter VPCS-10 m off") + + +@pytest.mark.asyncio +async def test_set_marker_filter_state_on(compute_project, manager): + + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_send = AsyncioMagicMock() + node._marker_filter_bridges["m", "L"] = "VPCS-10" + await node._ubridge_set_marker_filter_state("m", True) + node._ubridge_send.assert_called_with("bridge enable_packet_filter VPCS-10 m on") + + +@pytest.mark.asyncio +async def test_marker_pause_sends_command(compute_project, manager): + + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_hypervisor = AsyncioMagicMock() + await node._ubridge_marker_pause() + node._ubridge_hypervisor.send.assert_called_with("marker pause") + + +@pytest.mark.asyncio +async def test_marker_resume_sends_command(compute_project, manager): + + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_hypervisor = AsyncioMagicMock() + await node._ubridge_marker_resume() + node._ubridge_hypervisor.send.assert_called_with("marker resume") + + +@pytest.mark.asyncio +async def test_apply_markers_turns_disabled_filter_off(compute_project, manager): + # Part A: a disabled marker is installed (add_packet_filter) then turned off + # with enable_packet_filter … off, and its bridge is recorded for toggling. + + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_send = AsyncioMagicMock() + nio = NIOUDP(1234, "127.0.0.1", 4321) + nio.markers = {"m": {"bpf": "icmp", "tag": None, "link_id": "L1", "direction": None, "enabled": False}} + with patch("gns3server.compute.marker.marker_manager.MarkerManager") as mm: + mm.instance.return_value.register = MagicMock() + await node._ubridge_apply_markers("VPCS-10", nio) + node._ubridge_send.assert_any_call("bridge enable_packet_filter VPCS-10 m off") + assert node._marker_filter_bridges["m", "L1"] == "VPCS-10" + + +def test_marker_linktype_normalizes(): + # Ethernet / unset → None (linktype omitted; uBridge defaults to EN10MB). + assert VPCSVM._marker_linktype(None) is None + assert VPCSVM._marker_linktype("") is None + assert VPCSVM._marker_linktype("DLT_EN10MB") is None + # Serial DLTs from SerialPort.data_link_types, DLT_ prefix stripped. + assert VPCSVM._marker_linktype("DLT_C_HDLC") == "C_HDLC" + assert VPCSVM._marker_linktype("DLT_PPP_SERIAL") == "PPP_SERIAL" + assert VPCSVM._marker_linktype("DLT_FRELAY") == "FRELAY" + assert VPCSVM._marker_linktype("DLT_ATM_RFC1483") == "ATM_RFC1483" + # Case-insensitive input → canonical uppercase (pcap_datalink_name_to_val is + # case-sensitive and expects the uppercase form). Shared by base_node and IOU. + assert VPCSVM._marker_linktype("dlt_c_hdlc") == "C_HDLC" + + +@pytest.mark.asyncio +async def test_apply_markers_appends_linktype_for_serial(compute_project, manager): + # A serial data_link_type reaches the uBridge mark command as `linktype C_HDLC` + # so the BPF offsets and pcap decode match the WAN encapsulation. + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_send = AsyncioMagicMock() + nio = NIOUDP(1234, "127.0.0.1", 4321) + nio.markers = {"m": {"bpf": "icmp", "tag": None, "link_id": "L1", + "direction": None, "data_link_type": "DLT_C_HDLC", "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) + sent = [c.args[0] for c in node._ubridge_send.call_args_list] + assert any("linktype C_HDLC" in s for s in sent) + + +@pytest.mark.asyncio +async def test_apply_markers_omits_linktype_for_ethernet(compute_project, manager): + # Ethernet (DLT_EN10MB) → no linktype keyword; uBridge defaults to EN10MB. + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_send = AsyncioMagicMock() + nio = NIOUDP(1234, "127.0.0.1", 4321) + nio.markers = {"m": {"bpf": "icmp", "tag": None, "link_id": "L1", + "direction": None, "data_link_type": "DLT_EN10MB", "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) + sent = [c.args[0] for c in node._ubridge_send.call_args_list] + assert not any("linktype" in s for s in sent) + + +@pytest.mark.asyncio +async def test_delete_marker_capture_removes_pcap_and_entry(compute_project, manager): + # Deleting a marker's capture removes its pcap and forgets the bridge entry. + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + markers_dir = compute_project.markers_working_directory() + os.makedirs(markers_dir, exist_ok=True) + node._marker_filter_bridges["m", "L1"] = "VPCS-10" + pcap = os.path.join(markers_dir, f"{node.id}_L1_m.pcap") + open(pcap, "wb").write(b"data") + + await node.delete_marker_capture("m", "L1") + + assert not os.path.exists(pcap) + assert ("m", "L1") not in node._marker_filter_bridges + + +@pytest.mark.asyncio +async def test_delete_marker_capture_idempotent_when_missing(compute_project, manager): + # No file on disk → must not raise, and still clears the entry. + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._marker_filter_bridges["m", "L1"] = "VPCS-10" + + await node.delete_marker_capture("m", "L1") + + assert ("m", "L1") not in node._marker_filter_bridges + + +@pytest.mark.asyncio +async def test_delete_marker_capture_sends_delete_filter(compute_project, manager): + # With uBridge running, removing a marker issues a fine-grained + # delete_packet_filter (not a bridge-wide reset) so sibling pcaps stay open. + 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" + + await node.delete_marker_capture("m", "L1") + + node._ubridge_send.assert_any_call("bridge delete_packet_filter VPCS-10 m") + assert ("m", "L1") not in node._marker_filter_bridges + + +@pytest.mark.asyncio +async def test_delete_marker_capture_drops_from_nio_markers(compute_project, manager): + # The marker spec cached on the port NIO (nio.markers) is what + # _ubridge_apply_markers reads on node start. delete_marker_capture must drop + # it, else deleting a marker while the node is stopped leaves the spec in + # nio.markers and starting the node reinstalls it (empty pcap reappears). + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + nio = NIOUDP(1234, "127.0.0.1", 4321) + nio.markers = {"m": {"bpf": "icmp", "tag": None, "link_id": "L1", + "direction": None, "enabled": True}} + await node.delete_marker_capture("m", "L1", nio) + assert "m" not in nio.markers + + +@pytest.mark.asyncio +async def test_rebuild_marker_filter_delete_then_add(compute_project, manager): + # rebuild re-installs a single filter (delete_packet_filter + add) with the + # new params, no bridge reset; enabled=False turns it off after re-add. + 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" + + await node.rebuild_marker_filter("m", "L1", "tcp", tag=7, direction="rx", enabled=False) + + 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 any("add_packet_filter VPCS-10 m mark" in c and "tcp" in c for c in cmds) + assert any("enable_packet_filter VPCS-10 m off" in c for c in cmds) + + +@pytest.mark.asyncio +async def test_apply_markers_skips_already_installed(compute_project, manager): + # Incremental apply: a marker already in _marker_filter_bridges is not + # re-added (uBridge keeps it across reset), so its pcap isn't reopened. + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._ubridge_send = AsyncioMagicMock() + node._marker_filter_bridges["m", "L1"] = "VPCS-10" # already installed + nio = NIOUDP(1234, "127.0.0.1", 4321) + nio.markers = {"m": {"bpf": "icmp", "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 not any("add_packet_filter" in c for c in cmds) # skipped, not re-added + + +@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 + # apply re-installs them instead of skipping as "already installed". + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node._marker_filter_bridges["m", "L1"] = "VPCS-10" + await node._stop_ubridge() + assert node._marker_filter_bridges == {} diff --git a/tests/compute/ubridge/__init__.py b/tests/compute/ubridge/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/compute/ubridge/test_hypervisor.py b/tests/compute/ubridge/test_hypervisor.py new file mode 100644 index 000000000..126278ab0 --- /dev/null +++ b/tests/compute/ubridge/test_hypervisor.py @@ -0,0 +1,191 @@ +#!/usr/bin/env python +# +# Copyright (C) 2025 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +""" +Tests for the uBridge ``Hypervisor`` wrapper — the configurable control-channel +transport (AF_UNIX ``-U`` vs TCP ``-H``), command building, the human-readable +``endpoint``, socket cleanup on stop, and the fail-fast detection of an +immediately-exiting uBridge process (e.g. an old build that rejects ``-U``). +""" + +import os +import re +import stat +import logging + +import pytest +from unittest.mock import MagicMock, AsyncMock, patch + +from gns3server.compute.ubridge.hypervisor import Hypervisor +from gns3server.compute.ubridge.ubridge_error import UbridgeError + + +def _make(transport, tmp_path, monkeypatch, node_id="abc123", host="127.0.0.1"): + """Build a Hypervisor with ``XDG_RUNTIME_DIR`` pinned to ``tmp_path``. + + The unix transport creates its socket dir under ``$XDG_RUNTIME_DIR/gns3``; + pinning it keeps creation predictable and avoids touching the real runtime + dir. ``host`` is unused for the unix transport but always accepted. + """ + + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path)) + return Hypervisor(MagicMock(), "ubridge", str(tmp_path), transport, host=host, node_id=node_id) + + +# --------------------------------------------------------------------------- +# __init__: transport selection +# --------------------------------------------------------------------------- + +def test_init_unix_creates_socket_dir_and_path(tmp_path, monkeypatch): + + hyp = _make("unix", tmp_path, monkeypatch, node_id="abc123") + assert hyp._socket_path == str(tmp_path / "gns3" / "ubridge-abc123.sock") + + socket_dir = os.path.dirname(hyp._socket_path) + assert os.path.isdir(socket_dir) + # 0o700 regardless of umask — __init__ chmods explicitly. + assert stat.S_IMODE(os.stat(socket_dir).st_mode) == 0o700 + # TCP-only attributes are unused on the unix transport. + assert hyp._host is None + assert hyp._port is None + + +def test_init_unix_fallback_name_without_node_id(tmp_path, monkeypatch): + # node_id is normally always passed (one ubridge per node); the counter + # fallback only fires when it's missing. Match the numbered pattern so the + # assertion is independent of class-counter ordering across the suite. + hyp = _make("unix", tmp_path, monkeypatch, node_id=None) + assert re.search(r"ubridge-\d+\.sock$", hyp._socket_path) + + +def test_init_tcp_sets_host_port(tmp_path, monkeypatch): + + hyp = _make("tcp", tmp_path, monkeypatch) + assert hyp._socket_path is None + assert hyp._host == "127.0.0.1" + assert isinstance(hyp._port, int) and hyp._port > 0 + + +# --------------------------------------------------------------------------- +# _build_command + endpoint +# --------------------------------------------------------------------------- + +def test_build_command_unix(tmp_path, monkeypatch): + + hyp = _make("unix", tmp_path, monkeypatch) + cmd = hyp._build_command() + assert cmd[0] == "ubridge" + assert "-U" in cmd + assert hyp._socket_path in cmd + assert "-H" not in cmd + assert "-d" not in cmd # debug flag only at DEBUG level + + +def test_build_command_tcp(tmp_path, monkeypatch): + + hyp = _make("tcp", tmp_path, monkeypatch) + cmd = hyp._build_command() + assert cmd[0] == "ubridge" + assert "-H" in cmd + assert f"{hyp._host}:{hyp._port}" in cmd + assert "-U" not in cmd + + +def test_build_command_debug_flag(tmp_path, monkeypatch): + + hyp = _make("unix", tmp_path, monkeypatch) + logger = logging.getLogger("gns3server.compute.ubridge.hypervisor") + original = logger.level + logger.setLevel(logging.DEBUG) + try: + cmd = hyp._build_command() + assert "-d" in cmd and "1" in cmd + finally: + logger.setLevel(original) + + +def test_endpoint_unix(tmp_path, monkeypatch): + + hyp = _make("unix", tmp_path, monkeypatch) + assert hyp.endpoint == hyp._socket_path + + +def test_endpoint_tcp(tmp_path, monkeypatch): + + hyp = _make("tcp", tmp_path, monkeypatch) + assert hyp.endpoint == f"{hyp._host}:{hyp._port}" + + +# --------------------------------------------------------------------------- +# stop: AF_UNIX socket cleanup +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_stop_unlinks_unix_socket(tmp_path, monkeypatch): + + hyp = _make("unix", tmp_path, monkeypatch) + # Simulate the socket file ubridge would have created. + open(hyp._socket_path, "w").close() + # Stopped process => is_running() is False => skips UBridgeHypervisor.stop (no send). + hyp._process = MagicMock() + hyp._process.returncode = 0 + assert os.path.exists(hyp._socket_path) + + await hyp.stop() + + assert not os.path.exists(hyp._socket_path) + + +@pytest.mark.asyncio +async def test_stop_tcp_has_no_socket_to_unlink(tmp_path, monkeypatch): + # TCP transport: no socket_path, so stop must simply not raise. + hyp = _make("tcp", tmp_path, monkeypatch) + hyp._process = MagicMock() + hyp._process.returncode = 0 + await hyp.stop() + + +# --------------------------------------------------------------------------- +# start: fail-fast on an immediately-exiting uBridge +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_start_detects_immediate_exit(tmp_path, monkeypatch): + # An unsupported flag (e.g. -U on an old ubridge) makes the process exit at + # once. start() must surface that from ubridge.log instead of timing out in + # connect() with a confusing "couldn't connect" error. + hyp = _make("unix", tmp_path, monkeypatch) + proc = MagicMock() + proc.pid = 1234 + proc.returncode = 2 # already exited + with patch.object(Hypervisor, "_check_ubridge_version", new_callable=AsyncMock), \ + patch("asyncio.create_subprocess_exec", new_callable=AsyncMock, return_value=proc): + with pytest.raises(UbridgeError, match="exited immediately"): + await hyp.start() + + +@pytest.mark.asyncio +async def test_start_proceeds_when_process_keeps_running(tmp_path, monkeypatch): + # Healthy startup: the process stays up, so start() returns normally. + hyp = _make("unix", tmp_path, monkeypatch) + proc = MagicMock() + proc.pid = 1234 + proc.returncode = None # still running + with patch.object(Hypervisor, "_check_ubridge_version", new_callable=AsyncMock), \ + patch("asyncio.create_subprocess_exec", new_callable=AsyncMock, return_value=proc): + await hyp.start() # must NOT raise + assert hyp._process is proc diff --git a/tests/controller/test_marker.py b/tests/controller/test_marker.py index 7dc0ebe13..c5297aeff 100644 --- a/tests/controller/test_marker.py +++ b/tests/controller/test_marker.py @@ -24,36 +24,53 @@ Controller-layer tests for the traffic-insight marker feature: * Project.apply_defs_to_new_link and the markers aggregation property. """ +import uuid + import pytest from unittest.mock import MagicMock, patch +from contextlib import ExitStack from tests.utils import AsyncioMagicMock from gns3server.controller.udp_link import UDPLink from gns3server.controller.ports.ethernet_port import EthernetPort +from gns3server.controller.ports.serial_port import SerialPort from gns3server.controller.node import Node from gns3server.controller.controller_error import ControllerError, ControllerNotFoundError def _valid_bpf(): - """Bypass tcpdump-based BPF validation so tests don't depend on tcpdump.""" - return patch( + """Bypass tcpdump-based BPF validation so tests don't depend on tcpdump. + + Patches both namespaces that import ``validate_bpf_syntax`` by name: the + per-link ``udp_link`` (private marker create/update) and the project layer + (definition create/update/load), which is now the single validation point + for inherited copies. + """ + stack = ExitStack() + for target in ( "gns3server.controller.udp_link.validate_bpf_syntax", - return_value={"valid": True, "error": None}, - ) + "gns3server.controller.project.validate_bpf_syntax", + ): + stack.enter_context(patch(target, return_value={"valid": True, "error": None})) + return stack -async def _make_link(project): - """Build a created UDPLink between two VPCS nodes on a mocked compute.""" +async def _make_link(project, port_cls=EthernetPort): + """Build a created UDPLink between two VPCS nodes on a mocked compute. + + ``port_cls`` defaults to EthernetPort; pass SerialPort for a serial link + (the link's link_type follows the port). + """ compute = MagicMock() compute.id = "local" compute.host = "example.com" node1 = Node(project, compute, "n1", node_type="vpcs") - node1._ports = [EthernetPort("E0", 0, 0, 0)] + node1._ports = [port_cls("E0", 0, 0, 0)] node2 = Node(project, compute, "n2", node_type="vpcs") - node2._ports = [EthernetPort("E0", 0, 0, 1)] + node2._ports = [port_cls("E0", 0, 0, 1)] async def subnet(_other): return ("192.168.1.1", "192.168.1.2") @@ -98,6 +115,63 @@ async def test_start_marker_stores_entry(project): assert "inherited_from" not in entry +@pytest.mark.asyncio +async def test_start_marker_stores_data_link_type(project): + # data_link_type is stored on the marker and flows into the per-node spec + # (the compute-side source for the uBridge `linktype` keyword). Serial-only; + # defaults to DLT_EN10MB when omitted (Ethernet → linktype omitted). + with _valid_bpf(): + link = await _make_link(project) + await link.start_marker("ospf", "ospf", data_link_type="DLT_C_HDLC") + + assert link.markers["ospf"]["data_link_type"] == "DLT_C_HDLC" + capture_id = link.markers["ospf"]["capture_node_id"] + capture_side = next(n for n in link._nodes if n["node"].id == capture_id) + assert link._markers_for_node(capture_side["node"])["ospf"]["data_link_type"] == "DLT_C_HDLC" + + # Default when omitted = Ethernet. + with _valid_bpf(): + link2 = await _make_link(project) + await link2.start_marker("icmp", "icmp") + cid = link2.markers["icmp"]["capture_node_id"] + cside = next(n for n in link2._nodes if n["node"].id == cid) + assert link2._markers_for_node(cside["node"])["icmp"]["data_link_type"] == "DLT_EN10MB" + + +@pytest.mark.asyncio +async def test_start_marker_pins_capture_node(project): + # Auto-pick would choose node1 (first endpoint); pin to node2 explicitly. + with _valid_bpf(): + link = await _make_link(project) + chosen = link._nodes[1]["node"].id + auto = link._nodes[0]["node"].id + assert chosen != auto # sanity: the pin must actually mean something + await link.start_marker("icmp", "icmp", capture_node_id=chosen) + + assert link.markers["icmp"]["capture_node_id"] == chosen + + +@pytest.mark.asyncio +async def test_start_marker_rejects_non_endpoint_capture_node(project): + + with _valid_bpf(): + link = await _make_link(project) + with pytest.raises(ControllerNotFoundError): + await link.start_marker("icmp", "icmp", capture_node_id="11111111-2222-3333-4444-555555555555") + + +@pytest.mark.asyncio +async def test_start_marker_capture_node_ignored_for_inherited(project): + # Definitions are link-agnostic: an inherited marker must auto-pick even + # if a capture_node_id leaks through, never trusting the caller's pin. + with _valid_bpf(): + link = await _make_link(project) + leaked = link._nodes[1]["node"].id + await link.start_marker("m", "icmp", capture_node_id=leaked, inherited_from="arp") + + assert link.markers["m"]["capture_node_id"] == link._nodes[0]["node"].id + + @pytest.mark.asyncio async def test_start_marker_rejects_duplicate(project): @@ -252,6 +326,78 @@ async def test_persist_markers_excludes_inherited(project): assert "global-arp" not in persisted +@pytest.mark.asyncio +async def test_load_marker_preserves_direction_and_highlight_duration(project): + """Regression: a private marker's direction + highlight_duration must survive + a close/reopen round-trip through the topology file. + + _create_link_from_topology_data previously restored only bpf/tag/enabled/ + color/capture_node_id, silently dropping direction (→ reverted to "both") + and highlight_duration. + """ + compute = MagicMock() + compute.id = "local" + compute.host = "example.com" + + async def subnet(_other): + return ("192.168.1.1", "192.168.1.2") + + async def udp_cb(path, data={}, **kwargs): + response = MagicMock() + response.json = {"udp_port": 1234} + return response + + compute.get_ip_on_same_subnet.side_effect = subnet + compute.post.side_effect = udp_cb + # Attaching the 2nd node auto-creates the link (NIO round-trips). + compute.put = AsyncioMagicMock() + compute.delete = AsyncioMagicMock() + + node1 = Node(project, compute, "n1", node_type="vpcs") + node1._ports = [EthernetPort("E0", 0, 0, 0)] + node2 = Node(project, compute, "n2", node_type="vpcs") + node2._ports = [EthernetPort("E0", 0, 0, 1)] + # _create_link_from_topology_data resolves nodes via project.get_node(). + project._nodes[node1.id] = node1 + project._nodes[node2.id] = node2 + + capture_node_id = str(uuid.uuid4()) + link_id = str(uuid.uuid4()) + link_data = { + "link_id": link_id, + "nodes": [ + {"node_id": node1.id, "adapter_number": 0, "port_number": 0, "label": "a"}, + {"node_id": node2.id, "adapter_number": 0, "port_number": 1, "label": "b"}, + ], + "markers": { + "icmp": { + "bpf": "icmp", + "direction": "rx", + "highlight_duration": 800, + "tag": 7, + "color": "#ff5722", + "enabled": True, + "capture_node_id": capture_node_id, + } + }, + } + with patch( + "gns3server.controller.project.validate_bpf_syntax", + return_value={"valid": True, "error": None}, + ): + await project._create_link_from_topology_data(link_data) + + # The link survives (2 attached nodes); pull it back from the project. + link = project._links[link_id] + entry = link._markers["icmp"] + assert entry["direction"] == "rx" # dropped before the fix + assert entry["highlight_duration"] == 800 # dropped before the fix + assert entry["tag"] == 7 + assert entry["color"] == "#ff5722" + assert entry["capture_node_id"] == capture_node_id + assert entry["enabled"] is True + + @pytest.mark.asyncio async def test_asdict_markers_runtime_vs_dump(project): """Runtime asdict exposes all markers; topology dump drops inherited ones.""" @@ -287,6 +433,21 @@ async def test_create_marker_definition_fans_out(project): assert project.marker_definitions["arp"]["highlight_duration"] == 1200 +@pytest.mark.asyncio +async def test_definition_fans_out_over_many_links(project): + # The fan-out is concurrent (bounded) — a large topology must not serialize + # N compute round-trips — but behaviorally every link still receives the + # marker and per-link failures stay isolated. + with _valid_bpf(): + links = [await _make_link(project) for _ in range(20)] + await project.create_marker_definition("arp", "arp", highlight_duration=700) + + for link in links: + assert link.markers["global-arp"]["highlight_duration"] == 700 + assert link.markers["global-arp"]["inherited_from"] == "arp" + assert project.marker_definitions["arp"]["highlight_duration"] == 700 + + @pytest.mark.asyncio async def test_update_marker_definition_syncs(project): @@ -302,6 +463,46 @@ async def test_update_marker_definition_syncs(project): assert project.marker_definitions["arp"]["highlight_duration"] == 1500 +@pytest.mark.asyncio +async def test_definition_serial_dlt_fans_out_to_serial_link(project): + # A definition with a serial data_link_type covers serial links with that + # encapsulation AND ethernet links with EN10MB (one definition, mixed topo). + with _valid_bpf(): + serial_link = await _make_link(project, SerialPort) + eth_link = await _make_link(project) + await project.create_marker_definition("ospf", "ospf", data_link_type="DLT_C_HDLC") + + assert serial_link._link_type == "serial" + assert serial_link.markers["global-ospf"]["data_link_type"] == "DLT_C_HDLC" + assert eth_link.markers["global-ospf"]["data_link_type"] == "DLT_EN10MB" + + +@pytest.mark.asyncio +async def test_definition_default_skips_serial_link(project): + # Default (EN10MB) definition is Ethernet-only: serial links are skipped + # (an EN10MB pcap on a serial link would be undecodable). + with _valid_bpf(): + serial_link = await _make_link(project, SerialPort) + eth_link = await _make_link(project) + await project.create_marker_definition("arp", "arp") + + assert "global-arp" not in serial_link.markers + assert "global-arp" in eth_link.markers + + +@pytest.mark.asyncio +async def test_update_definition_data_link_type_refans_out(project): + # Changing data_link_type re-evaluates which links host the marker: a serial + # link skipped under the default gains the marker once a WAN DLT is chosen. + with _valid_bpf(): + serial_link = await _make_link(project, SerialPort) + await project.create_marker_definition("ospf", "ospf") + assert "global-ospf" not in serial_link.markers # default → serial skipped + await project.update_marker_definition("ospf", data_link_type="DLT_PPP_SERIAL") + + assert serial_link.markers["global-ospf"]["data_link_type"] == "DLT_PPP_SERIAL" + + @pytest.mark.asyncio async def test_delete_marker_definition_clears(project): """Regression: deleting a def must remove inherited copies from every link.""" @@ -330,6 +531,73 @@ async def test_apply_defs_to_new_link(project): assert new_link.markers["global-arp"]["inherited_from"] == "arp" +@pytest.mark.asyncio +async def test_create_marker_definition_validates_bpf_once(project): + # A definition validates its BPF once (project layer); the inherited fan-out + # to every link must NOT re-validate — no tcpdump subprocess per link. + with patch("gns3server.controller.project.validate_bpf_syntax", + return_value={"valid": True, "error": None}) as proj_val, \ + patch("gns3server.controller.udp_link.validate_bpf_syntax", + return_value={"valid": True, "error": None}) as link_val: + await _make_link(project) + await _make_link(project) + await project.create_marker_definition("arp", "arp") + + assert proj_val.call_count == 1 # validated once at the def layer + assert link_val.call_count == 0 # fan-out skipped per-link validation + + +@pytest.mark.asyncio +async def test_create_marker_definition_rejects_invalid_bpf(project): + + with patch("gns3server.controller.project.validate_bpf_syntax", + return_value={"valid": False, "error": "syntax error"}): + with pytest.raises(ControllerError): + await project.create_marker_definition("arp", "not a real bpf") + assert "arp" not in project.marker_definitions + + +@pytest.mark.asyncio +async def test_update_marker_definition_skips_per_link_validation(project): + # Updating a def's BPF validates once more (project); the per-link sync + # (update_marker with inherited=True) must NOT re-validate. + with patch("gns3server.controller.project.validate_bpf_syntax", + return_value={"valid": True, "error": None}) as proj_val, \ + patch("gns3server.controller.udp_link.validate_bpf_syntax", + return_value={"valid": True, "error": None}) as link_val: + await _make_link(project) + await _make_link(project) + await project.create_marker_definition("arp", "arp") + await project.update_marker_definition("arp", bpf="arp or rarp") + + assert proj_val.call_count == 2 # once on create, once on update + assert link_val.call_count == 0 # sync skipped per-link validation + + +@pytest.mark.asyncio +async def test_start_marker_skips_validation_for_inherited(project): + # An inherited marker rides an already-validated definition BPF, so + # start_marker must not call validate_bpf_syntax. + with patch("gns3server.controller.udp_link.validate_bpf_syntax", + return_value={"valid": True, "error": None}) as link_val: + link = await _make_link(project) + await link.inherit_marker("arp", {"bpf": "arp"}) + + assert link_val.call_count == 0 + assert link.markers["global-arp"]["bpf"] == "arp" + + +@pytest.mark.asyncio +async def test_start_marker_validates_for_private(project): + # A private (non-inherited) marker still validates inline. + with patch("gns3server.controller.udp_link.validate_bpf_syntax", + return_value={"valid": True, "error": None}) as link_val: + link = await _make_link(project) + await link.start_marker("icmp", "icmp") + + assert link_val.call_count == 1 + + @pytest.mark.asyncio async def test_markers_aggregation(project): @@ -344,3 +612,248 @@ async def test_markers_aggregation(project): assert agg[key]["highlight_duration"] == 800 assert agg[key]["link_id"] == link.id assert agg[key]["node_id"] == agg[key]["capture_node_id"] + + +# --------------------------------------------------------------------------- +# Direction clear/preserve semantics (sentinel _UNSET vs explicit None) +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_update_marker_clears_direction(project): + # Explicit direction=None clears the filter back to "both directions" — + # distinct from omitting the kwarg (which preserves the stored value). + with _valid_bpf(): + link = await _make_link(project) + await link.start_marker("m", "icmp", direction="tx") + assert link.markers["m"]["direction"] == "tx" + await link.update_marker("m", direction=None) + assert link.markers["m"]["direction"] is None + + +@pytest.mark.asyncio +async def test_update_marker_preserves_direction_when_omitted(project): + # Omitting direction entirely is a partial update: the stored value stays. + with _valid_bpf(): + link = await _make_link(project) + await link.start_marker("m", "icmp", direction="tx") + await link.update_marker("m", tag=9) + assert link.markers["m"]["direction"] == "tx" + assert link.markers["m"]["tag"] == 9 + + +@pytest.mark.asyncio +async def test_update_marker_definition_clears_direction(project): + # Clearing a definition's direction must propagate to every inherited copy. + # New defs can't carry tx/rx, but a legacy def loaded from an old topology + # could — so inject one and confirm a clear syncs every copy. + with _valid_bpf(): + link1 = await _make_link(project) + link2 = await _make_link(project) + await project.create_marker_definition("arp", "arp") + # Simulate a legacy directional value persisted before the restriction. + project._marker_definitions["arp"]["direction"] = "tx" + await link1.update_marker("global-arp", direction="tx", inherited=True) + await link2.update_marker("global-arp", direction="tx", inherited=True) + for link in (link1, link2): + assert link.markers["global-arp"]["direction"] == "tx" + await project.update_marker_definition("arp", direction=None) + + assert project.marker_definitions["arp"]["direction"] is None + for link in (link1, link2): + assert link.markers["global-arp"]["direction"] is None + + +# --------------------------------------------------------------------------- +# Capture-node routing + capability validation +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_pinned_marker_routes_only_to_chosen_node(project): + # The marker rides only the pinned capture node's NIO; the far endpoint sees nothing. + with _valid_bpf(): + link = await _make_link(project) + chosen = link._nodes[1]["node"] + other = link._nodes[0]["node"] + await link.start_marker("icmp", "icmp", capture_node_id=chosen.id) + + assert "icmp" in link._markers_for_node(chosen) + assert "icmp" not in link._markers_for_node(other) + + +@pytest.mark.asyncio +async def test_markers_for_node_carries_direction(project): + # The NIO-bound marker spec forwards direction so uBridge gets the dir token. + with _valid_bpf(): + link = await _make_link(project) + node = link._nodes[0]["node"] # auto-pick selects the first capable endpoint + await link.start_marker("m", "icmp", direction="rx") + + assert link._markers_for_node(node)["m"]["direction"] == "rx" + + +@pytest.mark.asyncio +async def test_start_marker_rejects_non_capable_capture_node(project): + # A NAT endpoint has no uBridge bridge. Pinning to it must fail even though + # it IS a link endpoint (distinct from the not-an-endpoint -> 404 case). + with _valid_bpf(): + link = await _make_link(project) + nat = Node(project, link._nodes[0]["node"].compute, "nat", node_type="nat") + link._nodes.append({"node": nat, "adapter_number": 0, "port_number": 0}) + with pytest.raises(ControllerError): + await link.start_marker("m", "icmp", capture_node_id=nat.id) + + +# --------------------------------------------------------------------------- +# Part A/B: enabled reaches uBridge + instant per-filter toggle +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_markers_for_node_keeps_disabled_and_carries_enabled(project): + # Part A: a disabled marker is NOT dropped from the NIO payload (so uBridge + # can install it then turn it off) and the spec carries `enabled`. + with _valid_bpf(): + link = await _make_link(project) + node = link._nodes[0]["node"] + await link.start_marker("m", "icmp") + await link.update_marker("m", enabled=False) + spec = link._markers_for_node(node).get("m") + assert spec is not None + assert spec["enabled"] is False + + +@pytest.mark.asyncio +async def test_update_marker_enabled_only_hits_toggle_route(project): + # Part B: an enabled-only change routes to the per-marker toggle endpoint, + # not a full NIO reset+reapply. + with _valid_bpf(): + link = await _make_link(project) + node = link._nodes[0]["node"] + await link.start_marker("m", "icmp") + compute = node.compute + compute.put.reset_mock() + await link.update_marker("m", enabled=False) + paths = [c.args[0] for c in compute.put.call_args_list] + assert any("/markers/m" in p for p in paths) + assert not any(p.endswith("/nio") for p in paths) + + +@pytest.mark.asyncio +async def test_update_marker_with_bpf_rebuilds_single_filter(project): + # A bpf change rebuilds just this marker's filter (delete + add), NOT a full + # NIO reapply, so sibling markers' pcaps stay open. + with _valid_bpf(): + link = await _make_link(project) + node = link._nodes[0]["node"] + await link.start_marker("m", "icmp") + compute = node.compute + compute.put.reset_mock() + await link.update_marker("m", bpf="tcp") + paths = [c.args[0] for c in compute.put.call_args_list] + assert any(p.endswith("/markers/m/rebuild") for p in paths) + assert not any(p.endswith("/nio") for p in paths) # no full NIO reapply + + +@pytest.mark.asyncio +async def test_update_marker_ui_only_does_not_push(project): + # color/highlight_duration are UI-only — stored, never pushed to uBridge. + with _valid_bpf(): + link = await _make_link(project) + node = link._nodes[0]["node"] + await link.start_marker("m", "icmp") + compute = node.compute + compute.put.reset_mock() + await link.update_marker("m", color="#ffffff", highlight_duration=1500) + assert compute.put.call_args_list == [] # nothing pushed to uBridge + assert link.markers["m"]["color"] == "#ffffff" + assert link.markers["m"]["highlight_duration"] == 1500 + + +# --------------------------------------------------------------------------- +# Per-definition pause/resume (toggle every inherited global-{name} copy) +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_pause_marker_definition_toggles_copies_off(project): + with _valid_bpf(): + link1 = await _make_link(project) + link2 = await _make_link(project) + await project.create_marker_definition("arp", "arp") + assert link1.markers["global-arp"]["enabled"] is True + assert link2.markers["global-arp"]["enabled"] is True + await project.pause_marker_definition("arp") + assert project.marker_definitions["arp"]["paused"] is True + assert link1.markers["global-arp"]["enabled"] is False + assert link2.markers["global-arp"]["enabled"] is False + + +@pytest.mark.asyncio +async def test_resume_marker_definition_toggles_copies_on(project): + with _valid_bpf(): + link = await _make_link(project) + await project.create_marker_definition("arp", "arp") + await project.pause_marker_definition("arp") + assert link.markers["global-arp"]["enabled"] is False + await project.resume_marker_definition("arp") + assert project.marker_definitions["arp"]["paused"] is False + assert link.markers["global-arp"]["enabled"] is True + + +@pytest.mark.asyncio +async def test_paused_definition_inherited_as_disabled(project): + # A link created after the definition was paused inherits it already off. + with _valid_bpf(): + await project.create_marker_definition("arp", "arp") + await project.pause_marker_definition("arp") + new_link = await _make_link(project) + assert new_link.markers["global-arp"]["enabled"] is False + + +# --------------------------------------------------------------------------- +# Marker definition direction (tx/rx rejected — it is capture-node-relative) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_create_marker_definition_rejects_directional(project): + # tx/rx is relative to the auto-selected capture node → rejected at the def level. + with pytest.raises(ControllerError): + await project.create_marker_definition("arp", "arp", direction="tx") + with pytest.raises(ControllerError): + await project.create_marker_definition("arp", "arp", direction="rx") + assert "arp" not in project.marker_definitions # nothing created + + +@pytest.mark.asyncio +async def test_create_marker_definition_allows_both(project): + await project.create_marker_definition("arp", "arp") # default both + await project.create_marker_definition("icmp", "icmp", direction=None) + assert project.marker_definitions["arp"]["direction"] is None + assert project.marker_definitions["icmp"]["direction"] is None + + +@pytest.mark.asyncio +async def test_update_marker_definition_rejects_directional(project): + await project.create_marker_definition("arp", "arp") # both + with pytest.raises(ControllerError): + await project.update_marker_definition("arp", direction="tx") + # omitted direction and explicit clear-to-both are both fine + await project.update_marker_definition("arp", color="#ffffff") + await project.update_marker_definition("arp", direction=None) + assert project.marker_definitions["arp"]["direction"] is None + + +@pytest.mark.asyncio +async def test_stop_marker_deletes_capture_pcap(project): + # Removing a marker asks the capture node's compute to delete its pcap, so + # the file is cleaned up even with the node stopped (the NIO reapply path + # only runs while uBridge is up). + with _valid_bpf(): + link = await _make_link(project) + capture = link._nodes[0]["node"] + await link.start_marker("icmp", "icmp", capture_node_id=capture.id) + capture.delete = AsyncioMagicMock() + await link.stop_marker("icmp") + + capture.delete.assert_called_once_with( + "/adapters/0/ports/0/markers/icmp", params={"link_id": link.id} + )