[PATCH net-next] qmi_wwan: Add rawip module param

[PATCH net-next] qmi_wwan: Add rawip module param

[Stefan x Nilsson]

Certain QMI modems will start communicating in rawip mode after
bootup, and will not work properly if communication starts off in
ethernet mode. So add a module parameter, rawip_as_default, that
can be used to load the qmi driver in rawip mode.

The advantage compared to changing rawip at a later point using
sysfs is that the os will not detect the device and start talking
to it while the driver is still in incorrect mode.

Signed-off-by: Stefan x Nilsson <stefan.x.nilsson@axis.com>
---
 drivers/net/usb/qmi_wwan.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 344af3c5c836..968c60ececf8 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -46,6 +46,10 @@
  * commands on a serial interface
  */
 
+/* Module parameters */
+static bool rawip_as_default;
+module_param(rawip_as_default, bool, 0644);
+
 /* driver specific data */
 struct qmi_wwan_state {
 	struct usb_driver *subdriver;
@@ -843,6 +847,13 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 	}
 	dev->net->netdev_ops = &qmi_wwan_netdev_ops;
 	dev->net->sysfs_groups[0] = &qmi_wwan_sysfs_attr_group;
+
+	/* Set the driver into rawip mode if requested by module param */
+	if (rawip_as_default) {
+		info->flags |= QMI_WWAN_FLAG_RAWIP;
+		qmi_wwan_netdev_setup(dev->net);
+	}
+
 err:
 	return status;
 }

---
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
change-id: 20230828-qmiraw-b8bcbaed14ab

Best regards,
-- 
Stefan x Nilsson <stefan.x.nilsson@axis.com>

Re: [PATCH net-next] qmi_wwan: Add rawip module param

[Bjørn Mork]

Stefan x Nilsson <stefan.x.nilsson@axis.com> writes:

> Certain QMI modems will start communicating in rawip mode after
> bootup, and will not work properly if communication starts off in
> ethernet mode. So add a module parameter, rawip_as_default, that
> can be used to load the qmi driver in rawip mode.
>
> The advantage compared to changing rawip at a later point using
> sysfs is that the os will not detect the device and start talking
> to it while the driver is still in incorrect mode.
>
> Signed-off-by: Stefan x Nilsson <stefan.x.nilsson@axis.com>
> ---
>  drivers/net/usb/qmi_wwan.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 344af3c5c836..968c60ececf8 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -46,6 +46,10 @@
>   * commands on a serial interface
>   */
>  
> +/* Module parameters */
> +static bool rawip_as_default;
> +module_param(rawip_as_default, bool, 0644);
> +
>  /* driver specific data */
>  struct qmi_wwan_state {
>  	struct usb_driver *subdriver;
> @@ -843,6 +847,13 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>  	}
>  	dev->net->netdev_ops = &qmi_wwan_netdev_ops;
>  	dev->net->sysfs_groups[0] = &qmi_wwan_sysfs_attr_group;
> +
> +	/* Set the driver into rawip mode if requested by module param */
> +	if (rawip_as_default) {
> +		info->flags |= QMI_WWAN_FLAG_RAWIP;
> +		qmi_wwan_netdev_setup(dev->net);
> +	}
> +
>  err:
>  	return status;
>  }
>

NAK

There is no reason to start communicating with the device before
changing the framing, using the existing sysfs interface.

This seems to be a workaround for some userspace bug.  I don't think we
yet another userspace knob for that.  And certainly not one that applies
to every device.



Bjørn

Re: [PATCH net-next] qmi_wwan: Add rawip module param

[Stefan x Nilsson]

On 9/12/23 09:22, Bjørn Mork wrote:
> Stefan x Nilsson <stefan.x.nilsson@axis.com> writes:
> 
>> Certain QMI modems will start communicating in rawip mode after
>> bootup, and will not work properly if communication starts off in
>> ethernet mode. So add a module parameter, rawip_as_default, that
>> can be used to load the qmi driver in rawip mode.
>>
>> The advantage compared to changing rawip at a later point using
>> sysfs is that the os will not detect the device and start talking
>> to it while the driver is still in incorrect mode.
>>
>> Signed-off-by: Stefan x Nilsson <stefan.x.nilsson@axis.com>
>> ---
>>   drivers/net/usb/qmi_wwan.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
>> index 344af3c5c836..968c60ececf8 100644
>> --- a/drivers/net/usb/qmi_wwan.c
>> +++ b/drivers/net/usb/qmi_wwan.c
>> @@ -46,6 +46,10 @@
>>    * commands on a serial interface
>>    */
>>   
>> +/* Module parameters */
>> +static bool rawip_as_default;
>> +module_param(rawip_as_default, bool, 0644);
>> +
>>   /* driver specific data */
>>   struct qmi_wwan_state {
>>   	struct usb_driver *subdriver;
>> @@ -843,6 +847,13 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>>   	}
>>   	dev->net->netdev_ops = &qmi_wwan_netdev_ops;
>>   	dev->net->sysfs_groups[0] = &qmi_wwan_sysfs_attr_group;
>> +
>> +	/* Set the driver into rawip mode if requested by module param */
>> +	if (rawip_as_default) {
>> +		info->flags |= QMI_WWAN_FLAG_RAWIP;
>> +		qmi_wwan_netdev_setup(dev->net);
>> +	}
>> +
>>   err:
>>   	return status;
>>   }
>>
> 
> NAK
> 
> There is no reason to start communicating with the device before
> changing the framing, using the existing sysfs interface.
> 
> This seems to be a workaround for some userspace bug.  I don't think we
> yet another userspace knob for that.  And certainly not one that applies
> to every device.
> 
> 
> 
> Bjørn

Right, got the message and I see your point. Thanks for the review.

Would a quirk on the affected modem be an better/acceptable solution?

Best Regards
Stefan x Nilsson

Re: [PATCH net-next] qmi_wwan: Add rawip module param

[Bjørn Mork]

Stefan x Nilsson <stefan.x.nilsson@axis.com> writes:

> Would a quirk on the affected modem be an better/acceptable solution?

More acceptable but still needs justification.

I'm probably missing something, but I don't see the described issue even
if I assume a device crashing if it sees an ethernet header.

The netdev and associated chardev are created when a USB device is
connected.  The device knows nothing about this.  It answers a few USB
descriptor requests and that's it.  The host is free to switch the
netdev to raw-ip framing before bringing it up.  There is no need to
send any frames with eithernet header.  There is no relationship between
framing and the QMI control channel.  But the host is in complete
control of any communication there too, so it wouldn't be a problam if
there were.


Bjørn