[review] aleksander/huawei-ndisdup-cdc-wdm-rebased-1.2

[Bjørn Mork]

Aleksander Morgado <aleksander at aleksander.es> writes:

> On Fri, Jun 13, 2014 at 4:39 PM, Bjørn Mork <bjorn at mork.no> wrote:
>> A real quick fix is just increasing the size of the buffer.
>>
>> I don't know about fixing the real problem. The cdc-wdm driver has
>> become a bit more messy than it should have been, making any such change
>> risky. I wonder how we could end up making this so complicated? ;-)
>>
>> And then there is the question whether this change is appropriate for
>> all protocols handled by the cdc-wdm driver.  WDM and MBIM have well
>> defined message sizes, so we know we'll get a notification for the next
>> message even if we read a full buffer. And WDM is obviously important
>> here...
>>
>> The problem is QMI and the Huawei NCM thing, where we really don't know
>> what the firmware will send.  For QMI we do depend on having big enough
>> buffers anyway.  Nothing is prepared to see a split QMI message, which
>> is why we have set the buffer size to as much as 4096 in qmi_wwan.  I am
>> tempted to just close my eyes and see if the problem goes away :-)
>>
>> Or maybe just go for the quick fix, like and increase the huawei_cdc_ncm
>> buffer size?  What do you say, Enrico? Is 512 bytes fine, or maybe 1024?
>> What is the largest single AT command response we can expect over this
>> channel?
>
>
> Puff... the largest AT command response may be pretty large. Not sure
> if will hit 1024, though.
>
> Another thing we could try is to detect the issue in userspace; and if
> we get 256 bytes from the AT-capable /dev/cdc-wdm port, just schedule
> another read() to be done in an idle. This is a non-blocking socket,
> so it shouldn't harm... unless the kernel doesn't fill the buffer
> again with the remaining data in the time we're doing the idle...

I could be wrong, ref the driver complexity, but I believe the driver
won't fetch the remaining data from the modem until the modem sends
another notification.  And I am guessing the modem won't do that twice
for the same message, even if the driver only read part of it.



Bjørn