Hi again Joe,
On Wed, Aug 10, 2016 at 05:36:16PM +0200, Willy Tarreau wrote:
> On Wed, Aug 10, 2016 at 07:11:44AM -0700, Joe Williams wrote:
> > Hello list,
> >
> > Adding on to Theirry's work (
> > http://git.haproxy.org/?p=haproxy.git;a=commit;h=6310bef51148b747f9019bd0bd67fd285eff0ae3)
> > I have added a few more fetchers for counters based on the tcp_info struct
> > maintained by the kernel.
>
> Thanks for this! As I told you initially I thought we wouldn't need
> the extra metrics, but you proved me wrong :-)
>
> I've merged it and added your comment as the commit message. However
> I was having a doubt about the presence of older fields on older kernels,
> so I gave it a try with linux-2.6.32 and glibc 2.3.6 and it failed on me
> like this :
>
> src/proto_tcp.c: In function `get_tcp_info':
> src/proto_tcp.c:2407: error: structure has no member named `tcpi_rcv_rtt'
> src/proto_tcp.c:2408: error: structure has no member named
> `tcpi_total_retrans'
> make: *** [src/proto_tcp.o] Error 1
> make: *** Waiting for unfinished jobs....
>
> So I'm seeing two possibilities :
> - either you don't need these ones and we simply drop them from the patch
> (the most likely solution given that total_retrans is meaningless in HTTP
> since it applies to the whole connection)
>
> - or we find a way to detect them and disable them at build time (I'm
> looking at this now).
>
> Please let me know, I've not pushed the commit yet, and I'd admit that
> the first option still seems the easiest to me :-/
>From what I'm seeing it also breaks FreeBSD where only Thierry's entries
are declared, and some of yours exist with a "__" in front of their name :
clang -Iinclude -Iebtree -Wall -O2 -g -fno-strict-aliasing
-Wdeclaration-after-statement -DTPROXY -DCONFIG_HAP_CRYPT -DENABLE_POLL
-DENABLE_KQUEUE -DCONFIG_HAPROXY_VERSION=\"1.7-dev3-f5f03ef\"
-DCONFIG_HAPROXY_DATE=\"2016/08/10\" -c -o src/proto_tcp.o src/proto_tcp.c
src/proto_tcp.c:2392:34: error: use of undeclared identifier 'SOL_TCP'
if (getsockopt(conn->t.sock.fd, SOL_TCP, TCP_INFO, &info, &optlen) ==
-1)
^
src/proto_tcp.c:2400:35: error: no member named 'tcpi_unacked' in 'struct
tcp_info'; did you mean '__tcpi_unacked'?
case 2: smp->data.u.sint = info.tcpi_unacked; break;
^~~~~~~~~~~~
__tcpi_unacked
/usr/include/netinet/tcp.h:212:12: note: '__tcpi_unacked' declared here
u_int32_t __tcpi_unacked;
^
src/proto_tcp.c:2401:35: error: no member named 'tcpi_sacked' in 'struct
tcp_info'; did you mean '__tcpi_sacked'?
case 3: smp->data.u.sint = info.tcpi_sacked; break;
^~~~~~~~~~~
__tcpi_sacked
/usr/include/netinet/tcp.h:213:12: note: '__tcpi_sacked' declared here
u_int32_t __tcpi_sacked;
^
src/proto_tcp.c:2402:35: error: no member named 'tcpi_lost' in 'struct
tcp_info'; did you mean '__tcpi_lost'?
case 4: smp->data.u.sint = info.tcpi_lost; break;
^~~~~~~~~
__tcpi_lost
/usr/include/netinet/tcp.h:214:12: note: '__tcpi_lost' declared here
u_int32_t __tcpi_lost;
^
src/proto_tcp.c:2403:35: error: no member named 'tcpi_retrans' in 'struct
tcp_info'; did you mean '__tcpi_retrans'?
case 5: smp->data.u.sint = info.tcpi_retrans; break;
^~~~~~~~~~~~
__tcpi_retrans
/usr/include/netinet/tcp.h:215:12: note: '__tcpi_retrans' declared here
u_int32_t __tcpi_retrans;
^
src/proto_tcp.c:2404:35: error: no member named 'tcpi_fackets' in 'struct
tcp_info'; did you mean '__tcpi_fackets'?
case 6: smp->data.u.sint = info.tcpi_fackets; break;
^~~~~~~~~~~~
__tcpi_fackets
/usr/include/netinet/tcp.h:216:12: note: '__tcpi_fackets' declared here
u_int32_t __tcpi_fackets;
^
src/proto_tcp.c:2405:35: error: no member named 'tcpi_reordering' in 'struct
tcp_info'; did you mean '__tcpi_reordering'?
case 7: smp->data.u.sint = info.tcpi_reordering; break;
^~~~~~~~~~~~~~~
__tcpi_reordering
/usr/include/netinet/tcp.h:232:12: note: '__tcpi_reordering' declared here
u_int32_t __tcpi_reordering;
^
src/proto_tcp.c:2406:35: error: no member named 'tcpi_rcv_rtt' in 'struct
tcp_info'
case 8: smp->data.u.sint = info.tcpi_rcv_rtt; break;
~~~~ ^
src/proto_tcp.c:2407:35: error: no member named 'tcpi_total_retrans' in 'struct
tcp_info'
case 9: smp->data.u.sint = info.tcpi_total_retrans; break;
~~~~ ^
9 errors generated.
Makefile:791: recipe for target 'src/proto_tcp.o' failed
gmake: *** [src/proto_tcp.o] Error 1
So I think we'll have to be tricky here :
- the basic info are accessible when TCP_INFO is declared. These ones
are common between Linux 2.4-4.x and FreeBSD 10.3 :
u_int8_t tcpi_state; /* TCP FSM state. */
u_int8_t tcpi_options; /* Options enabled on conn. */
u_int8_t tcpi_snd_wscale:4, /* RFC1323 send shift value. */
tcpi_rcv_wscale:4; /* RFC1323 recv shift value. */
u_int32_t tcpi_rto; /* Retransmission timeout
(usec). */
u_int32_t tcpi_snd_mss; /* Max segment size for send. */
u_int32_t tcpi_rcv_mss; /* Max segment size for
receive. */
u_int32_t tcpi_last_data_recv; /* Time since last recv data. */
u_int32_t tcpi_rtt; /* Smoothed RTT in usecs. */
u_int32_t tcpi_rttvar; /* RTT variance in usecs. */
u_int32_t tcpi_snd_ssthresh; /* Slow start threshold. */
u_int32_t tcpi_snd_cwnd; /* Send congestion window. */
Thus I consider it is safe to rely only on these ones by default for now.
- A number of other fields are available on FreeBSD with the same names as
on Linux but with "__" prepended (eg: __tcp_sacked). We could have a #define
for this in the compat.h file.
- some more fields are available and differ on these OSes. Afteri
"tcpi_reordering" which is the last common one, we find :
Linux 2.4 : (nothing)
Linux 2.6.12-3.14 :
__u32 tcpi_rcv_rtt;
__u32 tcpi_rcv_space;
__u32 tcpi_total_retrans;
Linux 3.15-4.0 further add :
__u64 tcpi_pacing_rate;
__u64 tcpi_max_pacing_rate;
Linux 4.1 adds :
__u64 tcpi_bytes_acked; /* RFC4898 tcpEStatsAppHCThruOctetsAcked */
__u64 tcpi_bytes_received; /* RFC4898
tcpEStatsAppHCThruOctetsReceived */
Linux 4.2-4.5 add :
__u32 tcpi_segs_out; /* RFC4898 tcpEStatsPerfSegsOut */
__u32 tcpi_segs_in; /* RFC4898 tcpEStatsPerfSegsIn */
Linux 4.6-4.7 add :
__u32 tcpi_notsent_bytes;
__u32 tcpi_min_rtt;
__u32 tcpi_data_segs_in; /* RFC4898 tcpEStatsDataSegsIn */
__u32 tcpi_data_segs_out; /* RFC4898 tcpEStatsDataSegsOut */
FreeBSD 10.3 has this instead :
u_int32_t __tcpi_rcv_rtt;
u_int32_t tcpi_rcv_space; /* Advertised recv window. */
/* FreeBSD extensions to tcp_info. */
u_int32_t tcpi_snd_wnd; /* Advertised send window. */
u_int32_t tcpi_snd_bwnd; /* No longer used. */
u_int32_t tcpi_snd_nxt; /* Next egress seqno */
u_int32_t tcpi_rcv_nxt; /* Next ingress seqno */
u_int32_t tcpi_toe_tid; /* HWTID for TOE endpoints */
u_int32_t tcpi_snd_rexmitpack; /* Retransmitted packets */
u_int32_t tcpi_rcv_ooopack; /* Out-of-order packets */
u_int32_t tcpi_snd_zerowin; /* Zero-sized windows sent */
/* Padding to grow without breaking ABI. */
u_int32_t __tcpi_pad[26]; /* Padding. */
u_int32_t tcpi_rcv_space; /* Advertised recv window. */
One interesting point is that FreeBSD explicitly mentions their intent to
be compatible with Linux's struct :
* The TCP_INFO socket option comes from the Linux 2.6 TCP API, and permits
* the caller to query certain information about the state of a TCP
* connection. We provide an overlapping set of fields with the Linux
* implementation, but since this is a fixed size structure, room has been
* left for growth. In order to maximize potential future compatibility with
* the Linux API, the same variable names and order have been adopted, and
* padding left to make room for omitted fields in case they are added later.
Given that we won't have the proper knowledge of these fields from the
libc itself which is not always in sync with the kernel (especially since
people run whatever OS in containers on a different kernel), I tend to think
that in order to maximize the portability we should simply define our own
structure with our own fields, and possibly adjust it a bit depending on
the operating system (eg: linux vs freebsd).
What do you think ? If you want, as a first step we can merge your patch
as-is with surrounding #ifdef __linux__ and drop the two parts that are
not compatible with 2.4. Probably that we'll have to think about dropping
support for linux < 2.6.something for version 1.8.
CCing Thierry in case he has other ideas on the subject.
Thanks,
Willy
PS: I'm appending your current patch in which I only adjusted the commit
message
>From f5f03ef39089aa4cf4913e6d3e0c5e158f7b718e Mon Sep 17 00:00:00 2001
From: Joe Williams <[email protected]>
Date: Wed, 10 Aug 2016 07:06:44 -0700
Subject: MINOR: tcp: add further tcp info fetchers
Adding on to Thierry's work (http://git.haproxy.org/?p=haproxy.git;h=6310bef5)
I have added a few more fetchers for counters based on the tcp_info struct
maintained by the kernel :
fc_unacked, fc_sacked, fc_retrans, fc_fackets, fc_lost,
fc_reordering, fc_rcv_rtt, fc_total_retrans
---
doc/configuration.txt | 51 +++++++++++++++++++++++++++
src/proto_tcp.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 143 insertions(+), 4 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 430b0ca..ef232b2 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -12767,6 +12767,57 @@ fc_rttvar(<unit>) : integer
operating system does not support TCP_INFO, for example Linux kernels before
2.4, the sample fetch fails.
+fc_unacked(<unit>) : integer
+ Returns the unacked counter measured by the kernel for the client connection.
+ If the server connection is not established, if the connection is not TCP or
+ if the operating system does not support TCP_INFO, for example Linux kernels
+ before 2.4, the sample fetch fails.
+
+fc_sacked(<unit>) : integer
+ Returns the sacked counter measured by the kernel for the client connection.
+ If the server connection is not established, if the connection is not TCP or
+ if the operating system does not support TCP_INFO, for example Linux kernels
+ before 2.4, the sample fetch fails.
+
+fc_retrans(<unit>) : integer
+ Returns the retransmits counter measured by the kernel for the client
+ connection. If the server connection is not established, if the connection is
+ not TCP or if the operating system does not support TCP_INFO, for example
+ Linux kernels before 2.4, the sample fetch fails.
+
+fc_fackets(<unit>) : integer
+ Returns the fack counter measured by the kernel for the client
+ connection. If the server connection is not established, if the connection is
+ not TCP or if the operating system does not support TCP_INFO, for example
+ Linux kernels before 2.4, the sample fetch fails.
+
+fc_lost(<unit>) : integer
+ Returns the lost counter measured by the kernel for the client
+ connection. If the server connection is not established, if the connection is
+ not TCP or if the operating system does not support TCP_INFO, for example
+ Linux kernels before 2.4, the sample fetch fails.
+
+fc_reordering(<unit>) : integer
+ Returns the reordering counter measured by the kernel for the client
+ connection. If the server connection is not established, if the connection is
+ not TCP or if the operating system does not support TCP_INFO, for example
+ Linux kernels before 2.4, the sample fetch fails.
+
+fc_rcv_rtt(<unit>) : integer
+ Returns the Receive side Round Trip Time (RTT) measured by the kernel for the
+ client connection. <unit> is facultative, by default the unit is
milliseconds.
+ <unit> can be set to "ms" for milliseconds or "us" for microseconds. If the
+ server connection is not established, if the connection is not TCP or if the
+ operating system does not support TCP_INFO, for example Linux kernels before
+ 2.4, the sample fetch fails.
+
+fc_total_retrans(<unit>) : integer
+ Returns the total retransmits counter measured by the kernel for the client
+ connection. If the server connection is not established, if the connection is
+ not TCP or if the operating system does not support TCP_INFO, for example
+ Linux kernels before 2.4, the sample fetch fails.
+
+
fe_id : integer
Returns an integer containing the current frontend's id. It can be used in
backends to check from which backend it was called, or to stick all users
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 797ddcb..13f09ef 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -2395,8 +2395,16 @@ static inline int get_tcp_info(const struct arg *args,
struct sample *smp,
/* extract the value. */
smp->data.type = SMP_T_SINT;
switch (val) {
- case 0: smp->data.u.sint = info.tcpi_rtt; break;
- case 1: smp->data.u.sint = info.tcpi_rttvar; break;
+ case 0: smp->data.u.sint = info.tcpi_rtt; break;
+ case 1: smp->data.u.sint = info.tcpi_rttvar; break;
+ case 2: smp->data.u.sint = info.tcpi_unacked; break;
+ case 3: smp->data.u.sint = info.tcpi_sacked; break;
+ case 4: smp->data.u.sint = info.tcpi_lost; break;
+ case 5: smp->data.u.sint = info.tcpi_retrans; break;
+ case 6: smp->data.u.sint = info.tcpi_fackets; break;
+ case 7: smp->data.u.sint = info.tcpi_reordering; break;
+ case 8: smp->data.u.sint = info.tcpi_rcv_rtt; break;
+ case 9: smp->data.u.sint = info.tcpi_total_retrans; break;
default: return 0;
}
@@ -2435,6 +2443,78 @@ smp_fetch_fc_rttvar(const struct arg *args, struct
sample *smp, const char *kw,
return 0;
return 1;
}
+
+/* get the unacked counter on a client connexion */
+static int
+smp_fetch_fc_unacked(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 2))
+ return 0;
+ return 1;
+}
+
+/* get the sacked counter on a client connexion */
+static int
+smp_fetch_fc_sacked(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 3))
+ return 0;
+ return 1;
+}
+
+/* get the lost counter on a client connexion */
+static int
+smp_fetch_fc_lost(const struct arg *args, struct sample *smp, const char *kw,
void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 4))
+ return 0;
+ return 1;
+}
+
+/* get the retrans counter on a client connexion */
+static int
+smp_fetch_fc_retrans(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 5))
+ return 0;
+ return 1;
+}
+
+/* get the fackets counter on a client connexion */
+static int
+smp_fetch_fc_fackets(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 6))
+ return 0;
+ return 1;
+}
+
+/* get the reordering counter on a client connexion */
+static int
+smp_fetch_fc_reordering(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 7))
+ return 0;
+ return 1;
+}
+
+/* get the receive side rtt */
+static int
+smp_fetch_fc_rcv_rtt(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 8))
+ return 0;
+ return 1;
+}
+
+/* get the total retransmits counter on a client connexion */
+static int
+smp_fetch_fc_total_retrans(const struct arg *args, struct sample *smp, const
char *kw, void *private)
+{
+ if (!get_tcp_info(args, smp, 0, 9))
+ return 0;
+ return 1;
+}
#endif
#ifdef IPV6_V6ONLY
@@ -2667,8 +2747,16 @@ static struct sample_fetch_kw_list sample_fetch_keywords
= {ILH, {
{ "src_is_local", smp_fetch_src_is_local, 0, NULL, SMP_T_BOOL,
SMP_USE_L4CLI },
{ "src_port", smp_fetch_sport, 0, NULL, SMP_T_SINT, SMP_USE_L4CLI },
#ifdef TCP_INFO
- { "fc_rtt", smp_fetch_fc_rtt, ARG1(0,STR), NULL, SMP_T_SINT,
SMP_USE_L4CLI },
- { "fc_rttvar", smp_fetch_fc_rttvar, ARG1(0,STR), NULL, SMP_T_SINT,
SMP_USE_L4CLI },
+ { "fc_rtt", smp_fetch_fc_rtt, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_rttvar", smp_fetch_fc_rttvar, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_unacked", smp_fetch_fc_unacked, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_sacked", smp_fetch_fc_sacked, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_retrans", smp_fetch_fc_retrans, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_fackets", smp_fetch_fc_fackets, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_lost", smp_fetch_fc_lost, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_reordering", smp_fetch_fc_reordering, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_rcv_rtt", smp_fetch_fc_rcv_rtt, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
+ { "fc_total_retrans", smp_fetch_fc_total_retrans, ARG1(0,STR), NULL,
SMP_T_SINT, SMP_USE_L4CLI },
#endif
{ /* END */ },
}};
--
1.7.12.1
