Re: Bug in computing data_len in tcp_sendmsg?

[Jesse Brandeburg <jesse.brandeburg@intel.com>]

On Thu, 1 Dec 2011 01:15:37 -0800
Eric Dumazet <eric.dumazet@gmail.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.