Re: [iccrg] editorial comments on draft-briscoe-iccrg-prague-congestion-control-02

[Bob Briscoe <[e-mail]>]

Neal, (and adding iccrg as suggested)

Thanks for these. All accepted except the one about connection v kernel.
I've given responses to some, but please take no response to mean 
'accepted'.

Anyone can find the diff in the editor's copy of the xml, linked via the 
draft's datatracker page.

On 28/07/2023 16:18, Neal Cardwell wrote:
> Hi,
>
> Spent the flight back home from the IETF reading the Prague CC draft 
> and skimming the tcp_prague.c code. Thanks for both! Below are some 
> editorial comments/suggestions from reading the draft. Hope some of 
> them are useful.
>
> Feel free to CC this e-mail to IETF mailing lists if you feel that 
> makes sense.
>
> best regards,
> neal
>
> -----------
>
> But if it is sharing the bottleneck with Classic ECN traffic, this is 
> more difficult to detect
>
> -->
>
> But if it is sharing a Classic ECN  bottleneckwith Classic ECN 
> traffic, this is more difficult to detect
>
>
> -----------
>
> A general response function has the form cwnd = K/p^B
>
> -->
>
> One family of response functionshas the form cwnd = K/p^B
>

[BB] Yes, not the only family - you're right.
Nonetheless, I made this 'A widely used family of response functions...' 
'cos otherwise it could have been read as some little-known family.

>
> -----------
>
> The meeting agreed a list
>
> -->
>
> The meeting agreed ona list
>

[BB] Happy to oblige, given British English allows both, but apparently 
a US ear doesn't.

>
> -----------
>
> Scaling down to a fractional window (no longer mandatory, see Section 3.5)
>
> -->
>
> [Perhaps this should be mandatory, to avoid having low-RTT flows 
> starve high-RTT flows? Sounds like IETF 117 testing had some 
> conclusions in this area? What were the findings there?]
>

[BB] For this draft, I've just added '(*Recommended but *no longer 
mandatory...' 'cos this debate really needs to be in the context of all 
scalable controls - i.e. a standards track update to RFC9331, not here.

>
> -----------
>
> An additional check is provided to verify that the kernel actually 
> does support AccECN
>
> -->
>
> An additional check is provided to verify that the connectionactually 
> does support AccECN
>

[BB] This does actually intend to mean the kernel.
I've clarified with "*The Prague CCA module  does a*n additional check 
to verify that the kernel actually does support AccECN"

>
> -----------
>
> A system wide option is available to disable AccECN negotiation, but 
> the Prague CC module will always override this setting, as it depends 
> on AccECN. Then, solely in this case, AccECN will only be active for 
> TCP flows using the Prague CCA.
>
> -->
>
> A system-wide sysctl is available to *enable or* disable AccECN 
> negotiation. However, the Prague CC module overrides this sysctl and 
> will always enable AccECN negotiation, since it depends on AccECN 
> (i.e., when the system-wide sysctl disables AccECN negotiation, TCP 
> flows using the Prague CCA will still attempt AccECN negotiation).
>

[BB] Yes, it was badly worded. I've had another go myself:

A system-wide option is available to enable or disable AccECN 
negotiation. However, TCP flows using the Prague CCA module depend on 
AccECN; so they  always ignore this system-wide sysctl and enable AccECN 
negotiation anyway.

>
> -----------
>
> A Prague CCA triggers update of its moving average once per RTT by 
> recording the packet it sent after the previous update, then watching 
> for the ACK of that packet to return.
>
> -->
>
> A Prague CCA triggers update of its moving average ECN mark rate once 
> per rtt_virt [see Section 2.4.4].
>
>

[BB] Thx for catching this.

> -----------
>
> To maintain its moving average, it measures the fraction, frac, of 
> ACKed bytes or ACKed packets
>
> -->
>
> [IMHO the spec should specify whether the CCA is measuring using bytes 
> or packets, since the answers may be very different depending on the 
> approach, leading to unfairness between implementations with different 
> approaches. I would argue for using the fraction of packets marked (as 
> IIRC  I have argued on some IETF mailing list or another). And Linux 
> TCP Prague is already doing this.]
>

[BB] Agreed that this ought to say just packets, to document what Linux 
Prague uses.

If packet sizes were independently and identically distributed (IID), on 
average any differences would cancel out, 'cos the distribution of 
packet sizes is in both the top and bottom of the fraction. That assumes 
all L4S AQMs mark packets independently of size, which is currently true 
(and recommended by RFC7141).

Nonetheless, if packet sizes do vary, they would very likely not be IID. 
For instance, if one end was sending ECN-capable pure ACKs, it would be 
likely to be sending a lot in a row, not just randomly. Then measuring 
bytes would be the right thing (adding a nominal header size to each 
packet if an exact one were not available).

BTW, I do remember you raising this on a list somewhere. I meant to 
reply, and I guess it's still in my todo list somewhere - I'll dig it out.

If we conclude that RFC7141 is OK on this point, then we'll need to 
write something in the future work section under congestion metrics 
about this (and we'll have to implement it).

>
> -----------
> that carried ECN feedback over the previous round trip
> -->
> that carried ECN feedback over the previous *rtt_virt*
>
>
> -----------
>
> where g is the gain of the EWMA (default 1/16).
>
> -->
>
> where *g=1/16* is the gain of the EWMA.
>
>
> -----------
>
> Linux represents alpha with a 10-bit integer (with resolution 1/1024).
>
> -->
>
> Linux DCTCP represents alpha with a 10-bit integer (with resolution 
> 1/1024).
>
>
> -----------
>
> Currently the above per-RTT update to the moving average, which was 
> inherited from DCTCP, is the default in the Linux Prague CCA.
>
> -->
>
> The above per-rtt_virtupdate to the moving average is the approach  in 
> the Linux Prague CCA.
>
>
> -----------
>
> Subsequently, as the window grows, RACK shifts to using a fraction of 
> the RTT for loss detection.
>
> -->
>
> If the TCP connection detects reordering, then RACK shifts to using a 
> fraction of the RTT for loss detection.
>
>
> -----------
>
> as long as the reductions, due to ECN and the loss, when multiplied 
> together result in the reduction that the implementation usually makes 
> in response to loss (e.g. 50% to emulate Reno or 30% to emulate CUBIC).
>
> ->
>
> as long as the reductions, due to ECN and the loss, when combined, 
> result in a reduction that is at least as large as theimplementation 
> usually makes in response to loss (e.g. 50% to emulate Reno or 30% to 
> emulate CUBIC).
>
> [In the spirit of the discussion in Section 3.2.1 "ECN with Loss", 
> with ECN+loss in a single round trip, presumably we want the reduction 
> to be at least as large as the max of the reductions dictated by the 
> ECN and loss responses separately. For example, if the ECN mark rate 
> is a sustained 100% and then there is a packet loss, we want a 
> CUBIC-ish Prague CCA to reduce cwnd by the ECN-dictated reduction of 
> 50% rather than the CUBIC-loss reduction of 30%].
>

[BB] Yes, good point.

>
>
> -----------
>
> Linux Prague CCA clocks its moving average of ECN-marking, alpha, once 
> per round trip
>
> -->
>
> Linux Prague CCA clocks its moving average of ECN-marking, alpha, once 
> per rtt_virt [see Section 2.4.4]
>

[BB] Thank you. I really didn't do a good job of finding all the 
occurrences of 'round trip' or 'RTT', did I!

>
> -----------
>
> Also, integer rounding bias ought to be removed from the 
> multiplicative decrease calculation.
>
> -->
>
> [I would suggest spelling out how to do this correctly to increase the 
> odds that this is implemented correctly by implementors that can't 
> look at the GPL tcp_prague.c reference code]
>

[BB] I introduced a pseudcode name for the carry variable into the 
previous sentence, Then added the pseudocode below:
     "... delay can be made significantly less jumpy by tracking a 
fractional value, cwnd_carry, alongside the integer window and carrying 
over any fractional remainder to the next reduction." ... Specifically:

#define ONE_CWND (1LL << 20)        /* Must be signed */
#define MAX_ALPHA (1ULL << 20)

/* On CE feedback, calculate the reduction in cwnd */
     /* Adding MAX_ALPHA to the numerator effectively adds 1/2
      *  which compensates for integer division always rounding down */
     reduction = (alpha * cwnd * ONE_CWND + MAX_ALPHA) / MAX_ALPHA / 2;
     cwnd_carry -= reduction;

/* Round reduction into whole segments and carry the remainder */
     if (cwnd_carry <= -ONE_CWND) {
         cwnd_carry += ONE_CWND;
         cwnd = max(cwnd - 1, MIN_CWND);
         ssthresh = cwnd;
     }


>
>
> -----------
>
> Example functions for the virtual RTT are:
>
>  *
>
>     rtt_virt = max(srtt, RTT_VIRT_MIN);
>
>  *
>
>     rtt_virt = srtt + AdditionalRTT;
>
> where RTT_VIRT_MIN and AdditionalRTT are constants. The current 
> default is rtt_virt = max(srtt, 25ms), which addresses the main Prague 
> requirement for when the RTT is smaller than typical.
>
> -->
>
> The virtual RTT, rtt_virt is computed as:
>
>  *
>
>     rtt_virt = max(srtt, RTT_VIRT_MIN);
>
> where RTT_VIRT_MIN = 25ms.This addresses the Prague requirement for 
> Reduced RTT-Dependence when the RTT is smaller than typical public 
> Internet RTTs.
>

[BB] The fluffiness is because this is a case where implementations 
might differ, so I've made it clearer what the Linux implementation does 
but also left in the other example. Also the constants depend on the 
deployment environment. Specifically:

Example functions that implementations might use for the virtual RTT are:
     rtt_virt = max(srtt, RTT_VIRT_MIN);
     rtt_virt = srtt + AdditionalRTT;
where the parameters RTT_VIRT_MIN or AdditionalRTT would be set for a 
particular deployment environment.

The Linux implementation of Prague uses the first example and, for the 
public Internet, it sets RTT_VIRT_MIN=25ms. Thus, Linux Prague defines
rtt_virt = max(srtt, 25ms), which addresses the Prague requirement for 
Reduced RTT-Dependence when the RTT is smaller than typical public 
Internet RTTs.

>
> -----------
>
> it spans 5 actual round trips
>
> ->
>
> cwnd_virt spans 5 actual round trips
>
> [the text looks correct; this is just a suggestion for clarity...]
>
>
> -----------
>
> bursts of packets that can occur, for example, when a jump in the 
> acknowledgement number opens up cwnd
>
> ->
> bursts of packets that can occur, for example, when restarting from 
> idle orwhen a jump in the acknowledgement number opens up cwnd
>
>
> -----------
>
> A Prague CCA SHOULD pace the packets it sends
>
> -->
>
> A Prague CCA MUST pace the packets it sends
>
> [Hard to see how an implementation can keep delays tolerable if it 
> bursts a full cwnd when restarting from idle?]
>
>
> -----------
>
> sleeping handset battery
>
> -->
>
> sleeping handset *radio*
>
>
> -----------
>
> it is also planned to allow the application
>
> -->
>
> It is also planned to allow the application
>
> [capitalization]
>
>
> -----------
>
> New EWMA and resposne algorithms
>
> -->
>
> New EWMA and responsealgorithms
>
>
> -----------
>
> stack ahs to be
>
> -->
>
> stack hasto be
>
>

Thanks again for all these.

Cheers


Bob

-- 
________________________________________________________________
Bob Briscoehttp://bobbriscoe.net/