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: <6556435.5o3fR8ZcBa@linux-lqwf.site>
Date:	Thu, 06 Sep 2012 10:50:13 +0200
From:	Oliver Neukum <oneukum@...e.de>
To:	Alexey ORISHKO <alexey.orishko@...ricsson.com>
Cc:	"bjorn@...k.no" <bjorn@...k.no>, Ming Lei <tom.leiming@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	Alexey Orishko <alexey.orishko@...il.com>
Subject: Re: changing usbnet's API to better deal with cdc-ncm

On Thursday 06 September 2012 10:13:01 Alexey ORISHKO wrote:
> > -----Original Message-----
> > From: Oliver Neukum [mailto:oneukum@...e.de]
> 
> 
> > 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.
> 
> Several issues need to be improved:
> Tx path:
> 1. IP packets must be accumulated in one NTB. Currently it's done via data copy.
> Preferred way would be a possibility to have a list of skb-s in resulting frame sent down.

I am afraid much HCD hardware is just not capable of doing DMA on a list of skbs.
I guess one copy will be necessary. We might consider not freeing the buffer used
to transfer over the bus.

> 2. Timer is needed to accumulate enough packets to get acceptable throughput and reducing amount of HW INTs on device side.
> If we get access to Tx queue size and can read skb from it, time can be removed. But Tx queue is above usbnet, so doesn't look possible in current framework.

I think we can use the number of transfers in flight to make that decision.
So if the choice is to not transfer data at all or to send transfers filled to the
max, we should send, otherwise we ought to accumulate.
That way we get rid of the timer.

> 3. While trying to get best throughput we also need to keep latency in mind, because in some setups it is quite important.
> As a simple solution configuration parameters could be used to address this issue.

I think we should just send the first (two) packages and use the time
to aggregate later packets.

> Rx path:
> 4. IP packets are cloned to separate skb and length of actual data set to eth packet size, while skb size is still the same as skb containing full NTB frame. This causes problems with TCP stack when throughput is high because of flow control in the stack, if too much data allocated.
> Someone suggested a patch for it, which was rejected. Anyway, I don't think copy data to a new skb would be a nice solution, but keeping several clones with size equal to incoming skb in not good either. 

Perhaps the problem is using an skb for aggregate reception at all.
Possibly enough buffers of fixed size should be allocated on open and reused,
not freed.

	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