On 4/25/23 18:26, Cheng Cui wrote:
The branch main has been updated by cc:
URL:
https://cgit.FreeBSD.org/src/commit/?id=d090464ecd4af5cd400ef5cbbfe8409d019eac34
commit d090464ecd4af5cd400ef5cbbfe8409d019eac34
Author: Cheng Cui <[email protected]>
AuthorDate: 2023-04-25 11:52:28 +0000
Commit: Cheng Cui <[email protected]>
CommitDate: 2023-04-25 17:26:39 +0000
Change the unit of srtt and rto to usec, inspired by these in struct
"tcp_info". Therefore, no need hz and tcp_rtt_scale in the headline of the log.
Update the man page as well.
Summary: Simplify srtt and rto values in siftr log.
Test Plan:
Tested in Emulab testbed:
cc@s1:~ % sudo sysctl net.inet.siftr
net.inet.siftr.port_filter: 0
net.inet.siftr.genhashes: 0
net.inet.siftr.ppl: 1
net.inet.siftr.logfile: /var/log/siftr.log
net.inet.siftr.enabled: 0
cc@s1:~ % sudo sysctl net.inet.siftr.port_filter=5001
net.inet.siftr.port_filter: 0 -> 5001
cc@s1:~ % sudo sysctl net.inet.siftr.enabled=1
net.inet.siftr.enabled: 0 -> 1
cc@s1:~ %
cc@s1:~ % iperf -c r1 -n 1M
------------------------------------------------------------
Client connecting to r1, TCP port 5001
TCP window size: 32.0 KByte (default)
------------------------------------------------------------
[ 1] local 10.1.1.2 port 33817 connected with 10.1.1.3 port 5001
[ ID] Interval Transfer Bandwidth
[ 1] 0.00-0.91 sec 1.00 MBytes 9.22 Mbits/sec
cc@s1:~ % sudo sysctl net.inet.siftr.enabled=0
net.inet.siftr.enabled: 1 -> 0
cc@s1:~ % ll /var/log/siftr.log
-rw-r--r-- 1 root wheel 91K Apr 25 09:38 /var/log/siftr.log
cc@s1:~ % cat /var/log/siftr.log
enable_time_secs=1682437111 enable_time_usecs=121115
siftrver=1.3.0 sysname=FreeBSD sysver=1400088 ipmode=4
o,0x00000000,1682437125.907343,10.1.1.2,33817,10.1.1.3,5001,1073725440,1073725440,2,0,0,0,0,2,536,0,1,672,1000000,32768,0,65536,0,0,0,0,0
i,0x00000000,1682437126.106759,10.1.1.2,33817,10.1.1.3,5001,1073725440,1073725440,2,0,0,0,0,2,536,0,1,672,1000000,32768,0,65536,0,1,0,0,0
o,0x00000000,1682437126.106767,10.1.1.2,33817,10.1.1.3,5001,1073725440,14480,2,65535,65700,9,9,4,1460,201000,1,16778209,803000,33580,0,65700,0,0,0,0,0
o,0x00000000,1682437126.107141,10.1.1.2,33817,10.1.1.3,5001,1073725440,14480,2,65535,65700,9,9,4,1460,201000,1,16778208,803000,33580,60,65700,0,0,0,0,0
...
i,0x00000000,1682437127.016754,10.1.1.2,33817,10.1.1.3,5001,1073725440,606109,1030,748544,66048,9,9,9,1460,100812,1,1008,303000,475948,0,65700,0,0,0,0,0
o,0x00000000,1682437127.016759,10.1.1.2,33817,10.1.1.3,5001,1073725440,606109,1030,748544,66048,9,9,10,1460,100812,1,1011,303000,475948,0,65700,0,0,0,0,0
disable_time_secs=1682437131 disable_time_usecs=767582
num_inbound_tcp_pkts=371 num_outbound_tcp_pkts=186
total_tcp_pkts=557 num_inbound_skipped_pkts_malloc=0
num_outbound_skipped_pkts_malloc=0 num_inbound_skipped_pkts_tcpcb=0
num_outbound_skipped_pkts_tcpcb=0 num_inbound_skipped_pkts_inpcb=0
num_outbound_skipped_pkts_inpcb=0 total_skipped_tcp_pkts=0
flow_list=10.1.1.2;33817-10.1.1.3;5001,
Reviewers: rscheff, tuexen
Approved by: rscheff, tuexen
Subscribers: imp, melifaro, glebius
Differential Revision: https://reviews.freebsd.org/D39803
---
share/man/man4/siftr.4 | 21 +++------------------
sys/netinet/siftr.c | 34 +++++++++++++++-------------------
2 files changed, 18 insertions(+), 37 deletions(-)
Hello,
This commit message is messy, and not easily understood. In particular
the subject line should be shorter, and the 'Test Plan' probably should
have been removed completely.
The full set of guidelines for commit messages is documented in the
Committers Guide:
https://docs.freebsd.org/en/articles/committers-guide/#commit-log-message
Next time you will need to clean up the message before pushing.
Unfortunately the default output of the 'arc' tool does not generate a
message that meets the guidelines. Ask your mentors for more information
if you need it. Now you know :)
Cheers,
Mitchell
diff --git a/share/man/man4/siftr.4 b/share/man/man4/siftr.4
index 0406a38779b6..78eb755ddef1 100644
--- a/share/man/man4/siftr.4
+++ b/share/man/man4/siftr.4
@@ -29,7 +29,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd April 24, 2023
+.Dd April 25, 2023
.Dt SIFTR 4
.Os
.Sh NAME
@@ -175,14 +175,6 @@ version of
.Nm .
.El
.Bl -tag -offset indent -width Va
-.It Va hz
-tick rate of the kernel in ticks per second.
-.El
-.Bl -tag -offset indent -width Va
-.It Va tcp_rtt_scale
-smoothed RTT estimate scaling factor.
-.El
-.Bl -tag -offset indent -width Va
.It Va sysname
operating system name.
.El
@@ -294,11 +286,7 @@ The maximum segment size for the flow, in bytes.
.El
.Bl -tag -offset indent -width Va
.It Va 17
-The current smoothed RTT estimate for the flow, in units of TCP_RTT_SCALE * HZ,
-where TCP_RTT_SCALE is a define found in tcp_var.h, and HZ is the kernel's tick
-timer.
-Divide by TCP_RTT_SCALE * HZ to get the RTT in secs.
-TCP_RTT_SCALE and HZ are reported in the enable log message.
+The current smoothed RTT estimate for the flow, in units of microsecond.
.El
.Bl -tag -offset indent -width Va
.It Va 18
@@ -313,10 +301,7 @@ for information about the various flags.
.El
.Bl -tag -offset indent -width Va
.It Va 20
-The current retransmission timeout length for the flow, in units of HZ, where
HZ
-is the kernel's tick timer.
-Divide by HZ to get the timeout length in seconds.
-HZ is reported in the enable log message.
+The current retransmission timeout length for the flow, in units microsecond.
.El
.Bl -tag -offset indent -width Va
.It Va 21
diff --git a/sys/netinet/siftr.c b/sys/netinet/siftr.c
index a23980704e17..05104627910d 100644
--- a/sys/netinet/siftr.c
+++ b/sys/netinet/siftr.c
@@ -207,11 +207,8 @@ struct pkt_node {
int conn_state;
/* Max Segment Size (bytes). */
u_int max_seg_size;
- /*
- * Smoothed RTT stored as found in the TCP control block
- * in units of (TCP_RTT_SCALE*hz).
- */
- int smoothed_rtt;
+ /* Smoothed RTT (usecs). */
+ uint32_t srtt;
/* Is SACK enabled? */
u_char sack_enabled;
/* Window scaling for snd window. */
@@ -220,8 +217,8 @@ struct pkt_node {
u_char rcv_scale;
/* TCP control block flags. */
u_int flags;
- /* Retransmit timeout length. */
- int rxt_length;
+ /* Retransmission timeout (usec). */
+ uint32_t rto;
/* Size of the TCP send buffer in bytes. */
u_int snd_buf_hiwater;
/* Current num bytes in the send socket buffer. */
@@ -453,7 +450,7 @@ siftr_process_pkt(struct pkt_node * pkt_node)
MAX_LOG_MSG_LEN,
"%c,0x%08x,%zd.%06ld,%x:%x:%x:%x:%x:%x:%x:%x,%u,%x:%x:%x:"
"%x:%x:%x:%x:%x,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,"
- "%u,%d,%u,%u,%u,%u,%u,%u,%u,%u\n",
+ "%u,%u,%u,%u,%u,%u,%u,%u,%u,%u\n",
direction[pkt_node->direction],
pkt_node->hash,
pkt_node->tval.tv_sec,
@@ -485,10 +482,10 @@ siftr_process_pkt(struct pkt_node * pkt_node)
pkt_node->rcv_scale,
pkt_node->conn_state,
pkt_node->max_seg_size,
- pkt_node->smoothed_rtt,
+ pkt_node->srtt,
pkt_node->sack_enabled,
pkt_node->flags,
- pkt_node->rxt_length,
+ pkt_node->rto,
pkt_node->snd_buf_hiwater,
pkt_node->snd_buf_cc,
pkt_node->rcv_buf_hiwater,
@@ -512,7 +509,7 @@ siftr_process_pkt(struct pkt_node * pkt_node)
log_buf->ae_bytesused = snprintf(log_buf->ae_data,
MAX_LOG_MSG_LEN,
"%c,0x%08x,%jd.%06ld,%u.%u.%u.%u,%u,%u.%u.%u.%u,%u,%u,%u,"
-
"%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%d,%u,%u,%u,%u,%u,%u,%u,%u\n",
+
"%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u,%u\n",
direction[pkt_node->direction],
pkt_node->hash,
(intmax_t)pkt_node->tval.tv_sec,
@@ -536,10 +533,10 @@ siftr_process_pkt(struct pkt_node * pkt_node)
pkt_node->rcv_scale,
pkt_node->conn_state,
pkt_node->max_seg_size,
- pkt_node->smoothed_rtt,
+ pkt_node->srtt,
pkt_node->sack_enabled,
pkt_node->flags,
- pkt_node->rxt_length,
+ pkt_node->rto,
pkt_node->snd_buf_hiwater,
pkt_node->snd_buf_cc,
pkt_node->rcv_buf_hiwater,
@@ -785,10 +782,10 @@ siftr_siftdata(struct pkt_node *pn, struct inpcb *inp,
struct tcpcb *tp,
pn->rcv_scale = tp->rcv_scale;
pn->conn_state = tp->t_state;
pn->max_seg_size = tp->t_maxseg;
- pn->smoothed_rtt = tp->t_srtt;
+ pn->srtt = ((u_int64_t)tp->t_srtt * tick) >> TCP_RTT_SHIFT;
pn->sack_enabled = (tp->t_flags & TF_SACK_PERMIT) != 0;
pn->flags = tp->t_flags;
- pn->rxt_length = tp->t_rxtcur;
+ pn->rto = tp->t_rxtcur * tick;
pn->snd_buf_hiwater = inp->inp_socket->so_snd.sb_hiwat;
pn->snd_buf_cc = sbused(&inp->inp_socket->so_snd);
pn->rcv_buf_hiwater = inp->inp_socket->so_rcv.sb_hiwat;
@@ -1270,10 +1267,9 @@ siftr_manage_ops(uint8_t action)
sbuf_printf(s,
"enable_time_secs=%jd\tenable_time_usecs=%06ld\t"
- "siftrver=%s\thz=%u\ttcp_rtt_scale=%u\tsysname=%s\t"
- "sysver=%u\tipmode=%u\n",
- (intmax_t)tval.tv_sec, tval.tv_usec, MODVERSION_STR, hz,
- TCP_RTT_SCALE, SYS_NAME, __FreeBSD_version, SIFTR_IPMODE);
+ "siftrver=%s\tsysname=%s\tsysver=%u\tipmode=%u\n",
+ (intmax_t)tval.tv_sec, tval.tv_usec, MODVERSION_STR,
+ SYS_NAME, __FreeBSD_version, SIFTR_IPMODE);
sbuf_finish(s);
alq_writen(siftr_alq, sbuf_data(s), sbuf_len(s), ALQ_WAITOK);
