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

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

[Daniele Palmas]

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(-)

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,
-- 
2.7.4

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

[Bjørn Mork]

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

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

[Daniele Palmas]

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

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

[David Miller]

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.