mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-10-12 21:45:41 +03:00
fix(compute): a passive absorbed anchor no longer resets the switch's tc state
The peer end of an Ethernet switch fast-path link is a NIOBridge with no bridge of its own (the switch absorbed its anchor into the switch bridge). Its update reconcile still ran the impairment pass with the empty filter set the controller routes to that end — and an empty set is a plain `tc reset` on the shared anchor, silently wiping the netem/cls_bpf/eBPF the switch had just applied. Any link update did it (filters, markers, suspend, capture start/stop), and the outcome depended on the link's endpoint order: UdpLink.update() PUTs node1 then node2, so with the switch first its apply ran first and the peer's reset ran last (API green, data plane unimpaired), while with the peer first the reset ran first and the filter survived. That order dependence is also why the shipped e2e never caught it — it only created peer-first links. Ownership is now explicit: only the end that owns the anchor's tc state applies the impairment reconcile (_owns_anchor_impairments — a NIOBridge with `bridge is None` is a passive carrier, the same idiom the bridge-delete path already used). Markers, capture and the suspend carrier still apply on both ends: markers reconcile per process, so neither end can strip the other's, and the carrier converges by agreement. Red/green: two new docker unit tests fail on the unfixed tree with `tc reset "<anchor>"` and pass after; the switch e2e also creates one link switch-first and asserts the netem qdisc and the RTT hold under that order (failed with the wipe, passes with the fix). Full unit suite 2110 passed; the switch e2e file passed end to end (378s).
This commit is contained in:
parent
7a9a514ea3
commit
d0b0cdbd38
@ -135,6 +135,21 @@ class KernelDatapathMixin:
|
||||
# Link attach / detach
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
@staticmethod
|
||||
def _owns_anchor_impairments(nio):
|
||||
"""
|
||||
Whether this end owns the tc state (netem, cls_bpf, eBPF) of the
|
||||
anchor it was handed. A NIOBridge with no bridge of its own
|
||||
(``bridge is None``) means the anchor is absorbed by an Ethernet
|
||||
switch: the switch owns the anchor's bridge membership and its
|
||||
impairment state — the controller routes the link's filters to the
|
||||
switch end, so this NIO always carries an empty filter set, whose
|
||||
reconcile is a plain ``tc reset``. Applying it here would silently
|
||||
wipe the netem/bpf/eBPF the switch just put on the shared anchor.
|
||||
"""
|
||||
|
||||
return not (isinstance(nio, NIOBridge) and nio.bridge is None)
|
||||
|
||||
async def _kernel_attach(self, anchor, nio):
|
||||
"""
|
||||
Attach an anchor to the per-link kernel bridge carried by *nio*
|
||||
@ -150,9 +165,11 @@ class KernelDatapathMixin:
|
||||
A NIO whose ``bridge`` is None means the anchor is bridged
|
||||
elsewhere — an Ethernet switch absorbed it into the switch's own
|
||||
bridge — so the membership work is skipped here and only the link
|
||||
state the anchor carries (capture, markers, impairment filters) is
|
||||
applied. Exactly one end of such a link owns the anchor's bridge
|
||||
state; the other end is a passive carrier.
|
||||
state this end owns is applied: capture and markers always, the
|
||||
impairment filters only when this end owns the anchor's tc state
|
||||
(see _owns_anchor_impairments). Exactly one end of such a link
|
||||
owns the anchor's bridge and impairment state; the other end is a
|
||||
passive carrier.
|
||||
"""
|
||||
|
||||
if nio.bridge is not None:
|
||||
@ -174,10 +191,13 @@ class KernelDatapathMixin:
|
||||
# (restored here on node restart, like the capture above); bpf
|
||||
# expressions become cls_bpf match-drop classifiers and
|
||||
# frequency_drop/quota/window_drop become the eBPF stateful
|
||||
# classifier.
|
||||
await self._ubridge_apply_netem(anchor, nio.filters)
|
||||
await self._ubridge_apply_bpf_drops(anchor, nio.filters)
|
||||
await self._ubridge_apply_ebpf_drops(anchor, nio.filters)
|
||||
# classifier. A passive end owns none of this: its empty filter
|
||||
# set is a tc reset, which would wipe the switch's qdisc off the
|
||||
# shared anchor.
|
||||
if self._owns_anchor_impairments(nio):
|
||||
await self._ubridge_apply_netem(anchor, nio.filters)
|
||||
await self._ubridge_apply_bpf_drops(anchor, nio.filters)
|
||||
await self._ubridge_apply_ebpf_drops(anchor, nio.filters)
|
||||
|
||||
async def _relay_attach(self, anchor, bridge_name, nio):
|
||||
"""
|
||||
@ -226,16 +246,19 @@ class KernelDatapathMixin:
|
||||
"""
|
||||
Re-apply everything a kernel link carries on an already-attached
|
||||
anchor (filters and markers changed): no re-enslaving, no bridge
|
||||
create.
|
||||
create. A passive end (bridge is None — an anchor absorbed by an
|
||||
Ethernet switch) leaves the anchor's impairment state to the
|
||||
switch that owns it.
|
||||
"""
|
||||
|
||||
await self._ubridge_apply_markers(anchor, nio)
|
||||
# The netem apply resets the interface first (the kernel merges
|
||||
# optional netem attrs on replace), so everything anchored on clsact
|
||||
# must re-apply after it: bpf drops flush + re-add, eBPF modes re-set.
|
||||
await self._ubridge_apply_netem(anchor, nio.filters)
|
||||
await self._ubridge_apply_bpf_drops(anchor, nio.filters)
|
||||
await self._ubridge_apply_ebpf_drops(anchor, nio.filters)
|
||||
if self._owns_anchor_impairments(nio):
|
||||
await self._ubridge_apply_netem(anchor, nio.filters)
|
||||
await self._ubridge_apply_bpf_drops(anchor, nio.filters)
|
||||
await self._ubridge_apply_ebpf_drops(anchor, nio.filters)
|
||||
|
||||
async def _remove_kernel_nio(self, nio, adapter_number, port_number=0):
|
||||
"""
|
||||
|
||||
@ -322,6 +322,32 @@ async def test_connect_nio_kernel_eexist_still_reapplies_everything(vm):
|
||||
vm._ubridge_send.assert_any_call(f'tc netem set "{host_ifc}" delay 10')
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_nio_kernel_passive_anchor_skips_impairments(vm):
|
||||
"""
|
||||
A NIOBridge with no bridge of its own (bridge is None) is the absorbed
|
||||
end of an Ethernet switch link: the anchor belongs to the switch, which
|
||||
owns its tc state and applies the link's filters there (the controller
|
||||
routes them to the switch end, so this NIO always carries {}). The
|
||||
passive end must not reconcile impairments — an empty filter set is a
|
||||
tc reset, which would silently wipe the switch's qdisc off the shared
|
||||
anchor at every node start.
|
||||
"""
|
||||
|
||||
vm._ubridge_hypervisor = MagicMock()
|
||||
vm._ubridge_send = AsyncioMagicMock()
|
||||
host_ifc, _ = vm._veth_names(0, 0)
|
||||
vm._kernel_veths[(0, 0)] = host_ifc
|
||||
nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": None})
|
||||
|
||||
await vm._connect_nio(0, nio)
|
||||
|
||||
tc_calls = [c.args[0] for c in vm._ubridge_send.call_args_list if str(c.args[0]).startswith("tc ")]
|
||||
assert tc_calls == [], tc_calls
|
||||
# The membership is the switch's as well: no per-link bridge work here.
|
||||
assert not any("brctl" in c.args[0] for c in vm._ubridge_send.call_args_list)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_nio_kernel_create_failure_propagates(vm):
|
||||
|
||||
@ -533,6 +559,33 @@ async def test_update_nio_kernel_turns_ebpf_modes_off(vm):
|
||||
assert "nth_drop" not in c.args[0] or "off" in c.args[0]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_nio_kernel_passive_anchor_skips_impairments(vm):
|
||||
"""
|
||||
The link-update reconcile on the absorbed (passive) end must leave the
|
||||
anchor's tc state to the switch: no tc reset / netem / drop classifiers
|
||||
from this end, or the reset would wipe what the switch applied moments
|
||||
earlier in the same link update (switch-first links PUT this side last).
|
||||
The carrier (suspend) is still driven here, and markers still reconcile
|
||||
(this end may host some).
|
||||
"""
|
||||
|
||||
vm._ubridge_hypervisor = MagicMock()
|
||||
vm._ubridge_send = AsyncioMagicMock()
|
||||
vm.status = "started"
|
||||
host_ifc, _ = vm._veth_names(0, 0)
|
||||
vm._kernel_veths[(0, 0)] = host_ifc
|
||||
nio = vm.manager.create_nio({"type": "nio_bridge", "bridge": None})
|
||||
vm._ethernet_adapters[0].add_nio(0, nio)
|
||||
|
||||
await vm.adapter_update_nio_binding(0, nio)
|
||||
|
||||
tc_calls = [c.args[0] for c in vm._ubridge_send.call_args_list if str(c.args[0]).startswith("tc ")]
|
||||
assert tc_calls == [], tc_calls
|
||||
# The carrier still follows the suspend flag on the shared anchor.
|
||||
vm._ubridge_send.assert_any_call(f'link set "{host_ifc}" up')
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_apply_ebpf_drops_requires_ebpf_capability(vm):
|
||||
"""
|
||||
|
||||
@ -29,7 +29,9 @@ Real ICMP crossing the switch proves the datapath:
|
||||
* VLAN programming applies to absorbed anchors: different access VLANs
|
||||
isolate the routers (ping dies), the same VLAN bridges them again;
|
||||
* a ``delay 100`` filter lands as netem on the absorbed anchor (single
|
||||
interface, switch-owned) and the measured RTT grows by ~100 ms;
|
||||
interface, switch-owned) and the measured RTT grows by ~100 ms — on both
|
||||
link endpoint orders (peer-first and switch-first; the order decides
|
||||
which side's NIO update runs last and must not decide the outcome);
|
||||
* suspend admin-downs the anchor and kills the traffic; resume restores;
|
||||
* capture (hosted by the switch side) writes a pcap of ICMP records;
|
||||
* deleting a link detaches the anchor (which survives — the router owns
|
||||
@ -128,8 +130,12 @@ def test_ethernet_switch_kernel_fast_path():
|
||||
# The switch bridge exists from the moment the switch node does; a
|
||||
# link against a stopped router must not fail — the join defers.
|
||||
assert harness.bridge_members(sw_bridge) == []
|
||||
# link1 is created peer-first, link2 switch-first: the endpoint order
|
||||
# decides which side's NIO update runs last, and only the switch-first
|
||||
# order used to expose the passive end's tc-reset wipe (its empty-filter
|
||||
# reconcile ran after the switch's apply). Both orders must work.
|
||||
link1 = compute.create_link(pid, (r1_id, *ETH), (switch_id, 0, 0))
|
||||
link2 = compute.create_link(pid, (r2_id, *ETH), (switch_id, 0, 1))
|
||||
link2 = compute.create_link(pid, (switch_id, 0, 1), (r2_id, *ETH))
|
||||
assert link1["kernel_datapath"] is True, link1
|
||||
assert link2["kernel_datapath"] is True, link2
|
||||
assert harness.bridge_members(sw_bridge) == []
|
||||
@ -223,6 +229,22 @@ def test_ethernet_switch_kernel_fast_path():
|
||||
fast = harness.wait_ping(c1, "10.1.1.2")
|
||||
assert fast["success"] == 100 and fast["avg"] < 50, fast
|
||||
|
||||
# The same single-owned impairment on link2 — created switch-first,
|
||||
# the order whose peer NIO update runs LAST. The passive end's empty
|
||||
# reconcile used to tc-reset the shared anchor right after the switch
|
||||
# applied the netem, wiping it (API green, data plane unimpaired);
|
||||
# the qdisc and the RTT must both hold under this order too.
|
||||
compute.call("PUT", f"/projects/{pid}/links/{link2['link_id']}", {"filters": {"delay": [100]}})
|
||||
assert "netem" in harness.qdiscs(a2), harness.qdiscs(a2)
|
||||
assert "netem" not in harness.qdiscs(a1), harness.qdiscs(a1)
|
||||
delayed2 = harness.wait_ping(c1, "10.1.1.2")
|
||||
assert delayed2["success"] == 100, delayed2["raw"]
|
||||
assert 60 <= delayed2["avg"] <= 250, (baseline, delayed2)
|
||||
compute.call("PUT", f"/projects/{pid}/links/{link2['link_id']}", {"filters": {}})
|
||||
assert "netem" not in harness.qdiscs(a2)
|
||||
fast2 = harness.wait_ping(c1, "10.1.1.2")
|
||||
assert fast2["success"] == 100 and fast2["avg"] < 50, fast2
|
||||
|
||||
# suspend: the anchor admin-downs, the link is dead; resume restores
|
||||
compute.call("PUT", f"/projects/{pid}/links/{link1['link_id']}", {"suspend": True})
|
||||
assert not harness.tap_up(a1)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user