Daniele Palmas <dnlplm@xxxxxxxxx> writes: > Il giorno mer 9 set 2020 alle ore 13:09 Carl Yin(殷张成) > <carl.yin@xxxxxxxxxxx> 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. This breaks the "one value per file" rule. Ref https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt And I really think this userspace ABI is complicated enough already without adding yet another variable governed by strict rules. I'd prefer this to just work, if possible. I liked the simplicity of your patch. If it is necessary to have a different value when QMAP is disabled, then I'd much rather see two fixed values, depending on QMI_WWAN_FLAG_MUX. >> 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. That's correct. But I believe 2 URBs might be enough. Still, would be nice if some of you with a fast enough modem could test this. At least if throughput issues are going to be an argument for a more complicated ABI. > 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. I'm not sure we want to do that. We haven't done it for other aggregating usbnet protocols. There is a reason we have the hard limit. Bjørn >> > -----邮件原件----- >> > 发件人: Daniele Palmas [mailto:dnlplm@xxxxxxxxx] >> > 发送时间: Wednesday, September 09, 2020 5:13 PM >> > 收件人: Bjørn Mork <bjorn@xxxxxxx> >> > 抄送: Kristian Evensen <kristian.evensen@xxxxxxxxx>; Paul Gildea >> > <paul.gildea@xxxxxxxxx>; Carl Yin(殷张成) <carl.yin@xxxxxxxxxxx>; David S . >> > Miller <davem@xxxxxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; >> > netdev@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Daniele Palmas >> > <dnlplm@xxxxxxxxx> >> > 主题: [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@xxxxxxxxx> >> > --- >> > 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 >>
