On 21/10/17(Sat) 15:17, Mike Belopuhov wrote: > 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.
Could you send the full/fixed diff? And what about TCP_FACK? It is disabled by default, is there a point in keeping it? > 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) > { > /* >