diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index f70e37edd..b4c88f62d 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -170,9 +170,11 @@ class DockerVM(BaseNode): self._volumes = [] # Keep a list of created bridge self._bridges = set() - # (adapter_number, port_number) -> veth host-end name for adapters - # wired on the kernel datapath (NIOBridge). The host end lives in the - # root namespace and survives container death, unlike relay TAPs. + # (adapter_number, port_number) -> veth host-end name. Every adapter + # is born as a veth pair (unified interface): guest end in the + # container, host end in the root namespace where it survives + # container death. Kernel links enslave it into a Linux bridge, + # relay links attach it to the uBridge relay via AF_PACKET. self._kernel_veths = {} if adapters is None: @@ -1735,12 +1737,23 @@ class DockerVM(BaseNode): async def _add_ubridge_connection(self, nio, adapter_number, port_number=0): """ - Creates a connection in uBridge. + Create the adapter's interfaces at container start. - :param nio: NIO instance or None if it's a dummy interface (if an interface is missing in ubridge you can't see it via ifconfig in the container) + Every adapter is born as a veth pair — the unified interface — + regardless of NIO type: the guest end moves into the container + namespace as eth{N}, the host end stays in the root namespace. A + kernel-datapath NIO (NIOBridge) enslaves the host end into the + per-link Linux bridge; a relay NIO (UDP) attaches it to the uBridge + relay via AF_PACKET. The datapath is thus a runtime decision: any + link type can be attached to a running container without touching + its interfaces. + + :param nio: NIO instance, or None for an unconnected adapter — the + veth still exists (carrier off) so the interface shows up inside + the container :param adapter_number: adapter number :param port_number: port number on the adapter (standard Docker - adapters have a single port, so this is always 0 on the TAP path) + adapters have a single port, so this is always 0) """ try: @@ -1753,38 +1766,7 @@ class DockerVM(BaseNode): f"Port {port_number} doesn't exist on adapter {adapter_number} of Docker container '{self.name}'" ) - if isinstance(nio, NIOBridge): - await self._add_ubridge_kernel_connection(nio, adapter_number, port_number) - return - - for index in range(4096): - if f"tap-gns3-e{index}" not in psutil.net_if_addrs(): - adapter.host_ifc = f"tap-gns3-e{index!s}" - break - if adapter.host_ifc is None: - raise DockerError( - f"Adapter {adapter_number} couldn't allocate interface on Docker container '{self.name}'. Too many Docker interfaces already exists" - ) - bridge_name = self._bridge_name(adapter_number, port_number) - await self._ubridge_send(f"bridge create {bridge_name}") - self._bridges.add(bridge_name) - await self._ubridge_send(f"bridge add_nio_tap {bridge_name} {adapter.host_ifc} off") - - mac_address = self._adapter_mac_address(adapter_number) - - try: - await self._ubridge_send(f"docker set_mac_addr {adapter.host_ifc} {mac_address}") - except UbridgeError: - log.warning(f"Could not set MAC address {mac_address} on interface {adapter.host_ifc}") - - ifname = self._get_container_ifname(adapter_number) - log.debug(f"Move container {self.name} adapter {adapter.host_ifc} -> {ifname} in ns {self._namespace}") - try: - await self._ubridge_send(f"docker move_to_ns {adapter.host_ifc} {self._namespace} {ifname}") - except UbridgeError as e: - raise UbridgeNamespaceError(e) - else: - log.debug(f"Created adapter {adapter_number} with MAC address {mac_address} in namespace {self._namespace}") + await self._create_veth(adapter_number, port_number) if nio: await self._connect_nio(adapter_number, nio, port_number) @@ -1817,15 +1799,14 @@ class DockerVM(BaseNode): suffix = f"{self._id.replace('-', '')[:8]}e{adapter_number}p{port_number}" return f"gv{suffix}", f"gc{suffix}" - async def _add_ubridge_kernel_connection(self, nio, adapter_number, port_number=0): + async def _create_veth(self, adapter_number, port_number=0): """ - Wire an adapter port on the kernel datapath (NIOBridge): a veth pair - whose host end will be enslaved to the per-link kernel bridge, and - whose guest end becomes the container interface (the role the TAP - plays on the relay path). No uBridge relay bridge is created — frames - flow entirely in the kernel. Filters are unavailable on this datapath - (they live in the uBridge relay); packet capture and markers are - provided by uBridge's AF_PACKET modules on the veth host end. + Create the adapter's unified veth pair: the guest end moves into the + container namespace as eth{N} (the role the TAP used to play on the + relay path), the host end stays in the root namespace with carrier + off until a link attaches. Both datapaths anchor here — kernel links + enslave the host end into a Linux bridge; relay links attach it to + the uBridge relay via AF_PACKET. """ try: @@ -1849,7 +1830,7 @@ class DockerVM(BaseNode): try: await self._ubridge_send(f'docker create_veth "{host_ifc}" "{guest_ifc}"') # create_veth leaves the host end UP; carrier must be off until - # a link attaches (mirrors "bridge add_nio_tap ... off"). + # a link attaches (mirrors the old "bridge add_nio_tap ... off"). await self._ubridge_send(f'link set "{host_ifc}" down') try: await self._ubridge_send(f"docker set_mac_addr {guest_ifc} {mac_address}") @@ -1874,16 +1855,12 @@ class DockerVM(BaseNode): adapter.host_ifc = host_ifc self._kernel_veths[(adapter_number, port_number)] = host_ifc log.debug( - "Created kernel-datapath adapter {adapter_number} port {port_number} with MAC address {mac_address} in namespace {namespace}".format( + "Created veth adapter {adapter_number} port {port_number} with MAC address {mac_address} in namespace {namespace}".format( adapter_number=adapter_number, port_number=port_number, mac_address=mac_address, namespace=self._namespace, ) ) - if nio: - await self._connect_nio(adapter_number, nio, port_number) - await self._set_adapter_carrier(adapter_number, not nio.suspend, port_number) - async def _remove_kernel_veths(self): """ Delete the root-namespace veth host ends. Unlike relay TAPs (which die @@ -1923,12 +1900,16 @@ class DockerVM(BaseNode): async def _connect_nio(self, adapter_number, nio, port_number=0): + host_ifc = self._kernel_veths.get((adapter_number, port_number)) + if isinstance(nio, NIOBridge): - host_ifc = self._kernel_veths.get((adapter_number, port_number)) if host_ifc is None: + # Only reachable for containers started before the + # unified-veth change (or unix-socket vendor containers, + # rejected earlier): the kernel path needs the veth host end. raise DockerError( - "Adapter {adapter_number} port {port_number} of container '{name}' has no " - "kernel-datapath interface (it was started on the relay datapath); " + "Adapter {adapter_number} port {port_number} of container '{name}' has no veth " + "interface (it was started before the unified-veth datapath); " "restart the node to attach a kernel link to it".format( adapter_number=adapter_number, port_number=port_number, name=self.name ) @@ -1952,18 +1933,17 @@ class DockerVM(BaseNode): await self._ubridge_apply_markers(host_ifc, nio) return + # Relay NIO. On a veth-backed adapter (the unified Docker interface) + # the uBridge bridge reads/writes the veth host end via AF_PACKET + # (bridge add_nio_ethernet) instead of owning a TAP fd. Vendor + # containers with unix-socket wiring have no veth: their bridge and + # its unix NIO already exist (added by their _add_ubridge_connection + # override), so only the UDP half is wired here. bridge_name = self._bridge_name(adapter_number, port_number) - if (adapter_number, port_number) in self._kernel_veths and bridge_name not in self._bridges: - # A veth adapter attaching a UDP NIO (kernel link deleted and a - # relay link created on the running node): relay through AF_PACKET - # on the veth host end instead of a TAP. + if host_ifc is not None and bridge_name not in self._bridges: await self._ubridge_send(f"bridge create {bridge_name}") self._bridges.add(bridge_name) - await self._ubridge_send( - 'bridge add_nio_ethernet {bridge_name} "{hostif}"'.format( - bridge_name=bridge_name, hostif=self._kernel_veths[(adapter_number, port_number)] - ) - ) + await self._ubridge_send(f'bridge add_nio_ethernet {bridge_name} "{host_ifc}"') await self._ubridge_send( "bridge add_nio_udp {bridge_name} {lport} {rhost} {rport}".format( bridge_name=bridge_name, lport=nio.lport, rhost=nio.rhost, rport=nio.rport @@ -2242,13 +2222,17 @@ class DockerVM(BaseNode): if not self.ubridge: raise DockerError("Cannot start the packet capture: uBridge is not running") - host_ifc = self._kernel_veths.get((adapter_number, port_number)) - if host_ifc is not None: + nio = self._ethernet_adapters[adapter_number].get_nio(port_number) + if isinstance(nio, NIOBridge): # Kernel datapath: no relay bridge exists — capture via uBridge's # AF_PACKET module bound to the veth host end. Single capture per # uBridge process: a concurrent second one returns EALREADY. + host_ifc = self._kernel_veths.get((adapter_number, port_number)) await self._ubridge_send(f'capture start_kernel {host_ifc} "{output_file}"') return + # Relay datapath (even though the adapter interface is a veth, the + # traffic flows through the uBridge bridge — key on the NIO type, + # not on the veth presence): capture at the relay. bridge_name = self._bridge_name(adapter_number, port_number) await self._ubridge_send(f'bridge start_capture {bridge_name} "{output_file}"') @@ -2262,7 +2246,8 @@ class DockerVM(BaseNode): if not self.ubridge: raise DockerError("Cannot stop the packet capture: uBridge is not running") - if (adapter_number, port_number) in self._kernel_veths: + nio = self._ethernet_adapters[adapter_number].get_nio(port_number) + if isinstance(nio, NIOBridge): # Idempotent on the uBridge side (no active capture is OK). await self._ubridge_send("capture stop_kernel") return diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index ea759a5a8..4d02cf9fd 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -138,8 +138,12 @@ class UDPLink(Link): relay. The kernel path has no userspace relay, so filters — which live in the relay — disqualify it. Markers and capture are served by uBridge's AF_PACKET modules on the veth host end. - It is host-local (same compute) and requires the adapter interfaces - to be created as veths at container start, hence stopped nodes only. + + Docker adapters are born as veth pairs (unified interface), so the + datapath is a runtime decision — links attach to running containers + too: the compute side enslaves the veth host end (brctl) or attaches + the relay to it (add_nio_ethernet) without touching the container's + interfaces. """ if not Config.instance().settings.Server.enable_kernel_datapath: @@ -150,10 +154,6 @@ class UDPLink(Link): return False if self.get_active_filters(): return False - # Running nodes already have relay TAPs wired into uBridge; the - # kernel path needs the veth variant created at container start. - if node1.status != "stopped" or node2.status != "stopped": - return False if _is_unix_socket_docker(node1) or _is_unix_socket_docker(node2): return False return True diff --git a/tests/compute/docker/test_docker_kernel_datapath.py b/tests/compute/docker/test_docker_kernel_datapath.py index e200590da..f09ba6591 100644 --- a/tests/compute/docker/test_docker_kernel_datapath.py +++ b/tests/compute/docker/test_docker_kernel_datapath.py @@ -125,10 +125,11 @@ async def test_add_ubridge_kernel_connection(vm): @pytest.mark.asyncio -async def test_add_ubridge_connection_none_nio_stays_on_tap_path(vm): +async def test_add_ubridge_connection_none_nio_creates_veth(vm): """ - Unconnected adapters keep the relay TAP: the veth variant is only created - for adapters whose NIO is a kernel one at start time. + Unconnected adapters are born as veths too (the unified interface): the + interface exists inside the container with carrier off, and any link + type can be attached later — at runtime — without rebirthing it. """ vm._ubridge_hypervisor = MagicMock() @@ -136,11 +137,42 @@ async def test_add_ubridge_connection_none_nio_stays_on_tap_path(vm): await vm._add_ubridge_connection(None, 0) + host_ifc, guest_ifc = vm._veth_names(0, 0) vm._ubridge_hypervisor.assert_has_calls([ - call.send("bridge create bridge0"), - call.send("bridge add_nio_tap bridge0 tap-gns3-e0 off"), + call.send(f'docker create_veth "{host_ifc}" "{guest_ifc}"'), + call.send(f'link set "{host_ifc}" down'), + call.send(f'docker move_to_ns {guest_ifc} 42 eth0'), ], any_order=True) - assert vm._kernel_veths == {} + assert vm._kernel_veths[(0, 0)] == host_ifc + # no relay bridge and no TAP for an unconnected adapter + assert "bridge0" not in vm._bridges + for c in vm._ubridge_hypervisor.method_calls: + assert "add_nio_tap" not in str(c) + + +@pytest.mark.asyncio +async def test_add_ubridge_connection_udp_relays_over_veth(vm): + """ + A relay NIO at start: the adapter is born as a veth and the uBridge + relay attaches to its host end via AF_PACKET (add_nio_ethernet) — + no TAP anywhere. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._namespace = 42 + + nio = vm.manager.create_nio({"type": "nio_udp", "lport": 4242, "rport": 4343, "rhost": "127.0.0.1"}) + await vm._add_ubridge_connection(nio, 0) + + host_ifc, _ = vm._veth_names(0, 0) + vm._ubridge_hypervisor.assert_has_calls([ + call.send(f'docker create_veth "{host_ifc}" "{vm._veth_names(0, 0)[1]}"'), + call.send("bridge create bridge0"), + call.send(f'bridge add_nio_ethernet bridge0 "{host_ifc}"'), + call.send("bridge add_nio_udp bridge0 4242 127.0.0.1 4343"), + call.send("bridge start bridge0"), + ], any_order=True) + assert "bridge0" in vm._bridges @pytest.mark.asyncio diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index c98998a81..51a1e4176 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -1541,14 +1541,17 @@ async def test_add_ubridge_connection(vm): vm._namespace = 42 await vm._add_ubridge_connection(nio, 0) + host_ifc, guest_ifc = vm._veth_names(0, 0) calls = [ + call.send(f'docker create_veth "{host_ifc}" "{guest_ifc}"'), + call.send(f'link set "{host_ifc}" down'), + call.send(f"docker move_to_ns {guest_ifc} 42 eth0"), call.send("bridge create bridge0"), - call.send("bridge add_nio_tap bridge0 tap-gns3-e0 off"), - call.send("docker move_to_ns tap-gns3-e0 42 eth0"), + call.send(f'bridge add_nio_ethernet bridge0 "{host_ifc}"'), call.send("bridge add_nio_udp bridge0 4242 127.0.0.1 4343"), call.send('bridge start_capture bridge0 "/tmp/capture.pcap"'), call.send("bridge start bridge0"), - call.send("bridge set_nio_tap_carrier bridge0 on"), + call.send(f'link set "{host_ifc}" up'), ] assert "bridge0" in vm._bridges # We need to check any_order otherwise mock is confused by asyncio @@ -1571,11 +1574,13 @@ async def test_add_ubridge_connections_with_base_mac_address(vm): nio = vm.manager.create_nio(nio_params) await vm._add_ubridge_connection(nio, 1) + _, guest0 = vm._veth_names(0, 0) + _, guest1 = vm._veth_names(1, 0) calls = [ call.send("bridge create bridge0"), call.send("bridge create bridge1"), - call.send("docker set_mac_addr tap-gns3-e0 02:42:42:42:42:00"), - call.send("docker set_mac_addr tap-gns3-e0 02:42:42:42:42:01"), + call.send(f"docker set_mac_addr {guest0} 02:42:42:42:42:00"), + call.send(f"docker set_mac_addr {guest1} 02:42:42:42:42:01"), ] # We need to check any_order otherwise mock is confused by asyncio @@ -1591,12 +1596,14 @@ async def test_add_ubridge_connection_none_nio(vm): await vm._add_ubridge_connection(nio, 0) + host_ifc, guest_ifc = vm._veth_names(0, 0) calls = [ - call.send("bridge create bridge0"), - call.send("bridge add_nio_tap bridge0 tap-gns3-e0 off"), - call.send("docker move_to_ns tap-gns3-e0 42 eth0"), + call.send(f'docker create_veth "{host_ifc}" "{guest_ifc}"'), + call.send(f'link set "{host_ifc}" down'), + call.send(f"docker move_to_ns {guest_ifc} 42 eth0"), ] - assert "bridge0" in vm._bridges + # unconnected adapter: interface exists (carrier off), no relay bridge + assert "bridge0" not in vm._bridges # We need to check any_order ortherwise mock is confused by asyncio vm._ubridge_hypervisor.assert_has_calls(calls, any_order=True) @@ -1626,19 +1633,6 @@ async def test_add_ubridge_connection_invalid_adapter_number(vm): await vm._add_ubridge_connection(nio, 12) -@pytest.mark.asyncio -async def test_add_ubridge_connection_no_free_interface(vm): - - nio = {"type": "nio_udp", "lport": 4242, "rport": 4343, "rhost": "127.0.0.1"} - nio = vm.manager.create_nio(nio) - with pytest.raises(DockerError): - # We create fake ethernet interfaces for docker - interfaces = [f"tap-gns3-e{index}" for index in range(4096)] - - with patch("psutil.net_if_addrs", return_value=interfaces): - await vm._add_ubridge_connection(nio, 0) - - @pytest.mark.asyncio async def test_adapter_add_nio_binding_1(vm): diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index 93ef04905..f508bcf29 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -314,7 +314,7 @@ async def test_move_to_ns_uses_renamed_interface(compute_project, manager): # adapter 0 should be renamed to mgmt0 move_calls = [c for c in vm._ubridge_hypervisor.method_calls if "move_to_ns" in str(c)] assert move_calls, "move_to_ns was not sent" - assert call.send("docker move_to_ns tap-gns3-e0 42 mgmt0") in move_calls + assert call.send(f"docker move_to_ns {vm._veth_names(0, 0)[1]} 42 mgmt0") in move_calls @pytest.mark.asyncio @@ -326,7 +326,7 @@ async def test_move_to_ns_falls_back_to_eth(compute_project, manager): nio = manager.create_nio({"type": "nio_udp", "lport": 4242, "rport": 4343, "rhost": "127.0.0.1"}) await vm._add_ubridge_connection(nio, 1) move_calls = [c for c in vm._ubridge_hypervisor.method_calls if "move_to_ns" in str(c)] - assert call.send("docker move_to_ns tap-gns3-e0 42 eth1") in move_calls + assert call.send(f"docker move_to_ns {vm._veth_names(1, 0)[1]} 42 eth1") in move_calls # --------------------------------------------------------------------------- diff --git a/tests/controller/test_kernel_datapath_link.py b/tests/controller/test_kernel_datapath_link.py index 755ad3bfa..b47532de6 100644 --- a/tests/controller/test_kernel_datapath_link.py +++ b/tests/controller/test_kernel_datapath_link.py @@ -123,11 +123,16 @@ async def test_kernel_datapath_eligible_with_project_marker_definitions(project) @pytest.mark.asyncio -async def test_kernel_datapath_not_eligible_for_running_nodes(project): +async def test_kernel_datapath_eligible_for_running_nodes(project): + """ + Docker adapters are born as veths (unified interface), so links attach + to running containers too — no stopped-node requirement. + """ link, node1, node2 = await _kernel_link(project) node1._status = "started" - assert link._kernel_datapath_eligible(node1, node2) is False + node2._status = "started" + assert link._kernel_datapath_eligible(node1, node2) is True @pytest.mark.asyncio