Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 62502 Build 59386: arc lint + arc unit
Event Timeline
Question: why does this function returns signed value? And why some of the sackhint counters are signed?
There are several calls to tcp_compute_pipe() that did not check the V_tcp_do_newsack: 2 in tcp_output and 1 in tcp_input. Their behavior will change with revised SACK turned off. Commit message should explain why this is okay.
Way back to the original tcp_compute_pipe() in commit 12eeb81fc1ce, it was returning signed value, which I would think that fits into many places that require signed comparison or signed operation.
for example, in tcp_default_output()
... /* * If we are doing retransmissions, then snd_nxt will * not reflect the first unsent octet. For ACK only * packets, we do not want the sequence number of the * retransmitted packet, we want the sequence number * of the next unsent octet. So, if there is no data * (and no SYN or FIN), use snd_max instead of snd_nxt * when filling in ti_seq. But if we are in persist * state, snd_max might reflect one byte beyond the * right edge of the window, so use snd_nxt in that * case, since we know we aren't doing a retransmission. * (retransmit and persist are mutually exclusive...) */ if (!sack_rxmit) { if (len || (flags & (TH_SYN|TH_FIN)) || tcp_timer_active(tp, TT_PERSIST)) th->th_seq = htonl(tp->snd_nxt); else th->th_seq = htonl(tp->snd_max); } else { th->th_seq = htonl(p->rxmit); p->rxmit += len; /* * Lost Retransmission Detection * trigger resending of a (then * still existing) hole, when * fack acks recoverypoint. */ if ((tp->t_flags & TF_LRD) && SEQ_GEQ(p->rxmit, p->end)) p->rxmit = tp->snd_recover; tp->sackhint.sack_bytes_rexmit += len; <<< } ...
And given that struct sackhint is hard coded into tcpcb without a toggle, so I think it does no harm or no bother at all with revised SACK turned off. And also I think the toggle V_tcp_do_rfc6675_pipe or V_tcp_do_newsack should be included in tcp_compute_pipe() from the original commit to avoid such later discrepancy.
For these newly added two calls of tcp_compute_pipe() in tcp_output from commit 7dc78150c730, we may double confirm in meeting.
sys/netinet/cc/cc.c | ||
---|---|---|
405–410 | Missing this part's update. |