Message-ID: <20110317142030.62af785a@nehalam>
Date:	Thu, 17 Mar 2011 14:20:30 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Jerry Chu <hkchu@...gle.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] Add useful per-connection TCP stats for diagnosis
 purpose.

On Thu, 17 Mar 2011 13:16:15 -0700
Jerry Chu <hkchu@...gle.com> wrote:

> Eric, thanks for the prompt feedback.
> 
> On Thu, Mar 17, 2011 at 1:42 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> > Le jeudi 17 mars 2011 à 01:06 -0700, H.K. Jerry Chu a écrit :
> >> From: Jerry Chu <hkchu@...gle.com>
> >>
> >> This patch add a number of very useful counters/stats (defined in
> >> tcp_stats.h) to help diagnosing TCP related problems.
> >>
> >> create_time     - when the connection was created (in jiffies)
> >> total_inbytes   - total inbytes as consumed by the receiving apps.
> >> total_outbytes  - total outbytes sent down from the transmitting apps.
> >>
> >> total_outdatasegs - total data carrying segments sent so far, including
> >>               retransmitted ones.
> >>
> >> total_xmit      - total accumulated time (usecs) when the connection
> >>               has something to send.
> >>
> >> total_retrans_time - total time (usecs, accumulated) the connection
> >>               spends trying to recover lost packets. For each
> >>               loss event the time is measured from the lost packet
> >>               was first sent till the retransmitted packet was
> >>               eventually ack'ed.
> >>
> >> total_cwnd_limit - total time (usecs, excluding time spent on loss
> >>               recovery) the xmit is stopped due to cwnd limited
> >>
> >> total_swnd_limit - total time (usecs) theconnection is swnd limited
> >>
> >> The following two counters are for listeners only:
> >>
> >> accepted_reqs   - total # of accepted connection requests.
> >> listen_drops    - total # of dropped SYN reqs (SYN cookies excluded) due
> >>               to listener's queue overflow.
> >>
> >> total_retrans_time/total_retrans ratio gives a rough picture of how
> >> quickly in average the connection can recover from a pkt loss. E.g.,
> >> when the network is more congested, or the traffic contains mainly
> >> smaller RPC where tail drop often requires RTO to recover,
> >> the total_retrans_time/total_retrans ratio tends to be higher.
> >>
> >> Currently the new counters/stats are exported through /proc/net/tcp.
> >
> > Please dont. Use iproute2 instead.
> >
> >> Some simple, abbreviated field names have been added to the output of
> >> /proc/net/tcp in order to allow backward/forward compatibility in the
> >> future. Obviously the new counters/stats can also be easily exported
> >> through other APIs.
> >>
> >
> > /proc/net/tcp is legacy. You should touch it eventually, but after
> > "other APIS" are done. It was the old way (quick but a bit ugly)
> 
> Understood. /proc/net/tcp is a much more expedient way of exporting these
> counters because it doesn't requires any additional, special tool to read it,
> unless other APIs (e.g., netlink). Note that backward compatibility to
> /proc/net/tcp has been ensured by adding field names in the heading.
> 
> >
> >
> >
> >> Signed-off-by: H.K. Jerry Chu <hkchu@...gle.com>
> >> ---
> >>  include/linux/ktime.h    |    3 ++
> >>  include/linux/tcp.h      |    1 +
> >>  include/net/tcp_stats.h  |   65 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  net/ipv4/tcp.c           |   30 ++++++++++++++++++---
> >>  net/ipv4/tcp_input.c     |   13 +++++++++
> >>  net/ipv4/tcp_ipv4.c      |   41 ++++++++++++++++++++++++++---
> >>  net/ipv4/tcp_minisocks.c |    9 ++++++
> >>  net/ipv4/tcp_output.c    |   47 +++++++++++++++++++++++++++++++--
> >>  net/ipv6/tcp_ipv6.c      |    8 +++++
> >>  9 files changed, 206 insertions(+), 11 deletions(-)
> >>  create mode 100644 include/net/tcp_stats.h
> >>
> >> diff --git a/include/linux/ktime.h b/include/linux/ktime.h
> >> index e1ceaa9..e60e758 100644
> >> --- a/include/linux/ktime.h
> >> +++ b/include/linux/ktime.h
> >> @@ -333,6 +333,9 @@ extern void ktime_get_ts(struct timespec *ts);
> >>  /* Get the real (wall-) time in timespec format: */
> >>  #define ktime_get_real_ts(ts)        getnstimeofday(ts)
> >
> > Hmm, this kind of changes are out of netdev scope and should be avoided
> 
> Ok. (It was moved out of tcp_stats.h only at the last minute.)
> 
> >
> >>
> >> +#define ktime_since(a)               ktime_to_us(ktime_sub(ktime_get(), (a)))
> >
> > us are implied in ktime_since() ? thats strange.
> 
> Ok.
> 
> >
> >> +#define ktime_zero(a)                ktime_equal((a), ktime_set(0, 0))
> >
> > ktime_zero() sounds like : "give me zero time" or "clear the ktime
> > field".
> 
> Yes I actually have been flip-flopping on the name...
> 
> >
> >> +
> >>  static inline ktime_t ns_to_ktime(u64 ns)
> >>  {
> >>       static const ktime_t ktime_zero = { .tv64 = 0 };
> >> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> >> index e64f4c6..ea5cb5d 100644
> >> --- a/include/linux/tcp.h
> >> +++ b/include/linux/tcp.h
> >> @@ -460,6 +460,7 @@ struct tcp_sock {
> >>        * contains related tcp_cookie_transactions fields.
> >>        */
> >>       struct tcp_cookie_values  *cookie_values;
> >> +     struct tcp_stats        *conn_stats;
> >>  };
> >
> > Really, using separate cache lines to store some stats is expensive.
> > You should add counters in existing structure, to avoid additional cache
> > line dirties. Carefully placing stats in already dirtied cache lines.
> 
> This was how it was done initially but then we wanted to allow future
> extension to include possibly a lot more counters, something like Web100
> (RFC4898). For the latter the memory/performance hit will likely require
> a config option, and a separate structure will make this easier. Does it
> make sense?
> 
> >
> > You also should use native ktime_t infrastructure, to make the maths
> > really fast in fast path.
> >
> > Only when stats are to be returned to user, you'll have to convert the
> > native timestamps to user exportable ones.
> 
> Good point! Will do. (I mistakenly thought ktime_t is a larger structure.)
> 
> >
> > Quite frankly, using u64 fields allow nanosec resolution.
> 
> I wish to use less bits because the final report only needs ms or even
> sec resolution but the intermediate computation needs to capture usec
> resolution.
> 
> >
> > BTW, we probably could 'export' sk->sk_drops for TCP, like we do for
> > UDP.
> 
> There are many other potentially useful counters/stats (spurious_retrans,
> min_rtt, total_rto,...) but there is a tradeoff against memory/performance hit
> so for the first round I'm focusing on what i feel is the most useful set.
> 
> Thanks,
> 
> Jerry

These stats are best added via netlink, the tool ss already prints lots
of similar stats. Look at INET_DIAG_INFO and the output of:
 ss -i -t






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

Powered by blists - more mailing lists