On Fri, Oct 20, 2017 at 22:59 +0200, Klemens Nanni wrote:
> The comments for both void tcp_{sack,newreno}_partialack() still mention
> tp->snd_last and return value bits.
>
Good eyes! It made me spot a mistake I made by folding two lines
into an incorrect ifdef in tcp_sack_partialack. I expected it to
say "ifdef TCP_FACK" while it says "ifNdef". The adjusted comment
didn't make sense and I found the bug.
diff --git sys/netinet/tcp_input.c sys/netinet/tcp_input.c
index 45aafee0d05..d5de9cb2407 100644
--- sys/netinet/tcp_input.c
+++ sys/netinet/tcp_input.c
@@ -2690,13 +2690,13 @@ tcp_clean_sackreport(struct tcpcb *tp)
tp->sackblks[i].start = tp->sackblks[i].end=0;
}
/*
- * Checks for partial ack. If partial ack arrives, turn off retransmission
- * timer, deflate the window, do not clear tp->t_dupacks, and return 1.
- * If the ack advances at least to tp->snd_last, return 0.
+ * Partial ack handling within a sack recovery episode. When a partial ack
+ * arrives, turn off retransmission timer, deflate the window, do not clear
+ * tp->t_dupacks.
*/
void
tcp_sack_partialack(struct tcpcb *tp, struct tcphdr *th)
{
/* Turn off retx. timer (will start again next segment) */
@@ -2711,16 +2711,16 @@ tcp_sack_partialack(struct tcpcb *tp, struct tcphdr *th)
if (tp->snd_cwnd > (th->th_ack - tp->snd_una)) {
tp->snd_cwnd -= th->th_ack - tp->snd_una;
tp->snd_cwnd += tp->t_maxseg;
} else
tp->snd_cwnd = tp->t_maxseg;
+ tp->snd_cwnd += tp->t_maxseg;
+ tp->t_flags |= TF_NEEDOUTPUT;
+#else
/* Force call to tcp_output */
if (tp->snd_awnd < tp->snd_cwnd)
tp->t_flags |= TF_NEEDOUTPUT;
-#else
- tp->snd_cwnd += tp->t_maxseg;
- tp->t_flags |= TF_NEEDOUTPUT;
#endif
}
/*
* Pull out of band byte out of a segment so
@@ -3078,14 +3078,14 @@ tcp_mss_update(struct tcpcb *tp)
}
}
/*
- * Checks for partial ack. If partial ack arrives, force the retransmission
- * of the next unacknowledged segment, do not clear tp->t_dupacks, and return
- * 1. By setting snd_nxt to ti_ack, this forces retransmission timer to
- * be started again. If the ack advances at least to tp->snd_last, return 0.
+ * When a partial ack arrives, force the retransmission of the
+ * next unacknowledged segment. Do not clear tp->t_dupacks.
+ * By setting snd_nxt to ti_ack, this forces retransmission timer
+ * to be started again.
*/
void
tcp_newreno_partialack(struct tcpcb *tp, struct tcphdr *th)
{
/*
