Re: changing usbnet's API to better deal with cdc-ncm

On Fri, Sep 7, 2012 at 1:56 AM, Oliver Neukum [off-list ref] wrote:

On Friday 07 September 2012 00:09:13 Ming Lei wrote:

quoted

On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork [off-list ref] wrote:

quoted

Ming Lei [off-list ref] writes:

quoted

quoted

quoted

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

Inside .tx_fixup, the low level driver will get the tx progress information.

That information changes after tx_fixup

You mean the low level drive can't see if the later transmission
for the aggregated frame is successful?  If so, there is no any complete
notification to all low level drivers (with aggregation capability
or not) now, isn't there?

If no aggregated frame is ready for transmission, the information
can't be changed after current tx_fixup and before the next .tx_fixup.

quoted

quoted

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

For cdc_ncm, the wait time is controlled by cdc_ncm driver, see
cdc_ncm_tx_timeout_start().

Well, that is the mistake. Using a timer is a bad idea.

Why is a bad idea?  Without a timer, how can usbnet or
low level driver aggregate the later coming transmit frames to
form a biger aggregated frame?

quoted

If we can abstract some common things about aggregation, it should be
meaningful. As far as I know, most of aggregation protocol is very different,
so almost all aggregation work is only done by low level driver, such as
cdc_ncm.

If we want to implement some aggregation framework, maybe below is
one solution, correct me if it is wrong.

It isn't so much wrong as incomplete.

It seems to me we can have

- can queue, buffer not full -> do nothing more

As I said above, without a timeout timer, the queued skb
might not be sent out even some long time passed if no
frame comes later.

So could you add the wait for more mechanism to your patch
for further discussion?

- can queue, buffer full -> transmit
- cannot queue, buffer full -> transmit and then try again to queue

This might not happen, the buffer full should have been observed
in last xmit path.

and an error case

- cannot queue, buffer not full

Thanks,
-- 
Ming Lei
--
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