From: "Bjørn Mork" <bjorn@mork.no>
To: Alexey ORISHKO <alexey.orishko@stericsson.com>
Cc: "netdev\@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-usb\@vger.kernel.org" <linux-usb@vger.kernel.org>,
Greg Suarez <gsuarez@smithmicro.com>,
Oliver Neukum <oneukum@suse.de>,
Alexey Orishko <alexey.orishko@gmail.com>
Subject: Re: [PATCH net 2/3] net: cdc_mbim: send ZLP after max sized NTBs
Date: Mon, 21 Jan 2013 23:36:49 +0100 [thread overview]
Message-ID: <878v7mb1tq.fsf@nemi.mork.no> (raw)
In-Reply-To: <2AC7D4AD8BA1C640B4C60C61C8E520154A8EE17D57@EXDCVYMBSTM006.EQ1STM.local> (Alexey ORISHKO's message of "Mon, 21 Jan 2013 17:31:37 +0100")
Alexey ORISHKO <alexey.orishko@stericsson.com> writes:
>> -----Original Message-----
>> From: Bjørn Mork [mailto:bjorn@mork.no]
>>
>> We normally avoid sending ZLPs by padding NTBs with a zero byte if the
>> NTB is shorter than dwNtbOutMaxSize, resulting in a short USB packet
>> instead of a ZLP. But in the case where the NTB length is exactly
>> dwNtbOutMaxSize and this is an exact multiplum of wMaxPacketSize, then
>> we must send a ZLP.
>>
>> This fixes an issue seen on a Sierra Wireless MC7710 device where the
>> transmission would fail whenever we ended up padding the NTBs to max
>> size.
>
> If you add ZLP for NTBs of dwNtbOutMaxSize, you are heavily affecting CPU
> load, increasing interrupt load by factor of 2 in high load traffic
> scenario and possibly decreasing throughput for all other devices
> which behaves correctly.
>
> Why should others pay for Sierra Wireless fault?
No, they shouldn't.
> At least, do check VID/PID in usbnet and send ZLP only for malfunctioning
> devices.
Yes, this would be easy to implement by simply creating a vendor/device
specific entry in cdc_mbim.
But I do worry that this isn't Sierra Wireless fault, but Qualcomms
fault. SW are using Qualcomm firmware to do the low level stuff, and I
know the Qualcomm firmware implement MBIM functionality so it may very
well be the one to blame (I don't know this for sure, and I have no idea
how to verify it). If this is correct than we can expect hundreds of
devices needing a device specific entry in a year or two. That's not
what I was looking forward to with a class driver like cdc_mbim.
And I still believe that there is reason to ask if this is really a
fault at all.
I don't know if this is particularily useful, but here is an usbmon
extract from the driver without the extra ZLP fix:
ffff88022aef3d40 3883111197 S Bo:8:005:1 -115 4096 = 4e434d48 0c000600 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff880213f2d2c0 3883111233 S Bi:8:005:2 -115 16384 <
ffff8802274eab40 3883111255 S Bi:8:005:2 -115 16384 <
ffff88022fd65980 3883111276 S Bi:8:005:2 -115 16384 <
ffff880204351440 3883111297 S Bi:8:005:2 -115 16384 <
ffff880213f2d8c0 3883111317 S Bi:8:005:2 -115 16384 <
ffff88022aef3d40 3883111460 C Bo:8:005:1 0 4096 >
ffff880204351e00 3883965516 S Bo:8:005:1 -115 4096 = 4e434d48 0c000700 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff880204351e00 3883965849 C Bo:8:005:1 0 4096 >
ffff880204351e00 3884973428 S Bo:8:005:1 -115 4096 = 4e434d48 0c000800 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff880204351e00 3884973752 C Bo:8:005:1 0 4096 >
ffff880204351d40 3885981392 S Bo:8:005:1 -115 4096 = 4e434d48 0c000900 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff880204351d40 3885981638 C Bo:8:005:1 0 4096 >
ffff880231b128c0 3886989391 S Bo:8:005:1 -115 4096 = 4e434d48 0c000a00 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff880231b128c0 3886989622 C Bo:8:005:1 0 4096 >
ffff8802041d8a40 3887997401 S Bo:8:005:1 -115 4096 = 4e434d48 0c000b00 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff8802041d8a40 3887997675 C Bo:8:005:1 0 4096 >
ffff8802041d8a40 3889005720 S Bo:8:005:1 -115 4096 = 4e434d48 0c000c00 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff8802041d8a40 3889006019 C Bo:8:005:1 0 4096 >
ffff8802041d8c80 3890013428 S Bo:8:005:1 -115 4096 = 4e434d48 0c000d00 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff8802041d8c80 3890013784 C Bo:8:005:1 0 4096 >
ffff880213f2d2c0 3892433962 C Bi:8:005:2 -104 0
ffff8802274eab40 3892434032 C Bi:8:005:2 -104 0
ffff88022fd65980 3892434078 C Bi:8:005:2 -104 0
ffff880204351440 3892434126 C Bi:8:005:2 -104 0
ffff880213f2d8c0 3892434172 C Bi:8:005:2 -104 0
same driver, with smaller packets keeping the NTB size below the magic
limit:
ffff88022b1de200 3896794916 S Bi:8:005:2 -115 16384 <
ffff88022767d780 3896794928 C Bo:8:005:1 0 262 >
ffff88022b1de140 3896794948 S Bi:8:005:2 -115 16384 <
ffff88022767d780 3896794971 S Bi:8:005:2 -115 16384 <
ffff88022b1deb00 3896794995 S Bi:8:005:2 -115 16384 <
ffff88022767d900 3896864077 C Bi:8:005:2 0 416 = 4e434d48 0c003500 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff88022767d900 3896864129 S Bi:8:005:2 -115 16384 <
ffff88022d639e40 3897644729 S Bo:8:005:1 -115 262 = 4e434d48 0c000f00 06010c00 49505300 10000000 b8004e00 00000000 00000000
ffff88022d639e40 3897644965 C Bo:8:005:1 0 262 >
ffff88022b1de380 3897711770 C Bi:8:005:2 0 112 = 4e434d48 0c003600 70000c00 49505300 10000000 1c004e00 00000000 4500004e
ffff88022b1de380 3897711812 S Bi:8:005:2 -115 16384 <
ffff88022b07c600 3898646616 S Bo:8:005:1 -115 262 = 4e434d48 0c001000 06010c00 49505300 10000000 b8004e00 00000000 00000000
ffff88022b07c600 3898646848 C Bo:8:005:1 0 262 >
ffff88022b1de200 3898697788 C Bi:8:005:2 0 112 = 4e434d48 0c003700 70000c00 49505300 10000000 1c004e00 00000000 4500004e
ffff88022b1de200 3898697829 S Bi:8:005:2 -115 16384 <
ffff88022b1de140 3901425927 C Bi:8:005:2 -104 0
driver with ZLP:
ffff8802041d82c0 3651255294 C Bo:8:005:1 0 4096 >
ffff88021a3ef380 3652103000 S Bo:8:005:1 -115 4096 = 4e434d48 0c003200 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff88021a3ef380 3652103315 C Bo:8:005:1 0 4096 >
ffff8802041af540 3652439129 C Bi:8:005:2 0 416 = 4e434d48 0c002c00 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff8802041af540 3652439183 S Bi:8:005:2 -115 16384 <
ffff8802041af240 3653099148 C Bi:8:005:2 0 416 = 4e434d48 0c002d00 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff8802041af240 3653099183 S Bi:8:005:2 -115 16384 <
ffff88021a3efb00 3653101946 S Bo:8:005:1 -115 4096 = 4e434d48 0c003300 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff88021a3efb00 3653102201 C Bo:8:005:1 0 4096 >
ffff8802041af9c0 3654039362 C Bi:8:005:2 0 416 = 4e434d48 0c002e00 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff8802041af9c0 3654039414 S Bi:8:005:2 -115 16384 <
ffff88021a3ef980 3654101353 S Bo:8:005:1 -115 4096 = 4e434d48 0c003400 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff88021a3ef980 3654101622 C Bo:8:005:1 0 4096 >
ffff88021a3efc80 3654999361 C Bi:8:005:2 0 416 = 4e434d48 0c002f00 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff88021a3efc80 3654999412 S Bi:8:005:2 -115 16384 <
ffff88021a3ef200 3655102396 S Bo:8:005:1 -115 4096 = 4e434d48 0c003500 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff88021a3ef200 3655102737 C Bo:8:005:1 0 4096 >
ffff88021a3efe00 3655949445 C Bi:8:005:2 0 416 = 4e434d48 0c003000 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff88021a3efe00 3655949491 S Bi:8:005:2 -115 16384 <
ffff88021a3ef500 3656103498 S Bo:8:005:1 -115 4096 = 4e434d48 0c003600 00100c00 49505300 10000000 b8007a01 00000000 00000000
ffff88021a3ef500 3656103779 C Bo:8:005:1 0 4096 >
ffff8802041af540 3657229210 C Bi:8:005:2 0 416 = 4e434d48 0c003100 a0010c00 49505300 10000000 1c007a01 00000000 4500017a
ffff8802041af540 3657229251 S Bi:8:005:2 -115 16384 <
ffff8802041af240 3659438084 C Bi:8:005:2 -104 0
ffff8802041af9c0 3659438159 C Bi:8:005:2 -104 0
ffff88021a3efc80 3659438206 C Bi:8:005:2 -104 0
Bjørn
next prev parent reply other threads:[~2013-01-21 22:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-21 15:50 [PATCH net 0/3] cdc_ncm and cdc_mbim fixes for 3.8 Bjørn Mork
2013-01-21 15:50 ` [PATCH net 1/3] net: cdc_ncm: workaround for missing CDC Union Bjørn Mork
2013-01-21 15:50 ` [PATCH net 2/3] net: cdc_mbim: send ZLP after max sized NTBs Bjørn Mork
[not found] ` <1358783440-11459-3-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2013-01-21 16:31 ` Alexey ORISHKO
2013-01-21 22:36 ` Bjørn Mork [this message]
[not found] ` <2AC7D4AD8BA1C640B4C60C61C8E520154A8EE17D57-8ZTw5gFVCTjVH5byLeRTJxkTb7+GphCuwzqs5ZKRSiY@public.gmane.org>
2013-01-22 9:54 ` Bjørn Mork
2013-01-22 15:51 ` Alexey ORISHKO
2013-01-21 16:56 ` Yauheni Kaliuta
[not found] ` <87wqv64gr9.fsf-jZyLUQyd5ymekGDzARDqUg@public.gmane.org>
2013-01-21 22:01 ` Bjørn Mork
2013-01-22 17:16 ` Yauheni Kaliuta
[not found] ` <1358783440-11459-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2013-01-21 15:50 ` [PATCH net 3/3] net: cdc_ncm: fix error path for single interface probing Bjørn Mork
2013-01-22 8:58 ` Oliver Neukum
2013-01-22 10:32 ` Bjørn Mork
2013-01-21 19:22 ` [PATCH net 0/3] cdc_ncm and cdc_mbim fixes for 3.8 David Miller
2013-01-21 19:42 ` Alexey ORISHKO
2013-01-21 20:16 ` Bjørn Mork
2013-01-21 20:34 ` Alexey Orishko
2013-01-21 22:12 ` Bjørn Mork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878v7mb1tq.fsf@nemi.mork.no \
--to=bjorn@mork.no \
--cc=alexey.orishko@gmail.com \
--cc=alexey.orishko@stericsson.com \
--cc=gsuarez@smithmicro.com \
--cc=linux-usb@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=oneukum@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox