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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1676538.CEPj2RtrPe@linux-lqwf.site>
Date:	Thu, 06 Sep 2012 19:56:50 +0200
From:	Oliver Neukum <oneukum@...e.de>
To:	Ming Lei <tom.leiming@...il.com>
Cc:	Bjørn Mork <bjorn@...k.no>,
	alexey.orishko@...ricsson.com, netdev@...r.kernel.org,
	linux-usb@...r.kernel.org
Subject: Re: changing usbnet's API to better deal with cdc-ncm

On Friday 07 September 2012 00:09:13 Ming Lei wrote:
> On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork <bjorn@...k.no> wrote:
> > Ming Lei <tom.leiming@...il.com> 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
 
> > 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.

> 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
- can queue, buffer full -> transmit
- cannot queue, buffer full -> transmit and then try again to queue

and an error case

- cannot queue, buffer not full

And that's the way I coded it.

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html