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) { /*