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, 1 Dec 2011 09:29:50 -0800
From:	Jesse Brandeburg <jesse.brandeburg@...el.com>
To:	Eric Dumazet <eric.dumazet@...il.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?

On Thu, 1 Dec 2011 01:15:37 -0800
Eric Dumazet <eric.dumazet@...il.com> wrote:

> Le mercredi 30 novembre 2011 à 21:09 -0800, Tom Herbert a écrit :
> > > We recently added commit f07d960df33c5aef
> > > (tcp: avoid frag allocation for small frames)
> > 
> > Yes, reverting that patch seems to fix the problem.  Eric, do you have
> > an idea what should be used to determine length of headers in an skb
> > now (mac through transport).

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