netdev - Re: Bug in computing data_len in tcp

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]

Date:	Thu, 01 Dec 2011 21:37:55 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Jesse Brandeburg <jesse.brandeburg@...el.com>
Cc:	Tom Herbert <therbert@...gle.com>,
	Linux Netdev List <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>, alexander.h.duyck@...el.com
Subject: Re: Bug in computing data_len in tcp_sendmsg?

Le jeudi 01 décembre 2011 à 09:29 -0800, Jesse Brandeburg a écrit :
> On Thu, 1 Dec 2011 01:15:37 -0800
> Eric Dumazet <eric.dumazet@...il.com> wrote:

> Tom, thanks very much for finding this subtle bug!  I bet that this
> commit (which I had missed) broke a lot of other drivers in
> very subtle ways due to changing a long standing behavior.  Auditing
> every driver's tx path is going to be a lot of work unless a way can be
> discovered to automate finding incorrect assumptions in drivers.
> 

I did a quick check an counted 3 intel drivers.

Others are fine. I'll redo a check.

> > I dont know why its even necessary :
> > 
> > TSO enabled NIC all provide hardware counters, so why even bother
> > computing tx_bytes ourself ?
> 
> because we have runtime logic for adjusting interrupt rate that depends
> on knowing how many bytes and packets were cleaned up in an interrupt
> and reading MMIO to get latest stats causes CPU stall in hot path.
> Counting the bytes sent on the wire via a segmented SKB was hard to get
> right to begin with, I think we took a different approach in ixgbe
> (recent versions) and compute the math in hard_start_xmit instead of tx
> cleanup.


> 
> > skb->len is appropriate for BQL, as long as producers/consumer use the
> > same skb->len. 1 or 2% error is not a problem if not cumulative ?
> 
> if skb->len access doesn't cause a cache miss in hot path (or at
> least doesn't increase misses) then I say sure.

You still can cache it like now in your bytecount field in start_xmit(),
but since you call skb_free(), I doubt this matters a lot.



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