Improve SACK support code for RFC6675 and PRR

Adding proper accounting of sacked_bytes and (per-ACK)
delivered data to the SACK scoreboard. This will
allow more aspects of RFC6675 to be implemented as well
as Proportional Rate Reduction (RFC6937).

Prior to this change, the pipe calculation controlled with
net.inet.tcp.rfc6675_pipe was also susceptible to incorrect
results when more than 3 (or 4) holes in the sequence space
were present, which can no longer all fit into a single
ACK's SACK option.

Reviewed by:	kbowling, rgrimes (mentor)
Approved by:	rgrimes (mentor, blanket)
MFC after:	3 weeks
Sponsored by:	NetApp, Inc.
Differential Revision:	https://reviews.freebsd.org/D18624
This commit is contained in:
Richard Scheffenegger 2020-08-13 16:30:09 +00:00
parent 958a094323
commit f359d6ebbc
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=364195
3 changed files with 36 additions and 14 deletions

View File

@ -2673,9 +2673,16 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so,
tp->t_dupacks = 0;
/*
* If this ack also has new SACK info, increment the
* counter as per rfc6675.
* counter as per rfc6675. The variable
* sack_changed tracks all changes to the SACK
* scoreboard, including when partial ACKs without
* SACK options are received, and clear the scoreboard
* from the left side. Such partial ACKs should not be
* counted as dupacks here.
*/
if ((tp->t_flags & TF_SACK_PERMIT) && sack_changed)
if ((tp->t_flags & TF_SACK_PERMIT) &&
(to.to_flags & TOF_SACK) &&
sack_changed)
tp->t_dupacks++;
}

View File

@ -535,9 +535,7 @@ tcp_sackhole_remove(struct tcpcb *tp, struct sackhole *hole)
* tp->snd_holes is an ordered list of holes (oldest to newest, in terms of
* the sequence space).
* Returns 1 if incoming ACK has previously unknown SACK information,
* 0 otherwise. Note: We treat (snd_una, th_ack) as a sack block so any changes
* to that (i.e. left edge moving) would also be considered a change in SACK
* information which is slightly different than rfc6675.
* 0 otherwise.
*/
int
tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
@ -545,16 +543,21 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
struct sackhole *cur, *temp;
struct sackblk sack, sack_blocks[TCP_MAX_SACK + 1], *sblkp;
int i, j, num_sack_blks, sack_changed;
int delivered_data, left_edge_delta;
INP_WLOCK_ASSERT(tp->t_inpcb);
num_sack_blks = 0;
sack_changed = 0;
delivered_data = 0;
left_edge_delta = 0;
/*
* If SND.UNA will be advanced by SEG.ACK, and if SACK holes exist,
* treat [SND.UNA, SEG.ACK) as if it is a SACK block.
* Account changes to SND.UNA always in delivered data.
*/
if (SEQ_LT(tp->snd_una, th_ack) && !TAILQ_EMPTY(&tp->snd_holes)) {
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;
}
@ -563,7 +566,6 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
* received new blocks from the other side.
*/
if (to->to_flags & TOF_SACK) {
tp->sackhint.sacked_bytes = 0; /* reset */
for (i = 0; i < to->to_nsacks; i++) {
bcopy((to->to_sacks + i * TCPOLEN_SACK),
&sack, sizeof(sack));
@ -576,8 +578,6 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
SEQ_GT(sack.end, tp->snd_una) &&
SEQ_LEQ(sack.end, tp->snd_max)) {
sack_blocks[num_sack_blks++] = sack;
tp->sackhint.sacked_bytes +=
(sack.end-sack.start);
}
}
}
@ -602,7 +602,7 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
}
}
}
if (TAILQ_EMPTY(&tp->snd_holes))
if (TAILQ_EMPTY(&tp->snd_holes)) {
/*
* Empty scoreboard. Need to initialize snd_fack (it may be
* uninitialized or have a bogus value). Scoreboard holes
@ -611,6 +611,8 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
* scoreboard).
*/
tp->snd_fack = SEQ_MAX(tp->snd_una, th_ack);
tp->sackhint.sacked_bytes = 0; /* reset */
}
/*
* In the while-loop below, incoming SACK blocks (sack_blocks[]) and
* SACK holes (snd_holes) are traversed from their tails with just
@ -634,6 +636,7 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
*/
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--;
@ -651,11 +654,15 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
SEQ_LT(tp->snd_fack, sblkp->start))
sblkp--;
if (sblkp >= sack_blocks &&
SEQ_LT(tp->snd_fack, sblkp->end))
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)) {
/* fack is advanced. */
delivered_data += sblkp->end - tp->snd_fack;
tp->snd_fack = sblkp->end;
sack_changed = 1;
}
@ -689,6 +696,7 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
/* Data acks at least the beginning of hole. */
if (SEQ_GEQ(sblkp->end, cur->end)) {
/* Acks entire hole, so delete hole. */
delivered_data += (cur->end - cur->start);
temp = cur;
cur = TAILQ_PREV(cur, sackhole_head, scblink);
tcp_sackhole_remove(tp, temp);
@ -700,6 +708,7 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
continue;
} else {
/* Move start of hole forward. */
delivered_data += (sblkp->end - cur->start);
cur->start = sblkp->end;
cur->rxmit = SEQ_MAX(cur->rxmit, cur->start);
}
@ -707,6 +716,7 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
/* Data acks at least the end of hole. */
if (SEQ_GEQ(sblkp->end, cur->end)) {
/* Move end of hole backward. */
delivered_data += (cur->end - sblkp->start);
cur->end = sblkp->start;
cur->rxmit = SEQ_MIN(cur->rxmit, cur->end);
} else {
@ -726,6 +736,7 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
cur->end = sblkp->start;
cur->rxmit = SEQ_MIN(cur->rxmit,
cur->end);
delivered_data += (sblkp->end - sblkp->start);
}
}
}
@ -740,6 +751,10 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
else
sblkp--;
}
tp->sackhint.delivered_data = delivered_data;
tp->sackhint.sacked_bytes += delivered_data - left_edge_delta;
KASSERT((delivered_data >= 0), ("delivered_data < 0"));
KASSERT((tp->sackhint.sacked_bytes >= 0), ("sacked_bytes < 0"));
return (sack_changed);
}

View File

@ -105,12 +105,12 @@ struct sackhole {
struct sackhint {
struct sackhole *nexthole;
int sack_bytes_rexmit;
int32_t sack_bytes_rexmit;
tcp_seq last_sack_ack; /* Most recent/largest sacked ack */
int ispare; /* explicit pad for 64bit alignment */
int sacked_bytes; /*
* Total sacked bytes reported by the
int32_t delivered_data; /* Newly acked data from last SACK */
int32_t sacked_bytes; /* Total sacked bytes reported by the
* receiver via sack option
*/
uint32_t _pad1[1]; /* TBD */