From a272bffcf16c6c5ced83fb19a35aa3c78bbf7bff Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 29 Sep 2026 13:41:40 +0000 Subject: wwan: T7487: identify modem by port ownership, not interface number modem_index() in python/vyos/utils/wwan.py derived the ModemManager modem index straight from the interface name (wwan0 -> 0). The kernel-assigned WWAN interface number and ModemManager's own modem index are two independently enumerated values and are not guaranteed to match - on a box with more than one modem, they can enumerate in different orders, so modem index 0 does not necessarily own wwan0. Every function in this module funnels through modem_index() (start_modem_manager, modem_disconnect, modem_connect, and both admin-disconnect marker helpers), and is_wwan_connected() in vyos.utils.network had its own separate copy of the same assumption - so a single reversed enumeration order silently commands the wrong physical modem for every one of them. modem_index() now lists every modem ModemManager currently knows about and returns whichever one actually has ifname in its own port list, instead of assuming the correspondence. A modem that has not shown up on the bus yet looks the same as one that will never exist, so this waits up to the same modem_wait_timeout budget start_modem_manager() already gives a modem to become dialable, then returns None if nothing claimed the interface. is_wwan_connected() now calls the same resolver instead of keeping its own copy of the old assumption. None is a real, expected return now, not just a theoretical one - the three callers that previously assumed modem_index() always produced *a* string (right or wrong) are updated: start_modem_manager() bails out early (matching the tolerance its own caller's w.exists() check already has for hardware that isn't detected yet), modem_disconnect() and modem_connect() treat it as nothing to disconnect / a failed dial, matching each function's own existing return contract. show_wwan.py's modem_info() raises a clear, specific error instead, which its own top-level exception handler already reports cleanly. set_admin_disconnected()/clear_admin_disconnected() only ever used modem_index() to validate the interface name looks like a WWAN interface in the first place, so they're unaffected. --- python/vyos/utils/network.py | 5 +++- python/vyos/utils/wwan.py | 65 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 5 deletions(-) (limited to 'python') diff --git a/python/vyos/utils/network.py b/python/vyos/utils/network.py index cb5863e51..93ed05f6b 100644 --- a/python/vyos/utils/network.py +++ b/python/vyos/utils/network.py @@ -290,6 +290,7 @@ def is_wwan_connected(interface): carrier network or not """ from vyos.utils.dict import dict_search from vyos.utils.process import is_systemd_service_active + from vyos.utils.wwan import modem_index if not interface.startswith('wwan'): raise ValueError(f'Specified interface "{interface}" is not a WWAN interface') @@ -299,7 +300,9 @@ def is_wwan_connected(interface): if not is_systemd_service_active('ModemManager.service'): return False - modem = interface.lstrip('wwan') + modem = modem_index(interface) + if modem is None: + return False try: tmp = cmdl(['mmcli', '--modem', modem, '--output-json']) diff --git a/python/vyos/utils/wwan.py b/python/vyos/utils/wwan.py index d8da34bfa..6b07e90bb 100644 --- a/python/vyos/utils/wwan.py +++ b/python/vyos/utils/wwan.py @@ -25,9 +25,11 @@ would have done. import os +from json import loads from time import sleep from vyos.utils.dict import dict_search +from vyos.utils.misc import wait_for from vyos.utils.process import call from vyos.utils.process import cmdl from vyos.utils.process import DEVNULL @@ -52,11 +54,54 @@ modem_wait_poll = 0.250 # survive a reboot, and a commit re-asserts the configured state too. admin_disconnect_dir = '/run/vyos-wwan' -def modem_index(ifname: str) -> str: - """ Return the ModemManager modem index backing an interface, wwan0 -> 0 """ +def _modem_owning_port(ifname: str): + """ Return the ModemManager modem index whose own port list includes + ifname, or None if no currently-known modem owns it. """ + try: + modem_list = loads(cmdl(['mmcli', '--list-modems', '--output-json'])).get('modem-list', []) + except OSError: + return None + + for modem_path in modem_list: + idx = modem_path.rsplit('/', 1)[-1] + try: + detail = loads(cmdl(['mmcli', '--modem', idx, '--output-json'])) + except OSError: + continue + ports = dict_search('modem.generic.ports', detail) or [] + if any(port.split(' ')[0] == ifname for port in ports): + return idx + + return None + +def modem_index(ifname: str): + """ Return the ModemManager modem index backing an interface, or None if + no modem currently owns it. + + The kernel-assigned WWAN interface number and ModemManager's own modem + index are independently enumerated and are not guaranteed to match - on + a box with more than one modem, wwan0 does not necessarily belong to + modem 0 (T7487). Resolved by real port ownership instead: every modem + ModemManager currently knows about is listed, and whichever one + actually has ifname among its own ports is returned. + + A modem that has not shown up on the bus yet looks the same here as one + that will never exist, so this waits up to modem_wait_timeout for it to + appear before giving up - the same budget start_modem_manager already + gives a modem to become dialable, since a modem slow enough to need + that budget is also slow enough to not be listed yet when this is first + called. """ if not ifname.startswith('wwan'): raise ValueError(f'Specified interface "{ifname}" is not a WWAN interface') - return ifname[len('wwan'):] + + index = None + def _resolve(): + nonlocal index + index = _modem_owning_port(ifname) + return index is not None + wait_for(_resolve, interval=modem_wait_poll, timeout=modem_wait_timeout) + + return index def modem_state(index: str) -> str: """ ModemManager state of a modem, empty if it cannot be read yet """ @@ -84,6 +129,13 @@ def start_modem_manager(ifname: str) -> None: cmdl(['systemctl', 'start', service_name]) index = modem_index(ifname) + if index is None: + # No modem currently owns ifname at all - nothing more this can do. + # Matches the pre-existing tolerance for hardware that isn't detected + # yet: interfaces_wwan.py's own w.exists() check right after this + # call already bails out the same way. + return None + counter = int(modem_wait_timeout / modem_wait_poll) while counter > 0: counter -= 1 @@ -150,13 +202,18 @@ def modem_disconnect(ifname: str, quiet: bool = False) -> None: """ Disconnect every bearer of the modem backing ifname. The number of bearers a modem can hold is limited, so we always disconnect before we dial again. """ - call(f'mmcli --modem {modem_index(ifname)} --simple-disconnect', + modem = modem_index(ifname) + if modem is None: + return None + call(f'mmcli --modem {modem} --simple-disconnect', stdout=DEVNULL if quiet else None, stderr=DEVNULL if quiet else None) def modem_connect(ifname: str, options: str) -> bool: """ Dial the modem backing ifname, returns True if the modem connected """ modem = modem_index(ifname) + if modem is None: + return False # Some networks only ever admit a single combined IPv4+IPv6 PDN context per # APN and reject a standalone IPv6 "Start Network" request outright (QMI -- cgit v1.2.3 From 90887491e9c0d122c8760195f37e01b1e7e8cdcc Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 29 Sep 2026 14:17:42 +0000 Subject: wwan: T7487: only wait for a modem in start_modem_manager() modem_index() unconditionally polled for up to modem_wait_timeout before returning None, so every caller that expects "no modem" as a routine outcome - interface teardown, admin-disconnect markers, disconnect/connect status checks, and vyos-netlinkd's reconcile loop checking every configured interface once per pass - blocked on it too. The reconcile loop in particular calls is_wwan_connected() synchronously, not per-interface, so one interface with no modem stalled the whole daemon's reconcile pass, including unrelated DHCP interfaces, every cycle. Add a wait parameter, default False (single check, returns immediately). Only start_modem_manager() passes wait=True, where waiting for a still-initializing modem is the actual point. --- python/vyos/utils/wwan.py | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) (limited to 'python') diff --git a/python/vyos/utils/wwan.py b/python/vyos/utils/wwan.py index 6b07e90bb..6b708e39c 100644 --- a/python/vyos/utils/wwan.py +++ b/python/vyos/utils/wwan.py @@ -54,11 +54,14 @@ modem_wait_poll = 0.250 # survive a reboot, and a commit re-asserts the configured state too. admin_disconnect_dir = '/run/vyos-wwan' + def _modem_owning_port(ifname: str): - """ Return the ModemManager modem index whose own port list includes - ifname, or None if no currently-known modem owns it. """ + """Return the ModemManager modem index whose own port list includes + ifname, or None if no currently-known modem owns it.""" try: - modem_list = loads(cmdl(['mmcli', '--list-modems', '--output-json'])).get('modem-list', []) + modem_list = loads(cmdl(['mmcli', '--list-modems', '--output-json'])).get( + 'modem-list', [] + ) except OSError: return None @@ -74,8 +77,9 @@ def _modem_owning_port(ifname: str): return None -def modem_index(ifname: str): - """ Return the ModemManager modem index backing an interface, or None if + +def modem_index(ifname: str, wait: bool = False): + """Return the ModemManager modem index backing an interface, or None if no modem currently owns it. The kernel-assigned WWAN interface number and ModemManager's own modem @@ -85,20 +89,24 @@ def modem_index(ifname: str): ModemManager currently knows about is listed, and whichever one actually has ifname among its own ports is returned. - A modem that has not shown up on the bus yet looks the same here as one - that will never exist, so this waits up to modem_wait_timeout for it to - appear before giving up - the same budget start_modem_manager already - gives a modem to become dialable, since a modem slow enough to need - that budget is also slow enough to not be listed yet when this is first - called. """ + "No modem owns this yet" is a routine result for most callers (deleted + interfaces, admin-disconnect markers, vyos-netlinkd's reconcile loop), so + by default this checks once and returns immediately. Only + start_modem_manager() passes wait=True, to give a modem that is still + enumerating up to modem_wait_timeout to show up.""" if not ifname.startswith('wwan'): raise ValueError(f'Specified interface "{ifname}" is not a WWAN interface') + if not wait: + return _modem_owning_port(ifname) + index = None + def _resolve(): nonlocal index index = _modem_owning_port(ifname) return index is not None + wait_for(_resolve, interval=modem_wait_poll, timeout=modem_wait_timeout) return index @@ -128,7 +136,7 @@ def start_modem_manager(ifname: str) -> None: if not is_systemd_service_active(service_name): cmdl(['systemctl', 'start', service_name]) - index = modem_index(ifname) + index = modem_index(ifname, wait=True) if index is None: # No modem currently owns ifname at all - nothing more this can do. # Matches the pre-existing tolerance for hardware that isn't detected @@ -205,9 +213,12 @@ def modem_disconnect(ifname: str, quiet: bool = False) -> None: modem = modem_index(ifname) if modem is None: return None - call(f'mmcli --modem {modem} --simple-disconnect', - stdout=DEVNULL if quiet else None, - stderr=DEVNULL if quiet else None) + call( + f'mmcli --modem {modem} --simple-disconnect', + stdout=DEVNULL if quiet else None, + stderr=DEVNULL if quiet else None, + ) + def modem_connect(ifname: str, options: str) -> bool: """ Dial the modem backing ifname, returns True if the modem connected """ -- cgit v1.2.3 From 002050d158356078931d1a196538b5803eab788e Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 29 Sep 2026 19:21:15 +0000 Subject: wwan: T7487: move json/wait_for imports into their only callers Requested by c-po: json.loads and wait_for were only used by _modem_owning_port() and modem_index()'s wait branch respectively, so importing them at module level cost every other caller of this module an import it does not need. --- python/vyos/utils/wwan.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'python') diff --git a/python/vyos/utils/wwan.py b/python/vyos/utils/wwan.py index 6b708e39c..af719571d 100644 --- a/python/vyos/utils/wwan.py +++ b/python/vyos/utils/wwan.py @@ -25,11 +25,9 @@ would have done. import os -from json import loads from time import sleep from vyos.utils.dict import dict_search -from vyos.utils.misc import wait_for from vyos.utils.process import call from vyos.utils.process import cmdl from vyos.utils.process import DEVNULL @@ -58,6 +56,8 @@ admin_disconnect_dir = '/run/vyos-wwan' def _modem_owning_port(ifname: str): """Return the ModemManager modem index whose own port list includes ifname, or None if no currently-known modem owns it.""" + from json import loads + try: modem_list = loads(cmdl(['mmcli', '--list-modems', '--output-json'])).get( 'modem-list', [] @@ -100,6 +100,8 @@ def modem_index(ifname: str, wait: bool = False): if not wait: return _modem_owning_port(ifname) + from vyos.utils.misc import wait_for + index = None def _resolve(): -- cgit v1.2.3 From 1aa2cd1df614925cf68964239eeaaf42ea74bc43 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 29 Sep 2026 19:30:18 +0000 Subject: wwan: T7487: catch malformed mmcli JSON, not just OSError mmcli can return empty or non-JSON output while ModemManager is still starting (T6604), and json.loads() raising on that is a ValueError, not an OSError - it was propagating straight out of _modem_owning_port() uncaught. Under wait=True that reaches wait_for()'s callback and aborts the poll loop instead of retrying, exactly the startup race this module exists to ride out. Co-authored-by: Christian Breunig --- python/vyos/utils/wwan.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'python') diff --git a/python/vyos/utils/wwan.py b/python/vyos/utils/wwan.py index af719571d..8ee6a5a9a 100644 --- a/python/vyos/utils/wwan.py +++ b/python/vyos/utils/wwan.py @@ -59,17 +59,25 @@ def _modem_owning_port(ifname: str): from json import loads try: - modem_list = loads(cmdl(['mmcli', '--list-modems', '--output-json'])).get( - 'modem-list', [] - ) - except OSError: + data = loads(cmdl(['mmcli', '--list-modems', '--output-json'])) + except (OSError, ValueError): + # ValueError also catches json.JSONDecodeError - mmcli can return + # empty or malformed output while ModemManager is still starting + # (T6604), and that must retry under wait=True, not abort it. + return None + + # Well-formed JSON of an unexpected shape is treated like no modems at all + modem_list = [] + if isinstance(data, dict) and 'modem-list' in data: + modem_list = data['modem-list'] + if not isinstance(modem_list, list): return None for modem_path in modem_list: idx = modem_path.rsplit('/', 1)[-1] try: detail = loads(cmdl(['mmcli', '--modem', idx, '--output-json'])) - except OSError: + except (OSError, ValueError): continue ports = dict_search('modem.generic.ports', detail) or [] if any(port.split(' ')[0] == ifname for port in ports): -- cgit v1.2.3