Subject: Re: [tsvwg] [AVTCORE] WG last call for draft-ietf-avtcore-rtp-circuit-breakers-11

From: Colin Perkins <[email-masked]>
Date: Tue, 1 Dec 2015 12:58:14 +0100

Hi Gorry,

Thanks for the feedback. Some comments in line, but these all seem like reasonable points to fix. 

Colin

> On 29 Nov 2015, at 16:27, [email-masked] wrote:
> 
> 
> I have reviewed draft-ietf-avtcore-rtp-circuit-breakers-11.
> 
> This document appears to be useful to publish as a Standards Track
> document, and appears to be complete. My review is from a  transport
> perspective (rather than based on RTP expertise). I have the following
> comments (below).
> 
> Gorry
> 
> ----
> /If congestion control is not implemented in the applications, then
> network congestion will deteriorate the user's multimedia experience./
> - This is likely true, but then it could be more important that a circuit
> breaker is used to protect other traffic sharing the bottleneck, than just
> the media of the flow. Is it possible to include something like or similar
> to:
>   This acts as a safety measure to prevent
>   starvation of network resources denying other flows from access to
>   the Internet, such measures are essential for an Internet that is
>   heterogeneous and for traffic that is hard to predict in advance.

Ok.

> ----
> /resort to avoid persistent congestion/
> - A requested change to the related draft in TSVWG is to use the text:
> /resort to avoid persistent excessive congestion/
> (This has the advantage of highlighting both persistent over time, and
> excessive not a low level of loss/mark.)

Ok.

> ----
> NiT:
> /so that it matches the available network bandwidth./
> - Id suggest the words for this is /not more than the capacity available
> along the network path /

Ok.

> Is /available bandwidth estimator/ also /available capacity estimator/?

Ok.

> ----
> / but there is potential to disrupt the network's operation/
> - The use of “disrupt" seems like a little vague.

Could change to “is potential to cause persistent congestion to the network if the application does not adapt its sending rate in a timely and effective manner”?

> ----
> /p is the loss event rate, between 0.0 and 1.0,/
> - strictly speaking this seem like it should be a ratio rather than a rate
> per second?

True, but RFC 5348 uses rate, and I think consistency is important.

> ----
> /Implementations that sometimes need to interwork with endpoints that do
> not support RTCP need to disable the RTP circuit breakers if they don't
> receive some confirmation via signalling that the remote endpoint
> implements RTCP (the presence of an SDP "a=rtcp:" attribute in an answer
> might be such an indication).
> - I think I understand the intention, but this somehow does not seem the
> correct thing to write,
> Later, the document says SHOULD monitor packet loss to ensure that the
> packet loss rate is within acceptable parameters, which I think is
> correct, hence this seems like a violation of a SHOULD, which to me seems
> to imply careful wording to say do-this-if-it-is-possible, and then say
> why this is clearly impossible in some cases.

Well, why it’s impossible in this case is that the endpoint doesn’t support RTCP and so doesn’t provide loss reports. Suggestions for better wording would be appreciated.

> ---
> /The remaining congestion signals are the packet loss fraction and the
> cumulative number of packets lost.  /
> - This para - or the set of paras here - could benefit from a few
> sentences that explain that while such signals can be used immediately as
> input to a CC, they need to be taken over a reasonable time scale when
> used for a circuit breaker, because the measured loss/marks/delay can be
> transient and could otherwise result in premature triggering.
> draft-ietf-tsvwg-circuit-breaker could be a reference here.

Will add.

> - It also may be worth considering adding a note saying that continuous,
> but low levels of loss/delay also may result from normal operation when
> flows sharing a path, and that these too shouldnt be used on their own as
> a basis for terminating a flow. (i.e. a threshold is needed), also
> draft-ietf-tsvwg-circuit-breaker could be a reference here.

Sure, will add. 

> ---
> NiT
> /When a sender receives an RTCP packet indicating that the media it's
> sending is being received/
> - Is there an English problem?
> Perhaps it may be possible to say something like:
> /When a sender receives an RTCP packet that indicates reception of the
> media it has been sending/

Okay.

> ---
> NiT
> /the throughput a TCP connection would achieve over the path/
> - I think this is the *maximum* throughput a TCP connection *could*
> achieve or better written as
> /the throughput a *bulk* TCP connection would achieve over the path/
> ---
> [RFC3448] has been obsoleted, do you specifically mean to refer to this
> rather than RFC 5348?

No, I’ll update the reference.

> ---
> /However, in pathological scenarios where the congestion
>   circuit breaker does not stop the flow, it is desirable that the RTP
>   application cease sending useless traffic. The role of the media
>   usability circuit breaker is to protect the network in such cases./
> - True, but protect the network is maybe a little veiled, this seems to
> be more about
> preventing the use of network resources denying other flows from access to
> the Internet… anyway just saying protect seems wrong to me.

“to prevent the application sending unnecessary traffic that might disrupt other applications”?

> ---
> NiT
> /What it means to cease transmission depends on the application, but
>   the intention is that/
> - A mangled sentence?

No, but might be clearer split into two: “What it means to cease transmission depends on the application. The intention is that…”?

> ---
> /restart the call/
> Section 4.5 could point to draft-ietf-tsvwg-circuit-breaker for rules on
> reseting
> circuit breakers, which seem to be similar, and could benefit from a
> cross-reference?
>     The reaction to a triggered Circuit Breaker MUST continue for a
>      period that is at least the triggering interval.  Operator
>      intervention will usually be required to restore a flow.  If an
>      automated response is needed to reset the trigger, then this needs
>      to not be immediate.  The design of an automated reset mechanism
>      needs to be sufficiently conservative that it does not adversely
>      interact with other mechanisms (including other Circuit Breaker
>      algorithms that control traffic over a common path).  It SHOULD
>      NOT perform an automated reset when there is evidence of continued
>      congestion.

Ok.

> ---
> Should the reference to I-D.ietf-tsvwg-circuit-breaker be normative, since
> this is a BCP on the same topic, and I would expect any update to this
> document would need to be consistent with this, or update this?

This is not clear. You don’t need to read the TSV draft to implement this, but it is relevant and does provide useful background and rationale. Happy to take chair/AD guidance on this; I don’t have a strong opinion.

-- 
Colin Perkins
https://csperkins.org/

---

From: "Black, David" <[email-masked]>
Date: Sat, 5 Dec 2015 03:19:49 +0000

> This is not clear. You don’t need to read the TSV draft to implement this, but
> it is relevant and does provide useful background and rationale. Happy to take
> chair/AD guidance on this; I don’t have a strong opinion.

Well, RFC 7322 (RFC Style Guide) has this to say in section 4.8.6 on this topic:

   Reference lists must indicate whether each reference is normative or
   informative, where normative references are essential to implementing
   or understanding the content of the RFC and informative references
   provide additional information.

That suggests that an informative reference is appropriate.

Thanks,
--David

> -----Original Message-----
> From: tsvwg [mailto:[email-masked]] On Behalf Of Colin Perkins
> Sent: Tuesday, December 01, 2015 6:58 AM
> To: Gorry Fairhurst
> Cc: Magnus Westerlund; [email-masked]; [email-masked]
> Subject: Re: [tsvwg] [AVTCORE] WG last call for draft-ietf-avtcore-rtp-
> circuit-breakers-11
> 
> Hi Gorry,
> 
> Could change to “is potential to cause persistent congestion to the network if
> the application does not adapt its sending rate in a timely and effective
> manner”?
> 
> > ----
> Well, why it’s impossible in this case is that the endpoint doesn’t support
> RTCP and so doesn’t provide loss reports. Suggestions for better wording would
> be appreciated.
> 
> > ---
> No, but might be clearer split into two: “What it means to cease transmission
> depends on the application. The intention is that…”?
> 
> > ---
> > /restart the call/
> This is not clear. You don’t need to read the TSV draft to implement this, but
> it is relevant and does provide useful background and rationale. Happy to take
> chair/AD guidance on this; I don’t have a strong opinion.
> 
> 
> --
> Colin Perkins
> https://csperkins.org/
> 
> 
>