Re: commit 662dc80a5e86 breaks rmnet over usb

[Laurent Vivier <lvivier@redhat.com>]

On 2/26/26 09:26, Daniele Palmas wrote:
> Hello Jakub,
> 
> Il giorno gio 26 feb 2026 alle ore 02:01 Jakub Kicinski
> <kuba@kernel.org> ha scritto:
>>
>> On Wed, 25 Feb 2026 08:19:46 +0100 Daniele Palmas wrote:
>>>> Could you try something like:
>>>>
>>>> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
>>>> index 3a4985b582cb..6b4796fac692 100644
>>>> --- a/drivers/net/usb/qmi_wwan.c
>>>> +++ b/drivers/net/usb/qmi_wwan.c
>>>> @@ -788,6 +788,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>>>>                   usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress);
>>>>           }
>>>>
>>>> +       dev->rx_urb_size = 32768;
>>>> +
>>>
>>> So far userspace tools (e.g. also the most important one which is
>>> ModemManager) rely on changing the rx_urb_size by changing the MTU: I
>>> know this is ugly, but it is a behavior that has been there since a
>>> lot of time, not sure how many tools based on this assumption could
>>> break.
>>
>> What's the policy in ModemManager to change the rx_urb_size?
>> Increase it to make sure it can hold some specific cmd / packet?
>>
>> I wonder if having rx_urb_size max of (mtu, 32k) would break anything.
>>
> 
> Typically the host sends a QMI request to the modem for setting the
> size of the maximum QMAP aggregated packets block. Then the modem
> replies with the maximum size it supports and that one is then set by
> the host through changing the MTU of the netdevice. Low-cat modems
> usually support 4-8 KB, while 5G 16-32KB.
> 
> On ModemManager side this is currently fixed at 16KB, but one can use
> other tools e.g. qmicli to set custom values as far as they are
> supported by the modem.
> 
>> Since we're talking about rx buffer config the right API to configure
>> it is likely ethtool -G rx-buf-len :(
>>
> 
> Thanks for the hint, I'll try to have a look at that to improve qmi_wwan.
> 
>>> There's also the chance that there are modems which require a higher
>>> rx_urb_size, so having this fixed could not work well.
>>>
>>> Unfortunately usbnet serves many drivers, I agree with Koen that a
>>> revert is the safest option.
>>
>> Then again the usbnet driver is brittle enough as is, if it's just qmi
>> that needs this workaround we would be making common code less robust
>> for a benefit of a single "hack" (for lack of a better term)
> 
> Makes sense, also Laurent proposed a solution to keep his fix in
> usbnet and make qmi_wwan the exception.

I was thinking to something like that (see below), but I'm not really able to test it. If 
everyone thinks it's the path to follow, I can send a patch.

Thanks,
Laurent

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 3a4985b582cb..05acac10cd2b 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -928,7 +928,7 @@ static int qmi_wwan_resume(struct usb_interface *intf)

  static const struct driver_info	qmi_wwan_info = {
  	.description	= "WWAN/QMI device",
-	.flags		= FLAG_WWAN | FLAG_SEND_ZLP,
+	.flags		= FLAG_WWAN | FLAG_NOMAXMTU | FLAG_SEND_ZLP,
  	.bind		= qmi_wwan_bind,
  	.unbind		= qmi_wwan_unbind,
  	.manage_power	= qmi_wwan_manage_power,
@@ -937,7 +937,7 @@ static const struct driver_info	qmi_wwan_info = {

  static const struct driver_info	qmi_wwan_info_quirk_dtr = {
  	.description	= "WWAN/QMI device",
-	.flags		= FLAG_WWAN | FLAG_SEND_ZLP,
+	.flags		= FLAG_WWAN | FLAG_NOMAXMTU | FLAG_SEND_ZLP,
  	.bind		= qmi_wwan_bind,
  	.unbind		= qmi_wwan_unbind,
  	.manage_power	= qmi_wwan_manage_power,
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ed86ba87ca4e..b72ba0803392 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1829,11 +1829,12 @@ usbnet_probe(struct usb_interface *udev, const struct 
usb_device_id *prod)
  		if ((dev->driver_info->flags & FLAG_NOARP) != 0)
  			net->flags |= IFF_NOARP;

-		if (net->max_mtu > (dev->hard_mtu - net->hard_header_len))
+		if ((dev->driver_info->flags & FLAG_NOMAXMTU) == 0 &&
+		    net->max_mtu > (dev->hard_mtu - net->hard_header_len))
  			net->max_mtu = dev->hard_mtu - net->hard_header_len;

-		if (net->mtu > net->max_mtu)
-			net->mtu = net->max_mtu;
+		if (net->mtu > (dev->hard_mtu - net->hard_header_len))
+			net->mtu = dev->hard_mtu - net->hard_header_len;

  	} else if (!info->in || !info->out)
  		status = usbnet_get_endpoints(dev, udev);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index b0e84896e6ac..bbf799ccf3b3 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -132,6 +132,7 @@ struct driver_info {
  #define FLAG_MULTI_PACKET	0x2000
  #define FLAG_RX_ASSEMBLE	0x4000	/* rx packets may span >1 frames */
  #define FLAG_NOARP		0x8000	/* device can't do ARP */
+#define FLAG_NOMAXMTU		0x10000	/* allow max_mtu above hard_mtu */

  	/* init device ... can sleep, or cause probe() failure */
  	int	(*bind)(struct usbnet *, struct usb_interface *);