The code above is my current understanding of the 6927bis draft. Note that this will make PRR loss recovery slightly less aggressive especially under heavy loss.

Also, this change probably doesn't change much in the pathological instance discueed in the draft, as the base stack has no capability at the moment, to retransmit lost retransmissions, and will instead run into an RTO and slow-start thereafter.

no magic numbers in sack-changed.

In D28822#868424, @rscheff wrote:

• make the sack return status an enum for nice debugging

Sorry for nit-picking, but this isn't enough for the goal of seeing the string literals in the debugger. The prototype of tcp_sack_doack() needs to be changed accordingly, as well as type of variable.

typedef enum {
        SACK_NOCHANGE = 0,
        SACK_CHANGE,
        SACK_NEWLOSS
} sack_status;

sack_status  tcp_sack_doack(struct tcpcb *, struct tcpopt *, tcp_seq);

And in tcp_do_segment():

sack_status sack_changed;

Thanks Gleb! While at it, I also found that most other typedef enums use a nomenclature of a _t suffix. Also fixed up all relevant variables - haven't checked gdb to verify the proper decoding though.

Did more unit testing as promised. Found that the hardcoded CRB during dupack processing needs to be changed against the proper toggle (SACK_NEWLOSS or SACK_CHANGE) - to conform with RFC6937bis