Only the plural ATTRS form was checked but we also need to support
the single ATTR for matching device's attribute only.
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
The wwan subsystem is a new bus agnostic framework exposing wwan
device and its components (ports, netdev...). It can be useful to
get the wwan device a device belongs to
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
ModemManager[6954]: <warn> [1618260091.319919] [br0] invalid sysfs path read for net/br0
**
ERROR:kerneldevice/mm-kernel-device-generic.c:235:ptr_array_add_sysfs_attribute_link_basename: assertion failed: (array && sysfs_path && attribute)
Bail out! ERROR:kerneldevice/mm-kernel-device-generic.c:235:ptr_array_add_sysfs_attribute_link_basename: assertion failed: (array && sysfs_path && attribute)
Thread 1 "ModemManager" received signal SIGABRT, Aborted.
(gdb) bt
#0 0x00007ffff7438ef5 in raise () at /usr/lib/libc.so.6
#1 0x00007ffff7422862 in abort () at /usr/lib/libc.so.6
#2 0x00007ffff76c2084 in () at /usr/lib/libglib-2.0.so.0
#3 0x00007ffff771d85d in g_assertion_message_expr () at /usr/lib/libglib-2.0.so.0
#4 0x00005555556a5337 in ptr_array_add_sysfs_attribute_link_basename (array=0x5555557de520, sysfs_path=0x0, attribute=0x555555703bd8 "driver", out_value=0x0)
at kerneldevice/mm-kernel-device-generic.c:235
#5 0x00005555556a542b in preload_contents_other (self=0x5555557a6c60) at kerneldevice/mm-kernel-device-generic.c:255
#6 0x00005555556a6485 in preload_contents (self=0x5555557a6c60) at kerneldevice/mm-kernel-device-generic.c:517
#7 0x00005555556a7fb6 in check_preload (self=0x5555557a6c60) at kerneldevice/mm-kernel-device-generic.c:980
#8 0x00005555556a855f in initable_init (initable=0x5555557a6c60, cancellable=0x0, error=0x7fffffffe5c8) at kerneldevice/mm-kernel-device-generic.c:1127
#9 0x00007ffff78a2f41 in g_initable_new_valist () at /usr/lib/libgio-2.0.so.0
#10 0x00007ffff78a2ffa in g_initable_new () at /usr/lib/libgio-2.0.so.0
#11 0x00005555556a8205 in mm_kernel_device_generic_new_with_rules (props=0x5555557a5fa0, rules=0x7fffec001e30, error=0x7fffffffe5c8) at kerneldevice/mm-kernel-device-generic.c:1042
#12 0x00005555556a827d in mm_kernel_device_generic_new (props=0x5555557a5fa0, error=0x7fffffffe5c8) at kerneldevice/mm-kernel-device-generic.c:1063
#13 0x00005555555aba9c in handle_kernel_event (self=0x555555793220, properties=0x5555557a5fa0, error=0x7fffffffe5c8) at mm-base-manager.c:414
We already have methods to query for interface specific attributes
like class/subclass/protocol, so add a new one for the interface
number, and make sure we use ATTRS{bInterfaceNumber} to load it
always, instead of assuming the ID_USB_INTERFACE_NUM property is set.
In addition to loading port and device properties, we now also allow
loading sysfs properties that are assumed to be static (i.e. their
values won't change since loaded the first time).
Instead of the custom simple implementation which only supported the
'*' modifier in the pattern. This allows us to support e.g. attribute
value matches like e.g. 'DATA[0-9]*_CNTL'.
Until now we did not support ATTRS{} matches against attributes we
don't support in the core codebase. Implement now a simple lookup
mechanism which traverses the tree of sysfs path from the port sysfs
path to the physical device sysfs path, looking up the attribute
requested.
This is not completely equivalent to what udev does, because the udev
rules matching ATTRS would usually also include an additional previous
matching e.g. SUBSYSTEMS and such, so that the ATTRS is looked up
exactly in the device that also matches the additional previous
rules. In our case, we just simplify the logic and return the first
one found.
We were applying the DRIVERS check looking only at the single port
driver. Instead, we now lookup and cache all drivers found in the
device tree, and apply the loose DRIVERS check properly looking at all
of them.
We were applying the SUBSYSTEMS and SUBSYSTEM check looking at the
sysfs path and just hoping the subsystem we're looking for is included
in the path itself. Instead, we now lookup and cache all susystems
found in the device tree, and apply the loose SUBSYSTEMS check
properly looking at all of them.
E.g. we can now match SUBSYSTEMS="mhi_q" in the following device tree,
without needing it to be found in the sysfs path:
looking at device '/devices/pci0000:00/0000:00:1b.0/0000:01:00.0/1001_00.01.00_MBIM/mhi_uci_q/mhi_MBIM':
SUBSYSTEM=="mhi_uci_q"
looking at parent device '/devices/pci0000:00/0000:00:1b.0/0000:01:00.0/1001_00.01.00_MBIM':
SUBSYSTEMS=="mhi_q"
looking at parent device '/devices/pci0000:00/0000:00:1b.0/0000:01:00.0':
SUBSYSTEMS=="pci"
The generic backend implementation was really based on detecting USB
devices, not so much devices in other subsystems. This patch puts the
generic backend at the same level as the udev backend to support
non-USB modems.
We can just subclass the methods to check whether a given property
exists and to get it as a string, and then implement in the generic
class the actual boolean/int/hex type getters common for all.
kerneldevice/mm-kernel-device-generic.c: In function ‘mm_kernel_device_generic_new_with_rules’:
kerneldevice/mm-kernel-device-generic.c:954:68: error: declaration of ‘properties’ shadows a global declaration [-Werror=shadow]
954 | mm_kernel_device_generic_new_with_rules (MMKernelEventProperties *properties,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c:46:20: note: shadowed declaration is here
46 | static GParamSpec *properties[PROP_LAST];
| ^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c: In function ‘mm_kernel_device_generic_new’:
kerneldevice/mm-kernel-device-generic.c:971:57: error: declaration of ‘properties’ shadows a global declaration [-Werror=shadow]
971 | mm_kernel_device_generic_new (MMKernelEventProperties *properties,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c:46:20: note: shadowed declaration is here
46 | static GParamSpec *properties[PROP_LAST];
| ^~~~~~~~~~
This prevents errors due to nasty typos in the strings.
We define all symbols in a single header file that is NOT considered
part of the API, as there is no need for MM clients to know about
these tags code-wise. These tags are only meaningful when associated
to devices in udev.
Information of each tag is included in the general API documentation.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/issues/88
This patch implicitly enables in the generic device backend the
manual-only greylist (ID_MM_DEVICE_MANUAL_SCAN_ONLY tag) and the
platform TTY whitelist (ID_MM_PLATFORM_DRIVER_PROBE), which were not
being applied.
A USB device may report its manufacturer through the Manufacturer String
Descriptor. That information can sometimes be useful, e.g. MBIM doesn't
define a standard command to query the manufacturer. This patch adds a
method to MMKernelDevice to retrieve the manufacturer information from
the underlying physical device, which can later be used by
MMBroadbandModemMbim to report the manufacturer of a MBIM modem.
==5075== 198 bytes in 18 blocks are definitely lost in loss record 4,486 of 4,761
==5075== at 0x4C2CE5F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5075== by 0x66E3028: g_malloc (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x66CAD42: g_path_get_basename (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x20BB7B: preload_interface_sysfs_path (mm-kernel-device-generic.c:184)
==5075== by 0x20C885: preload_contents (mm-kernel-device-generic.c:383)
==5075== by 0x20E2E1: check_preload (mm-kernel-device-generic.c:830)
==5075== by 0x20EA6F: set_property (mm-kernel-device-generic.c:953)
==5075== by 0x6455359: ??? (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6456DBC: g_object_new_valist (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6105E95: g_initable_new_valist (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x6105F58: g_initable_new (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x20E848: mm_kernel_device_generic_new_with_rules (mm-kernel-device-generic.c:901)
==5075== 333 bytes in 37 blocks are definitely lost in loss record 4,612 of 4,761
==5075== at 0x4C2CE5F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5075== by 0x66E3028: g_malloc (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x66CAD42: g_path_get_basename (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x20BE20: preload_driver (mm-kernel-device-generic.c:242)
==5075== by 0x20C8E5: preload_contents (mm-kernel-device-generic.c:391)
==5075== by 0x20E2E1: check_preload (mm-kernel-device-generic.c:830)
==5075== by 0x20EA6F: set_property (mm-kernel-device-generic.c:953)
==5075== by 0x6455359: ??? (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6456DBC: g_object_new_valist (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6105E95: g_initable_new_valist (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x6105F58: g_initable_new (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x20E848: mm_kernel_device_generic_new_with_rules (mm-kernel-device-generic.c:901)
There are 2 main types of udev properties: device-specific and
port-specific.
The port-specific properties are set independently per port (e.g. port
type hints set per interface number for a given vid:pid).
The device-specific properties apply to all ports in the device. Some
of these properties are currently expected in the physical device
(e.g. ID_MM_PLATFORM_DRIVER_PROBE) while some others are expected in
each port (e.g. the plugin udev tag filters).
This patch tries to simplify the logic and just assume that the device
specific tags may be given in either the physical device or the port
device, by providing separate APIs to retrieve port-specific or
device-specific (global) properties. If the same tag is given in both
the device and the port, the one in the device takes preference.
For the generic backend, these new APIs are really useless, as all
device-specific and port-specific properties are always stored in the
port object themselves (there is no 'tree' of devices in the generic
backend, no 'physdev' device).
For the udev backend, though, there really is a difference, as the
tags may be set in port or device.
https://bugs.freedesktop.org/show_bug.cgi?id=100156
This is so that "make check" doesn't depend on having the rule files
installed in the default udev rules directory, which currently would
break the build:
TEST: test-service-generic... (pid=2601)
/MM/Service/Generic/enable-disable: Activating service name='org.freedesktop.ModemManager1'
Successfully activated service 'org.freedesktop.ModemManager1'
** (/home/aleksander/Development/foss/ModemManager/plugins/.libs/lt-test-service-generic:2601):
ERROR **: Error setting test profile: GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Failed: Failed to find primary AT port
FAIL
GTester: last random seed: R02S3897abaae9df36f8d2eeb679406ab675
make[3]: *** [Makefile:3804: test-nonrecursive] Terminated
The original g_udev_device_get_property_as_int() uses strtol() without
an explicit base (i.e. 0) so that the base is autodetected from the
string whenever possible (e.g. if prefixes with '0x' it is treated as a
hexadecimal string).
But, for ID_USB_INTERFACE_NUM, we explicitly require reading the number
as an hex string, even if we don't have any '0x' prefix.
Reported-by: Matthew Stanger <stangerm2@gmail.com>
We need at least ID_USB_INTERFACE_NUM, as some plugins (Huawei, Telit, Option)
require this property when gathering port type hints.
We also add support for other properties (ID_VENDOR_ID, ID_MODEL_ID...) as we
already have those values preloaded.
Instead of relying on the udev daemon and GUDev to manage the devices reported
by the kernel, we can now run ModemManager relying solely on the kernel events
reported via the new ReportKernelEvent() API. Therefore, the '--no-auto-scan'
option is implicit for the ModemManager daemon when udev is disabled in the
build.
Additionally, a new custom implementation of the kernel device object is
provided, which uses sysfs to load the properties and attributes required in
each kernel device, instead of using a GUdevDevice.
The udev rule files are kept in place, and a simple custom parser is provided
which preloads all rules in memory once and then applies them to the different
kernel objects reported via ReportKernelEvent(), e.g. to set port type hints.
A simple unit test setup is prepared to validate the udev rules during the
`check' Makefile target.
The mm_base_modem_grab_port() now receives a MMKernelDevice directly from the
plugin, which is then stored in the MMPort corresponding to the port.
This means that we have direct access to e.g. all properties set by udev rules
everywhere, and we don't need additional GUdevClient objects (e.g. like the one
used in the Huawei plugin to detect NDISDUP support during runtime).
For virtual ports (e.g. generated during unit tests), we have a new 'generic'
kernel device object which just provides the values from the kernel device
properties given during its creation.
Loic Poulain
Aleksander Morgado
Baruch Siach
Ben Chan