On Fri, Sep 7, 2012 at 1:56 AM, Oliver Neukum <oneukum@xxxxxxx> wrote: > On Friday 07 September 2012 00:09:13 Ming Lei wrote: >> On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork <bjorn@xxxxxxx> wrote: >> > Ming Lei <tom.leiming@xxxxxxxxx> writes: > >> >> 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. > >> > 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? > >> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
