Re: [tcpm] [EXTERNAL] Review: draft-balasubramanian-tcpm-hystartplusplus-01

[Praveen Balasubramanian <[e-mail]>]

Thanks for the review Martin! Sorry for the really long delay in response. Just published version 03. Comments inline prefixed by >>.

From: tcpm <[e-mail]> On Behalf Of Martin Duke
Sent: Saturday, February 15, 2020 6:20 PM
To: [e-mail] Extensions <[e-mail]>
Subject: [EXTERNAL] [tcpm] Review: draft-balasuramanian-tcpm-hystartplusplus-01


Hi Praveen,

Thanks for writing this up. It's great to hear what's actually happening out there with major implementations.

1. I think this draft has a somewhat uncomfortable relationship with ABC (RFC 3465), which for some reason is still Experimental. In each description of Slow Start (Sec 1 and 3.2) you say one MSS per ack, which is the non-ABC Reno behavior. But then the actual LSS algorithm uses bytes counted.

>> Windows TCP does use ABC but with a limit L value that’s different than RFC 3465.  We have incorporated some text that suggests use of ABC along with HyStart++. I agree that it is quite strange that ABC is experimental.

Note that for a non-ABC implementation that also uses HyStart++, this means an LSS_DIVISOR as low as 0.5 can cause LSS to be *more* aggressive than regular slow start if the receiver acks every other packet. In the non-RFC but common case where ACKs are aggregated even more aggressively, LSS can be more aggressive than slow start for arbitrarily low divisor values. I'm not sure the design has to change, but there ought to be some discussion of these considerations.

2. in section 3.2, you set the ssthresh to cwnd upon entering LSS. But then later you say "HyStart++ ends when cwnd exceeds ssthresh..." which by definition is immediately. I think you mean the normal Reno/Cubic ssthresh; perhaps it would be better to invent a new name for the value that is used in computing K.

>> That was a mistake. This is now fixed to say that HyStart++ only ends upon observing a congestion signal.

Similarly, I don't believe MIN_SSTHRESH has any relationship to the conventional ssthresh (i.e. it is not a minimum) and therefore should also be renamed.

>> That’s fair. Now renamed to LOW_CWND.

3. Section 3.3 could use some further considerations for setting these parameters. In particular, values of LSS_DIVISOR > 1 will be more aggressive than slow start, at least for low values of cwnd/ssthresh. Depending on the result of #1 above, the warning may need to be for a lower value.

>> Added text to suggest that values greater than 0.5 (value suggested in the original LSS RFC) should not be used.

4. Though not necessary to make this draft worthwhile, some data from Microsoft's deployment showing the value of this would be helpful. If I were to try to sell this feature to our customers, they would probably want to see that it was providing benefits to large traffic generators rather than backing off for the good of the internet. In particular, I wonder whether Reno/Cubic is fair to Hystart++ as regular slow start would seem to cause LSS to cut the throttle.

>> We presented lab measurement data at the last IETF. We are in the process of doing some A/B experiments at scale with real world workloads. We did fairness comparisons in our lab measurements between Hystart and non-Hystart and we didn’t see any problems.

Nits:
- Please spell out SMSS before using it.

>> Done

- Title of 3.3: s/Constant/Constants

>> Fixed


- 3.3. You are little sloppy with units. MIN_SSTHRESH is clearly measured in bytes in Sec 3.2 but is reported in segments in this section.
>> Fair. Fixed LOW_CWND to be in packets and fixed the equation to use the SMSS multiplier.