From owner-dev-commits-src-all@freebsd.org Mon Apr 19 22:16:18 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 7590F5D9321; Mon, 19 Apr 2021 22:16:18 +0000 (UTC) (envelope-from glebius@freebsd.org) Received: from cell.glebi.us (glebi.us [162.251.186.162]) (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 (2048 bits) client-digest SHA256) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FPLkV1ZPgz3BvF; Mon, 19 Apr 2021 22:16:17 +0000 (UTC) (envelope-from glebius@freebsd.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.16.1/8.16.1) with ESMTPS id 13JMGG1u018269 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Mon, 19 Apr 2021 15:16:16 -0700 (PDT) (envelope-from glebius@freebsd.org) Received: (from glebius@localhost) by cell.glebi.us (8.16.1/8.16.1/Submit) id 13JMGG1A018268; Mon, 19 Apr 2021 15:16:16 -0700 (PDT) (envelope-from glebius@freebsd.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@freebsd.org using -f Date: Mon, 19 Apr 2021 15:16:16 -0700 From: Gleb Smirnoff To: Richard Scheffenegger Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: b87cf2bc841b - main - tcp: keep SACK scoreboard sorted when doing rescue retransmission Message-ID: References: <202104182112.13ILCMoW039801@gitrepo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202104182112.13ILCMoW039801@gitrepo.freebsd.org> X-Rspamd-Queue-Id: 4FPLkV1ZPgz3BvF X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [0.00 / 15.00]; local_wl_from(0.00)[freebsd.org]; ASN(0.00)[asn:27348, ipnet:162.251.186.0/24, country:US] 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: Mon, 19 Apr 2021 22:16:18 -0000 On Sun, Apr 18, 2021 at 09:12:22PM +0000, Richard Scheffenegger wrote: R> The branch main has been updated by rscheff: R> R> URL: https://cgit.FreeBSD.org/src/commit/?id=b87cf2bc841b2a336b7f0c6cd89573610412a84f R> R> commit b87cf2bc841b2a336b7f0c6cd89573610412a84f R> Author: Richard Scheffenegger R> AuthorDate: 2021-04-18 20:14:14 +0000 R> Commit: Richard Scheffenegger R> CommitDate: 2021-04-18 21:11:10 +0000 R> R> tcp: keep SACK scoreboard sorted when doing rescue retransmission R> R> Reviewed By: tuexen, kbowling, #transport R> MFC after: 3 days R> Sponsored by: NetApp, Inc. R> Differential Revision: https://reviews.freebsd.org/D29825 R> --- R> sys/netinet/tcp_sack.c | 12 ++++++++++-- R> 1 file changed, 10 insertions(+), 2 deletions(-) R> R> diff --git a/sys/netinet/tcp_sack.c b/sys/netinet/tcp_sack.c R> index a28bb40a8332..fffba2e75045 100644 R> --- a/sys/netinet/tcp_sack.c R> +++ b/sys/netinet/tcp_sack.c R> @@ -497,7 +497,7 @@ static struct sackhole * R> tcp_sackhole_insert(struct tcpcb *tp, tcp_seq start, tcp_seq end, R> struct sackhole *after) R> { R> - struct sackhole *hole; R> + struct sackhole *hole, *tail; R> R> /* Allocate a new SACK hole. */ R> hole = tcp_sackhole_alloc(tp, start, end); R> @@ -508,7 +508,15 @@ tcp_sackhole_insert(struct tcpcb *tp, tcp_seq start, tcp_seq end, R> if (after != NULL) R> TAILQ_INSERT_AFTER(&tp->snd_holes, after, hole, scblink); R> else R> - TAILQ_INSERT_TAIL(&tp->snd_holes, hole, scblink); R> + /* R> + * With Rescue Retransmission, new holes may need to R> + * be inserted just before the tail. R> + */ R> + if (((tail = TAILQ_LAST_FAST(&tp->snd_holes, sackhole, R> + scblink)) != NULL) && SEQ_LEQ(end, tail->start)) R> + TAILQ_INSERT_BEFORE(tail, hole, scblink); R> + else R> + TAILQ_INSERT_TAIL(&tp->snd_holes, hole, scblink); IMHO, formatting here violates style and also may be misleading. Should be: if (after != NULL) TAILQ_INSERT_AFTER(&tp->snd_holes, after, hole, scblink); else if (((tail = TAILQ_LAST_FAST(&tp->snd_holes, sackhole, scblink)) != NULL) && SEQ_LEQ(end, tail->start)) /* * With Rescue Retransmission, new holes may need to * be inserted just before the tail. */ TAILQ_INSERT_BEFORE(tail, hole, scblink); else TAILQ_INSERT_TAIL(&tp->snd_holes, hole, scblink); -- Gleb Smirnoff