From owner-dev-commits-src-all@freebsd.org Tue Apr 20 12:56:30 2021 Return-Path: Delivered-To: dev-commits-src-all@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 6131B5F4F70; Tue, 20 Apr 2021 12:56:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (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 "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FPkG626CHz4hpJ; Tue, 20 Apr 2021 12:56:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 3B39612CFE; Tue, 20 Apr 2021 12:56:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 13KCuUkj005597; Tue, 20 Apr 2021 12:56:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 13KCuU63005596; Tue, 20 Apr 2021 12:56:30 GMT (envelope-from git) Date: Tue, 20 Apr 2021 12:56:30 GMT Message-Id: <202104201256.13KCuU63005596@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Richard Scheffenegger Subject: git: a649f1f6fd7a - main - tcp: Deal with DSACKs, and adjust rescue hole on success. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rscheff X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: a649f1f6fd7a098ab173a69fe87916c04a8c6f8d Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Apr 2021 12:56:30 -0000 The branch main has been updated by rscheff: URL: https://cgit.FreeBSD.org/src/commit/?id=a649f1f6fd7a098ab173a69fe87916c04a8c6f8d commit a649f1f6fd7a098ab173a69fe87916c04a8c6f8d Author: Richard Scheffenegger AuthorDate: 2021-04-20 12:53:56 +0000 Commit: Richard Scheffenegger CommitDate: 2021-04-20 12:54:28 +0000 tcp: Deal with DSACKs, and adjust rescue hole on success. When a rescue retransmission is successful, rather than inserting new holes to the left of it, adjust the old rescue entry to cover the missed sequence space. Also, as snd_fack may be stale by that point, pull it forward in order to never create a hole left of snd_una/th_ack. Finally, with DSACKs, tcp_sack_doack() may be called with new full ACKs but a DSACK block. Account for this eventuality properly to keep sacked_bytes >= 0. MFC after: 3 days Reviewed By: kbowling, tuexen, #transport Sponsored by: NetApp, Inc. Differential Revision: https://reviews.freebsd.org/D29835 --- sys/netinet/tcp_sack.c | 76 +++++++++++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/sys/netinet/tcp_sack.c b/sys/netinet/tcp_sack.c index fffba2e75045..bfe40854c76b 100644 --- a/sys/netinet/tcp_sack.c +++ b/sys/netinet/tcp_sack.c @@ -497,7 +497,7 @@ static struct sackhole * tcp_sackhole_insert(struct tcpcb *tp, tcp_seq start, tcp_seq end, struct sackhole *after) { - struct sackhole *hole, *tail; + struct sackhole *hole; /* Allocate a new SACK hole. */ hole = tcp_sackhole_alloc(tp, start, end); @@ -508,15 +508,7 @@ tcp_sackhole_insert(struct tcpcb *tp, tcp_seq start, tcp_seq end, if (after != NULL) TAILQ_INSERT_AFTER(&tp->snd_holes, after, hole, scblink); else - /* - * With Rescue Retransmission, new holes may need to - * be inserted just before the tail. - */ - if (((tail = TAILQ_LAST_FAST(&tp->snd_holes, sackhole, - scblink)) != NULL) && SEQ_LEQ(end, tail->start)) - TAILQ_INSERT_BEFORE(tail, hole, scblink); - else - TAILQ_INSERT_TAIL(&tp->snd_holes, hole, scblink); + TAILQ_INSERT_TAIL(&tp->snd_holes, hole, scblink); /* Update SACK hint. */ if (tp->sackhint.nexthole == NULL) @@ -573,6 +565,15 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack) left_edge_delta = th_ack - tp->snd_una; sack_blocks[num_sack_blks].start = tp->snd_una; sack_blocks[num_sack_blks++].end = th_ack; + /* + * Pulling snd_fack forward if we got here + * due to DSACK blocks + */ + if (SEQ_LT(tp->snd_fack, th_ack)) { + delivered_data += th_ack - tp->snd_una; + tp->snd_fack = th_ack; + sack_changed = 1; + } } /* * Append received valid SACK blocks to sack_blocks[], but only if we @@ -642,35 +643,52 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack) tp->sackhint.last_sack_ack = sblkp->end; if (SEQ_LT(tp->snd_fack, sblkp->start)) { /* - * The highest SACK block is beyond fack. Append new SACK - * hole at the tail. If the second or later highest SACK - * blocks are also beyond the current fack, they will be - * inserted by way of hole splitting in the while-loop below. + * The highest SACK block is beyond fack. First, + * check if there was a successful Rescue Retransmission, + * and move this hole left. With normal holes, snd_fack + * is always to the right of the end. */ - temp = tcp_sackhole_insert(tp, tp->snd_fack,sblkp->start,NULL); - if (temp != NULL) { + if (((temp = TAILQ_LAST(&tp->snd_holes, sackhole_head)) != NULL) && + SEQ_LEQ(tp->snd_fack,temp->end)) { + temp->start = SEQ_MAX(tp->snd_fack, SEQ_MAX(tp->snd_una, th_ack)); + temp->end = sblkp->start; + temp->rxmit = temp->start; delivered_data += sblkp->end - sblkp->start; tp->snd_fack = sblkp->end; - /* Go to the previous sack block. */ sblkp--; sack_changed = 1; } else { /* - * We failed to add a new hole based on the current - * sack block. Skip over all the sack blocks that - * fall completely to the right of snd_fack and - * proceed to trim the scoreboard based on the - * remaining sack blocks. This also trims the - * scoreboard for th_ack (which is sack_blocks[0]). + * Append a new SACK hole at the tail. If the + * second or later highest SACK blocks are also + * beyond the current fack, they will be inserted + * by way of hole splitting in the while-loop below. */ - while (sblkp >= sack_blocks && - SEQ_LT(tp->snd_fack, sblkp->start)) - sblkp--; - if (sblkp >= sack_blocks && - SEQ_LT(tp->snd_fack, sblkp->end)) { - delivered_data += sblkp->end - tp->snd_fack; + temp = tcp_sackhole_insert(tp, tp->snd_fack,sblkp->start,NULL); + if (temp != NULL) { + delivered_data += sblkp->end - sblkp->start; tp->snd_fack = sblkp->end; + /* Go to the previous sack block. */ + sblkp--; sack_changed = 1; + } else { + /* + * We failed to add a new hole based on the current + * sack block. Skip over all the sack blocks that + * fall completely to the right of snd_fack and + * proceed to trim the scoreboard based on the + * remaining sack blocks. This also trims the + * scoreboard for th_ack (which is sack_blocks[0]). + */ + while (sblkp >= sack_blocks && + SEQ_LT(tp->snd_fack, sblkp->start)) + sblkp--; + if (sblkp >= sack_blocks && + SEQ_LT(tp->snd_fack, sblkp->end)) { + delivered_data += sblkp->end - tp->snd_fack; + tp->snd_fack = sblkp->end; + sack_changed = 1; + } } } } else if (SEQ_LT(tp->snd_fack, sblkp->end)) {