netdev - Re: [PATCH net-next 1/1] net: usb: qmi_wwan: add default rx_urb

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAGRyCJFuMv5PmLC2iUGOgbBufWUKhmVoYgrK_hXTgqmj1P1Yjw@mail.gmail.com>
Date:   Wed, 9 Sep 2020 13:57:58 +0200
From:   Daniele Palmas <dnlplm@...il.com>
To:     Carl Yin(殷张成) <carl.yin@...ctel.com>
Cc:     Bjørn Mork <bjorn@...k.no>,
        Kristian Evensen <kristian.evensen@...il.com>,
        Paul Gildea <paul.gildea@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH net-next 1/1] net: usb: qmi_wwan: add default rx_urb_size

Hi Carl,

Il giorno mer 9 set 2020 alle ore 13:09 Carl Yin(殷张成)
<carl.yin@...ctel.com> ha scritto:
>
> Hi Deniele:
>
>         I have an idea, by now in order to use QMAP,
>         must execute shell command 'echo mux_id > /sys/class/net/<iface>/add_mux' in user space,
>         maybe we can expand usage of sys attribute 'add_mux', like next:
>         'echo mux_id mux_size mux_version > /sys/class/net/<iface>/add_mux'.
>         Users can set correct 'mux_size and mux_version' according to the response of 'QMI_WDA_SET_DATA_FORMAT '.
>         If 'mux_size and mux_version' miss, qmi_wwan can use default values.
>

not sure this is something acceptable, let's wait for Bjørn to comment.

>         If fixed set as 32KB, but MDM9x07 chips only support 4KB, or uses do not enable QMAP,
>         Maybe cannot reach max throughput for no enough rx urbs.
>

I did not test for performance, but you could be right since for
example, if I'm not wrong, rx_qlen for an high-speed device would be 2
with the changed rx_urb_size.

So, we'll probably need to modify also usbnet_update_max_qlen, but not
sure about the direction (maybe fallback to a default value to
guarantee a minimum number if rx_qlen is < than a threshold?). And
this is also a change affecting all the drivers using usbnet, so it
requires additional care.

Let's wait for the maintainers' advice also on this.

Regards,
Daniele

>
> > -----邮件原件-----
> > 发件人: Daniele Palmas [mailto:dnlplm@...il.com]
> > 发送时间: Wednesday, September 09, 2020 5:13 PM
> > 收件人: Bjørn Mork <bjorn@...k.no>
> > 抄送: Kristian Evensen <kristian.evensen@...il.com>; Paul Gildea
> > <paul.gildea@...il.com>; Carl Yin(殷张成) <carl.yin@...ctel.com>; David S .
> > Miller <davem@...emloft.net>; Jakub Kicinski <kuba@...nel.org>;
> > netdev@...r.kernel.org; linux-usb@...r.kernel.org; Daniele Palmas
> > <dnlplm@...il.com>
> > 主题: [PATCH net-next 1/1] net: usb: qmi_wwan: add default rx_urb_size
> >
> > Add default rx_urb_size to support QMAP download data aggregation without
> > needing additional setup steps in userspace.
> >
> > The value chosen is the current highest one seen in available modems.
> >
> > The patch has the side-effect of fixing a babble issue in raw-ip mode reported by
> > multiple users.
> >
> > Signed-off-by: Daniele Palmas <dnlplm@...il.com>
> > ---
> > Resending with mailing lists added: sorry for the noise.
> >
> > Hi Bjørn and all,
> >
> > this patch tries to address the issue reported in the following threads
> >
> > https://www.spinics.net/lists/netdev/msg635944.html
> > https://www.spinics.net/lists/linux-usb/msg198846.html
> > https://www.spinics.net/lists/linux-usb/msg198025.html
> >
> > so I'm adding the people involved, maybe you can give it a try to double check if
> > this is good for you.
> >
> > On my side, I performed tests with different QC chipsets without experiencing
> > problems.
> >
> > Thanks,
> > Daniele
> > ---
> >  drivers/net/usb/qmi_wwan.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c index
> > 07c42c0719f5..92d568f982b6 100644
> > --- a/drivers/net/usb/qmi_wwan.c
> > +++ b/drivers/net/usb/qmi_wwan.c
> > @@ -815,6 +815,10 @@ 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 rx_urb_size to allow QMAP rx data aggregation */
> > +     dev->rx_urb_size = 32768;
> > +
> >  err:
> >       return status;
> >  }
> > --
> > 2.17.1
>