Re: commit 662dc80a5e86 breaks rmnet over usb

[Daniele Palmas]

Il giorno gio 26 feb 2026 alle ore 10:09 Laurent Vivier
<lvivier@xxxxxxxxxx> ha scritto:
>
> On 2/26/26 09:26, Daniele Palmas wrote:
> > Hello Jakub,
> >
> > Il giorno gio 26 feb 2026 alle ore 02:01 Jakub Kicinski
> > <kuba@xxxxxxxxxx> 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.

I think I can test this by the end of the week and let you know.

Thanks,
Daniele

>
> 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 *);
>
>