diff --git a/e2e/README.md b/e2e/README.md index d8d858702..b2cf5be12 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -134,7 +134,11 @@ offering the `dut-network` driver (nftables NAT/masquerade + DHCP/DNS). CI insta | should return error for unknown MAC | `j dut-network get-ip ff:ff:ff:ff:ff:ff` | errors; "No lease found" | | should add and remove an address entry via CLI | `add-address 192.168.200.99 --mac 02:00:00:00:00:99` then `remove-address` | "Added" then "Removed" | | should add, list, and remove DNS entries via CLI | `add-dns e2e-test.lab.local 10.0.0.42`, `dns-entries`, `remove-dns`, `dns-entries` | entry appears then disappears | -| should allow TCP connections from DUT to external via NAT | start Python TCP server in ext ns, connect from DUT ns via NAT | client receives "E2E_OK" | +| should allow TCP connections from DUT to external via NAT | start Python TCP echo in ext ns, connect from DUT ns via NAT | client receives "E2E_OK" | +| should allow TCP from DUT via VLAN PBR | create `jmp-vext.100` with `10.100.0.1/24` in ext ns; `add-address 192.168.200.50 --vlan-id 100 --public-ip 10.100.0.50 --public-gateway 10.100.0.1`; add DUT IP; TCP from `192.168.200.50` to `10.100.0.1` | client receives "E2E_OK" | +| should allow TCP from DUT via untagged source-IP PBR | add `10.99.1.1/32` on ext-ns loopback (PBR-only destination); `add-address 192.168.200.51 --public-gateway 10.99.0.1`; add DUT IP; TCP from `192.168.200.51` to `10.99.1.1`; ping from main DUT IP to `10.99.1.1` | TCP succeeds (PBR routes via gateway); ping from non-PBR source fails (proves PBR is required) | +| should not reach a VLAN-only peer without public_gateway | create `jmp-vext.101` with `10.101.0.1/24` in ext ns; `add-address 192.168.200.52 --vlan-id 101`; add DUT IP; ping VLAN-only `10.101.0.1` and untagged `10.99.0.1` | ping to `10.101.0.1` fails; ping to `10.99.0.1` succeeds (`Eventually`) | +| should masquerade unregistered DUT alongside VLAN-registered DUT | register `192.168.200.50` with VLAN 100 + PBR; verify VLAN TCP echo works; add unregistered `192.168.200.60` on DUT bridge (no `add-address`); ping external `10.99.0.1` from unregistered IP | VLAN DUT gets "E2E_OK"; unregistered DUT ping succeeds via upstream masquerade | --- diff --git a/e2e/test/dut_network_test.go b/e2e/test/dut_network_test.go index 4cb426da4..433bf29a5 100644 --- a/e2e/test/dut_network_test.go +++ b/e2e/test/dut_network_test.go @@ -53,6 +53,24 @@ func sudoArgs(args ...string) (string, []string) { // Serial: builds veth pairs, bridges and nftables rules in the host network // namespace, and drives dnsmasq. There is only one host to share. +// +// Baseline topology (created by setupNetworkNamespaces): +// +// netns: jmp-e2e-dut HOST (exporter) netns: jmp-e2e-ext +// ┌─────────────────┐ ┌──────────────────────────────┐ ┌──────────────────────┐ +// │ │ │ │ │ │ +// │ jmp-vdut │ │ jmp-vhost jmp-vup │ │ jmp-vext │ +// │ 192.168.200.10 │◄─►│ (DUT iface) 10.99.0.2/24 │◄─►│ 10.99.0.1/24 │ +// │ │ │ 02:00:...:01 │ │ │ +// │ default via │ │ │ │ │ route: 192.168.200 │ +// │ 192.168.200.1 │ │ nftables: masquerade │ │ .0/24 via 10.99 │ +// │ │ │ dnsmasq: DHCP on jmp-vhost │ │ .0.2 │ +// └─────────────────┘ └──────────────────────────────┘ └──────────────────────┘ +// veth pair veth pair +// jmp-vdut ◄─► jmp-vhost jmp-vup ◄─► jmp-vext +// +// VLAN/PBR tests add per-test overlays on top of this baseline. +// See the diagram above each test for details. var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, ContinueOnFailure, Serial, func() { var ( tracker *ProcessTracker @@ -61,18 +79,43 @@ var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, Continu ) const ( - dutNs = "jmp-e2e-dut" - extNs = "jmp-e2e-ext" - vethHost = "jmp-vhost" - vethDut = "jmp-vdut" - vethUp = "jmp-vup" - vethExt = "jmp-vext" - nftTable = "jumpstarter_jmp_vhost" - dutIP = "192.168.200.10" - gatewayIP = "192.168.200.1" - extIP = "10.99.0.1" - upstreamIP = "10.99.0.2" + // Namespaces + dutNs = "jmp-e2e-dut" // simulates the DUT side of the network + extNs = "jmp-e2e-ext" // simulates the external/LAN side + + // Veth pairs + vethHost = "jmp-vhost" // host-side DUT interface (exporter manages this) + vethDut = "jmp-vdut" // DUT-side end (lives in dutNs) + vethUp = "jmp-vup" // host-side upstream interface + vethExt = "jmp-vext" // ext-side end (lives in extNs) + + // nftables + nftTable = "jumpstarter_jmp_vhost" // driver's nft table name + + // Baseline IPs + dutIP = "192.168.200.10" // pre-configured DUT address + gatewayIP = "192.168.200.1" // gateway on the DUT interface + extIP = "10.99.0.1" // external network address (ext-ns) + upstreamIP = "10.99.0.2" // upstream address (host-side) subnet = "192.168.200.0/24" + + // VLAN PBR test (vlan_id=100) + vlanID = 100 + vlanDutIP = "192.168.200.50" // DUT private IP for VLAN test + vlanPubIP = "10.100.0.50" // public IP alias on VLAN sub-iface + vlanExtIP = "10.100.0.1" // ext-ns address on VLAN 100 + + // Untagged PBR test + pbrDutIP = "192.168.200.51" // DUT IP with source-IP PBR + pbrOnlyIP = "10.99.1.1" // destination reachable only via PBR + + // No-PBR VLAN test (vlan_id=101, no public_gateway) + noPbrVlan = 101 + noPbrDutIP = "192.168.200.52" // DUT IP on VLAN without PBR + noPbrExtIP = "10.101.0.1" // ext-ns address on VLAN 101 + + // Unregistered DUT test + unregisteredIP = "192.168.200.60" // IP never added via add-address ) setupNetworkNamespaces := func() { @@ -169,6 +212,17 @@ var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, Continu return raw[start:] } + addDutAddr := func(ip string) { + runInNs(dutNs, "ip", "addr", "replace", ip+"/24", "dev", vethDut) + } + delDutAddr := func(ip string) { + _, _ = runInNsCapture(dutNs, "ip", "addr", "del", ip+"/24", "dev", vethDut) + } + + setupExtVLAN := func(id int, cidr string) string { + return setupVLANInNs(extNs, vethExt, id, cidr) + } + Context("Network status", func() { It("should report network status via CLI", func() { out, err := jmpShell("j", "dut-network", "status") @@ -202,11 +256,7 @@ var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, Continu Context("Connectivity", func() { It("should allow DUT to reach external via NAT", func() { - Eventually(func() error { - _, err := runInNsCapture(dutNs, "ping", "-c", "1", "-W", "2", extIP) - return err - }, 10*time.Second, 1*time.Second).Should(Succeed(), - "DUT should be able to ping external IP %s via NAT", extIP) + expectPingNS(dutNs, "", extIP) }) }) @@ -255,39 +305,163 @@ var _ = Describe("DUT Network E2E Tests", Label("dut-network"), Ordered, Continu Context("TCP connectivity", func() { It("should allow TCP connections from DUT to external via NAT", func() { - serverScript := "import socket; " + - "s=socket.socket(); " + - "s.setsockopt(socket.SOL_SOCKET,socket.SO_REUSEADDR,1); " + - "s.bind(('',9998)); " + - "s.listen(1); " + - "s.settimeout(10); " + - "conn,_=s.accept(); " + - "conn.sendall(b'E2E_OK'); " + - "conn.close(); " + - "s.close()" - - fullArgs := []string{"ip", "netns", "exec", extNs, "python3", "-c", serverScript} - bin, cmdArgs := sudoArgs(fullArgs...) - listener := exec.Command(bin, cmdArgs...) //nolint:gosec - listener.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - Expect(listener.Start()).To(Succeed()) + expectTCPEcho(dutNs, extNs, "", extIP, 9998) + }) + }) + + Context("VLAN and policy-based routing", func() { + // Test: VLAN PBR (tagged traffic with public IP + gateway) + // + // DUT ns HOST ext ns + // ┌──────────────┐ ┌────────────────────────┐ ┌────────────────────┐ + // │ .200.50/24 │ │ jmp-vup.100 │ │ jmp-vext.100 │ + // │ (vlanDutIP) │──►│ 10.100.0.50/24 │◄─►│ 10.100.0.1/24 │ + // │ │ │ (public_ip alias) │ │ (vlanExtIP) │ + // │ ip rule: │ │ │ │ │ + // │ from .200.50│ │ PBR table 100: │ │ TCP echo server │ + // │ lookup 100 │ │ default via 10.100.0.1│ │ on :9998 │ + // └──────────────┘ └────────────────────────┘ └────────────────────┘ + // + // Traffic: .200.50 → SNAT to 10.100.0.50 → PBR table 100 + // → via 10.100.0.1 (VLAN gateway) → ext ns → echo OK + It("should allow TCP from DUT via VLAN PBR", func() { + extVlan := setupExtVLAN(vlanID, vlanExtIP+"/24") + defer deleteLinkInNs(extNs, extVlan) + + out, err := jmpShell("j", "dut-network", "add-address", + vlanDutIP, "--public-ip", vlanPubIP, + "--vlan-id", fmt.Sprintf("%d", vlanID), "--public-gateway", vlanExtIP) + Expect(err).NotTo(HaveOccurred(), out) + + addDutAddr(vlanDutIP) + defer func() { + delDutAddr(vlanDutIP) + _, _ = jmpShell("j", "dut-network", "remove-address", vlanDutIP) + }() + + expectTCPEcho(dutNs, extNs, vlanDutIP, vlanExtIP, 9998) + }) + + // Test: Untagged source-IP PBR (no VLAN, gateway on upstream) + // + // DUT ns HOST ext ns + // ┌──────────────┐ ┌──────────────────────┐ ┌──────────────────┐ + // │ .200.51/24 │ │ jmp-vup │ │ jmp-vext │ + // │ (pbrDutIP) │──►│ 10.99.0.2/24 │◄─►│ 10.99.0.1/24 │ + // │ │ │ (upstream, untagged)│ │ lo: 10.99.1.1 │ + // │ ip rule: │ │ │ │ (pbrOnlyIP) │ + // │ from .200.51│ │ PBR table N: │ │ │ + // │ lookup N │ │ default via 10.99.0.│ │ TCP echo on │ + // └──────────────┘ └──────────────────────┘ │ :9998 binds │ + // │ 10.99.1.1 │ + // N = int(192.168.200.51) = 3232286771 └──────────────────┘ + // + // 10.99.1.1 is on ext-ns loopback — NO route in host main table. + // Only the PBR table (default via 10.99.0.1) can reach it. + // + // Positive: .200.51 → PBR → via 10.99.0.1 → ext ns lo → echo OK + // Negative: .200.10 (no PBR rule) → main table → no route → FAIL + It("should allow TCP from DUT via untagged source-IP PBR", func() { + // 10.99.1.1 on ext-ns loopback: reachable ONLY through PBR. + runInNs(extNs, "ip", "addr", "add", pbrOnlyIP+"/32", "dev", "lo") + defer func() { + _, _ = runInNsCapture(extNs, "ip", "addr", "del", pbrOnlyIP+"/32", "dev", "lo") + }() + + out, err := jmpShell("j", "dut-network", "add-address", + pbrDutIP, "--public-gateway", extIP) + Expect(err).NotTo(HaveOccurred(), out) + + addDutAddr(pbrDutIP) + defer func() { + delDutAddr(pbrDutIP) + _, _ = jmpShell("j", "dut-network", "remove-address", pbrDutIP) + }() + + // PBR source: traffic from pbrDutIP uses the PBR table + // whose default route goes via extIP (10.99.0.1) — the + // ext namespace delivers 10.99.1.1 locally on its loopback. + expectTCPEcho(dutNs, extNs, pbrDutIP, pbrOnlyIP, 9998) + + // Non-PBR source: the main DUT IP has no PBR rule, so + // 10.99.1.1 is unreachable through the main routing table. + Expect(pingNS(dutNs, dutIP, pbrOnlyIP)).To(HaveOccurred(), + "main DUT IP should NOT reach %s without PBR", pbrOnlyIP) + }) + + // Test: VLAN without PBR (negative — proves gateway is required) + // + // DUT ns HOST ext ns + // ┌──────────────┐ ┌──────────────────────┐ ┌──────────────────┐ + // │ .200.52/24 │ │ jmp-vup.101 │ │ jmp-vext.101 │ + // │ (noPbrDutIP) │──►│ (no IP, no gateway) │◄─►│ 10.101.0.1/24 │ + // │ │ │ │ │ (noPbrExtIP) │ + // │ NO ip rule │ │ NO PBR table │ │ │ + // │ for .200.52 │ │ for VLAN 101 │ │ │ + // └──────────────┘ └──────────────────────┘ └──────────────────┘ + // + // VLAN 101 exists but has no public_gateway → no PBR route. + // Ping .200.52 → 10.101.0.1: FAIL (no route through VLAN) + // Ping .200.52 → 10.99.0.1: OK (falls back to upstream masquerade) + It("should not reach a VLAN-only peer without public_gateway", func() { + extVlan := setupExtVLAN(noPbrVlan, noPbrExtIP+"/24") + defer deleteLinkInNs(extNs, extVlan) + + out, err := jmpShell("j", "dut-network", "add-address", + noPbrDutIP, "--vlan-id", fmt.Sprintf("%d", noPbrVlan)) + Expect(err).NotTo(HaveOccurred(), out) + + addDutAddr(noPbrDutIP) + defer func() { + delDutAddr(noPbrDutIP) + _, _ = jmpShell("j", "dut-network", "remove-address", noPbrDutIP) + }() + + Expect(pingNS(dutNs, noPbrDutIP, noPbrExtIP)).To(HaveOccurred(), + "DUT should not reach VLAN-only %s without public_gateway/PBR", noPbrExtIP) + expectPingNS(dutNs, noPbrDutIP, extIP) + }) + + // Test: Unregistered DUT still masqueraded when VLAN is active + // + // DUT ns HOST ext ns + // ┌──────────────┐ ┌───────────────────────┐ ┌─────────────────┐ + // │ .200.50/24 │ │ jmp-vup.100 │ │ jmp-vext.100 │ + // │ (registered, │──►│ 10.100.0.50/24 │◄─►│ 10.100.0.1/24 │ + // │ VLAN PBR) │ │ PBR table 100 │ │ │ + // │ │ │ │ │ │ + // │ .200.60/24 │ │ jmp-vup │ │ jmp-vext │ + // │ (unregistered│──►│ 10.99.0.2/24 │◄─►│ 10.99.0.1/24 │ + // │ no add-addr)│ │ masquerade (upstream)│ │ │ + // └──────────────┘ └───────────────────────┘ └─────────────────┘ + // + // .200.50 is registered with VLAN 100 → TCP echo via PBR: OK + // .200.60 is never add-address'd → must still reach 10.99.0.1 + // via upstream masquerade (upstream always in outbound list) + It("should masquerade unregistered DUT alongside VLAN-registered DUT", func() { + extVlan := setupExtVLAN(vlanID, vlanExtIP+"/24") + defer deleteLinkInNs(extNs, extVlan) + + out, err := jmpShell("j", "dut-network", "add-address", + vlanDutIP, "--public-ip", vlanPubIP, + "--vlan-id", fmt.Sprintf("%d", vlanID), "--public-gateway", vlanExtIP) + Expect(err).NotTo(HaveOccurred(), out) + + addDutAddr(vlanDutIP) defer func() { - _ = syscall.Kill(-listener.Process.Pid, syscall.SIGKILL) - _ = listener.Wait() + delDutAddr(vlanDutIP) + _, _ = jmpShell("j", "dut-network", "remove-address", vlanDutIP) }() - time.Sleep(500 * time.Millisecond) - - clientScript := fmt.Sprintf( - "import socket; "+ - "s=socket.create_connection(('%s',9998),timeout=5); "+ - "data=s.recv(10); "+ - "s.close(); "+ - "print(data.decode())", - extIP) - out, err := runInNsCapture(dutNs, "python3", "-c", clientScript) - Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("TCP connection failed: %s", out)) - Expect(out).To(ContainSubstring("E2E_OK")) + // The registered VLAN DUT should work through PBR. + expectTCPEcho(dutNs, extNs, vlanDutIP, vlanExtIP, 9998) + + // An unregistered DUT IP on the same bridge — never added via + // add-address — should still reach external via upstream masquerade. + addDutAddr(unregisteredIP) + defer delDutAddr(unregisteredIP) + + expectPingNS(dutNs, unregisteredIP, extIP) }) }) }) @@ -322,3 +496,113 @@ func runInNsCapture(ns string, args ...string) (string, error) { out, err := cmd.CombinedOutput() return string(out), err } + +func deleteLinkInNs(ns, name string) { + _, _ = runInNsCapture(ns, "ip", "link", "del", name) +} + +func setupVLANInNs(ns, parent string, id int, cidr string) string { + name := fmt.Sprintf("%s.%d", parent, id) + deleteLinkInNs(ns, name) + runInNs(ns, "ip", "link", "add", "link", parent, "name", name, + "type", "vlan", "id", fmt.Sprintf("%d", id)) + runInNs(ns, "ip", "addr", "replace", cidr, "dev", name) + runInNs(ns, "ip", "link", "set", name, "up") + return name +} + +func pingNS(ns, src, dst string) error { + args := []string{"ping", "-c", "1", "-W", "2"} + if src != "" { + args = append(args, "-I", src) + } + args = append(args, dst) + _, err := runInNsCapture(ns, args...) + return err +} + +func expectPingNS(ns, src, dst string) { + GinkgoHelper() + Eventually(func() error { + return pingNS(ns, src, dst) + }, 10*time.Second, 1*time.Second).Should(Succeed(), + "namespace %s src %q should ping %s", ns, src, dst) +} + +func tcpEchoServerScript(bind string, port int) string { + return fmt.Sprintf( + "import socket; "+ + "s=socket.socket(); "+ + "s.setsockopt(socket.SOL_SOCKET,socket.SO_REUSEADDR,1); "+ + "s.bind(('%s',%d)); "+ + "s.listen(1); "+ + "s.settimeout(10); "+ + "conn,_=s.accept(); "+ + "conn.sendall(b'E2E_OK'); "+ + "conn.close(); "+ + "s.close()", + bind, port) +} + +func tcpEchoClientScript(src, dst string, port int) string { + if src == "" { + return fmt.Sprintf( + "import socket; "+ + "s=socket.create_connection(('%s',%d),timeout=5); "+ + "data=s.recv(10); "+ + "s.close(); "+ + "print(data.decode())", + dst, port) + } + return fmt.Sprintf( + "import socket; "+ + "s=socket.socket(); "+ + "s.settimeout(5); "+ + "s.bind(('%s',0)); "+ + "s.connect(('%s',%d)); "+ + "data=s.recv(10); "+ + "s.close(); "+ + "print(data.decode())", + src, dst, port) +} + +func startPythonInNs(ns, script string) (*exec.Cmd, error) { + fullArgs := []string{"ip", "netns", "exec", ns, "python3", "-c", script} + bin, cmdArgs := sudoArgs(fullArgs...) + cmd := exec.Command(bin, cmdArgs...) //nolint:gosec + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + return nil, err + } + return cmd, nil +} + +func stopProcessGroup(cmd *exec.Cmd) { + if cmd == nil || cmd.Process == nil { + return + } + _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + _ = cmd.Wait() +} + +func tcpEchoBetweenNS(dutNs, extNs, src, dst string, port int) (string, error) { + bind := "" + if src != "" { + bind = dst + } + listener, err := startPythonInNs(extNs, tcpEchoServerScript(bind, port)) + if err != nil { + return "", err + } + defer stopProcessGroup(listener) + time.Sleep(500 * time.Millisecond) + return runInNsCapture(dutNs, "python3", "-c", tcpEchoClientScript(src, dst, port)) +} + +func expectTCPEcho(dutNs, extNs, src, dst string, port int) { + GinkgoHelper() + out, err := tcpEchoBetweenNS(dutNs, extNs, src, dst, port) + Expect(err).NotTo(HaveOccurred(), + fmt.Sprintf("TCP %s -> %s:%d failed: %s", src, dst, port, out)) + Expect(out).To(ContainSubstring("E2E_OK")) +} diff --git a/python/packages/jumpstarter-driver-dut-network/README.md b/python/packages/jumpstarter-driver-dut-network/README.md index 7cc06e20b..d5f6b3661 100644 --- a/python/packages/jumpstarter-driver-dut-network/README.md +++ b/python/packages/jumpstarter-driver-dut-network/README.md @@ -64,17 +64,104 @@ export: - mac: "8a:12:4e:25:f4:8e" ip: "192.168.100.10" hostname: "sa8775p-1" - public_ip: "10.26.28.84" + public_ip: "198.51.100.84" - mac: "8a:12:4e:25:f4:8f" ip: "192.168.100.11" hostname: "sa8775p-2" - public_ip: "10.26.28.85" + public_ip: "198.51.100.85" # Entry without MAC: 1:1 NAT mapping only, no DHCP static lease - ip: "192.168.100.12" hostname: "nxp-board-03" - public_ip: "10.26.28.86" + public_ip: "198.51.100.86" ``` +### VLAN sub-interfaces and policy-based routing + +When an address entry sets `vlan_id`, the driver creates a tagged sub-interface +on `upstream_interface` (for example `end0.905` when upstream is `end0` and +the VLAN is 905), assigns `public_ip` to that sub-interface, and generates +nftables NAT/forward rules against it instead of the untagged parent. + +If `public_gateway` is also set, policy-based routing (PBR) forces traffic +from that DUT's private IP out via the VLAN: a default route in routing table +`` and an `ip rule` matching the DUT source address. + +`vlan_id` without `public_gateway` still creates the VLAN and NAT rules, but +logs a warning: without PBR, DUT traffic may not egress via the tagged +interface. + +`public_gateway` without `vlan_id` is supported on the untagged upstream: +source-IP PBR uses routing table `int()`. + +**Untagged source-IP PBR:** + +```yaml + addresses: + - mac: "8a:12:4e:25:f4:8e" + ip: "192.168.100.125" + public_gateway: "203.0.113.254" +``` + +Omit both fields to keep today's untagged-upstream behaviour. + +**1:1 NAT on a VLAN:** + +```yaml +export: + dut-network: + type: jumpstarter_driver_dut_network.driver.DutNetwork + config: + interface: "enp1s0u1" + subnet: "192.168.100.0/24" + gateway_ip: "192.168.100.1" + upstream_interface: "end0" + nat_mode: "1to1" + addresses: + - mac: "8a:12:4e:25:f4:8e" + ip: "192.168.100.125" + hostname: "sa8775p" + public_ip: "203.0.113.1" + vlan_id: 905 + public_gateway: "203.0.113.254" +``` + +**Masquerade on a VLAN:** + +```yaml +export: + dut-network: + type: jumpstarter_driver_dut_network.driver.DutNetwork + config: + interface: "enp1s0u1" + subnet: "192.168.100.0/24" + gateway_ip: "192.168.100.1" + upstream_interface: "end0" + nat_mode: "masquerade" + addresses: + - mac: "8a:12:4e:25:f4:8e" + ip: "192.168.100.125" + hostname: "sa8775p" + public_ip: "203.0.113.1" + vlan_id: 905 + public_gateway: "203.0.113.254" +``` + +Linux interface names are limited to 15 characters, so +`.` must fit that limit. VLAN IDs 253–255 cannot be used +with `public_gateway` because those routing-table IDs are reserved by the kernel. + +#### Mixed VLAN and untagged addresses + +Tagged and untagged entries can coexist on the same exporter. The +upstream (untagged) interface is **always** included in the masquerade +and forwarding rules so that unexpected or unregistered DUT hosts on +the bridge are still NATed via the default upstream. VLAN +sub-interfaces are added when at least one address entry carries a +`vlan_id`. Adding or removing addresses at runtime (via `add-address` +/ `remove-address`) triggers a full rebuild, so the rule set always +matches the current address list. + + ### Disabled NAT (DHCP only) DHCP works normally but no NAT rules or IP forwarding are configured. Useful for pure L2 isolation or when routing is handled externally: @@ -102,9 +189,9 @@ export: nat_mode: "masquerade" dns_entries: - hostname: "controller.lab.local" - ip: "10.26.28.1" + ip: "198.51.100.1" - hostname: "registry.lab.local" - ip: "10.26.28.2" + ip: "198.51.100.2" ``` ### Reference @@ -118,7 +205,7 @@ export: | `dhcp_enabled` | bool | `true` | Whether to run DHCP on the interface | | `dhcp_range_start` | str | `192.168.100.100` | DHCP dynamic range start | | `dhcp_range_end` | str | `192.168.100.200` | DHCP dynamic range end | -| `addresses` | list | `[]` | Address entries: `{ip, mac?, hostname?, public_ip?}`. Entries with `mac` generate DHCP static leases; entries without `mac` are used for 1:1 NAT only. | +| `addresses` | list | `[]` | Address entries: `{ip, mac?, hostname?, public_ip?, vlan_id?, public_gateway?}`. Entries with `mac` generate DHCP static leases; entries without `mac` are used for 1:1 NAT only. | | `dns_servers` | list | `[8.8.8.8, 8.8.4.4]` | DNS servers for DHCP clients | | `dns_entries` | list | `[]` | Custom DNS records: `{hostname, ip}` | | `state_dir` | str | `/var/lib/jumpstarter/dut-network-{interface}/` | Directory for dnsmasq state files | @@ -132,7 +219,9 @@ export: | `ip` | yes | Private IP to assign | | `mac` | no | MAC address of the DUT. Required for DHCP static lease; omit for 1:1 NAT-only entries | | `hostname` | no | Hostname for DHCP | -| `public_ip` | no | Public IP for 1:1 NAT (per-entry). At least one entry must have `public_ip` when `nat_mode=1to1` | +| `public_ip` | no | Public IP for 1:1 NAT (per-entry). At least one entry must have `public_ip` when `nat_mode=1to1`. Also assigned to the VLAN sub-interface when `vlan_id` is set | +| `vlan_id` | no | 802.1Q VLAN ID (1–4094). When set, NAT uses `.` instead of the untagged upstream. Omit for untagged behaviour | +| `public_gateway` | no | Gateway for policy-based routing. With `vlan_id`, DUT traffic exits via the VLAN (table ID = VLAN ID). Without `vlan_id`, traffic exits via the untagged upstream (table ID = DUT private IPv4 as an integer) | ## Usage @@ -153,8 +242,8 @@ j dut-network get-ip 8a:12:4e:25:f4:8e # Add an address entry with a MAC (creates a DHCP static lease) j dut-network add-address 192.168.100.50 --mac 02:00:00:aa:bb:cc --hostname my-dut -# Add an address entry without MAC (1:1 NAT mapping only, no DHCP lease) -j dut-network add-address 192.168.100.51 --public-ip 10.26.28.90 +# Add an address entry with VLAN and PBR +j dut-network add-address 192.168.100.125 --public-ip 203.0.113.1 --vlan-id 905 --public-gateway 203.0.113.254 # Remove an address entry by IP j dut-network remove-address 192.168.100.50 @@ -166,7 +255,7 @@ j dut-network nat-rules j dut-network dns-entries # Add a custom DNS entry -j dut-network add-dns controller.lab.local 10.26.28.1 +j dut-network add-dns controller.lab.local 198.51.100.1 # Remove a DNS entry j dut-network remove-dns controller.lab.local @@ -194,7 +283,14 @@ with env() as client: # With MAC: creates a DHCP static lease + optional 1:1 NAT mapping client.dut_network.add_address("192.168.100.50", mac="02:00:00:aa:bb:cc", hostname="new-dut") # Without MAC: 1:1 NAT mapping only (no DHCP lease) - client.dut_network.add_address("192.168.100.51", public_ip="10.26.28.90") + client.dut_network.add_address("192.168.100.51", public_ip="198.51.100.90") + # VLAN + PBR + client.dut_network.add_address( + "192.168.100.125", + public_ip="203.0.113.1", + vlan_id=905, + public_gateway="203.0.113.254", + ) client.dut_network.remove_address("192.168.100.50") # Manage DNS entries at runtime @@ -297,3 +393,58 @@ sysctl net.ipv4.conf..forwarding sysctl net.ipv4.conf..forwarding ``` +## Host System Side Effects + +```{warning} +The DUT network driver modifies host networking state. It is designed for +dedicated exporter hosts or containers, **not** shared workstations or +laptops. Running it on a multi-purpose machine may interfere with other +network configurations. +``` + +The driver creates and removes several types of host-level networking +resources. Under normal operation these are cleaned up when the exporter +shuts down, but an unclean exit (crash, `kill -9`, power loss) will leave +them behind. Re-starting the exporter recreates the resources from +scratch, so orphaned state from a previous run is overwritten — but if +the exporter is never restarted, manual cleanup may be necessary. + +### What the driver creates on the host + +| Resource | Created when | Cleaned up on shutdown | Survives a crash | +|----------|-------------|----------------------|------------------| +| **VLAN sub-interfaces** (e.g. `eth0.905`) | Address entry has `vlan_id` | Yes — deleted by `cleanup()` | Yes | +| **nftables table** (`jumpstarter_`) | NAT mode is not `disabled` | Yes — flushed on cleanup | Yes | +| **FORWARD chain accept rules** (in `ip filter`) | Docker sets FORWARD policy to `drop` | Yes — removed by handle | Yes | +| **IP forwarding sysctls** (`net.ipv4.conf..forwarding`) | NAT mode is not `disabled` | Yes — restored to previous value | Yes | +| **IP aliases** (e.g. `203.0.113.1/24` on upstream) | 1:1 NAT with `public_ip` | Yes — removed on cleanup | Yes | +| **Policy routes and IP rules** | `public_gateway` is set | Yes — flushed on cleanup | Yes | +| **dnsmasq process** | DHCP is enabled | Yes — stopped on cleanup | No (orphan process) | + +### Cleaning up after a crash + +If the exporter crashes, the simplest recovery is to restart it — the +driver recreates all resources idempotently. To clean up manually: + +```shell +# Remove orphan VLAN interfaces +sudo ip link del eth0.905 + +# Flush the driver's nftables table +sudo nft delete table ip jumpstarter_eth2 + +# Remove stale FORWARD chain rules (find handles first) +sudo nft -a list chain ip filter FORWARD | grep jmp +sudo nft delete rule ip filter FORWARD handle + +# Remove IP aliases +sudo ip addr del 203.0.113.1/24 dev eth0 + +# Flush policy routing tables +sudo ip route flush table 905 +sudo ip rule del from 192.168.100.125 table 905 + +# Kill orphan dnsmasq +sudo pkill -f "dnsmasq.*jumpstarter" +``` + diff --git a/python/packages/jumpstarter-driver-dut-network/examples/exporter-1to1-nat.yaml b/python/packages/jumpstarter-driver-dut-network/examples/exporter-1to1-nat.yaml index 16be59226..0c751decf 100644 --- a/python/packages/jumpstarter-driver-dut-network/examples/exporter-1to1-nat.yaml +++ b/python/packages/jumpstarter-driver-dut-network/examples/exporter-1to1-nat.yaml @@ -21,16 +21,16 @@ export: - mac: "8a:12:4e:25:f4:8e" ip: "192.168.100.10" hostname: "sa8775p-1" - public_ip: "10.26.28.84" + public_ip: "198.51.100.84" - mac: "8a:12:4e:25:f4:8f" ip: "192.168.100.11" hostname: "sa8775p-2" - public_ip: "10.26.28.85" + public_ip: "198.51.100.85" # Entry without MAC: 1:1 NAT mapping only, no DHCP static lease - ip: "192.168.100.12" hostname: "nxp-board-03" - public_ip: "10.26.28.86" + public_ip: "198.51.100.86" dns_servers: ["8.8.8.8", "8.8.4.4"] dns_entries: - hostname: "controller.lab.local" - ip: "10.26.28.1" + ip: "198.51.100.1" diff --git a/python/packages/jumpstarter-driver-dut-network/examples/exporter-vlan.yaml b/python/packages/jumpstarter-driver-dut-network/examples/exporter-vlan.yaml new file mode 100644 index 000000000..b33234091 --- /dev/null +++ b/python/packages/jumpstarter-driver-dut-network/examples/exporter-vlan.yaml @@ -0,0 +1,32 @@ +apiVersion: jumpstarter.dev/v1alpha1 +kind: ExporterConfig +metadata: + namespace: default + name: automotive-lab-vlan +endpoint: grpc.jumpstarter.example.com:8082 +token: "" +export: + dut-network: + type: jumpstarter_driver_dut_network.driver.DutNetwork + config: + interface: "enp1s0u1" + subnet: "192.168.100.0/24" + gateway_ip: "192.168.100.1" + upstream_interface: "end0" + nat_mode: "1to1" + dhcp_enabled: true + addresses: + # Tagged 1:1 NAT: public IP lives on end0.905, PBR sends DUT + # traffic out via 203.0.113.254. + - mac: "8a:12:4e:25:f4:8e" + ip: "192.168.100.125" + hostname: "sa8775p" + public_ip: "203.0.113.1" + vlan_id: 905 + public_gateway: "203.0.113.254" + # Untagged entry on the same exporter: omit vlan_id / public_gateway + # and behaviour matches the pre-VLAN driver. + - mac: "8a:12:4e:25:f4:8f" + ip: "192.168.100.126" + hostname: "untagged-dut" + public_ip: "198.51.100.86" diff --git a/python/packages/jumpstarter-driver-dut-network/examples/exporter.yaml b/python/packages/jumpstarter-driver-dut-network/examples/exporter.yaml index 4b021faa1..35a8e0310 100644 --- a/python/packages/jumpstarter-driver-dut-network/examples/exporter.yaml +++ b/python/packages/jumpstarter-driver-dut-network/examples/exporter.yaml @@ -23,7 +23,7 @@ export: dns_servers: ["8.8.8.8", "8.8.4.4"] dns_entries: - hostname: "registry.lab.local" - ip: "10.26.28.2" + ip: "198.51.100.2" # Optional: egress/ingress traffic filtering (exporter-enforced, not # exposed to remote clients). When omitted, no filtering is applied. filter: @@ -40,6 +40,6 @@ export: policy: "drop" # default verdict for inbound traffic rules: - action: "accept" # allow SSH from the lab network - source: "10.26.28.0/24" + source: "198.51.100.0/24" port: 22 protocol: "tcp" diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/client.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/client.py index ceb5f0ba3..1ca6afd0c 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/client.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/client.py @@ -28,9 +28,17 @@ def get_leases(self) -> list[dict]: """List all current DHCP leases (dynamic + static).""" return self.call("get_leases") - def add_address(self, ip: str, mac: str | None = None, hostname: str = "", public_ip: str | None = None) -> None: + def add_address( + self, + ip: str, + mac: str | None = None, + hostname: str = "", + public_ip: str | None = None, + vlan_id: int | None = None, + public_gateway: str | None = None, + ) -> None: """Add an address entry (with optional MAC for DHCP static lease).""" - self.call("add_address", ip, mac, hostname, public_ip) + self.call("add_address", ip, mac, hostname, public_ip, vlan_id, public_gateway) def remove_address(self, ip: str) -> None: """Remove an address entry by IP.""" @@ -71,6 +79,7 @@ def tcpdump(self, args: list[str] | None = None) -> Generator[str, None, None]: yield line def cli(self): # noqa: C901 + """Build the Click CLI command group for this driver.""" @driver_click_group(self) def base(): """DUT Network Isolation""" @@ -109,9 +118,20 @@ def get_ip(mac: str): @click.option("--mac", "-m", default=None, help="MAC address for DHCP static lease") @click.option("--hostname", "-n", default="", help="Hostname for the entry") @click.option("--public-ip", default=None, help="Public IP for 1:1 NAT mapping") - def add_address(ip: str, mac: str | None, hostname: str, public_ip: str | None): + @click.option("--vlan-id", type=int, default=None, help="VLAN ID for a tagged sub-interface") + @click.option("--public-gateway", default=None, help="Gateway for policy-based routing") + def add_address( + ip: str, + mac: str | None, + hostname: str, + public_ip: str | None, + vlan_id: int | None, + public_gateway: str | None, + ): """Add an address entry (with optional MAC for DHCP static lease).""" - self.add_address(ip, mac, hostname, public_ip) + self.add_address( + ip, mac, hostname, public_ip, vlan_id, public_gateway, + ) msg = f"Added address: {ip}" if mac: msg += f" (mac={mac})" diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver.py index cf876ac37..249002187 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver.py @@ -8,7 +8,7 @@ from collections.abc import AsyncGenerator from dataclasses import dataclass, field from pathlib import Path -from typing import Literal, TypedDict +from typing import Any, Literal, TypedDict from . import dnsmasq, iproute, nftables from .ntp_server import NtpServer @@ -113,6 +113,83 @@ def from_dict(cls, data: dict) -> "FilterConfig": return cls(**{k: v for k, v in data.items() if k in ("egress", "ingress")}) +_VLAN_ID_MIN = 1 +_VLAN_ID_MAX = 4094 +_RESERVED_ROUTE_TABLES = frozenset({0, 253, 254, 255}) +_PBR_PRIORITY = 100 +_ADDRESS_FIELDS = frozenset({"ip", "mac", "hostname", "public_ip", "vlan_id", "public_gateway"}) + + +@dataclass +class AddressEntry: + """A single DUT address mapping (DHCP lease and/or NAT/VLAN config).""" + + ip: str + mac: str | None = None + hostname: str = "" + public_ip: str | None = None + vlan_id: int | None = None + public_gateway: str | None = None + + def __post_init__(self) -> None: + if self.vlan_id is not None: + try: + self.vlan_id = int(self.vlan_id) + except (TypeError, ValueError) as exc: + raise ValueError(f"vlan_id must be an integer, got {self.vlan_id!r}") from exc + if not _VLAN_ID_MIN <= self.vlan_id <= _VLAN_ID_MAX: + raise ValueError( + f"vlan_id must be between {_VLAN_ID_MIN} and {_VLAN_ID_MAX}, got {self.vlan_id}" + ) + if self.public_gateway is not None: + try: + ipaddress.ip_address(self.public_gateway) + except ValueError as exc: + raise ValueError( + f"public_gateway is not a valid IP address: {self.public_gateway!r}" + ) from exc + if self.vlan_id is None: + try: + table = int(ipaddress.IPv4Address(self.ip)) + except ValueError as exc: + raise ValueError( + f"public_gateway without vlan_id requires a valid IPv4 " + f"address in ip, got {self.ip!r}" + ) from exc + if table in _RESERVED_ROUTE_TABLES: + raise ValueError( + f"DUT IP {self.ip!r} maps to routing table {table} which " + "is reserved by the kernel (tables 0, 253, 254, 255)" + ) + elif self.vlan_id in _RESERVED_ROUTE_TABLES: + raise ValueError( + f"vlan_id {self.vlan_id} cannot be used as a routing table ID " + "(tables 0, 253, 254, 255 are reserved by the kernel)" + ) + + @classmethod + def from_dict(cls, data: "AddressEntry | dict[str, Any]") -> "AddressEntry": + """Create an AddressEntry from a YAML/config dictionary.""" + if isinstance(data, AddressEntry): + return data + return cls(**{k: v for k, v in data.items() if k in _ADDRESS_FIELDS}) + + def to_dict(self) -> dict[str, Any]: + """Return a dict suitable for dnsmasq config and serialization.""" + result: dict[str, Any] = {"ip": self.ip} + if self.mac: + result["mac"] = self.mac + if self.hostname: + result["hostname"] = self.hostname + if self.public_ip: + result["public_ip"] = self.public_ip + if self.vlan_id is not None: + result["vlan_id"] = self.vlan_id + if self.public_gateway: + result["public_gateway"] = self.public_gateway + return result + + @dataclass(kw_only=True) class DutNetwork(Driver): """DUT network isolation with bridge, DHCP, DNS, and NAT.""" @@ -127,7 +204,7 @@ class DutNetwork(Driver): dhcp_enabled: bool = True dhcp_range_start: str = "192.168.100.100" dhcp_range_end: str = "192.168.100.200" - addresses: list[dict[str, str]] = field(default_factory=list) + addresses: list[AddressEntry] = field(default_factory=list) dns_servers: list[str] = field(default_factory=lambda: ["8.8.8.8", "8.8.4.4"]) dns_entries: list[dict[str, str]] = field(default_factory=list) @@ -151,6 +228,10 @@ class DutNetwork(Driver): _prev_fwd_upstream: str = field(init=False, default="0") _upstream_prefix_len: int = field(init=False, default=24) _added_aliases: set[str] = field(init=False, default_factory=set) + _alias_ifaces: dict[str, str] = field(init=False, default_factory=dict) + _created_vlans: set[str] = field(init=False, default_factory=set) + _pbr_rules: list[tuple[str, int]] = field(init=False, default_factory=list) + _pbr_tables: set[int] = field(init=False, default_factory=set) _fwd_rule_handles: list[int] = field(init=False, default_factory=list) _ntp_server: NtpServer | None = field(init=False, default=None) _tcpdump_process: asyncio.subprocess.Process | None = field(init=False, default=None) @@ -160,6 +241,7 @@ def client(cls) -> str: return "jumpstarter_driver_dut_network.client.DutNetworkClient" def __post_init__(self): + """Initialise the driver: validate config, set up the network stack.""" if hasattr(super(), "__post_init__"): super().__post_init__() self._table_name = nftables._table_name_for(self.interface) @@ -172,6 +254,7 @@ def __post_init__(self): raise def _check_system_requirements(self) -> None: + """Verify that required Linux tools (ip, nft, dnsmasq, …) are installed.""" if sys.platform != "linux": raise RuntimeError("DutNetwork driver requires Linux (network namespaces, nftables)") @@ -194,6 +277,7 @@ def _check_system_requirements(self) -> None: ) def _nat_disabled(self) -> bool: + """Return True when NAT is explicitly disabled.""" return self.nat_mode in ("disabled", "none") @staticmethod @@ -228,6 +312,7 @@ def _resolve_ip(value: str) -> str: return address def _validate_config(self) -> None: + """Parse and validate the driver configuration fields.""" network = ipaddress.ip_network(self.subnet, strict=False) self._prefix_len = network.prefixlen @@ -235,8 +320,11 @@ def _validate_config(self) -> None: if gateway not in network: raise ValueError(f"Gateway {self.gateway_ip} is not within subnet {self.subnet}") + # Convert raw dicts to AddressEntry instances (YAML deserialization produces dicts). + self.addresses = [AddressEntry.from_dict(e) for e in self.addresses] + if self.nat_mode == "1to1": - has_public = any(entry.get("public_ip") for entry in self.addresses) + has_public = any(entry.public_ip for entry in self.addresses) if not has_public: raise ValueError("At least one address entry must have public_ip for 1:1 NAT mode") @@ -248,6 +336,7 @@ def _validate_config(self) -> None: self.filter = None def _setup_network(self) -> None: + """Configure interface, forwarding, VLAN/PBR, DHCP, NAT, and NTP.""" if not self._nat_disabled(): self._upstream = self.upstream_interface or iproute.detect_upstream_interface() if not self._upstream: @@ -270,7 +359,14 @@ def _setup_network(self) -> None: self._prev_fwd_upstream = iproute.get_interface_forwarding(self._upstream) iproute.set_interface_forwarding(self.interface, True) iproute.set_interface_forwarding(self._upstream, True) - self._fwd_rule_handles = nftables.ensure_filter_forward(self.interface, self._upstream) + self._setup_vlans_and_pbr() + extra_ifaces = list(self._created_vlans) or None + if extra_ifaces: + self._fwd_rule_handles = nftables.ensure_filter_forward( + self.interface, self._upstream, extra_interfaces=extra_ifaces, + ) + else: + self._fwd_rule_handles = nftables.ensure_filter_forward(self.interface, self._upstream) if self.dhcp_enabled: dnsmasq.write_config( @@ -278,32 +374,17 @@ def _setup_network(self) -> None: interface=self.interface, range_start=self.dhcp_range_start, range_end=self.dhcp_range_end, - static_leases=[e for e in self.addresses if e.get("mac")], + static_leases=[e.to_dict() for e in self.addresses if e.mac], dns_servers=self.dns_servers, gateway_ip=self.gateway_ip, dns_entries=self.dns_entries, ) self._dnsmasq_process = dnsmasq.start(self._state_path) - upstream_for_nat = self._upstream if self.nat_mode == "masquerade": - nftables.apply_masquerade_rules( - self.interface, upstream_for_nat, self.subnet, - table_name=self._table_name, - filter_config=self.filter, - ) + self._apply_masquerade_rules() elif self.nat_mode == "1to1": - mappings = self._get_1to1_mappings() - upstream_for_alias = self.public_interface or self._upstream - for m in mappings: - ip = m["public_ip"] - iproute.add_ip_alias(upstream_for_alias, ip, self._upstream_prefix_len) - self._added_aliases.add(ip) - nftables.apply_1to1_rules( - self.interface, upstream_for_alias, mappings, self.subnet, - table_name=self._table_name, - filter_config=self.filter, - ) + self._apply_1to1_aliases_and_rules() if self.local_ntp: self._ntp_server = NtpServer(self.gateway_ip) @@ -318,14 +399,195 @@ def _setup_network(self) -> None: self.local_ntp, ) + def _vlan_parent(self) -> str: + """Parent interface on which VLAN sub-interfaces are created.""" + return self._upstream or "" + + def _alias_parent(self) -> str: + """Interface that receives 1:1 IP aliases when no VLAN is set.""" + return self.public_interface or self._upstream or "" + + def _vlan_name(self, vlan_id: int) -> str: + """Return the ``.`` sub-interface name.""" + return f"{self._vlan_parent()}.{vlan_id}" + + def _nat_iface_for(self, entry: AddressEntry) -> str: + """Return the NAT/upstream interface for a given address entry.""" + if entry.vlan_id is not None: + return self._vlan_name(entry.vlan_id) + return self._alias_parent() + + @staticmethod + def _pbr_table_id(entry: AddressEntry) -> int: + """Routing-table ID for policy-based routing. + + VLAN mappings use the VLAN ID so tagged traffic shares one table. + Untagged mappings use the DUT private IPv4 address as a unique + 32-bit table ID (``from lookup ``). + """ + if entry.vlan_id is not None: + return entry.vlan_id + return int(ipaddress.IPv4Address(entry.ip)) + + def _outbound_interfaces(self) -> list[str]: + """NAT interfaces used for masquerade/forward rules. + + The upstream (untagged) interface is **always** included so that + unexpected or unregistered DUT hosts whose traffic arrives on the + bridge are still masqueraded via the default upstream — matching + the behaviour of ``apply_1to1_rules`` which keeps the upstream + for unmapped-DUT fallback. VLAN sub-interfaces are appended when + at least one address entry carries a ``vlan_id``. + + Runtime ``add_address`` / ``remove_address`` trigger a full rebuild + via ``_sync_nat``, so the list stays consistent. + """ + parent = self._upstream or "" + result: list[str] = [parent] if parent else [] + for entry in self.addresses: + if entry.vlan_id is not None: + name = self._vlan_name(entry.vlan_id) + if name not in result: + result.append(name) + return result + + def _setup_vlan_interface(self, parent: str, entry: "AddressEntry", name: str) -> None: + """Create a single VLAN sub-interface with sysctls, alias, and warnings.""" + assert entry.vlan_id is not None + iproute.create_vlan_interface(parent, entry.vlan_id) + self._created_vlans.add(name) + try: + iproute.set_interface_forwarding(name, True) + iproute.set_interface_rp_filter(name, 2) + if entry.public_ip: + resolved = self._resolve_ip(entry.public_ip) + iproute.add_ip_alias(name, resolved, self._upstream_prefix_len) + self._added_aliases.add(resolved) + self._alias_ifaces[resolved] = name + if entry.public_gateway is None: + self.logger.warning( + "Address %s has vlan_id=%s but no public_gateway; " + "VLAN interface %s is configured without policy-based " + "routing, so DUT traffic may not egress via the VLAN", + entry.ip, + entry.vlan_id, + name, + ) + except Exception: + # Roll back: undo alias bookkeeping and delete the VLAN + # interface (deleting it also removes any addresses on it). + for alias_ip in list(self._added_aliases): + if self._alias_ifaces.get(alias_ip) == name: + self._added_aliases.discard(alias_ip) + self._alias_ifaces.pop(alias_ip, None) + self._created_vlans.discard(name) + iproute.delete_vlan_interface(name) + raise + + def _setup_vlans_and_pbr(self) -> None: + """Create VLAN sub-interfaces, sysctls, IP aliases, and PBR rules.""" + parent = self._vlan_parent() + if not parent: + return + for entry in self.addresses: + nat_if = self._nat_iface_for(entry) + if entry.vlan_id is not None: + nat_if = nat_if + self._setup_vlan_interface(parent, entry, nat_if) + if entry.public_gateway: + gateway = self._resolve_ip(entry.public_gateway) + table = self._pbr_table_id(entry) + iproute.add_policy_route(gateway, nat_if, table) + try: + iproute.add_ip_rule(entry.ip, table, priority=_PBR_PRIORITY) + except RuntimeError: + iproute.flush_routing_table(table) + raise + self._pbr_rules.append((entry.ip, table)) + self._pbr_tables.add(table) + + def _refresh_fwd_rule_handles(self) -> None: + """Re-insert nft FORWARD ACCEPT rules for any new VLAN sub-interfaces. + + Docker sets the FORWARD chain policy to drop; we need an explicit + accept for each interface we create. At init time this is done + once, but runtime ``add_address`` / ``remove_address`` may add or + remove VLAN sub-interfaces, so we tear down the old handles and + re-create them. + """ + if self._fwd_rule_handles: + nftables.remove_filter_forward(self._fwd_rule_handles) + self._fwd_rule_handles = [] + extra_ifaces = list(self._created_vlans) or None + if extra_ifaces: + self._fwd_rule_handles = nftables.ensure_filter_forward( + self.interface, self._upstream, extra_interfaces=extra_ifaces, + ) + else: + self._fwd_rule_handles = nftables.ensure_filter_forward( + self.interface, self._upstream, + ) + + def _teardown_vlans_and_pbr(self) -> None: + """Reverse VLAN sub-interfaces, PBR rules, and custom routing tables.""" + for from_ip, table in self._pbr_rules: + iproute.delete_ip_rule(from_ip, table) + self._pbr_rules.clear() + for table in list(self._pbr_tables): + iproute.flush_routing_table(table) + self._pbr_tables.clear() + for name in list(self._created_vlans): + iproute.delete_vlan_interface(name) + self._created_vlans.clear() + + def _apply_masquerade_rules(self) -> None: + """Apply nftables masquerade NAT rules for outbound DUT traffic.""" + kwargs: dict[str, Any] = { + "table_name": self._table_name, + "filter_config": self.filter, + } + outbound = self._outbound_interfaces() + if outbound != [self._upstream]: + kwargs["nat_interfaces"] = outbound + nftables.apply_masquerade_rules(self.interface, self._upstream, self.subnet, **kwargs) + + def _apply_1to1_aliases_and_rules(self) -> None: + """Add IP aliases and apply nftables 1:1 NAT rules per mapping.""" + mappings = self._get_1to1_mappings() + parent = self._alias_parent() + for m in mappings: + ip = m["public_ip"] + if ip in self._added_aliases: + continue + nat_if = m.get("nat_interface") or parent + iproute.add_ip_alias(nat_if, ip, self._upstream_prefix_len) + self._added_aliases.add(ip) + self._alias_ifaces[ip] = nat_if + nftables.apply_1to1_rules( + self.interface, parent, mappings, self.subnet, + table_name=self._table_name, + filter_config=self.filter, + ) + def _get_1to1_mappings(self) -> list[dict[str, str]]: - return [ - {"private_ip": entry["ip"], "public_ip": self._resolve_ip(entry["public_ip"])} - for entry in self.addresses - if entry.get("public_ip") - ] + """Build the list of private→public IP mappings for 1:1 NAT.""" + mappings: list[dict[str, str]] = [] + parent = self._alias_parent() + for entry in self.addresses: + if not entry.public_ip: + continue + mapping: dict[str, str] = { + "private_ip": entry.ip, + "public_ip": self._resolve_ip(entry.public_ip), + } + nat_if = self._nat_iface_for(entry) + if nat_if != parent: + mapping["nat_interface"] = nat_if + mappings.append(mapping) + return mappings def _stop_tcpdump(self) -> None: + """Terminate any running tcpdump process.""" if self._tcpdump_process is not None: try: self._tcpdump_process.terminate() @@ -334,12 +596,14 @@ def _stop_tcpdump(self) -> None: self._tcpdump_process = None def _stop_ntp(self) -> None: + """Stop the local NTP server and remove its redirect rules.""" if self._ntp_server is not None: self._ntp_server.stop() self._ntp_server = None nftables.remove_ntp_redirect(self._table_name) def cleanup(self) -> None: + """Tear down all network state created by this driver instance.""" self.logger.info("Cleaning up DUT network configuration") self._stop_ntp() @@ -354,12 +618,15 @@ def cleanup(self) -> None: nftables.flush_rules(self._table_name) - if self.nat_mode == "1to1": - upstream_for_alias = self.public_interface or self._upstream - if upstream_for_alias: - for ip in list(self._added_aliases): - iproute.remove_ip_alias(upstream_for_alias, ip, self._upstream_prefix_len) - self._added_aliases.clear() + if self._added_aliases: + for ip in list(self._added_aliases): + iface = self._alias_ifaces.get(ip) or self._alias_parent() + if iface: + iproute.remove_ip_alias(iface, ip, self._upstream_prefix_len) + self._added_aliases.clear() + self._alias_ifaces.clear() + + self._teardown_vlans_and_pbr() if self._fwd_rule_handles: nftables.remove_filter_forward(self._fwd_rule_handles) @@ -375,11 +642,13 @@ def cleanup(self) -> None: iproute.nm_set_managed(self.interface) def close(self): + """Clean up and close the driver.""" self.cleanup() super().close() @export def status(self) -> NetworkStatus: + """Return the current network status (interface, leases, NAT rules).""" iface_exists = iproute.interface_exists(self.interface) addresses = iproute.get_interface_addresses(self.interface) if iface_exists else [] leases = self._get_leases_list() @@ -411,6 +680,7 @@ def ntp_status(self) -> dict: @export def get_dut_ip(self, mac: str) -> str | None: + """Look up the assigned IP for a DUT by its MAC address.""" if not self._state_path: return None lease = dnsmasq.get_lease_by_mac(self._state_path, mac) @@ -418,9 +688,11 @@ def get_dut_ip(self, mac: str) -> str | None: @export def get_leases(self) -> list[dict]: + """Return all current DHCP leases (dynamic + static).""" return self._get_leases_list() def _get_leases_list(self) -> list[dict]: + """Parse lease file and return a list of lease dictionaries.""" if not self._state_path: return [] leases = dnsmasq.parse_leases(self._state_path) @@ -429,62 +701,102 @@ def _get_leases_list(self) -> list[dict]: ] @export - def add_address(self, ip: str, mac: str | None = None, hostname: str = "", public_ip: str | None = None) -> None: - new_entry: dict[str, str] = {"ip": ip} - if mac: - new_entry["mac"] = mac - if hostname: - new_entry["hostname"] = hostname - if public_ip: - new_entry["public_ip"] = self._resolve_ip(public_ip) - - self.addresses = [entry for entry in self.addresses if entry["ip"] != ip] + def add_address( + self, + ip: str, + mac: str | None = None, + hostname: str = "", + public_ip: str | None = None, + vlan_id: int | None = None, + public_gateway: str | None = None, + ) -> None: + """Add or replace an address entry, then resync DHCP and NAT.""" + new_entry = AddressEntry( + ip=ip, + mac=mac, + hostname=hostname or "", + public_ip=self._resolve_ip(public_ip) if public_ip else None, + vlan_id=vlan_id, + public_gateway=public_gateway, + ) + + self.addresses = [entry for entry in self.addresses if entry.ip != ip] self.addresses.append(new_entry) self._reload_dnsmasq_config() - if self.nat_mode == "1to1": - self._sync_1to1_nat() + self._sync_nat() self.logger.info("Added address: ip=%s mac=%s hostname=%s", ip, mac, hostname) @export def remove_address(self, ip: str) -> None: - self.addresses = [entry for entry in self.addresses if entry["ip"] != ip] + """Remove an address entry by IP and resync DHCP and NAT.""" + self.addresses = [entry for entry in self.addresses if entry.ip != ip] self._reload_dnsmasq_config() + self._sync_nat() + self.logger.info("Removed address for ip=%s", ip) + + def _sync_nat(self) -> None: + """Re-apply VLAN/PBR/NAT state after a runtime address change.""" + if self._nat_disabled(): + return + needs_vlan = bool(self._created_vlans) or any(e.vlan_id is not None for e in self.addresses) + needs_pbr = bool(self._pbr_rules) or any(e.public_gateway is not None for e in self.addresses) + if needs_vlan or needs_pbr: + self._teardown_vlans_and_pbr() + self._setup_vlans_and_pbr() + self._refresh_fwd_rule_handles() if self.nat_mode == "1to1": self._sync_1to1_nat() - self.logger.info("Removed address for ip=%s", ip) + elif self.nat_mode == "masquerade" and (needs_vlan or needs_pbr): + self._apply_masquerade_rules() def _sync_1to1_nat(self) -> None: - upstream_for_alias = self.public_interface or self._upstream - if not upstream_for_alias: + """Rebuild 1:1 NAT aliases and nftables rules after address changes.""" + parent = self._alias_parent() + if not parent: return mappings = self._get_1to1_mappings() - wanted = {m["public_ip"] for m in mappings} - stale = self._added_aliases - wanted - new = wanted - self._added_aliases + wanted: dict[str, str] = { + m["public_ip"]: m.get("nat_interface") or parent for m in mappings + } - for ip in stale: - iproute.remove_ip_alias(upstream_for_alias, ip, self._upstream_prefix_len) - for ip in new: - iproute.add_ip_alias(upstream_for_alias, ip, self._upstream_prefix_len) - self._added_aliases = wanted + for ip in list(self._added_aliases): + if ip not in wanted: + iface = self._alias_ifaces.get(ip, parent) + iproute.remove_ip_alias(iface, ip, self._upstream_prefix_len) + self._added_aliases.discard(ip) + self._alias_ifaces.pop(ip, None) + elif self._alias_ifaces.get(ip) != wanted[ip]: + old_iface = self._alias_ifaces.get(ip, parent) + iproute.remove_ip_alias(old_iface, ip, self._upstream_prefix_len) + iproute.add_ip_alias(wanted[ip], ip, self._upstream_prefix_len) + self._alias_ifaces[ip] = wanted[ip] + + for ip, iface in wanted.items(): + if ip not in self._added_aliases: + iproute.add_ip_alias(iface, ip, self._upstream_prefix_len) + self._added_aliases.add(ip) + self._alias_ifaces[ip] = iface nftables.flush_rules(self._table_name) nftables.apply_1to1_rules( - self.interface, upstream_for_alias, mappings, self.subnet, + self.interface, parent, mappings, self.subnet, table_name=self._table_name, filter_config=self.filter, ) @export def get_nat_rules(self) -> str: + """Return the active nftables rules for this driver's table.""" return nftables.list_rules(self._table_name) @export def get_dns_entries(self) -> list[dict[str, str]]: + """Return the list of configured DNS entries.""" return list(self.dns_entries) @export def add_dns_entry(self, hostname: str, ip: str) -> None: + """Add or replace a custom DNS entry, then reload dnsmasq.""" self.dns_entries = [e for e in self.dns_entries if e["hostname"] != hostname] self.dns_entries.append({"hostname": hostname, "ip": ip}) self._reload_dnsmasq_config() @@ -492,18 +804,20 @@ def add_dns_entry(self, hostname: str, ip: str) -> None: @export def remove_dns_entry(self, hostname: str) -> None: + """Remove a DNS entry by hostname, then reload dnsmasq.""" self.dns_entries = [e for e in self.dns_entries if e["hostname"] != hostname] self._reload_dnsmasq_config() self.logger.info("Removed DNS entry: %s", hostname) def _reload_dnsmasq_config(self) -> None: + """Rewrite the dnsmasq config and signal the daemon to reload.""" if self._state_path and self.dhcp_enabled: dnsmasq.write_config( state_dir=self._state_path, interface=self.interface, range_start=self.dhcp_range_start, range_end=self.dhcp_range_end, - static_leases=[e for e in self.addresses if e.get("mac")], + static_leases=[e.to_dict() for e in self.addresses if e.mac], dns_servers=self.dns_servers, gateway_ip=self.gateway_ip, dns_entries=self.dns_entries, diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/iproute.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/iproute.py index 526034faf..e3af40a44 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/iproute.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/iproute.py @@ -8,6 +8,27 @@ logger = logging.getLogger(__name__) +# Linux IFNAMSIZ is 16 including the terminating NUL. +_IFNAMSIZ = 15 + +# ip rule / ip route table IDs reserved by the kernel. +_RESERVED_ROUTE_TABLES = frozenset({0, 253, 254, 255}) + + +def vlan_subinterface_name(parent: str, vlan_id: int) -> str: + """Return the conventional ``.`` sub-interface name.""" + return f"{parent}.{vlan_id}" + + +def _sysctl_conf_key(iface: str, setting: str) -> str: + """Build a ``net.ipv4.conf..`` sysctl key. + + Sysctl uses ``.`` as a path separator, so dots in the interface name + (e.g. VLAN sub-interfaces like ``end0.905``) must be written as + slashes: ``net.ipv4.conf.end0/905.forwarding``. + """ + return f"net.ipv4.conf.{iface.replace('.', '/')}.{setting}" + def _run(cmd: list[str], check: bool = True) -> subprocess.CompletedProcess: """Run a read-only command (no sudo).""" @@ -81,20 +102,134 @@ def remove_ip_alias(interface: str, ip: str, prefix_len: int) -> None: def get_interface_forwarding(iface: str) -> str: """Return the current per-interface forwarding value ("0" or "1").""" - result = _run(["sysctl", "-n", f"net.ipv4.conf.{iface}.forwarding"], check=False) + result = _run(["sysctl", "-n", _sysctl_conf_key(iface, "forwarding")], check=False) return result.stdout.strip() or "0" def set_interface_forwarding(iface: str, enabled: bool) -> None: """Enable or disable IPv4 forwarding on a specific interface. - Uses the per-interface sysctl (net.ipv4.conf./forwarding) rather + Uses the per-interface sysctl (net.ipv4.conf..forwarding) rather than the global net.ipv4.ip_forward to avoid turning the host into a - full router on all interfaces. + full router on all interfaces. Dots in *iface* are translated to + slashes so VLAN names such as ``end0.905`` map to + ``net.ipv4.conf.end0/905.forwarding``. """ value = "1" if enabled else "0" - logger.info("Setting net.ipv4.conf.%s.forwarding=%s", iface, value) - _run_priv(["sysctl", "-w", f"net.ipv4.conf.{iface}.forwarding={value}"]) + key = _sysctl_conf_key(iface, "forwarding") + logger.info("Setting %s=%s", key, value) + _run_priv(["sysctl", "-w", f"{key}={value}"]) + + +def get_interface_rp_filter(iface: str) -> str: + """Return the current per-interface rp_filter value.""" + result = _run(["sysctl", "-n", _sysctl_conf_key(iface, "rp_filter")], check=False) + return result.stdout.strip() or "0" + + +def set_interface_rp_filter(iface: str, value: int) -> None: + """Set reverse-path filtering mode on a specific interface. + + Common values: ``0`` (disabled), ``1`` (strict), ``2`` (loose). + Dots in *iface* are translated to slashes for the sysctl key. + """ + key = _sysctl_conf_key(iface, "rp_filter") + logger.info("Setting %s=%s", key, value) + _run_priv(["sysctl", "-w", f"{key}={value}"]) + + +def create_vlan_interface(parent: str, vlan_id: int) -> str: + """Create (if needed) and bring up a VLAN sub-interface on *parent*. + + Returns the sub-interface name (``.``). The + operation is idempotent: an existing sub-interface is left in place + and brought up. + """ + name = vlan_subinterface_name(parent, vlan_id) + if len(name) > _IFNAMSIZ: + raise ValueError( + f"VLAN interface name {name!r} exceeds the Linux {_IFNAMSIZ}-character limit" + ) + if not interface_exists(name): + logger.info("Creating VLAN interface %s on %s id %d", name, parent, vlan_id) + _run_priv( + ["ip", "link", "add", "link", parent, "name", name, "type", "vlan", "id", str(vlan_id)] + ) + else: + logger.info("VLAN interface %s already exists", name) + nm_set_unmanaged(name) + _run_priv(["ip", "link", "set", name, "up"]) + return name + + +def delete_vlan_interface(name: str) -> None: + """Delete a VLAN sub-interface. Missing interfaces are ignored.""" + logger.info("Deleting VLAN interface %s", name) + _run_priv(["ip", "link", "del", name], check=False) + + +def add_policy_route(gateway: str, device: str, table: int) -> None: + """Install a default route in a custom routing table for PBR. + + *table* is the numeric routing-table ID (VLAN ID, or the DUT + private IPv4 address as an integer for untagged PBR). + + Raises :class:`RuntimeError` if the ``ip route add`` command fails + (unless the route already exists, which is treated as idempotent). + """ + if table in _RESERVED_ROUTE_TABLES: + raise ValueError( + f"Routing table {table} is reserved by the kernel (local/main/default)" + ) + logger.info("Adding policy route default via %s dev %s table %d", gateway, device, table) + # ``replace`` is idempotent: if an identical route exists it is a no-op, + # and if a *different* default exists it is updated to the requested + # gateway/device. ``add`` would return "File exists" in both cases, + # making it impossible to distinguish a harmless duplicate from a + # conflicting stale route. + result = _run_priv( + ["ip", "route", "replace", "default", "via", gateway, "dev", device, + "table", str(table), "onlink"], + check=False, + ) + if result.returncode != 0: + stderr = result.stderr.strip() + raise RuntimeError( + f"Failed to add policy route (table {table}): {stderr}" + ) + + +def add_ip_rule(from_ip: str, table: int, priority: int = 100) -> None: + """Add a policy routing rule sending traffic from *from_ip* to *table*. + + Raises :class:`RuntimeError` if the ``ip rule add`` command fails + (unless an identical rule already exists, which is treated as idempotent). + """ + logger.info("Adding ip rule from %s table %d priority %d", from_ip, table, priority) + result = _run_priv( + ["ip", "rule", "add", "from", from_ip, "table", str(table), "priority", str(priority)], + check=False, + ) + if result.returncode != 0: + stderr = result.stderr.strip() + if "File exists" in stderr: + logger.info("IP rule from %s table %d already exists, skipping", from_ip, table) + else: + raise RuntimeError( + f"Failed to add ip rule (from {from_ip} table {table}): {stderr}" + ) + + +def delete_ip_rule(from_ip: str, table: int) -> None: + """Delete a policy routing rule previously added by :func:`add_ip_rule`.""" + logger.info("Deleting ip rule from %s table %d", from_ip, table) + _run_priv(["ip", "rule", "del", "from", from_ip, "table", str(table)], check=False) + + +def flush_routing_table(table: int) -> None: + """Flush all routes from a custom routing table.""" + logger.info("Flushing routing table %d", table) + _run_priv(["ip", "route", "flush", "table", str(table)], check=False) def detect_upstream_interface() -> str | None: diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/nftables.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/nftables.py index a1225ff7b..0f2d65754 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/nftables.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/nftables.py @@ -18,25 +18,30 @@ def _validate_iface(name: str) -> None: + """Raise if *name* is not a valid Linux interface name.""" if not _IFACE_RE.match(name): raise ValueError(f"Invalid interface name: {name!r}") def _validate_subnet(subnet: str) -> None: + """Raise if *subnet* is not a valid CIDR network.""" ipaddress.ip_network(subnet, strict=False) def _validate_ip(ip: str) -> None: + """Raise if *ip* is not a valid IP address.""" ipaddress.ip_address(ip) def _run_nft(args: list[str], check: bool = True) -> subprocess.CompletedProcess: + """Execute an ``nft`` command, using sudo when not root.""" cmd = sudo_cmd(["nft"] + args) logger.debug("Running: %s", " ".join(cmd)) return subprocess.run(cmd, capture_output=True, text=True, check=check) def _load_ruleset(ruleset: str) -> None: + """Feed an nftables ruleset via ``nft -f -`` and raise on failure.""" logger.debug("Loading nftables ruleset:\n%s", ruleset) result = subprocess.run( sudo_cmd(["nft", "-f", "-"]), @@ -50,6 +55,7 @@ def _load_ruleset(ruleset: str) -> None: def _table_name_for(interface: str) -> str: + """Derive the nftables table name from the interface name.""" return f"jumpstarter_{interface}".replace("-", "_") @@ -73,9 +79,20 @@ def _render_filter_rule( return " ".join(parts) +def _normalize_upstreams(upstream: str | list[str]) -> list[str]: + """Return a de-duplicated list of upstream/NAT interface names.""" + if isinstance(upstream, str): + return [upstream] + seen: list[str] = [] + for name in upstream: + if name not in seen: + seen.append(name) + return seen + + def _build_forward_chain( interface: str, - upstream: str, + upstream: str | list[str], filter_config: FilterConfig | None = None, extra_forward_rules: list[str] | None = None, ) -> str: @@ -85,6 +102,10 @@ def _build_forward_chain( to the legacy (pre-filter) behaviour so that existing setups are not affected. + *upstream* may be a single interface name or a list of NAT interfaces + (untagged upstream plus any VLAN sub-interfaces). A single string + produces the same rules as before. + *extra_forward_rules* are additional pre-formatted nftables rule lines (e.g. per-mapping DNAT accept rules for 1:1 NAT). In legacy mode they are appended after the conntrack line; in filtered mode they are inserted @@ -102,50 +123,69 @@ def _build_forward_chain( The chain *policy* is always ``accept`` so that forwarded traffic unrelated to this interface pair is not disturbed. """ - lines: list[str] = [ + extras = extra_forward_rules or [] + upstreams = _normalize_upstreams(upstream) + body = ( + _legacy_forward_body(interface, upstreams, extras) + if filter_config is None + else _filtered_forward_body(interface, upstreams, filter_config, extras) + ) + lines = [ " chain forward {", " type filter hook forward priority filter; policy accept;", + *body, + " }", ] + return "\n".join(lines) - extras = extra_forward_rules or [] - if filter_config is None: - # Legacy behaviour - no filtering. - lines.append(f' iifname "{interface}" oifname "{upstream}" accept') +def _legacy_forward_body( + interface: str, + upstreams: list[str], + extras: list[str], +) -> list[str]: + """Build forward-chain rules in legacy (no filter) mode.""" + lines: list[str] = [] + for up in upstreams: + lines.append(f' iifname "{interface}" oifname "{up}" accept') lines.append( - f' iifname "{upstream}" oifname "{interface}" ct state related,established accept' + f' iifname "{up}" oifname "{interface}" ct state related,established accept' ) - lines.extend(extras) - lines.append(" }") - return "\n".join(lines) + lines.extend(extras) + return lines - # --- Filtered mode ------------------------------------------------- - lines.append(" ct state related,established accept") +def _filtered_forward_body( + interface: str, + upstreams: list[str], + filter_config: FilterConfig, + extras: list[str], +) -> list[str]: + """Build forward-chain rules when a traffic filter is configured.""" + lines: list[str] = [" ct state related,established accept"] egress = filter_config.egress ingress = filter_config.ingress - # - Egress (DUT -> upstream) ---------------------------------------- if egress: - for rule in egress.rules: - lines.append(_render_filter_rule(rule, interface, upstream, "destination")) + for up in upstreams: + for rule in egress.rules: + lines.append(_render_filter_rule(rule, interface, up, "destination")) egress_policy = egress.policy if egress else "accept" - lines.append(f' iifname "{interface}" oifname "{upstream}" {egress_policy}') + for up in upstreams: + lines.append(f' iifname "{interface}" oifname "{up}" {egress_policy}') - # - Extra forward rules (e.g. 1:1 NAT per-mapping accepts) --------- lines.extend(extras) - # - Ingress (upstream -> DUT) - new connections only ---------------- if ingress: - for rule in ingress.rules: - lines.append(_render_filter_rule(rule, upstream, interface, "source")) + for up in upstreams: + for rule in ingress.rules: + lines.append(_render_filter_rule(rule, up, interface, "source")) ingress_policy = ingress.policy if ingress else "accept" - lines.append(f' iifname "{upstream}" oifname "{interface}" {ingress_policy}') - - lines.append(" }") - return "\n".join(lines) + for up in upstreams: + lines.append(f' iifname "{up}" oifname "{interface}" {ingress_policy}') + return lines def apply_masquerade_rules( @@ -154,24 +194,37 @@ def apply_masquerade_rules( subnet: str, table_name: str | None = None, filter_config: FilterConfig | None = None, + nat_interfaces: list[str] | None = None, ) -> None: + """Apply nftables masquerade NAT and forwarding rules. + + *nat_interfaces* overrides *upstream* as the list of outbound + interfaces for masquerade (e.g. VLAN sub-interfaces). + """ _validate_iface(interface) _validate_iface(upstream) _validate_subnet(subnet) + outbound = _normalize_upstreams(nat_interfaces if nat_interfaces else upstream) + for oif in outbound: + _validate_iface(oif) table = table_name or _table_name_for(interface) logger.info( - "Applying masquerade rules: interface=%s upstream=%s subnet=%s table=%s", + "Applying masquerade rules: interface=%s upstream=%s outbound=%s subnet=%s table=%s", interface, upstream, + outbound, subnet, table, ) - forward_chain = _build_forward_chain(interface, upstream, filter_config) + forward_chain = _build_forward_chain(interface, outbound, filter_config) + masq_lines = "\n".join( + f' oifname "{oif}" ip saddr {subnet} masquerade' for oif in outbound + ) ruleset = ( f"table ip {table} {{\n" f" chain postrouting {{\n" f" type nat hook postrouting priority srcnat; policy accept;\n" - f' oifname "{upstream}" ip saddr {subnet} masquerade\n' + f"{masq_lines}\n" f" }}\n" f"{forward_chain}\n" f"}}\n" @@ -188,6 +241,7 @@ def apply_1to1_rules( table_name: str | None = None, filter_config: FilterConfig | None = None, ) -> None: + """Apply nftables 1:1 NAT (DNAT/SNAT) and forwarding rules.""" _validate_iface(interface) _validate_iface(upstream) _validate_subnet(subnet) @@ -208,20 +262,33 @@ def apply_1to1_rules( postrouting_rules = [] extra_forward_rules = [] output_rules = [] + outbound: list[str] = [] for m in mappings: private_ip = m["private_ip"] public_ip = m["public_ip"] - prerouting_rules.append(f' iifname "{upstream}" ip daddr {public_ip} dnat to {private_ip}') - postrouting_rules.append(f' ip saddr {private_ip} oifname "{upstream}" snat to {public_ip}') - extra_forward_rules.append(f' iifname "{upstream}" oifname "{interface}" ip daddr {private_ip} accept') + nat_if = m.get("nat_interface") or upstream + _validate_iface(nat_if) + if nat_if not in outbound: + outbound.append(nat_if) + prerouting_rules.append(f' iifname "{nat_if}" ip daddr {public_ip} dnat to {private_ip}') + postrouting_rules.append(f' ip saddr {private_ip} oifname "{nat_if}" snat to {public_ip}') + extra_forward_rules.append(f' iifname "{nat_if}" oifname "{interface}" ip daddr {private_ip} accept') output_rules.append(f" ip daddr {public_ip} dnat to {private_ip}") + # Keep the original upstream in the forward/masquerade set so unmapped + # DUTs (no public_ip) still egress via the untagged interface. + if upstream not in outbound: + outbound.append(upstream) + prerouting_block = "\n".join(prerouting_rules) postrouting_block = "\n".join(postrouting_rules) output_block = "\n".join(output_rules) + masq_lines = "\n".join( + f' oifname "{oif}" ip saddr {subnet} masquerade' for oif in outbound + ) - forward_chain = _build_forward_chain(interface, upstream, filter_config, extra_forward_rules) + forward_chain = _build_forward_chain(interface, outbound, filter_config, extra_forward_rules) ruleset = ( f"table ip {table} {{\n" @@ -236,7 +303,7 @@ def apply_1to1_rules( f" chain postrouting {{\n" f" type nat hook postrouting priority srcnat; policy accept;\n" f"{postrouting_block}\n" - f' oifname "{upstream}" ip saddr {subnet} masquerade\n' + f"{masq_lines}\n" f" }}\n" f"{forward_chain}\n" f"}}\n" @@ -258,16 +325,27 @@ def is_filter_forward_drop() -> bool: return "policy drop" in result.stdout -def ensure_filter_forward(interface: str, upstream: str) -> list[int]: +def ensure_filter_forward( + interface: str, + upstream: str, + extra_interfaces: list[str] | None = None, +) -> list[int]: """Insert nft ACCEPT rules into ``ip filter FORWARD`` if its policy is drop. Returns a list of rule handles so they can be removed on cleanup. + *extra_interfaces* are additional names (e.g. VLAN sub-interfaces) + that also need accept rules when Docker has set FORWARD policy drop. """ if not is_filter_forward_drop(): return [] + ifaces: list[str] = [] + for name in (interface, upstream, *(extra_interfaces or [])): + if name and name not in ifaces: + ifaces.append(name) + handles: list[int] = [] - for iface in (interface, upstream): + for iface in ifaces: for direction in ("iifname", "oifname"): result = _run_nft( ["-e", "-a", "insert", "rule", "ip", "filter", "FORWARD", @@ -333,11 +411,13 @@ def remove_ntp_redirect(table_name: str) -> None: def flush_rules(table_name: str = "jumpstarter") -> None: + """Delete the named nftables table (silently ignored if absent).""" logger.info("Flushing nftables table %s", table_name) _run_nft(["delete", "table", "ip", table_name], check=False) def list_rules(table_name: str = "jumpstarter") -> str: + """Return the nftables rules for *table_name* as text (empty if absent).""" result = _run_nft(["list", "table", "ip", table_name], check=False) if result.returncode != 0: return "" diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_cli.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_cli.py index 3bb6c62e3..9db4c4a8e 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_cli.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_cli.py @@ -182,7 +182,7 @@ def test_add_address_without_mac(self, tmp_path: Path, runner: CliRunner): ]) assert result.exit_code == 0 mock_add.assert_called_once_with( - "192.168.100.50", None, "", "10.0.0.50", + "192.168.100.50", None, "", "10.0.0.50", None, None, ) def test_add_address_with_hostname(self, tmp_path: Path, runner: CliRunner): @@ -194,7 +194,21 @@ def test_add_address_with_hostname(self, tmp_path: Path, runner: CliRunner): ) assert result.exit_code == 0 mock_add.assert_called_once_with( - "192.168.100.50", "aa:bb:cc:dd:ee:ff", "my-dut", None, + "192.168.100.50", "aa:bb:cc:dd:ee:ff", "my-dut", None, None, None, + ) + + def test_add_address_with_vlan_options(self, tmp_path: Path, runner: CliRunner): + with _make_client(tmp_path) as client: + with patch.object(client, "add_address") as mock_add: + result = runner.invoke(client.cli(), [ + "add-address", "192.168.100.50", + "--public-ip", "203.0.113.1", + "--vlan-id", "905", + "--public-gateway", "203.0.113.254", + ]) + assert result.exit_code == 0 + mock_add.assert_called_once_with( + "192.168.100.50", None, "", "203.0.113.1", 905, "203.0.113.254", ) def test_requires_ip_argument(self, tmp_path: Path, runner: CliRunner): diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver.py index c472a261d..16c45f79b 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver.py @@ -1,3 +1,5 @@ +import ipaddress +import logging import socket from pathlib import Path from unittest.mock import MagicMock, patch @@ -92,7 +94,7 @@ def test_valid_filter_config_from_dict(self, tmp_path: Path): "ingress": { "policy": "drop", "rules": [ - {"action": "accept", "source": "10.26.28.0/24", "port": 22, "protocol": "tcp"}, + {"action": "accept", "source": "198.51.100.0/24", "port": 22, "protocol": "tcp"}, ], }, } @@ -424,22 +426,22 @@ def test_add_address_with_mac(self, tmp_path: Path): driver, _, _, _ = _make_driver(tmp_path) with patch(f"{_DRIVER_MODULE}.dnsmasq"): driver.add_address("192.168.100.50", mac="aa:bb:cc:dd:ee:ff", hostname="new-dut") - assert any(entry["mac"] == "aa:bb:cc:dd:ee:ff" for entry in driver.addresses) + assert any(entry.mac == "aa:bb:cc:dd:ee:ff" for entry in driver.addresses) def test_add_address_without_mac(self, tmp_path: Path): driver, _, _, _ = _make_driver(tmp_path) with patch(f"{_DRIVER_MODULE}.dnsmasq"): driver.add_address("192.168.100.50", hostname="nat-only", public_ip="10.0.0.50") entry = driver.addresses[0] - assert "mac" not in entry - assert entry["public_ip"] == "10.0.0.50" + assert entry.mac is None + assert entry.public_ip == "10.0.0.50" def test_add_address_with_public_ip(self, tmp_path: Path): driver, _, _, _ = _make_driver(tmp_path) with patch(f"{_DRIVER_MODULE}.dnsmasq"): driver.add_address("192.168.100.50", mac="aa:bb:cc:dd:ee:ff", hostname="dut", public_ip="10.0.0.50") entry = driver.addresses[0] - assert entry["public_ip"] == "10.0.0.50" + assert entry.public_ip == "10.0.0.50" def test_add_replaces_existing_ip(self, tmp_path: Path): addrs = [{"mac": "AA:BB:CC:DD:EE:FF", "ip": "192.168.100.10"}] @@ -447,7 +449,7 @@ def test_add_replaces_existing_ip(self, tmp_path: Path): with patch(f"{_DRIVER_MODULE}.dnsmasq"): driver.add_address("192.168.100.10", mac="11:22:33:44:55:66") assert len(driver.addresses) == 1 - assert driver.addresses[0]["mac"] == "11:22:33:44:55:66" + assert driver.addresses[0].mac == "11:22:33:44:55:66" def test_remove_address(self, tmp_path: Path): addrs = [{"mac": "aa:bb:cc:dd:ee:ff", "ip": "192.168.100.10"}] @@ -548,3 +550,408 @@ def test_unresolvable_hostname_raises_during_setup(self, tmp_path: Path): with patch(f"{_DRIVER_MODULE}.socket.getaddrinfo", side_effect=socket.gaierror("fail")): with pytest.raises(ValueError, match="Cannot resolve hostname"): _make_driver(tmp_path, nat_mode="1to1", addresses=leases) + + +class TestAddressEntryValidation: + def test_omitted_vlan_fields_ok(self, tmp_path: Path): + driver, _, _, _ = _make_driver( + tmp_path, + addresses=[{"mac": "aa:bb:cc:dd:ee:ff", "ip": "192.168.100.10"}], + ) + entry = driver.addresses[0] + assert entry.vlan_id is None + assert entry.public_gateway is None + + def test_vlan_id_out_of_range(self, tmp_path: Path): + with pytest.raises(ValueError, match="vlan_id"): + _make_driver( + tmp_path, + addresses=[{"ip": "192.168.100.10", "vlan_id": 5000}], + ) + + def test_public_gateway_without_vlan_is_allowed(self, tmp_path: Path): + driver, mock_ip, _, _ = _make_driver( + tmp_path, + addresses=[{ + "ip": "192.168.100.10", + "public_gateway": "203.0.113.254", + }], + ) + table = int(ipaddress.IPv4Address("192.168.100.10")) + mock_ip.create_vlan_interface.assert_not_called() + mock_ip.add_policy_route.assert_called_once_with("203.0.113.254", "eth-up", table) + mock_ip.add_ip_rule.assert_called_once_with("192.168.100.10", table, priority=100) + assert driver._pbr_tables == {table} + + def test_public_gateway_without_vlan_requires_ipv4(self, tmp_path: Path): + with pytest.raises(ValueError, match="valid IPv4"): + _make_driver( + tmp_path, + addresses=[{"ip": "not-an-ip", "public_gateway": "10.0.0.1"}], + ) + + def test_reserved_vlan_table_rejected_with_gateway(self, tmp_path: Path): + with pytest.raises(ValueError, match="reserved"): + _make_driver( + tmp_path, + addresses=[{ + "ip": "192.168.100.10", + "vlan_id": 254, + "public_gateway": "203.0.113.254", + }], + ) + + def test_invalid_public_gateway(self, tmp_path: Path): + with pytest.raises(ValueError, match="public_gateway"): + _make_driver( + tmp_path, + addresses=[{ + "ip": "192.168.100.10", + "vlan_id": 905, + "public_gateway": "not-an-ip", + }], + ) + + +class TestVlanSetupMasquerade: + def test_creates_vlan_and_sysctls(self, tmp_path: Path): + addrs = [{ + "mac": "aa:bb:cc:dd:ee:ff", + "ip": "192.168.100.125", + "vlan_id": 905, + "public_ip": "203.0.113.1", + "public_gateway": "203.0.113.254", + }] + _, mock_ip, mock_nft, _ = _make_driver(tmp_path, nat_mode="masquerade", addresses=addrs) + mock_ip.create_vlan_interface.assert_called_once_with("eth-up", 905) + mock_ip.set_interface_forwarding.assert_any_call("eth-up.905", True) + mock_ip.set_interface_rp_filter.assert_called_once_with("eth-up.905", 2) + mock_ip.add_ip_alias.assert_called_once_with("eth-up.905", "203.0.113.1", 24) + mock_ip.add_policy_route.assert_called_once_with("203.0.113.254", "eth-up.905", 905) + mock_ip.add_ip_rule.assert_called_once_with("192.168.100.125", 905, priority=100) + call_kwargs = mock_nft.apply_masquerade_rules.call_args[1] + # Upstream is always included so unexpected/unregistered DUTs are + # still masqueraded via the default upstream interface. + assert call_kwargs["nat_interfaces"] == ["eth-up", "eth-up.905"] + + def test_no_vlan_does_not_pass_nat_interfaces(self, tmp_path: Path): + _, _, mock_nft, _ = _make_driver(tmp_path, nat_mode="masquerade") + mock_nft.apply_masquerade_rules.assert_called_once_with( + "eth-dut", "eth-up", "192.168.100.0/24", + table_name="jumpstarter_eth_dut", + filter_config=None, + ) + + def test_vlan_only_still_includes_upstream_for_unexpected_duts(self, tmp_path: Path): + """Even when all addresses carry a vlan_id, the upstream interface + must be in _outbound_interfaces() so that unexpected / unregistered + DUTs on the bridge are still masqueraded via the default upstream. + """ + addrs = [ + {"ip": "192.168.100.125", "vlan_id": 905, + "public_ip": "203.0.113.1", "public_gateway": "203.0.113.254"}, + {"ip": "192.168.100.126", "vlan_id": 906, + "public_ip": "203.0.113.2", "public_gateway": "203.0.113.254"}, + ] + driver, _, mock_nft, _ = _make_driver( + tmp_path, nat_mode="masquerade", addresses=addrs, + ) + call_kwargs = mock_nft.apply_masquerade_rules.call_args[1] + outbound = call_kwargs["nat_interfaces"] + assert "eth-up" in outbound, "upstream must be present for unexpected DUTs" + assert "eth-up.905" in outbound + assert "eth-up.906" in outbound + + def test_ensure_filter_forward_includes_vlan(self, tmp_path: Path): + addrs = [{"ip": "192.168.100.125", "vlan_id": 905}] + _, mock_ip, mock_nft, _ = _make_driver(tmp_path, nat_mode="masquerade", addresses=addrs) + mock_nft.ensure_filter_forward.assert_called_once_with( + "eth-dut", "eth-up", extra_interfaces=["eth-up.905"], + ) + mock_ip.add_policy_route.assert_not_called() + mock_ip.add_ip_rule.assert_not_called() + + def test_add_address_creates_vlan_and_pbr(self, tmp_path: Path): + driver, _, _, _ = _make_driver(tmp_path) + with patch(f"{_DRIVER_MODULE}.dnsmasq"), \ + patch(f"{_DRIVER_MODULE}.iproute") as mock_ip, \ + patch(f"{_DRIVER_MODULE}.nftables") as mock_nft: + mock_nft.list_rules.return_value = "" + driver.add_address( + "192.168.100.50", + public_ip="10.99.0.50", + vlan_id=100, + public_gateway="10.99.0.1", + ) + mock_ip.create_vlan_interface.assert_called_with("eth-up", 100) + mock_ip.add_policy_route.assert_called_with("10.99.0.1", "eth-up.100", 100) + mock_ip.add_ip_rule.assert_called_with("192.168.100.50", 100, priority=100) + mock_nft.apply_masquerade_rules.assert_called() + + def test_vlan_without_gateway_emits_warning(self, tmp_path: Path, caplog: pytest.LogCaptureFixture): + addrs = [{"ip": "192.168.100.125", "vlan_id": 905}] + with caplog.at_level(logging.WARNING, logger="driver.DutNetwork"): + _make_driver(tmp_path, nat_mode="masquerade", addresses=addrs) + assert any( + "no public_gateway" in rec.message and "192.168.100.125" in rec.message + for rec in caplog.records + ) + + def test_vlan_with_gateway_does_not_warn(self, tmp_path: Path, caplog: pytest.LogCaptureFixture): + addrs = [{ + "ip": "192.168.100.125", + "vlan_id": 905, + "public_gateway": "203.0.113.254", + }] + with caplog.at_level(logging.WARNING, logger="driver.DutNetwork"): + _make_driver(tmp_path, nat_mode="masquerade", addresses=addrs) + assert not any("no public_gateway" in rec.message for rec in caplog.records) + + +class TestVlanSetup1to1: + def test_alias_and_rules_use_vlan_interface(self, tmp_path: Path): + addrs = [{ + "mac": "aa:bb:cc:dd:ee:ff", + "ip": "192.168.100.125", + "public_ip": "203.0.113.1", + "vlan_id": 905, + "public_gateway": "203.0.113.254", + }] + driver, mock_ip, mock_nft, _ = _make_driver(tmp_path, nat_mode="1to1", addresses=addrs) + mock_ip.create_vlan_interface.assert_called_once_with("eth-up", 905) + mock_ip.add_ip_alias.assert_called_once_with("eth-up.905", "203.0.113.1", 24) + mappings = mock_nft.apply_1to1_rules.call_args[0][2] + assert mappings == [{ + "private_ip": "192.168.100.125", + "public_ip": "203.0.113.1", + "nat_interface": "eth-up.905", + }] + assert driver._added_aliases == {"203.0.113.1"} + assert driver._alias_ifaces["203.0.113.1"] == "eth-up.905" + + def test_untagged_1to1_unchanged(self, tmp_path: Path): + addrs = [{ + "mac": "aa:bb:cc:dd:ee:ff", + "ip": "192.168.100.10", + "public_ip": "10.0.0.50", + }] + _, mock_ip, mock_nft, _ = _make_driver(tmp_path, nat_mode="1to1", addresses=addrs) + mock_ip.create_vlan_interface.assert_not_called() + mock_ip.add_ip_alias.assert_called_once_with("eth-up", "10.0.0.50", 24) + mappings = mock_nft.apply_1to1_rules.call_args[0][2] + assert mappings == [{"private_ip": "192.168.100.10", "public_ip": "10.0.0.50"}] + + +class TestVlanCreationRollback: + """VLAN interface is deleted if post-creation setup fails.""" + + def test_sysctl_failure_rolls_back_vlan(self, tmp_path: Path): + """set_interface_forwarding raises on VLAN iface -> VLAN deleted.""" + addrs = [{ + "ip": "192.168.100.125", + "vlan_id": 905, + "public_ip": "203.0.113.1", + "public_gateway": "203.0.113.254", + }] + + from .driver import DutNetwork + + # Allow the first two calls (eth-dut, eth-up) but fail on the + # third call which targets the VLAN sub-interface. + _fwd_calls: list[int] = [0] + def _fwd_side_effect(iface, enabled): + _fwd_calls[0] += 1 + if _fwd_calls[0] >= 3: + raise RuntimeError("sysctl boom") + + with patch(f"{_DRIVER_MODULE}.sys") as mock_sys, \ + patch(f"{_DRIVER_MODULE}.shutil") as mock_shutil, \ + patch(f"{_DRIVER_MODULE}.iproute") as mock_ip, \ + patch(f"{_DRIVER_MODULE}.nftables") as mock_nft, \ + patch(f"{_DRIVER_MODULE}.dnsmasq") as mock_dns: + mock_sys.platform = "linux" + mock_shutil.which.return_value = "/usr/bin/fake" + mock_dns.state_dir_for_interface.return_value = tmp_path + mock_dns.start.return_value = MagicMock() + mock_ip.detect_upstream_interface.return_value = "eth-up" + mock_ip.interface_exists.return_value = False + mock_ip.get_interface_addresses.return_value = [] + mock_ip.get_interface_forwarding.return_value = "0" + mock_ip.get_interface_prefix_len.return_value = 24 + mock_nft.ensure_filter_forward.return_value = [] + mock_nft.list_rules.return_value = "" + mock_nft._table_name_for.return_value = "jumpstarter_eth_dut" + mock_ip.set_interface_forwarding.side_effect = _fwd_side_effect + + with pytest.raises(RuntimeError, match="sysctl boom"): + DutNetwork( + interface="eth-dut", + subnet="192.168.100.0/24", + gateway_ip="192.168.100.1", + upstream_interface="eth-up", + nat_mode="masquerade", + dhcp_enabled=True, + dhcp_range_start="192.168.100.100", + dhcp_range_end="192.168.100.200", + addresses=addrs, + dns_servers=["8.8.8.8"], + state_dir=str(tmp_path), + ) + + mock_ip.create_vlan_interface.assert_called_once_with("eth-up", 905) + mock_ip.delete_vlan_interface.assert_called_once_with("eth-up.905") + + def test_alias_failure_rolls_back_vlan_and_bookkeeping(self, tmp_path: Path): + """add_ip_alias raises -> VLAN deleted and alias bookkeeping undone.""" + addrs = [{ + "ip": "192.168.100.125", + "vlan_id": 905, + "public_ip": "203.0.113.1", + "public_gateway": "203.0.113.254", + }] + + from .driver import DutNetwork + + with patch(f"{_DRIVER_MODULE}.sys") as mock_sys, \ + patch(f"{_DRIVER_MODULE}.shutil") as mock_shutil, \ + patch(f"{_DRIVER_MODULE}.iproute") as mock_ip, \ + patch(f"{_DRIVER_MODULE}.nftables") as mock_nft, \ + patch(f"{_DRIVER_MODULE}.dnsmasq") as mock_dns: + mock_sys.platform = "linux" + mock_shutil.which.return_value = "/usr/bin/fake" + mock_dns.state_dir_for_interface.return_value = tmp_path + mock_dns.start.return_value = MagicMock() + mock_ip.detect_upstream_interface.return_value = "eth-up" + mock_ip.interface_exists.return_value = False + mock_ip.get_interface_addresses.return_value = [] + mock_ip.get_interface_forwarding.return_value = "0" + mock_ip.get_interface_prefix_len.return_value = 24 + mock_nft.ensure_filter_forward.return_value = [] + mock_nft.list_rules.return_value = "" + mock_nft._table_name_for.return_value = "jumpstarter_eth_dut" + mock_ip.add_ip_alias.side_effect = RuntimeError("alias boom") + + with pytest.raises(RuntimeError, match="alias boom"): + DutNetwork( + interface="eth-dut", + subnet="192.168.100.0/24", + gateway_ip="192.168.100.1", + upstream_interface="eth-up", + nat_mode="masquerade", + dhcp_enabled=True, + dhcp_range_start="192.168.100.100", + dhcp_range_end="192.168.100.200", + addresses=addrs, + dns_servers=["8.8.8.8"], + state_dir=str(tmp_path), + ) + + mock_ip.create_vlan_interface.assert_called_once_with("eth-up", 905) + mock_ip.set_interface_forwarding.assert_called() + mock_ip.delete_vlan_interface.assert_called_once_with("eth-up.905") + + +class TestVlanCleanup: + def test_cleanup_reverses_vlan_and_pbr(self, tmp_path: Path): + addrs = [{ + "ip": "192.168.100.125", + "public_ip": "203.0.113.1", + "vlan_id": 905, + "public_gateway": "203.0.113.254", + }] + driver, _, _, _ = _make_driver(tmp_path, nat_mode="1to1", addresses=addrs) + with patch(f"{_DRIVER_MODULE}.iproute") as mock_ip2, \ + patch(f"{_DRIVER_MODULE}.nftables"), \ + patch(f"{_DRIVER_MODULE}.dnsmasq"): + driver.cleanup() + mock_ip2.remove_ip_alias.assert_called_once_with("eth-up.905", "203.0.113.1", 24) + mock_ip2.delete_ip_rule.assert_called_once_with("192.168.100.125", 905) + mock_ip2.flush_routing_table.assert_called_once_with(905) + mock_ip2.delete_vlan_interface.assert_called_once_with("eth-up.905") + assert driver._created_vlans == set() + assert driver._pbr_rules == [] + assert driver._added_aliases == set() + + +class TestUntaggedPbr: + def test_uses_private_ip_as_table_id(self, tmp_path: Path): + table = int(ipaddress.IPv4Address("192.168.100.10")) + addrs = [{ + "mac": "aa:bb:cc:dd:ee:ff", + "ip": "192.168.100.10", + "public_gateway": "10.0.0.1", + }] + _, mock_ip, mock_nft, _ = _make_driver(tmp_path, nat_mode="masquerade", addresses=addrs) + mock_ip.create_vlan_interface.assert_not_called() + mock_ip.add_policy_route.assert_called_once_with("10.0.0.1", "eth-up", table) + mock_ip.add_ip_rule.assert_called_once_with("192.168.100.10", table, priority=100) + mock_nft.apply_masquerade_rules.assert_called_once_with( + "eth-dut", "eth-up", "192.168.100.0/24", + table_name="jumpstarter_eth_dut", + filter_config=None, + ) + + def test_cleanup_removes_untagged_pbr(self, tmp_path: Path): + table = int(ipaddress.IPv4Address("192.168.100.10")) + addrs = [{"ip": "192.168.100.10", "public_gateway": "10.0.0.1"}] + driver, _, _, _ = _make_driver(tmp_path, nat_mode="masquerade", addresses=addrs) + with patch(f"{_DRIVER_MODULE}.iproute") as mock_ip2, \ + patch(f"{_DRIVER_MODULE}.nftables"), \ + patch(f"{_DRIVER_MODULE}.dnsmasq"): + driver.cleanup() + mock_ip2.delete_ip_rule.assert_called_once_with("192.168.100.10", table) + mock_ip2.flush_routing_table.assert_called_once_with(table) + mock_ip2.delete_vlan_interface.assert_not_called() + + def test_add_address_applies_untagged_pbr(self, tmp_path: Path): + table = int(ipaddress.IPv4Address("192.168.100.51")) + driver, _, _, _ = _make_driver(tmp_path) + with patch(f"{_DRIVER_MODULE}.dnsmasq"), \ + patch(f"{_DRIVER_MODULE}.iproute") as mock_ip, \ + patch(f"{_DRIVER_MODULE}.nftables"): + driver.add_address("192.168.100.51", public_gateway="10.99.0.1") + mock_ip.create_vlan_interface.assert_not_called() + mock_ip.add_policy_route.assert_called_once_with("10.99.0.1", "eth-up", table) + mock_ip.add_ip_rule.assert_called_once_with("192.168.100.51", table, priority=100) + + def test_reserved_untagged_table_rejected(self, tmp_path: Path): + """Untagged PBR where int(IPv4) hits a reserved table should fail.""" + with pytest.raises(ValueError, match="reserved"): + _make_driver( + tmp_path, + addresses=[{"ip": "0.0.0.253", "public_gateway": "10.0.0.1"}], + ) + + +class TestSyncNatRefreshesFwdHandles: + """Verify _sync_nat re-creates FORWARD ACCEPT rules for new VLAN interfaces.""" + + def test_runtime_add_address_refreshes_fwd_handles(self, tmp_path: Path): + driver, _, _, _ = _make_driver(tmp_path) + driver._fwd_rule_handles = [42, 43] + with patch(f"{_DRIVER_MODULE}.dnsmasq"), \ + patch(f"{_DRIVER_MODULE}.iproute"), \ + patch(f"{_DRIVER_MODULE}.nftables") as mock_nft: + mock_nft.list_rules.return_value = "" + driver.add_address( + "192.168.100.50", + public_ip="10.99.0.50", + vlan_id=100, + public_gateway="10.99.0.1", + ) + mock_nft.remove_filter_forward.assert_called_once_with([42, 43]) + mock_nft.ensure_filter_forward.assert_called_once_with( + "eth-dut", "eth-up", extra_interfaces=["eth-up.100"], + ) + + def test_runtime_untagged_pbr_does_not_add_extra_ifaces(self, tmp_path: Path): + driver, _, _, _ = _make_driver(tmp_path) + driver._fwd_rule_handles = [42] + with patch(f"{_DRIVER_MODULE}.dnsmasq"), \ + patch(f"{_DRIVER_MODULE}.iproute"), \ + patch(f"{_DRIVER_MODULE}.nftables") as mock_nft: + mock_nft.list_rules.return_value = "" + driver.add_address("192.168.100.51", public_gateway="10.99.0.1") + mock_nft.remove_filter_forward.assert_called_once_with([42]) + call_kwargs = mock_nft.ensure_filter_forward.call_args + assert call_kwargs == (("eth-dut", "eth-up"),) diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver_test.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver_integration.py similarity index 90% rename from python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver_test.py rename to python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver_integration.py index 64b2dc641..431e829ed 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/driver_test.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_driver_integration.py @@ -4,6 +4,7 @@ and interface operations. They are skipped when neither is available. """ +import ipaddress import os import re import shlex @@ -1126,3 +1127,118 @@ def test_no_filter_allows_all_traffic(self, net_env: NetworkTestEnv): assert result.returncode == 0, f"No-filter should allow all traffic: {result.stderr}" finally: driver.cleanup() + + +@requires_linux +@requires_privileges +@requires_nft +@requires_dnsmasq +class TestVlanAndPbr: + """Control-plane tests for VLAN sub-interfaces and policy-based routing.""" + + VLAN_ID = 100 + PUBLIC_IP = "10.99.0.50" + PUBLIC_GW = "10.99.0.1" + + def _vlan_name(self, net_env: NetworkTestEnv) -> str: + return f"{net_env.VETH_UPSTREAM}.{self.VLAN_ID}" + + def _vlan_addresses(self, net_env: NetworkTestEnv) -> list[dict]: + return [{ + "mac": net_env.DUT_MAC, + "ip": net_env.DUT_IP, + "hostname": "test-dut", + "public_ip": self.PUBLIC_IP, + "vlan_id": self.VLAN_ID, + "public_gateway": self.PUBLIC_GW, + }] + + def test_vlan_interface_created_and_cleaned_up(self, net_env: NetworkTestEnv): + vlan = self._vlan_name(net_env) + driver = net_env.create_driver( + nat_mode="1to1", + addresses=self._vlan_addresses(net_env), + ) + try: + result = _run(f"ip link show {vlan}") + assert result.returncode == 0 + addr = _run(f"ip -o -4 addr show dev {vlan}") + assert self.PUBLIC_IP in addr.stdout + fwd = _run(f"sysctl -n net.ipv4.conf.{net_env.VETH_UPSTREAM}/{self.VLAN_ID}.forwarding") + assert fwd.stdout.strip() == "1" + rp = _run(f"sysctl -n net.ipv4.conf.{net_env.VETH_UPSTREAM}/{self.VLAN_ID}.rp_filter") + assert rp.stdout.strip() == "2" + rules = _run("ip rule show") + assert net_env.DUT_IP in rules.stdout + assert f"lookup {self.VLAN_ID}" in rules.stdout + table = _run(f"ip route show table {self.VLAN_ID}") + assert self.PUBLIC_GW in table.stdout + assert vlan in table.stdout + nft = _run(f"nft list table ip {net_env.NFT_TABLE}") + assert vlan in nft.stdout + assert self.PUBLIC_IP in nft.stdout + finally: + driver.cleanup() + + gone = _run(f"ip link show {vlan}", check=False) + assert gone.returncode != 0 + rules = _run("ip rule show") + assert f"from {net_env.DUT_IP} lookup {self.VLAN_ID}" not in rules.stdout + + def test_masquerade_nftables_use_vlan_interface(self, net_env: NetworkTestEnv): + vlan = self._vlan_name(net_env) + driver = net_env.create_driver( + nat_mode="masquerade", + addresses=self._vlan_addresses(net_env), + ) + try: + nft = _run(f"nft list table ip {net_env.NFT_TABLE}") + assert f'oifname "{vlan}"' in nft.stdout + assert "masquerade" in nft.stdout + finally: + driver.cleanup() + + def test_vlan_without_gateway_skips_pbr(self, net_env: NetworkTestEnv): + vlan = self._vlan_name(net_env) + driver = net_env.create_driver( + nat_mode="masquerade", + addresses=[{ + "mac": net_env.DUT_MAC, + "ip": net_env.DUT_IP, + "hostname": "test-dut", + "vlan_id": self.VLAN_ID, + }], + ) + try: + result = _run(f"ip link show {vlan}") + assert result.returncode == 0 + rules = _run("ip rule show") + assert f"from {net_env.DUT_IP}" not in rules.stdout + finally: + driver.cleanup() + + gone = _run(f"ip link show {vlan}", check=False) + assert gone.returncode != 0 + + def test_untagged_pbr_uses_private_ip_table(self, net_env: NetworkTestEnv): + pbr_ip = "192.168.200.51" + table = int(ipaddress.IPv4Address(pbr_ip)) + driver = net_env.create_driver( + nat_mode="masquerade", + addresses=[{ + "ip": pbr_ip, + "public_gateway": self.PUBLIC_GW, + }], + ) + try: + rules = _run("ip rule show") + assert f"from {pbr_ip}" in rules.stdout + assert f"lookup {table}" in rules.stdout + table_routes = _run(f"ip route show table {table}") + assert self.PUBLIC_GW in table_routes.stdout + assert net_env.VETH_UPSTREAM in table_routes.stdout + finally: + driver.cleanup() + + rules = _run("ip rule show") + assert f"from {pbr_ip} lookup {table}" not in rules.stdout diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_iproute.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_iproute.py index ea8a851d0..0ff92f805 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_iproute.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_iproute.py @@ -1,6 +1,8 @@ import subprocess from unittest.mock import patch +import pytest + from . import iproute @@ -118,3 +120,131 @@ def test_set_interface_forwarding(self): mock.assert_called_once_with( ["sysctl", "-w", "net.ipv4.conf.eth0.forwarding=1"] ) + + def test_set_interface_forwarding_translates_vlan_dots(self): + with patch.object(iproute, "_run_priv") as mock: + iproute.set_interface_forwarding("end0.905", True) + mock.assert_called_once_with( + ["sysctl", "-w", "net.ipv4.conf.end0/905.forwarding=1"] + ) + + def test_get_interface_forwarding_translates_vlan_dots(self): + fake = subprocess.CompletedProcess(args=[], returncode=0, stdout="1\n") + with patch.object(iproute, "_run", return_value=fake) as mock_run: + iproute.get_interface_forwarding("end0.905") + mock_run.assert_called_once_with( + ["sysctl", "-n", "net.ipv4.conf.end0/905.forwarding"], check=False + ) + + +class TestRpFilter: + def test_set_rp_filter_translates_vlan_dots(self): + with patch.object(iproute, "_run_priv") as mock: + iproute.set_interface_rp_filter("end0.905", 2) + mock.assert_called_once_with( + ["sysctl", "-w", "net.ipv4.conf.end0/905.rp_filter=2"] + ) + + +class TestVlanInterface: + def test_vlan_subinterface_name(self): + assert iproute.vlan_subinterface_name("end0", 905) == "end0.905" + + def test_create_vlan_interface(self): + with patch.object(iproute, "interface_exists", return_value=False), \ + patch.object(iproute, "nm_set_unmanaged") as mock_nm, \ + patch.object(iproute, "_run_priv") as mock: + name = iproute.create_vlan_interface("end0", 905) + assert name == "end0.905" + mock.assert_any_call( + ["ip", "link", "add", "link", "end0", "name", "end0.905", "type", "vlan", "id", "905"] + ) + mock.assert_any_call(["ip", "link", "set", "end0.905", "up"]) + mock_nm.assert_called_once_with("end0.905") + + def test_create_vlan_interface_idempotent(self): + with patch.object(iproute, "interface_exists", return_value=True), \ + patch.object(iproute, "nm_set_unmanaged"), \ + patch.object(iproute, "_run_priv") as mock: + iproute.create_vlan_interface("end0", 905) + add_calls = [c for c in mock.call_args_list if c.args[0][:3] == ["ip", "link", "add"]] + assert add_calls == [] + + def test_create_vlan_rejects_long_name(self): + with pytest.raises(ValueError, match="15-character"): + iproute.create_vlan_interface("enx00e04c683af1", 905) + + def test_delete_vlan_interface(self): + with patch.object(iproute, "_run_priv") as mock: + iproute.delete_vlan_interface("end0.905") + mock.assert_called_once_with(["ip", "link", "del", "end0.905"], check=False) + + +class TestPolicyRouting: + def test_add_policy_route(self): + ok = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + with patch.object(iproute, "_run_priv", return_value=ok) as mock: + iproute.add_policy_route("203.0.113.254", "end0.905", 905) + mock.assert_called_once_with( + ["ip", "route", "replace", "default", "via", "203.0.113.254", + "dev", "end0.905", "table", "905", "onlink"], + check=False, + ) + + def test_add_policy_route_replaces_conflicting_route(self): + """``replace`` is idempotent — a pre-existing route is overwritten.""" + ok = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + with patch.object(iproute, "_run_priv", return_value=ok): + iproute.add_policy_route("10.0.0.1", "eth0", 100) + + def test_add_policy_route_raises_on_failure(self): + fail = subprocess.CompletedProcess( + args=[], returncode=2, stdout="", stderr="Error: some failure\n", + ) + with patch.object(iproute, "_run_priv", return_value=fail): + with pytest.raises(RuntimeError, match="some failure"): + iproute.add_policy_route("10.0.0.1", "eth0", 100) + + def test_add_policy_route_rejects_reserved_table(self): + with pytest.raises(ValueError, match="reserved"): + iproute.add_policy_route("10.0.0.1", "eth0", 254) + + def test_add_ip_rule(self): + ok = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + with patch.object(iproute, "_run_priv", return_value=ok) as mock: + iproute.add_ip_rule("192.168.100.125", 905, priority=100) + mock.assert_called_once_with( + ["ip", "rule", "add", "from", "192.168.100.125", + "table", "905", "priority", "100"], + check=False, + ) + + def test_add_ip_rule_idempotent_on_exists(self): + exists = subprocess.CompletedProcess( + args=[], returncode=2, stdout="", stderr="RTNETLINK answers: File exists\n", + ) + with patch.object(iproute, "_run_priv", return_value=exists): + iproute.add_ip_rule("192.168.100.10", 100) + + def test_add_ip_rule_raises_on_failure(self): + fail = subprocess.CompletedProcess( + args=[], returncode=2, stdout="", stderr="Error: some failure\n", + ) + with patch.object(iproute, "_run_priv", return_value=fail): + with pytest.raises(RuntimeError, match="some failure"): + iproute.add_ip_rule("192.168.100.10", 100) + + def test_delete_ip_rule(self): + with patch.object(iproute, "_run_priv") as mock: + iproute.delete_ip_rule("192.168.100.125", 905) + mock.assert_called_once_with( + ["ip", "rule", "del", "from", "192.168.100.125", "table", "905"], + check=False, + ) + + def test_flush_routing_table(self): + with patch.object(iproute, "_run_priv") as mock: + iproute.flush_routing_table(905) + mock.assert_called_once_with( + ["ip", "route", "flush", "table", "905"], check=False + ) diff --git a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_nftables.py b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_nftables.py index dd60874a4..da91a29ab 100644 --- a/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_nftables.py +++ b/python/packages/jumpstarter-driver-dut-network/jumpstarter_driver_dut_network/test_nftables.py @@ -56,12 +56,12 @@ def test_ingress_only_with_port_protocol(self): ingress=FilterDirection( policy="drop", rules=[ - FilterRule(action="accept", source="10.26.28.0/24", port=22, protocol="tcp"), + FilterRule(action="accept", source="198.51.100.0/24", port=22, protocol="tcp"), ], ), ) result = nftables._build_forward_chain("br-jmp0", "eth0", filter_config=fc) - assert "ip saddr 10.26.28.0/24 tcp dport 22 accept" in result + assert "ip saddr 198.51.100.0/24 tcp dport 22 accept" in result # Ingress catch-all should be drop lines = result.strip().splitlines() # The last rule before "}" should be the ingress catch-all @@ -76,7 +76,7 @@ def test_combined_egress_ingress(self): ), ingress=FilterDirection( policy="drop", - rules=[FilterRule(action="accept", source="10.26.28.0/24", port=22, protocol="tcp")], + rules=[FilterRule(action="accept", source="198.51.100.0/24", port=22, protocol="tcp")], ), ) result = nftables._build_forward_chain("br-jmp0", "eth0", filter_config=fc) @@ -87,7 +87,7 @@ def test_combined_egress_ingress(self): i for i, ln in enumerate(lines) if 'iifname "br-jmp0"' in ln and "daddr" not in ln ) - ingress_rule = next(i for i, ln in enumerate(lines) if "10.26.28.0/24" in ln) + ingress_rule = next(i for i, ln in enumerate(lines) if "198.51.100.0/24" in ln) ingress_catchall = next( i for i, ln in enumerate(lines) if 'iifname "eth0"' in ln and "saddr" not in ln and "ct state" not in ln @@ -273,6 +273,58 @@ def test_without_filter_backward_compatible(self): assert "ip daddr 192.168.100.10 accept" in ruleset +class TestVlanNatRules: + """VLAN sub-interface names must appear in nftables instead of the parent.""" + + def test_masquerade_uses_vlan_interface(self): + with patch.object(nftables, "_run_nft"), \ + patch.object(nftables, "_load_ruleset") as mock_load: + nftables.apply_masquerade_rules( + "enp1s0u1", "end0", "192.168.100.0/24", + nat_interfaces=["end0.905"], + ) + ruleset = mock_load.call_args[0][0] + assert 'oifname "end0.905" ip saddr 192.168.100.0/24 masquerade' in ruleset + assert 'iifname "enp1s0u1" oifname "end0.905" accept' in ruleset + assert 'iifname "end0.905" oifname "enp1s0u1" ct state related,established accept' in ruleset + assert 'oifname "end0" ip saddr' not in ruleset + + def test_1to1_uses_vlan_interface(self): + mappings = [{ + "private_ip": "192.168.100.125", + "public_ip": "203.0.113.1", + "nat_interface": "end0.905", + }] + with patch.object(nftables, "_run_nft"), \ + patch.object(nftables, "_load_ruleset") as mock_load: + nftables.apply_1to1_rules("enp1s0u1", "end0", mappings, "192.168.100.0/24") + ruleset = mock_load.call_args[0][0] + assert 'iifname "end0.905" ip daddr 203.0.113.1 dnat to 192.168.100.125' in ruleset + assert 'ip saddr 192.168.100.125 oifname "end0.905" snat to 203.0.113.1' in ruleset + assert 'iifname "end0.905" oifname "enp1s0u1" ip daddr 192.168.100.125 accept' in ruleset + assert 'iifname "enp1s0u1" oifname "end0.905" accept' in ruleset + assert 'iifname "end0.905" oifname "enp1s0u1" ct state related,established accept' in ruleset + # Untagged parent is kept for unmapped-DUT masquerade fallback + assert 'oifname "end0" ip saddr 192.168.100.0/24 masquerade' in ruleset + + def test_1to1_without_nat_interface_stays_on_upstream(self): + mappings = [{"private_ip": "192.168.100.10", "public_ip": "10.0.0.50"}] + with patch.object(nftables, "_run_nft"), \ + patch.object(nftables, "_load_ruleset") as mock_load: + nftables.apply_1to1_rules("br-jmp0", "eth0", mappings, "192.168.100.0/24") + ruleset = mock_load.call_args[0][0] + assert 'iifname "eth0" ip daddr 10.0.0.50 dnat to 192.168.100.10' in ruleset + assert 'oifname "eth0" snat to 10.0.0.50' in ruleset + + def test_masquerade_without_nat_interfaces_unchanged(self): + with patch.object(nftables, "_run_nft"), \ + patch.object(nftables, "_load_ruleset") as mock_load: + nftables.apply_masquerade_rules("br-jmp0", "eth0", "192.168.100.0/24") + ruleset = mock_load.call_args[0][0] + assert 'oifname "eth0" ip saddr 192.168.100.0/24 masquerade' in ruleset + assert 'iifname "br-jmp0" oifname "eth0" accept' in ruleset + + class TestInterfaceNameValidation: def test_rejects_invalid_names(self): with pytest.raises(ValueError, match="Invalid interface name"): @@ -339,6 +391,15 @@ def test_ensure_inserts_rules_when_drop(self): assert all(h == 42 for h in handles) assert mock_nft.call_count == 4 + def test_ensure_inserts_extra_interfaces(self): + insert_output = 'insert rule ip filter FORWARD iifname "end0.905" accept # handle 7\n' + fake_insert = subprocess.CompletedProcess(args=[], returncode=0, stdout=insert_output) + with patch.object(nftables, "is_filter_forward_drop", return_value=True), \ + patch.object(nftables, "_run_nft", return_value=fake_insert) as mock_nft: + handles = nftables.ensure_filter_forward("br-jmp0", "eth0", extra_interfaces=["end0.905"]) + assert len(handles) == 6 + assert mock_nft.call_count == 6 + def test_ensure_returns_empty_when_accept(self): with patch.object(nftables, "is_filter_forward_drop", return_value=False): handles = nftables.ensure_filter_forward("br-jmp0", "eth0")