Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 19 Apr 2021 15:16:16 -0700
From:      Gleb Smirnoff <glebius@freebsd.org>
To:        Richard Scheffenegger <rscheff@freebsd.org>
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:  <YH4BMAvUbMZNkGW8@FreeBSD.org>
In-Reply-To: <202104182112.13ILCMoW039801@gitrepo.freebsd.org>
References:  <202104182112.13ILCMoW039801@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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 <rscheff@FreeBSD.org>
R> AuthorDate: 2021-04-18 20:14:14 +0000
R> Commit:     Richard Scheffenegger <rscheff@FreeBSD.org>
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YH4BMAvUbMZNkGW8>