From owner-svn-src-stable@freebsd.org Thu May 21 23:34:16 2020 Return-Path: Delivered-To: svn-src-stable@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id E33F32F85C8; Thu, 21 May 2020 23:34:16 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 49SmD84Swkz4VjG; Thu, 21 May 2020 23:34:16 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id 04LNYELm018984; Thu, 21 May 2020 16:34:14 -0700 (PDT) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: (from freebsd@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id 04LNYEY9018983; Thu, 21 May 2020 16:34:14 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <202005212334.04LNYEY9018983@gndrsh.dnsmgr.net> Subject: Re: svn commit: r361342 - in stable/12/sys/netinet: . tcp_stacks In-Reply-To: <202005211946.04LJkBme027322@repo.freebsd.org> To: Richard Scheffenegger Date: Thu, 21 May 2020 16:34:14 -0700 (PDT) CC: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: 49SmD84Swkz4VjG X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:13868, ipnet:69.59.192.0/19, country:US] X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 May 2020 23:34:17 -0000 > Author: rscheff > Date: Thu May 21 19:46:11 2020 > New Revision: 361342 > URL: https://svnweb.freebsd.org/changeset/base/361342 > > Log: Partial merge of r360477: > MFC r360477: Correctly set up the initial TCP congestion window in all cases > > by not including the SYN bit sequence space in cwnd related calculations. > Snd_und is adjusted explicitly in all cases, outside the cwnd update, instead. > > This fixes an off-by-one conformance issue with regular TCP sessions > not using Appropriate Byte Counting (RFC3465), sending one more > packet during the initial window than expected. There should of been a note here: This does NOT contain the merge of the change to bbrv1 since at this time that code does not exist in stable/12, and there is no plan to merge that code to stable/12. > > PR: 235256 > Reviewed by: tuexen (mentor), rgrimes (mentor, blanket) > Approved by: tuexen (mentor), rgrimes (mentor, blanket) > MFC after: 3 weeks > Sponsored by: NetApp, Inc. > Differential Revision: https://reviews.freebsd.org/D19000 > > Modified: > stable/12/sys/netinet/tcp_input.c > stable/12/sys/netinet/tcp_stacks/rack.c > Directory Properties: > stable/12/ (props changed) > > Modified: stable/12/sys/netinet/tcp_input.c > ============================================================================== > --- stable/12/sys/netinet/tcp_input.c Thu May 21 19:45:14 2020 (r361341) > +++ stable/12/sys/netinet/tcp_input.c Thu May 21 19:46:11 2020 (r361342) > @@ -1519,7 +1519,7 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru > struct tcpcb *tp, int drop_hdrlen, int tlen, uint8_t iptos) > { > int thflags, acked, ourfinisacked, needoutput = 0, sack_changed; > - int rstreason, todrop, win; > + int rstreason, todrop, win, incforsyn = 0; > uint32_t tiwin; > uint16_t nsegs; > char *s; > @@ -2432,12 +2432,6 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru > if (IS_FASTOPEN(tp->t_flags) && tp->t_tfo_pending) { > tcp_fastopen_decrement_counter(tp->t_tfo_pending); > tp->t_tfo_pending = NULL; > - > - /* > - * Account for the ACK of our SYN prior to > - * regular ACK processing below. > - */ > - tp->snd_una++; > } > if (tp->t_flags & TF_NEEDFIN) { > tcp_state_change(tp, TCPS_FIN_WAIT_1); > @@ -2458,6 +2452,13 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru > tcp_timer_activate(tp, TT_KEEP, TP_KEEPIDLE(tp)); > } > /* > + * Account for the ACK of our SYN prior to > + * regular ACK processing below, except for > + * simultaneous SYN, which is handled later. > + */ > + if (SEQ_GT(th->th_ack, tp->snd_una) && !(tp->t_flags & TF_NEEDSYN)) > + incforsyn = 1; > + /* > * If segment contains data or ACK, will call tcp_reass() > * later; if not, do so now to pass queued data to user. > */ > @@ -2751,6 +2752,15 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru > process_ACK: > INP_WLOCK_ASSERT(tp->t_inpcb); > > + /* > + * Adjust for the SYN bit in sequence space, > + * but don't account for it in cwnd calculations. > + * This is for the SYN_RECEIVED, non-simultaneous > + * SYN case. SYN_SENT and simultaneous SYN are > + * treated elsewhere. > + */ > + if (incforsyn) > + tp->snd_una++; > acked = BYTES_THIS_ACK(tp, th); > KASSERT(acked >= 0, ("%s: acked unexepectedly negative " > "(tp->snd_una=%u, th->th_ack=%u, tp=%p, m=%p)", __func__, > > Modified: stable/12/sys/netinet/tcp_stacks/rack.c > ============================================================================== > --- stable/12/sys/netinet/tcp_stacks/rack.c Thu May 21 19:45:14 2020 (r361341) > +++ stable/12/sys/netinet/tcp_stacks/rack.c Thu May 21 19:46:11 2020 (r361342) > @@ -5580,12 +5580,6 @@ rack_do_syn_recv(struct mbuf *m, struct tcphdr *th, st > if (IS_FASTOPEN(tp->t_flags) && tp->t_tfo_pending) { > tcp_fastopen_decrement_counter(tp->t_tfo_pending); > tp->t_tfo_pending = NULL; > - > - /* > - * Account for the ACK of our SYN prior to > - * regular ACK processing below. > - */ > - tp->snd_una++; > } > if (tp->t_flags & TF_NEEDFIN) { > tcp_state_change(tp, TCPS_FIN_WAIT_1); > @@ -5603,6 +5597,13 @@ rack_do_syn_recv(struct mbuf *m, struct tcphdr *th, st > if (!IS_FASTOPEN(tp->t_flags)) > cc_conn_init(tp); > } > + /* > + * Account for the ACK of our SYN prior to > + * regular ACK processing below, except for > + * simultaneous SYN, which is handled later. > + */ > + if (SEQ_GT(th->th_ack, tp->snd_una) && !(tp->t_flags & TF_NEEDSYN)) > + tp->snd_una++; > /* > * If segment contains data or ACK, will call tcp_reass() later; if > * not, do so now to pass queued data to user. > -- Rod Grimes rgrimes@freebsd.org