diff mbox

[1/1] net: usb: cdc_mbim: add flag FLAG_SEND_ZLP

Message ID 1527758309-9614-1-git-send-email-dnlplm@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniele Palmas May 31, 2018, 9:18 a.m. UTC
Testing Telit LM940 with ICMP packets > 14552 bytes revealed that
the modem needs FLAG_SEND_ZLP to properly work, otherwise the cdc
mbim data interface won't be anymore responsive.

Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
---
 drivers/net/usb/cdc_mbim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bjørn Mork May 31, 2018, 9:56 a.m. UTC | #1
Daniele Palmas <dnlplm@gmail.com> writes:

> Testing Telit LM940 with ICMP packets > 14552 bytes revealed that
> the modem needs FLAG_SEND_ZLP to properly work, otherwise the cdc
> mbim data interface won't be anymore responsive.
>
> Signed-off-by: Daniele Palmas <dnlplm@gmail.com>

Acked-by: Bjørn Mork <bjorn@mork.no>

Should have thought of this... I noticed your discussion, but couldn't
reproduce the issues myself.  This explains why.

Do you happen to know if the device announces larger buffers than the
driver wants to use, or if this happens with the max sized buffers too?

You can easily check these values by comparing dwNtbInMaxSize and
dwNtbOutMaxSize (device maximum values) with rx_max and tx_max
(neogtiated values) using e.g

 grep . /sys/class/net/wwan0/cdc_ncm/*


It has never been 100% clear to me whether we should send the ZLP by
default if we've negotiated a smaller than max buffer. But the ZLP ought
to be redundant in any case, since the device knows the negotiated
buffer size. So I do believe our current interpretation makes sense.

Not that it matters.  There are obviously more than enough device
implementations violating this requirement to make it completely
pointless.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniele Palmas May 31, 2018, 11:52 a.m. UTC | #2
2018-05-31 11:56 GMT+02:00 Bjørn Mork <bjorn@mork.no>:
> Daniele Palmas <dnlplm@gmail.com> writes:
>
>> Testing Telit LM940 with ICMP packets > 14552 bytes revealed that
>> the modem needs FLAG_SEND_ZLP to properly work, otherwise the cdc
>> mbim data interface won't be anymore responsive.
>>
>> Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
>
> Acked-by: Bjørn Mork <bjorn@mork.no>
>
> Should have thought of this... I noticed your discussion, but couldn't
> reproduce the issues myself.  This explains why.
>
> Do you happen to know if the device announces larger buffers than the
> driver wants to use, or if this happens with the max sized buffers too?
>
> You can easily check these values by comparing dwNtbInMaxSize and
> dwNtbOutMaxSize (device maximum values) with rx_max and tx_max
> (neogtiated values) using e.g
>
>  grep . /sys/class/net/wwan0/cdc_ncm/*
>

This seems to happen with the max sized buffers according to the output:

daniele@L2122:/home/daniele$ grep . /sys/class/net/wwp0s20u6i2/cdc_ncm/*

/sys/class/net/wwp0s20u6i2/cdc_ncm/bmNtbFormatsSupported:0x0001
/sys/class/net/wwp0s20u6i2/cdc_ncm/dwNtbInMaxSize:16384
/sys/class/net/wwp0s20u6i2/cdc_ncm/dwNtbOutMaxSize:16384
/sys/class/net/wwp0s20u6i2/cdc_ncm/min_tx_pkt:13312
/sys/class/net/wwp0s20u6i2/cdc_ncm/ndp_to_end:N
/sys/class/net/wwp0s20u6i2/cdc_ncm/rx_max:16384
/sys/class/net/wwp0s20u6i2/cdc_ncm/tx_max:16384
/sys/class/net/wwp0s20u6i2/cdc_ncm/tx_timer_usecs:400
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNdpInAlignment:4
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNdpInDivisor:1
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNdpInPayloadRemainder:0
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNdpOutAlignment:4
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNdpOutDivisor:4
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNdpOutPayloadRemainder:0
/sys/class/net/wwp0s20u6i2/cdc_ncm/wNtbOutMaxDatagrams:16

Thanks,
Daniele

>
> It has never been 100% clear to me whether we should send the ZLP by
> default if we've negotiated a smaller than max buffer. But the ZLP ought
> to be redundant in any case, since the device knows the negotiated
> buffer size. So I do believe our current interpretation makes sense.
>
> Not that it matters.  There are obviously more than enough device
> implementations violating this requirement to make it completely
> pointless.
>
>
> Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 1, 2018, 6:02 p.m. UTC | #3
From: Daniele Palmas <dnlplm@gmail.com>
Date: Thu, 31 May 2018 11:18:29 +0200

> Testing Telit LM940 with ICMP packets > 14552 bytes revealed that
> the modem needs FLAG_SEND_ZLP to properly work, otherwise the cdc
> mbim data interface won't be anymore responsive.
> 
> Signed-off-by: Daniele Palmas <dnlplm@gmail.com>

Applied and queued up for -stable.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 7220cd6..0362acd 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -609,7 +609,7 @@  static const struct driver_info cdc_mbim_info_ndp_to_end = {
  */
 static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
 	.description = "CDC MBIM",
-	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN | FLAG_SEND_ZLP,
 	.bind = cdc_mbim_bind,
 	.unbind = cdc_mbim_unbind,
 	.manage_power = cdc_mbim_manage_power,