OK, I double checked the code. I think the way to go is taking your change to tcp_hc_get() and not do the initialisations in cc_conn_init() and tcp_mss_update().
It is clear that tcp_hc_get() always returns the memory located at hc_metrics_lite in a correct state. So there is no need to initialise the memory before calling tcp_hc_get().
The code should be clear on this.

I think the proposed fix tcp_hc_get() is the correct thing to do. I don't think we need the initialisers in cc_conn_init() and tcp_mss_update. Which compiler warning are you referring to?

Why are you enforcing a lower bond of two MSS instead of one MSS?

Would it make sense to make the default value for the delay ACK timer sysctl'able?

The same change is needed for RACK. I will add it when committing.

In D23212#508834, @rscheff_gmx.at wrote:

There is a special case handling in DSACK when start and end (2nd and 3rd param) are equal. Placing the todrop before the block will prevent DSACKs from generated if the last in-sequence packet was sent twice (is to be dsacked).

Please note that todrop is positive when we enter the block. It is only reduced when a SYN segment is received. So the case where todrop is zero and caught by the condition I added is only when todrop is initially 1 and reduced to 0 because we received a SYN segment. The condition todrop > 0 and tlen == 0 would be handled wrong. Can it happen?

I suggest to move the (todrop > 0) check just over the tf_ACKNOW statement, possibly with a check if we are not to send out a (D)SACK block (tp->rcv_numsacks >= 1).

Same change for RACK and BBR.

Unpredictability is not important here, it is more the distribution. Any reason why the change has to happen?

In D22704#496723, @rscheff_gmx.at wrote:

Why would cleaning up trailing whitespaces make MFCing harder? Other than possibly adding one adjacent line?

If you don't MFC the whitespace cleanup, you might get merge conflicts...

I made this huge diff, because sneaking these in one-by-one whenever I encounter them around the lines I was modifying was shot down so far ;)

As I said, doing this once is a nice thing in my opinion...

BTW - since lint was removed, what other tool could I use to validate the fbsd code style(9)? Any hint as to how i could integrate that into git or arcanist would be appreciated.

I"m not doing this automated... I just try to follow it (as much as I think is a good thing) when coding... But this is not perfect and others are also not perfect.

When I did similar things in the past, one comment I received was that this does not improve the functionality, but
might make MFCing harder... However, only dealing with this in a single commit is definitely a good idea in my view.
The question is whether to do it at all. Maybe rrs@ or rgrimes@ have an opinion on this.

@jhibbits Thanks for the explanation. I'm using it on a blackbird. I have also a G5 Powermac in my lab, so if there is a need for testing, just let me know. I would like to run an unmodified version of FreeBSD just to make sure I don't commit anything by accident...

Don't we need a similar change to BBR?