tcp: fix detection of bad RTOs

[tuexen]

Summary

If timestamps are enabled, the actions does by a retransmission timeout were rolled back,
when they should not. We need to make sure the incoming packet advances the SND.UNA.

To do this, remove the incorrect upfront check and extend the check in the fast path to
handle also the case of timestamps.

This addresses the recently observed panics with the message: sent too much.

[rscheff]

sys/netinet/tcp_input.c

1799

Any specific reason to write the or statement as two separate if clauses?

I believe in other parts we do it ((true & x & y & z) | (false & a & b &c )) - as multiline checks;

I don't like them both get executed "always", so at the very least, an "else" just before the 2nd if would be good.

The logic looks ok to me.

[glebius]

sys/netinet/tcp_input.c

1799

I agree with Richard, better one complex if clause. The two clauses are mutually exclusive as one wants TOF_TS and other !TOF_TS, so cc_cong_signal() can be executed only once. My first reading was that it is intentionally written this way cause we may want two calls into cc_cong_singal(), so it was a misleading reading.

Oh, the upper level embracing if can also be merged into one big clause:

if ((tp->t_rxtshift == 1 &&
    tp->t_flags & TF_PREVVALID &&
    tp->t_badrxtwin != 0) &&
    (((to.to_flags & TOF_TS) != 0 &&
    (to.to_tsecr != 0) &&
    TSTMP_LT(to.to_tsecr, tp->t_badrxtwin)) ||
    ((to.to_flags & TOF_TS) == 0 &&
    TSTMP_LT(ticks, tp->t_badrxtwin))))
        cc_cong_signal(tp, th, CC_RTO_ERR);

This will also fix lines longer than 79 chars. IMHO, we may slightly violate style(9) and add single space indentation to highlight our three main large blocks (a && (b || c)).

[tuexen]

Use one big condition and try to use indentation to structure the boolean expression.

[tuexen]

sys/netinet/tcp_input.c

1799

It was a try to break things up for improved readability by humans.

1799

Now using one and two spaces of indentation to make the expression more readable.

[rscheff]