fix(net): detect interfaces without following /sys symlinks
/sys/class/net/<if> is always a symlink into /sys/devices; in nested containers (sysbox runtime) that target is not visible, so the link is dead. iface_exists() used Path.exists(), which follows the link and therefore tested the visibility of the target instead of the existence of the interface. detect_egress_iface() discarded every interface it had correctly read from the routing table, so automtu aborted with "Could not detect egress interface" and rc=2 even though `ip -4 route show default` and `ip link show dev eth0` both worked. The documented escape hatch --egress-if was equally dead, because core.py validates it through the same call. Chosen fix: os.path.lexists() rather than probing netlink for existence. It asks the right question -- "is there an entry named <if>" -- and costs no subprocess on a healthy host. Netlink (`ip link show`) is only the fallback for when /sys/class/net itself is unavailable, so sysfs stays the preferred path everywhere. read_iface_mtu() gains the same cascade: sysfs first, MTU parsed from `ip link show dev <if>` when the sysfs path is unreadable, RuntimeError naming both sources when neither answers - no silent default. core.py turns that into an error line plus rc=3 instead of a traceback. list_ifaces() had the same defect (is_dir() on a dead symlink) and would have left Docker bridge detection blind in the same environments. Regression tests build a /sys replacement whose class/net/eth0 points at a missing target and mock the ip command: they fail if lexists becomes exists again, if the MTU fallback is removed, or if the symlink filter is dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2,8 +2,8 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import statistics
|
import statistics
|
||||||
import sys
|
import sys
|
||||||
|
from collections.abc import Iterable
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Iterable, Optional
|
|
||||||
|
|
||||||
from .docker import detect_docker_ifaces
|
from .docker import detect_docker_ifaces
|
||||||
from .net import (
|
from .net import (
|
||||||
@@ -28,7 +28,7 @@ class Result:
|
|||||||
wg_mtu: int
|
wg_mtu: int
|
||||||
|
|
||||||
|
|
||||||
def _split_targets(items: Optional[list[str]]) -> list[str]:
|
def _split_targets(items: list[str] | None) -> list[str]:
|
||||||
raw: list[str] = []
|
raw: list[str] = []
|
||||||
for item in items or []:
|
for item in items or []:
|
||||||
raw.extend([x.strip() for x in item.split(",") if x.strip()])
|
raw.extend([x.strip() for x in item.split(",") if x.strip()])
|
||||||
@@ -119,7 +119,11 @@ def run_automtu(args) -> int:
|
|||||||
set_iface_mtu(egress, args.force_egress_mtu, args.dry_run)
|
set_iface_mtu(egress, args.force_egress_mtu, args.dry_run)
|
||||||
base_mtu = int(args.force_egress_mtu)
|
base_mtu = int(args.force_egress_mtu)
|
||||||
else:
|
else:
|
||||||
base_mtu = int(read_iface_mtu(egress))
|
try:
|
||||||
|
base_mtu = int(read_iface_mtu(egress))
|
||||||
|
except RuntimeError as exc:
|
||||||
|
print(f"[automtu][ERROR] {exc}", file=sys.stderr)
|
||||||
|
return 3
|
||||||
log(f"[automtu] Egress base MTU: {base_mtu}")
|
log(f"[automtu] Egress base MTU: {base_mtu}")
|
||||||
|
|
||||||
# Targets (explicit + optional WG auto targets)
|
# Targets (explicit + optional WG auto targets)
|
||||||
@@ -140,8 +144,8 @@ def run_automtu(args) -> int:
|
|||||||
|
|
||||||
# PMTU probing
|
# PMTU probing
|
||||||
effective_mtu = base_mtu
|
effective_mtu = base_mtu
|
||||||
probe_results: dict[str, Optional[int]] = {}
|
probe_results: dict[str, int | None] = {}
|
||||||
chosen_pmtu: Optional[int] = None
|
chosen_pmtu: int | None = None
|
||||||
|
|
||||||
if targets:
|
if targets:
|
||||||
log(
|
log(
|
||||||
@@ -186,7 +190,7 @@ def run_automtu(args) -> int:
|
|||||||
f"[automtu] Computed {args.wg_if} MTU: {wg_mtu} (overhead={args.wg_overhead}, min={args.wg_min})"
|
f"[automtu] Computed {args.wg_if} MTU: {wg_mtu} (overhead={args.wg_overhead}, min={args.wg_min})"
|
||||||
)
|
)
|
||||||
|
|
||||||
wg_mtu_set: Optional[int] = None
|
wg_mtu_set: int | None = None
|
||||||
wg_mtu_clamped = False
|
wg_mtu_clamped = False
|
||||||
|
|
||||||
if args.set_wg_mtu is not None:
|
if args.set_wg_mtu is not None:
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ import pathlib
|
|||||||
import re
|
import re
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
from typing import Optional
|
|
||||||
|
|
||||||
|
|
||||||
def _run(cmd: list[str]) -> str:
|
def _run(cmd: list[str]) -> str:
|
||||||
@@ -14,27 +13,55 @@ def _run(cmd: list[str]) -> str:
|
|||||||
).stdout.strip()
|
).stdout.strip()
|
||||||
|
|
||||||
|
|
||||||
|
SYSFS_NET = pathlib.Path("/sys/class/net")
|
||||||
|
|
||||||
|
|
||||||
|
def _ip_link_show(iface: str) -> str:
|
||||||
|
return _run(["ip", "link", "show", "dev", iface])
|
||||||
|
|
||||||
|
|
||||||
def iface_exists(iface: str) -> bool:
|
def iface_exists(iface: str) -> bool:
|
||||||
return pathlib.Path(f"/sys/class/net/{iface}").exists()
|
"""
|
||||||
|
True if the interface exists. Does not follow the /sys/class/net symlink;
|
||||||
|
falls back to netlink when sysfs is unavailable.
|
||||||
|
"""
|
||||||
|
if os.path.lexists(SYSFS_NET / iface):
|
||||||
|
return True
|
||||||
|
return bool(_ip_link_show(iface))
|
||||||
|
|
||||||
|
|
||||||
def list_ifaces() -> list[str]:
|
def list_ifaces() -> list[str]:
|
||||||
"""
|
"""
|
||||||
Return a sorted list of all network interfaces visible under /sys/class/net.
|
Return a sorted list of all network interfaces, netlink as fallback.
|
||||||
|
Plain files in /sys/class/net (e.g. bonding_masters) are not interfaces.
|
||||||
"""
|
"""
|
||||||
base = pathlib.Path("/sys/class/net")
|
try:
|
||||||
if not base.exists():
|
return sorted(
|
||||||
return []
|
p.name for p in SYSFS_NET.iterdir() if p.is_symlink() or p.is_dir()
|
||||||
names: list[str] = []
|
)
|
||||||
for p in base.iterdir():
|
except OSError:
|
||||||
if p.is_dir():
|
out = _run(["ip", "-o", "link", "show"])
|
||||||
names.append(p.name)
|
return sorted(re.findall(r"^\d+:\s+([^:@\s]+)", out, flags=re.MULTILINE))
|
||||||
names.sort()
|
|
||||||
return names
|
|
||||||
|
|
||||||
|
|
||||||
def read_iface_mtu(iface: str) -> int:
|
def read_iface_mtu(iface: str) -> int:
|
||||||
return int(pathlib.Path(f"/sys/class/net/{iface}/mtu").read_text().strip())
|
"""
|
||||||
|
Read the interface MTU from sysfs, falling back to netlink.
|
||||||
|
Raises RuntimeError if neither source reports an MTU.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
return int((SYSFS_NET / iface / "mtu").read_text().strip())
|
||||||
|
except (OSError, ValueError):
|
||||||
|
pass
|
||||||
|
|
||||||
|
m = re.search(r"\bmtu\s+(\d+)\b", _ip_link_show(iface))
|
||||||
|
if not m:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"Could not read MTU of {iface}: "
|
||||||
|
f"{SYSFS_NET / iface / 'mtu'} is unreadable and "
|
||||||
|
f"'ip link show dev {iface}' reported no MTU."
|
||||||
|
)
|
||||||
|
return int(m.group(1))
|
||||||
|
|
||||||
|
|
||||||
def set_iface_mtu(iface: str, mtu: int, dry: bool) -> None:
|
def set_iface_mtu(iface: str, mtu: int, dry: bool) -> None:
|
||||||
@@ -53,7 +80,7 @@ def require_root(*, dry: bool, needs_root: bool) -> None:
|
|||||||
raise SystemExit(1)
|
raise SystemExit(1)
|
||||||
|
|
||||||
|
|
||||||
def detect_egress_iface(ignore_vpn: bool = True) -> Optional[str]:
|
def detect_egress_iface(ignore_vpn: bool = True) -> str | None:
|
||||||
devs: list[str] = []
|
devs: list[str] = []
|
||||||
for cmd in (
|
for cmd in (
|
||||||
["ip", "-4", "route", "show", "default"],
|
["ip", "-4", "route", "show", "default"],
|
||||||
|
|||||||
@@ -1,12 +1,46 @@
|
|||||||
import io
|
import io
|
||||||
|
import pathlib
|
||||||
|
import tempfile
|
||||||
import unittest
|
import unittest
|
||||||
from contextlib import redirect_stdout
|
from contextlib import redirect_stdout
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
from automtu import net
|
||||||
from automtu.core import run_automtu
|
from automtu.core import run_automtu
|
||||||
|
|
||||||
|
|
||||||
|
def _args(**over) -> SimpleNamespace:
|
||||||
|
base = {
|
||||||
|
"dry_run": True,
|
||||||
|
"egress_if": None,
|
||||||
|
"prefer_wg_egress": False,
|
||||||
|
"force_egress_mtu": None,
|
||||||
|
"pmtu_target": None,
|
||||||
|
"auto_pmtu_from_wg": False,
|
||||||
|
"pmtu_min_payload": 1200,
|
||||||
|
"pmtu_max_payload": 1472,
|
||||||
|
"pmtu_timeout": 1.0,
|
||||||
|
"pmtu_policy": "min",
|
||||||
|
"apply_egress_mtu": False,
|
||||||
|
"apply_wg_mtu": False,
|
||||||
|
"apply_docker_mtu": False,
|
||||||
|
"apply_all": False,
|
||||||
|
"docker_if": None,
|
||||||
|
"docker_no_user_bridges": False,
|
||||||
|
"wg_if": "wg0",
|
||||||
|
"wg_overhead": 80,
|
||||||
|
"wg_min": 1280,
|
||||||
|
"set_wg_mtu": None,
|
||||||
|
"persist": None,
|
||||||
|
"uninstall": False,
|
||||||
|
"print_mtu": None,
|
||||||
|
"print_json": False,
|
||||||
|
}
|
||||||
|
base.update(over)
|
||||||
|
return SimpleNamespace(**base)
|
||||||
|
|
||||||
|
|
||||||
class TestCore(unittest.TestCase):
|
class TestCore(unittest.TestCase):
|
||||||
def test_run_automtu_happy_path_all_mocked(self) -> None:
|
def test_run_automtu_happy_path_all_mocked(self) -> None:
|
||||||
args = SimpleNamespace(
|
args = SimpleNamespace(
|
||||||
@@ -164,5 +198,53 @@ class TestCore(unittest.TestCase):
|
|||||||
mock_set.assert_not_called()
|
mock_set.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
class TestCoreInSysboxContainer(unittest.TestCase):
|
||||||
|
"""Container with dead /sys/class/net symlinks: routing and ip link work."""
|
||||||
|
|
||||||
|
def setUp(self) -> None:
|
||||||
|
tmp = tempfile.TemporaryDirectory()
|
||||||
|
self.addCleanup(tmp.cleanup)
|
||||||
|
root = pathlib.Path(tmp.name)
|
||||||
|
self.netdir = root / "class" / "net"
|
||||||
|
self.netdir.mkdir(parents=True)
|
||||||
|
for name in ("eth0", "lo"):
|
||||||
|
(self.netdir / name).symlink_to(root / "devices" / "virtual" / "net" / name)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _fake_run(cmd: list[str]) -> str:
|
||||||
|
if cmd[:5] == ["ip", "-4", "route", "show", "default"]:
|
||||||
|
return "default via 172.28.0.1 dev eth0"
|
||||||
|
if cmd[:3] == ["ip", "link", "show"] and cmd[-1] == "eth0":
|
||||||
|
return (
|
||||||
|
"2: eth0@if5: <BROADCAST,MULTICAST,UP> mtu 1450 qdisc noqueue state UP"
|
||||||
|
)
|
||||||
|
return ""
|
||||||
|
|
||||||
|
def test_print_mtu_effective_yields_number_and_rc0(self) -> None:
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", self.netdir),
|
||||||
|
patch.object(net, "_run", side_effect=self._fake_run),
|
||||||
|
patch("automtu.core.probe_pmtu", return_value=1400),
|
||||||
|
):
|
||||||
|
buf = io.StringIO()
|
||||||
|
with redirect_stdout(buf):
|
||||||
|
rc = run_automtu(_args(pmtu_target=["1.1.1.1"], print_mtu="effective"))
|
||||||
|
|
||||||
|
self.assertEqual(rc, 0)
|
||||||
|
self.assertEqual(int(buf.getvalue().strip()), 1400)
|
||||||
|
|
||||||
|
def test_explicit_egress_if_is_accepted(self) -> None:
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", self.netdir),
|
||||||
|
patch.object(net, "_run", side_effect=self._fake_run),
|
||||||
|
):
|
||||||
|
buf = io.StringIO()
|
||||||
|
with redirect_stdout(buf):
|
||||||
|
rc = run_automtu(_args(egress_if="eth0", print_mtu="egress"))
|
||||||
|
|
||||||
|
self.assertEqual(rc, 0)
|
||||||
|
self.assertEqual(int(buf.getvalue().strip()), 1450)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main(verbosity=2)
|
unittest.main(verbosity=2)
|
||||||
|
|||||||
@@ -1,8 +1,120 @@
|
|||||||
|
import pathlib
|
||||||
|
import tempfile
|
||||||
import unittest
|
import unittest
|
||||||
from pathlib import Path
|
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import automtu.net as net
|
from automtu import net
|
||||||
|
|
||||||
|
IP_LINK_ETH0 = (
|
||||||
|
"2: eth0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1420 qdisc noqueue state UP "
|
||||||
|
"mode DEFAULT group default\n link/ether 02:42:ac:1c:00:02 brd ff:ff:ff:ff:ff:ff"
|
||||||
|
)
|
||||||
|
IP_LINK_ALL = (
|
||||||
|
"1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN\n"
|
||||||
|
"2: eth0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1420 qdisc noqueue state UP"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _sysbox_sysfs(root: pathlib.Path, ifaces=("eth0", "lo")) -> pathlib.Path:
|
||||||
|
"""/sys replacement as seen inside a sysbox container: dead symlinks."""
|
||||||
|
netdir = root / "class" / "net"
|
||||||
|
netdir.mkdir(parents=True)
|
||||||
|
for name in ifaces:
|
||||||
|
(netdir / name).symlink_to(root / "devices" / "virtual" / "net" / name)
|
||||||
|
return netdir
|
||||||
|
|
||||||
|
|
||||||
|
class SysboxSysfsBase(unittest.TestCase):
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self._tmp = tempfile.TemporaryDirectory()
|
||||||
|
self.root = pathlib.Path(self._tmp.name)
|
||||||
|
self.addCleanup(self._tmp.cleanup)
|
||||||
|
|
||||||
|
|
||||||
|
class TestDeadSymlinkSysfs(SysboxSysfsBase):
|
||||||
|
def test_iface_exists_true_for_dead_symlink_without_netlink(self) -> None:
|
||||||
|
netdir = _sysbox_sysfs(self.root)
|
||||||
|
self.assertFalse((netdir / "eth0").exists())
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", netdir),
|
||||||
|
patch.object(net, "_run", return_value=""),
|
||||||
|
):
|
||||||
|
self.assertTrue(net.iface_exists("eth0"))
|
||||||
|
self.assertFalse(net.iface_exists("eth9"))
|
||||||
|
|
||||||
|
def test_iface_exists_falls_back_to_netlink_without_sysfs(self) -> None:
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", self.root / "absent" / "net"),
|
||||||
|
patch.object(net, "_run", return_value=IP_LINK_ETH0),
|
||||||
|
):
|
||||||
|
self.assertTrue(net.iface_exists("eth0"))
|
||||||
|
|
||||||
|
def test_detect_egress_iface_accepts_dead_symlink_iface(self) -> None:
|
||||||
|
netdir = _sysbox_sysfs(self.root)
|
||||||
|
|
||||||
|
def fake_run(cmd: list[str]) -> str:
|
||||||
|
if cmd[:5] == ["ip", "-4", "route", "show", "default"]:
|
||||||
|
return "default via 172.28.0.1 dev eth0"
|
||||||
|
return ""
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", netdir),
|
||||||
|
patch.object(net, "_run", side_effect=fake_run),
|
||||||
|
):
|
||||||
|
self.assertEqual(net.detect_egress_iface(), "eth0")
|
||||||
|
|
||||||
|
def test_list_ifaces_keeps_dead_symlinks_and_skips_plain_files(self) -> None:
|
||||||
|
netdir = _sysbox_sysfs(self.root, ("eth0", "lo", "br-abc123"))
|
||||||
|
(netdir / "bonding_masters").write_text("")
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", netdir),
|
||||||
|
patch.object(net, "_run", return_value=""),
|
||||||
|
):
|
||||||
|
self.assertEqual(net.list_ifaces(), ["br-abc123", "eth0", "lo"])
|
||||||
|
|
||||||
|
def test_list_ifaces_falls_back_to_netlink_without_sysfs(self) -> None:
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", self.root / "absent" / "net"),
|
||||||
|
patch.object(net, "_run", return_value=IP_LINK_ALL),
|
||||||
|
):
|
||||||
|
self.assertEqual(net.list_ifaces(), ["eth0", "lo"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestReadIfaceMtu(SysboxSysfsBase):
|
||||||
|
def test_prefers_sysfs_over_netlink(self) -> None:
|
||||||
|
netdir = self.root / "class" / "net"
|
||||||
|
(netdir / "eth0").mkdir(parents=True)
|
||||||
|
(netdir / "eth0" / "mtu").write_text("1500\n")
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", netdir),
|
||||||
|
patch.object(net, "_run", return_value=IP_LINK_ETH0) as run,
|
||||||
|
):
|
||||||
|
self.assertEqual(net.read_iface_mtu("eth0"), 1500)
|
||||||
|
run.assert_not_called()
|
||||||
|
|
||||||
|
def test_netlink_fallback_on_dead_symlink(self) -> None:
|
||||||
|
netdir = _sysbox_sysfs(self.root)
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", netdir),
|
||||||
|
patch.object(net, "_run", return_value=IP_LINK_ETH0),
|
||||||
|
):
|
||||||
|
self.assertEqual(net.read_iface_mtu("eth0"), 1420)
|
||||||
|
|
||||||
|
def test_raises_when_neither_sysfs_nor_netlink_answers(self) -> None:
|
||||||
|
netdir = _sysbox_sysfs(self.root)
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(net, "SYSFS_NET", netdir),
|
||||||
|
patch.object(net, "_run", return_value=""),
|
||||||
|
self.assertRaises(RuntimeError) as ctx,
|
||||||
|
):
|
||||||
|
net.read_iface_mtu("eth0")
|
||||||
|
|
||||||
|
self.assertIn("eth0", str(ctx.exception))
|
||||||
|
|
||||||
|
|
||||||
class TestNet(unittest.TestCase):
|
class TestNet(unittest.TestCase):
|
||||||
@@ -46,18 +158,10 @@ class TestNet(unittest.TestCase):
|
|||||||
self.assertFalse(net.default_route_uses_iface("wg0"))
|
self.assertFalse(net.default_route_uses_iface("wg0"))
|
||||||
|
|
||||||
def test_list_ifaces_returns_sorted_names(self) -> None:
|
def test_list_ifaces_returns_sorted_names(self) -> None:
|
||||||
fake = [
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
Path("/sys/class/net/eth0"),
|
netdir = _sysbox_sysfs(pathlib.Path(tmp), ("wg0", "eth0", "lo"))
|
||||||
Path("/sys/class/net/lo"),
|
with patch.object(net, "SYSFS_NET", netdir):
|
||||||
Path("/sys/class/net/wg0"),
|
self.assertEqual(net.list_ifaces(), ["eth0", "lo", "wg0"])
|
||||||
]
|
|
||||||
|
|
||||||
with (
|
|
||||||
patch("automtu.net.pathlib.Path.exists", return_value=True),
|
|
||||||
patch("automtu.net.pathlib.Path.iterdir", return_value=fake),
|
|
||||||
patch.object(Path, "is_dir", return_value=True),
|
|
||||||
):
|
|
||||||
self.assertEqual(net.list_ifaces(), ["eth0", "lo", "wg0"])
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user