From: "Bjørn Mork" <bjorn-yOkvZcmFvRU@public.gmane.org>
To: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>,
alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: changing usbnet's API to better deal with cdc-ncm
Date: Thu, 06 Sep 2012 10:30:29 +0200 [thread overview]
Message-ID: <87a9x33656.fsf@nemi.mork.no> (raw)
In-Reply-To: <CACVXFVMXz1mqANBYR56KZsh3RgN5M_og_OggDEHT02w0Pe-UiA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> (Ming Lei's message of "Thu, 6 Sep 2012 11:23:51 +0800")
Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> On Thu, Sep 6, 2012 at 4:12 AM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>> Hi,
>>
>> looking at cdc-ncm it seeems to me that cdc-ncm is forced to play
>> very dirty games because usbnet doesn't have a notion about aggregating
>> packets for a single transfer.
>
> The Ethernet API we are using does not support transmitting multiple Ethernet
> frames in a single call, so the aggregation things should be done inside low
> level driver, in fact it is just what some wlan(802.11n) drivers have
> been doing.
>
> IMO, the current .tx_fixup is intelligent enough to handle aggregation:
>
> - return a skb_out for sending if low level drivers think it is
> ready to send
> the aggregated frames
> - return NULL if the low level drivers think they need to wait
> for more frames
>
> Of course, the low level drivers need to start a timer to trigger sending
> remainder frames in case of timeout and no further frames come from
> upper protocol stack.
>
> Looks the introduced .tx_bundle is not necessary since .tx_fixup is OK.
The minidriver does not have any information about tx in progress. The
strategy above, which is what cdc_ncm uses today, is fine as long as you
always want to wait as long as you always want to fill as many frames as
possible in each packet. But what if the queue is empty and the device
is just sitting there doing nothing while you are waiting for more
frames? Then you are just adding unnecessary latency.
There are also several usbnet minidrivers for protocols with frame
aggregation support. Most of them do not currently implement tx
aggregation, possibly due to the complexity. This makes a common
aggregation framework interesting in any case. Reimplementing similar
loginc in multiple minidrivers is meaningless.
I believe Oliver is adding very useful functionality to usbnet here.
Functionality which at least cdc_ncm and possibly other existing
minidrivers can take advantage of immediately. There also seems to be a
trend towards more complex aggregating protocols as the devices get
smarter and speeds go up.
BJørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-09-06 8:30 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-05 20:12 changing usbnet's API to better deal with cdc-ncm Oliver Neukum
[not found] ` <2791550.LhGu6po6Xy-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
2012-09-06 3:23 ` Ming Lei
[not found] ` <CACVXFVMXz1mqANBYR56KZsh3RgN5M_og_OggDEHT02w0Pe-UiA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-09-06 8:30 ` Bjørn Mork [this message]
[not found] ` <87a9x33656.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2012-09-06 16:09 ` Ming Lei
2012-09-06 17:56 ` Oliver Neukum
[not found] ` <1676538.CEPj2RtrPe-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
2012-09-07 3:55 ` Ming Lei
2012-09-07 7:35 ` Oliver Neukum
[not found] ` <1446113.XC2Qbo4flT-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
2012-09-07 12:01 ` Alexey ORISHKO
[not found] ` <2AC7D4AD8BA1C640B4C60C61C8E520154A6AA5741E-8ZTw5gFVCTjVH5byLeRTJxkTb7+GphCuwzqs5ZKRSiY@public.gmane.org>
2012-09-07 13:08 ` Oliver Neukum
2012-09-07 15:23 ` Alexey ORISHKO
[not found] ` <2AC7D4AD8BA1C640B4C60C61C8E520154A6AA5753D-8ZTw5gFVCTjVH5byLeRTJxkTb7+GphCuwzqs5ZKRSiY@public.gmane.org>
2012-09-07 18:23 ` Oliver Neukum
[not found] ` <1609320.M17H7UceTZ-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
2012-09-10 17:10 ` Alexey ORISHKO
2012-09-06 8:13 ` Alexey ORISHKO
2012-09-06 8:50 ` Oliver Neukum
[not found] ` <6556435.5o3fR8ZcBa-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
2012-09-06 9:11 ` Eric Dumazet
2012-09-06 9:22 ` Eric Dumazet
2012-09-07 12:58 ` Ming Lei
2012-09-07 13:10 ` Alexey ORISHKO
2012-09-06 8:17 ` Bjørn Mork
[not found] ` <87ehmf36qd.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2012-09-07 7:40 ` MBIM (was: Re: changing usbnet's API to better deal with cdc-ncm) Oliver Neukum
2012-09-07 8:53 ` MBIM Bjørn Mork
[not found] ` <87392u1afl.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2012-09-07 17:02 ` MBIM Dan Williams
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=87a9x33656.fsf@nemi.mork.no \
--to=bjorn-yokvzcmfvru@public.gmane.org \
--cc=alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org \
--cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=oneukum-l3A5Bk7waGM@public.gmane.org \
--cc=tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.