netdev - Re: [RFC PATCH 4/6] net: wwan: add NMEA port support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8dcfecf3-f9ce-402b-8589-c1c1c770990b@gmail.com>
Date: Sat, 19 Apr 2025 02:20:25 +0300
From: Sergey Ryazanov <ryazanov.s.a@...il.com>
To: Loic Poulain <loic.poulain@....qualcomm.com>
Cc: Johannes Berg <johannes@...solutions.net>,
 Andrew Lunn <andrew+netdev@...n.ch>, Eric Dumazet <edumazet@...gle.com>,
 "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 Slark Xiao <slark_xiao@....com>, Muhammad Nuzaihan <zaihan@...ealasia.net>,
 Qiang Yu <quic_qianyu@...cinc.com>,
 Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 Johan Hovold <johan@...nel.org>
Subject: Re: [RFC PATCH 4/6] net: wwan: add NMEA port support

On 16.04.2025 23:04, Loic Poulain wrote:
> On Wed, Apr 9, 2025 at 1:31 AM Sergey Ryazanov <ryazanov.s.a@...il.com> wrote:
>> Many WWAN modems come with embedded GNSS receiver inside and have a
>> dedicated port to output geopositioning data. On the one hand, the
>> GNSS receiver has little in common with WWAN modem and just shares a
>> host interface and should be exported using the GNSS subsystem. On the
>> other hand, GNSS receiver is not automatically activated and needs a
>> generic WWAN control port (AT, MBIM, etc.) to be turned on. And a user
>> space software needs extra information to find the control port.
>>
>> Introduce the new type of WWAN port - NMEA. When driver asks to register
>> a NMEA port, the core allocates common parent WWAN device as usual, but
>> exports the NMEA port via the GNSS subsystem and acts as a proxy between
>> the device driver and the GNSS subsystem.
>>
>>  From the WWAN device driver perspective, a NMEA port is registered as a
>> regular WWAN port without any difference. And the driver interacts only
>> with the WWAN core. From the user space perspective, the NMEA port is a
>> GNSS device which parent can be used to enumerate and select the proper
>> control port for the GNSS receiver management.
>>
>> CC: Slark Xiao <slark_xiao@....com>
>> CC: Muhammad Nuzaihan <zaihan@...ealasia.net>
>> CC: Qiang Yu <quic_qianyu@...cinc.com>
>> CC: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
>> CC: Johan Hovold <johan@...nel.org>
>> Suggested-by: Loic Poulain <loic.poulain@....qualcomm.com>
>> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@...il.com>
>> ---
>>   drivers/net/wwan/Kconfig     |   1 +
>>   drivers/net/wwan/wwan_core.c | 157 ++++++++++++++++++++++++++++++++++-
>>   include/linux/wwan.h         |   2 +
>>   3 files changed, 156 insertions(+), 4 deletions(-)
>>
> [...]
>> +static void wwan_port_unregister_gnss(struct wwan_port *port)
>> +{
>> +       struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
>> +       struct gnss_device *gdev = port->gnss;
>> +
>> +       dev_info(&wwandev->dev, "port %s disconnected\n", dev_name(&gdev->dev));
>> +
>> +       gnss_deregister_device(gdev);
>> +       gnss_put_device(gdev);
>> +
>> +       __wwan_port_destroy(port);
>> +}
>> +#else
>> +static inline int wwan_port_register_gnss(struct wwan_port *port)
>> +{
>> +       __wwan_port_destroy(port);
>> +       return -EOPNOTSUPP;
>> +}
> 
> I don't think the wwan core should return an error in case GNSS_CONFIG
> is not enabled, a wwan driver may consider aborting the full
> probing/registration if one of the port registrations is failing.
> Maybe we should silently ignore such ports, and/or simply print a
> warning.

Good point. We can end up with the case of driver aborting the whole 
registration.

On the other hand, a driver author is supposed to know that for the GNSS 
functionality the corresponding subsystem should be enabled. And either 
don't even try to register the GNSS (NMEA) port if the system is 
disabled or handle the error properly. See for example, the Intel's NIC 
driver, which skips an embedded GNSS receiver exporting if the GNSS 
subsystem is disabled. We can catch such issues on the review stage or 
treat like bug if something like this going to sneak into the code.

So here, the WWAN core does its best to kindly inform that functionality 
is not enabled. This is what many other components do, and I personally 
like this approach.

Is it any good justification to clearly indicate the error in case of 
GNSS subsystem is not enabled?

--
Sergey