Subject: [rmcat] Spencer Dawkins' Yes on draft-ietf-rmcat-scream-cc-12: (with COMMENT)

From: Spencer Dawkins <[email-masked]>
Date: Wed, 25 Oct 2017 06:04:15 -0700

Spencer Dawkins has entered the following ballot position for
draft-ietf-rmcat-scream-cc-12: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-rmcat-scream-cc/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I'm a Yes with lots of questions and comments. You may be educating me, rather
than making text changes, as seems appropriate.

Because

4.1.1.1.  Constants

   The RECOMMENDED values, within (), for the constants are deduced from
   experiments.

provides recommended, but not required, constant values (which is fine), could
you say something about, or just provide a reference to, those experiments, so
that implementers and deployers could verify that they are in an environment
that was considered by the working group?

I'd also note that most of the constants, but not all, have associated text
that says basically "if you dork with this constant, here's what's likely to
change". Some of those constants are pretty self-evident even to me, but not
all of them. That could have been intentional, but the working group is likely
to have a better grip on that than J. Typical Coder On A Deadline in a couple
of years, so if there are useful things to say, I'd imagine people would listen
to you.

Is

  QDELAY_TREND_TH (0.2)
     Averaging factor for qdelay_fraction_avg.

   QDELAY_TREND_TH (0.2)
     Averaging factor for qdelay_fraction_avg.

a duplicate?

In this text,

  rtp_queue_size (0 bits)
     Size of RTP packets in queue.

   rtp_size (0 byte)
     Size of the last transmitted RTP packet.

is "size" being used in the same way (so, rtp_queue_size would be the total
number of bytes in queue)? But I'm guessing. Maybe "Sum of the sizes of RTP
packets in queue"?

ISTM that

   Note that the pseudo code does not show all
   details for reasons of readability, the reader is encouraged to look
   into the C++ code in [SCReAM-CPP-implementation] for the details.

might make SCReAM-CPP-implementation a normative reference. Does "encouraged to
look at" mean "really needs to look at"?

In this text,

   It is desired to avoid the case that the qdelay
   target is increased due to self-congestion, indicated by a changing
   qdelay and consequently an increased qdelay_norm_var_t.  Still it
   SHOULD be possible to increase the qdelay target if the qdelay
   continues to be high.

I got lost in the passive tense. Is this saying that *an implementation* should
be able to increase the qdelay target algorithmically?

This text,

   If it is deemed
   unlikely that competing flows occur over the same bottleneck, the
   algorithm described in this section MAY be turned off.  However, when
   sending over the Internet, often the network conditions are not known
   for sure.  Therefore turning this algorithm off must be considered
   with caution as that can lead to basically zero throughput if
   competing with other, loss based, traffic.

bothers me a bit, because I'm not sure how a transport implementation knows
that it's sending over the Internet. We have discussions from time to time
about networks that are playing games with non-RFC 1918 addresses and NATs, for
instance, and networks that are directly interconnected can be using routable
addresses, while not "sending over the Internet". Is the point that an
implementation that turns the algorithm off is well-advised to detect that it
should turn the algorithm on?

I couldn't parse

   A strict rule can lead to that the
      media bitrate will have difficulties to increase as the congestion
      window puts a too hard restriction on the media frame size
      variation.

without guessing. I lost my way at "can lead to that the media bitrate".

---

From: Ingemar Johansson S <[email-masked]>
Date: Thu, 26 Oct 2017 07:11:03 +0000

Hi
And thanks for the review. Comments inline marked [IJ]

/Ingemar

> -----Original Message-----
> From: Spencer Dawkins [mailto:[email-masked]]
> Sent: den 25 oktober 2017 15:04
> To: The IESG <[email-masked]>
> Cc: [email-masked]; Martin Stiemerling
> <[email-masked]>; [email-masked]; [email-masked];
> [email-masked]
> Subject: Spencer Dawkins' Yes on draft-ietf-rmcat-scream-cc-12: (with
> COMMENT)
> 
> When responding, please keep the subject line intact and reply to all email
> addresses included in the To and CC lines. (Feel free to cut this introductory
> paragraph, however.)
> 
> 
> I'm a Yes with lots of questions and comments. You may be educating me,
> rather than making text changes, as seems appropriate.
> 
> Because
> 
> 4.1.1.1.  Constants
> 
> provides recommended, but not required, constant values (which is fine),
> could you say something about, or just provide a reference to, those
> experiments, so that implementers and deployers could verify that they are
> in an environment that was considered by the working group?
> 
> I'd also note that most of the constants, but not all, have associated text that
> says basically "if you dork with this constant, here's what's likely to change".
> Some of those constants are pretty self-evident even to me, but not all of
> them. That could have been intentional, but the working group is likely to
> have a better grip on that than J. Typical Coder On A Deadline in a couple of
> years, so if there are useful things to say, I'd imagine people would listen to
> you.
> 
> Is
> 
>   QDELAY_TREND_TH (0.2)
[IJ] Yes, obviously a duplicate, will fix this

> number of bytes in queue)? But I'm guessing. Maybe "Sum of the sizes of
> RTP packets in queue"?
[IJ] Good suggestion, will change as suggested

> might make SCReAM-CPP-implementation a normative reference. Does
> "encouraged to look at" mean "really needs to look at"?
[IJ] No, it is not necessary to look in the code. I realize that I balance on a thin line here. It is more that the additional lines of comments in the C++ code can give the extra insight, a bonus is that it is possible to run the code in the minimalistic simulator and learn how it works. But the code is not normative and it is listed among the informative references, if you have a suggestions how to rewrite this then that would be more that welcome. 
  
> bothers me a bit, because I'm not sure how a transport implementation
> knows that it's sending over the Internet. We have discussions from time to
> time about networks that are playing games with non-RFC 1918 addresses
> and NATs, for instance, and networks that are directly interconnected can be
> using routable addresses, while not "sending over the Internet". Is the point
> that an implementation that turns the algorithm off is well-advised to detect
> that it should turn the algorithm on?
[IJ] Yes, that is probably as far as one can take it. One such example is remote controlled vehicles with multiple cameras that we are experimenting with. In this case we can configure a QoS bearer for service and we know that at least in the radio network there will be no competing traffic over the same radio bearer (~radio channel for a given terminal). In the general case however, we don't have that luxury

I would suggest that I rewrite the whole text a bit. If I rewrite the 1st paragraph (before the pseudo code) in section 4.1.2.3 as
=BEGIN=
It is likely that a flow using SCReAM algorithm will have to share
 congested bottlenecks with other flows that use a more aggressive
 congestion control algorithm, examples of such ar large FTP flows using loss based congestion control. 
 The worst condition occurs when the bottleneck queues are of tail-drop tupe with a large buffer size.  
 SCReAM takes care of such situations by adjusting the qdelay_target when loss based flows are detected, 
 as given by the pseudo code below.
==END==

And the text after the pseudo code as 
=BEGIN=
Two temporary variables are calculated, qdelay_norm_avg_t is the long term 
 average queue delay, qdelay_norm_var_t is the long term variance of the queue delay.
 A high qdelay_norm_var_t indicates that the queue delay changes, this can be an indication of 
 reduced bottleneck bandwidth or that a competing flow has just entered. This indicates that 
 it is not safe to adjust the queue delay target.        

 A low qdelay_norm_var_t indicates that the queue delay is relatively stable, 
 the reason can be that the queue delay is low but it can also be an indication that a 
 competing flow is filling up the bottleneck to the limit where packet losses may start to occur, 
 in which case the queue delay will stay relatively high for a longer time.

 The queue delay target is allowed to be increased if, 
 either the loss event rate is above a given threshold or that qdelay_norm_var_t is low. 
 Both these conditions indicate that a competing flow may be present. 
 In all other cases the queue delay target is decreased.
 
 The function that adjusts the qdelay_target is simple and has a certain risk to 
 produce both false positive and negatives, The case that self-inflicted congestion by the SCReAM algorithm 
 may be falsely interpreted as the presence of competing loss based FTP flows is a false positive. 
 The opposite case where the algorithm fails to detect the presence of a competing FTP flow is a false negative.

 Extensive simulations have shown that the algorithm performs well in LTE test 
 cases and that it also performs well in simple bandwidth limited bottleneck test cases with competing FTP flows.
 It can however not be completely ruled out that 
 this algorithm can fail. Especially the false positives can be problematic as the end to end delay can 
 increase dramatically if the target queue delay is increased by accident as a result of self-inflicted congestion.
 
 If it is deemed unlikely that competing flows occur over the same bottleneck, the
 algorithm described in this section MAY be turned off. One such case is QoS controlled bearers 
 in 3GPP based access such as LTE. 
 However, when sending over the Internet, often the network conditions are not known
 for sure and  it is in general not possible to make safe assumptions on how a 
 network is used and whether or not competing flows share the same bottleneck.  
 Therefore turning this algorithm off must be considered
 with caution as that can lead to basically zero throughput if
 competing with other, loss based, traffic.
==END==

Would these changes improve things ?

> without guessing. I lost my way at "can lead to that the media bitrate".
[IJ] The problem here is that, in order to keep the end to end delay low , the RTP queue delay must be kept short as well. 
This is an extra complication that does not apply for e.g. large file (FTP) transfers, in which case an object is simply put in the TCP transmit buffer, application level delay is not an issue

Is this more clear if I rewrite as below ?

=BEGIN=
Bitrate variations: The media frame size is always varying to a
larger or smaller extent. The RTP queue absorbs the natural variations in frame sizes. 
The RTP queue should however be as short as possible, to achieve that the media rate 
control takes the RTP queue size into account when the target bitrate for the media is computed.
A strict 'send only when bytes in flight is less than the congestion window' rule can lead to that 
the RTP queue grows simply because the send window is limited, the effect of 
which would be that the target bitrate is pushed down. The consequence of this is that the congestion window 
will not increase, or will increase very slowly, because the congestion window is only allowed to increase when there is a sufficient amount of data in flight. The end effect is then that the media bitrate increases very slowly 
or not at all.
==END==