From nobody Tue Apr 25 21:52:56 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Q5bMt1Bvpz46mYC; Tue, 25 Apr 2023 21:52:58 +0000 (UTC) (envelope-from mhorne@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Q5bMt0G29z3hn8; Tue, 25 Apr 2023 21:52:58 +0000 (UTC) (envelope-from mhorne@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682459578; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xLTxdPuel4FDYopvTe/21gLy4JQa5VkkUGNfT4Pwo9M=; b=TRN8hZqYXWFzX+s4/ZyHsjQOzGFFHqrnO+MWFlWImltja9L4kRMkTBKKSIsZFuCnrfU+2A Lr7zjo7vfkMp+iLZ6EmyIkJjCYcHtB0nip10pnpz+H5DXYhMav07Tjrupf7Mk6Kfgt8gJp a2eUL1B5d+db09KiEahIF5SCCowXRCQAiy4qqUqpYGcD+RrNSS/yhi0yAbg3XwE6lNuw5P 6Ho8hqvSxrbO7+ezuR2ZZ8nyor9iFnjywqM9gdzTYBKtI0piWRkEFctb4KKrK9HL2b0aio 36fXR+s3AmsBu8rhbu1s0jxFCV6JBlaXaQWYcAdcMx15uGJ6ScTZInfMynSemg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682459578; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xLTxdPuel4FDYopvTe/21gLy4JQa5VkkUGNfT4Pwo9M=; b=k/bnbq1+XASZ3M49DfFavjYumILBteDs6ZWAvFRMb1IFfdu40gqGaraHKTc1H+qdIuttlx jsJLJj2WlT5kkVinwIThp/IO8Lx8EBVFZurIP8PDv6g+4N85pX87YzXCzJu7p3zCaldh3A PfFpGNE1y/SPruC8lBpDQmv5yQLkJlTYpSf/ZR5emjxswtjBVIeDBny7C18Ub9rSFr0j8p I+UH1voc7/Kdxa3jWqamuZX8+0JAOUq4n5eTaphuvR8/4baCJeKffRtPOTR26G8hKBByhq NStrYFcXwpuFs4pxCze31GCGeKpq4txWcIqyxg3WIUzrMVzzfnll3ANAyrj7ww== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1682459578; a=rsa-sha256; cv=none; b=GeiI5ARghMv/pj0PhjFl3EmwGuOJG+9fpuKnyCj+162lrvVXb+zfbnDT7lrCkjjB0kHXVS Ip2cbfKR0+HzeVtHbtWTnAVbaidZ72S2Wktva6d5f22WGuU2gzYnVqVgGqntSvotGNhPER MKJopjoI91RtBrI28E40NbV1VRpvFVjclMw5UO1jr4DVUle4olOts6ByxD5j7mnZd6PpRs 4Gx0CH/oCDiSLV34orK1Vz+a+eWYYKOFToBV15O/ctINChbwN6dmDzaJOZ9/1bIVp/GYaI 3nbBPOdssjPOnWNdalY8vAO0fZ/8+l2iVWQIFESuvYxJx5KYy2bYKLoXkQSD6w== Received: from [192.168.1.151] (host-173-212-76-127.public.eastlink.ca [173.212.76.127]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: mhorne) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Q5bMs4ZbXzJtb; Tue, 25 Apr 2023 21:52:57 +0000 (UTC) (envelope-from mhorne@freebsd.org) Message-ID: <1ead933c-2567-7b79-8761-2f900ccf9b76@freebsd.org> Date: Tue, 25 Apr 2023 18:52:56 -0300 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: git: d090464ecd4a - main - 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. Content-Language: en-CA To: Cheng Cui , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202304252126.33PLQsKe024634@gitrepo.freebsd.org> From: Mitchell Horne In-Reply-To: <202304252126.33PLQsKe024634@gitrepo.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ThisMailContainsUnwantedMimeParts: N 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 > AuthorDate: 2023-04-25 11:52:28 +0000 > Commit: Cheng Cui > 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);