netdev - Re: [PATCH] usbnet: drop unneeded check for NULL

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: <1346883275.5325.28.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Wed, 5 Sep 2012 23:14:35 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	David Miller <davem@...emloft.net>
CC:	<oneukum@...e.de>, <richardcochran@...il.com>,
	<netdev@...r.kernel.org>,
	Alexander Duyck <alexander.h.duyck@...el.com>
Subject: Re: [PATCH] usbnet: drop unneeded check for NULL

On Wed, 2012-09-05 at 12:50 -0400, David Miller wrote:
> From: Oliver Neukum <oneukum@...e.de>
> Date: Wed, 05 Sep 2012 08:24:25 +0200
> 
> > On Wednesday 05 September 2012 06:47:12 Richard Cochran wrote:
> >> and so I think the problem that the test addresses is still present,
> >> or am I missing something?
> > 
> > No,
> > 
> > you are right. Thank you.
> > 
> > Dave, for now, please don't apply this patch. In the long run, this crap
> > in cdc-ncm needs to go. I am starting rewriting this driver right now.
> 
> I already applied it several days ago, someone send me a revert with a
> verbose commit message explaining the situation.

cdc-ncm is aggregating skbs into jumbo USB packets and then, because the
netdev is not signalled when the qdisc stops transmitting, flushing them
after one scheduler tick.  Flushing is done by calling
usbnet_start_xmit() with a null skb pointer and then substituting a real
skb in the tx_fixup callback.

Perhaps the skb_tx_timestamp() call should be moved below the
'if (info->tx_fixup)' block, at which point skb is definitely non-null.
It doesn't look like cdc-ncm will provide useful timestamps either way.

cdc-ncm's aggregation could be improved by either (1) implementing some
type of GSO with appropriate gso_max_size and gso_max_segs limits (2)
adding an explicit transmit flushing operation, similar to that
Alexander Duyck proposed.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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