Subject: Re: [AVTCORE] AD Evaluation of draft-ietf-avtcore-rtp-circuit-breakers-13

From: Colin Perkins <[email-masked]>
Date: Mon, 7 Mar 2016 18:36:57 +0000

Hi,

Apologies for the slow reply…

> On 24 Feb 2016, at 22:19, Ben Campbell <[email-masked]> wrote:
> 
> Hi,
> 
> This is my AD Evaluation of draft-ietf-avtcore-rtp-circuit-breakers-13. While I do have some comments and questions, I don't think anything here needs to block IETF last call. Please address these along with any other last call feedback.
> 
> Thanks!
> 
> Ben.
> 
> 
> Substantive
> ===========
> 
> -4, third paragraph, sentence starting with "This approach SHOULD NOT be used..."
> 
> Does this mean that, in addition to the fact that non-RTCP peers shouldn’t be on the network in the first place, a circuit-breaker implementation shouldn’t talk to them even if they are? That is, even if the peer has a “good reason" (in an RFC 2119 sense) to violate that SHOULD, circuit-breaker implementations should ostracize it? :-)

An RTP endpoint that implements the circuit breaker can’t talk to non-RTCP peers unless the circuit breaker is disabled, since the lack of RTCP will trigger the RTCP timeout circuit breaker. This sentence says you SHOULD NOT disable the RTP circuit breaker to talk to non-RTCP peers, unless you have some other means of determining whether you’re causing excessive congestion. 

> -4.2, last paragraph, sentence starting with "In this case"
> 
> Does this imply that the SHOULD only applies if the sender has reason to believe that the SR or RR packets will be too large? or should the sender always behave this way?

Assuming you mean last paragraph of §4.1, the reason I’m hesitant to say that it always applies is due to draft-ietf-tsvwg-rtcweb-qos-13 allowing different DSCP markings on different RTP flows on a 5-tuple, making me wonder if there could be differential queuing meaning the circuit breaker could trigger for RTP flows but not for others. Aside from that, it’s safe to always behave that way. 

> How does this relate to the guidance (MAY) in section 8?

I guess you could treat this as one example of treating flows as a group. If the DSCP issue noted above is a concern, it might make sense to update Section 8 to say you can treat RTP flows of the same media type (audio, video, etc.) as a group, rather than any subset of RTP flows.

> - 4.3, 6th paragraph:"Implementations that desire this extra sensitivity MAY use the full TCP throughput equation in the RTP circuit breaker. "
> 
> Is there potential for the extra sensitivity to do harm (beyond the higher calculation complexity)?

The full equation has been shown to be a better model of TCP throughput, and the simplified model can overestimate the rate a TCP flow will achieve in some cases (the [Padhye] reference in the paper goes into details). 

As a result, the RTP circuit breaker is slightly more likely to trigger if using the full equation, since the extra terms in the full equation will always give a slightly lower estimate for the TCP throughput than the simplified equation (and the RTP circuit breaker triggers when sending >10x the TCP throughput estimate). This is safe for the network, but potentially terminates slightly more flows. I don’t see this as a concern, given how conservative the design is for the triggering rate.

> -4.4, 2nd to last paragraph, last sentence:
> 
> Is there any potential guidance on reasonable lower limits to the time period considered “significant"?

Several seconds, and perhaps tens of seconds.

> - 12.2:
> 
> It seems like the following references might should be normative: I-D.ietf-tsvwg-circuit-breaker, RFC3168, RFC6679

I think you can implement the RTP circuit breaker without reading any of those, so informative seems correct. However, I don’t much care either way.

> Editorial
> =========
> 
> - Abstract
> s/“will deteriorate"/"will cause the deterioration of"

Okay.

> -- "This acts as a safety measure..."
> What is the antecedent for “this”? Congestion control? The deterioration? (Also, this sentence contains a comma splice)

That sentence is a mess. Will rewrite. 

> - 1:
> A very short definition of what we mean by “circuit breaker" might be useful.

Sure.

> - 3:
> It's a little odd to find 2119 keywords imbedded in term definitions. You might consider moving those to procedure sections. (OTOH, it may not be worth changing this late in the process.)

No strong opinion, but I thought it clearer to define how the term is calculated when the term was defined. 

> -4.2, last paragraph:
> What would it even mean to maintain a timeout past the end of the related stream?

It wouldn’t be meaningful. This is just stating the obvious for completeness. 

> -4.3,
> 
> -- paragraph 6:
> typo: s/throughout/throughput

Will fix.

> -- paragraph 9: "MUST record the value of the fraction
>   lost field in the report block"
> 
> Does this mean the field _from_ the report block? (That is, you aren’t _writing_ the value in the report block?)

Correct. Perhaps “MUST record the value of the fraction lost field from the report block” would be clearer?

> -9, last paragraph:
> 
> Is this paragraph really related to security? It sounds more like an operational consideration.

It’s related to security if an attacker is able to change the signalling, to effectively disable the circuit breaker in furtherance of a DDoS attack.

Cheers,
Colin

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

---

From: Colin Perkins <[email-masked]>
Date: Mon, 7 Mar 2016 18:47:53 +0000

> On 26 Feb 2016, at 22:02, Simon Perreault <[email-masked]> wrote:
> Le 2016-02-24 17:39, Ben Campbell a écrit :
>> There was a brief thread on the wg list about practical implementation
>> experience. Simon started the conversation starting with [1]. Has that
>> conversation been resolved, at least to the extent it can?
> 
> As far as I am concerned:
> 
> - The authors added good text about how to handle non-conforming
> implementations.

Thanks.

> - Even with that text, non-conforming implementations are so common in
> practice as to make this whole idea inapplicable to the SIP world in
> general. I admit WebRTC might be different and having WebRTC endpoints
> implement this algorithm would probably make sense, because they would
> be less likely to have to interoperate with broken implementations.

I’m hopeful that CLUE and WebRTC will push endpoints in the direction of implementing RTP and RTCP more fully, but I certainly agree there are legacy issues. 

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

---

From: "Ben Campbell" <[email-masked]>
Date: Mon, 07 Mar 2016 16:51:07 -0600

Thanks for the response. Comments inline; I've deleted sections that 
don't seem to need further discussion.

Ben.

On 7 Mar 2016, at 12:36, Colin Perkins wrote:

[...]

>> Does this mean that, in addition to the fact that non-RTCP peers 
>> shouldn’t be on the network in the first place, a circuit-breaker 
>> implementation shouldn’t talk to them even if they are? That is, 
>> even if the peer has a “good reason" (in an RFC 2119 sense) to 
>> violate that SHOULD, circuit-breaker implementations should ostracize 
>> it? :-)
>
> An RTP endpoint that implements the circuit breaker can’t talk to 
> non-RTCP peers unless the circuit breaker is disabled, since the lack 
> of RTCP will trigger the RTCP timeout circuit breaker. This sentence 
> says you SHOULD NOT disable the RTP circuit breaker to talk to 
> non-RTCP peers, unless you have some other means of determining 
> whether you’re causing excessive congestion.

My concern is the interaction with the SHOULD NOT in paragraph 3, which 
says the non RTPC peer SHOULD NOT be on the network in the first place, 
unless you have another way to report congestion. Do you consider the 
"unless you have other means..." parts to be exhaustive? That is, are 
they the only circumstances in which one might violate the SHOULD? If 
that's the case, then both should probably say "MUST NOT... unless you 
have some other means...".

On the other hand, if the intent is really mean SHOULD NOT, which would 
allow implementors to find other "good reasons" to violate it, then we 
end up with the situation where a non-RTCP peer chooses to join the 
network for some "other good reason", but other peers that support 
circuit-breakers will not honor that choice. The point is, we probably 
don't need both SHOULD NOTs. And since implementations that do not 
implement rtcp are also unlikely to implement _this_ draft, it probably 
makes more sense to keep the one in paragraph 4.

>> Does this imply that the SHOULD only applies if the sender has reason 
>> to believe that the SR or RR packets will be too large? or should the 
>> sender always behave this way?
>
> Assuming you mean last paragraph of §4.1,

Yes, sorry.

> the reason I’m hesitant to say that it always applies is due to 
> draft-ietf-tsvwg-rtcweb-qos-13 allowing different DSCP markings on 
> different RTP flows on a 5-tuple, making me wonder if there could be 
> differential queuing meaning the circuit breaker could trigger for RTP 
> flows but not for others. Aside from that, it’s safe to always 
> behave that way.

I guess my concern is that "In this case..." can be interpreted to mean 
the sender should choose or not choose this behavior based on inferences 
about whether the receiver's RR packets will be too big. Is it 
reasonable to expect the sender to draw that inference?  (Or would it 
make sense to chance "In this case..." to "For this reason..."?)

> I guess you could treat this as one example of treating flows as a 
> group. If the DSCP issue noted above is a concern, it might make sense 
> to update Section 8 to say you can treat RTP flows of the same media 
> type (audio, video, etc.) as a group, rather than any subset of RTP 
> flows.

So the point would be that the receiver MAY always treat flows as a 
group, and SHOULD do so if it thinks the RRs would be too big? I guess 
that makes sense, depending on your answer about whether the sender can 
reasonably infer that.

[...]

> Several seconds, and perhaps tens of seconds.

Does it make sense to say that in the text?

>
>> - 12.2:
>>
>> It seems like the following references might should be normative: 
>> I-D.ietf-tsvwg-circuit-breaker, RFC3168, RFC6679
>
> I think you can implement the RTP circuit breaker without reading any 
> of those, so informative seems correct. However, I don’t much care 
> either way.

On reflection I agree for tsvwg-circuit-breaker and RFC 3168. However, 
section 7 seems to describe an optional feature that depends on RFC 
6679. IMO, normative dependences, even in optional features, need a 
normative reference.

>> Editorial

[...]

>> Does this mean the field _from_ the report block? (That is, you 
>> aren’t _writing_ the value in the report block?)
>

It seems more clear to me.

[...]

---

From: Colin Perkins <[email-masked]>
Date: Wed, 16 Mar 2016 15:12:13 +0000

Hi,

Replies inline.

> On 7 Mar 2016, at 22:51, Ben Campbell <[email-masked]> wrote:
> 
> Thanks for the response. Comments inline; I've deleted sections that don't seem to need further discussion.
> 
> Ben.
> 
> My concern is the interaction with the SHOULD NOT in paragraph 3, which says the non RTPC peer SHOULD NOT be on the network in the first place, unless you have another way to report congestion. Do you consider the "unless you have other means..." parts to be exhaustive? That is, are they the only circumstances in which one might violate the SHOULD? If that’s the case, then both should probably say "MUST NOT... unless you have some other means...".

The SHOULD NOT is intended to apply to the device implementing the circuit breaker, not the peer that doesn’t implement RTCP. That is, it’s saying you shouldn’t disable the RTP circuit breaker unless you have some other way of getting congestion feedback. It’s not saying that non-RTCP devices shouldn’t be on the network. Would it help to change it to “The RTP circuit breaker SHOULD NOT be disabled on networks that might be subject to congestion, unless…”?

> On the other hand, if the intent is really mean SHOULD NOT, which would allow implementors to find other "good reasons" to violate it, then we end up with the situation where a non-RTCP peer chooses to join the network for some "other good reason", but other peers that support circuit-breakers will not honor that choice. The point is, we probably don't need both SHOULD NOTs. And since implementations that do not implement rtcp are also unlikely to implement _this_ draft, it probably makes more sense to keep the one in paragraph 4.

Implementations that don’t implement RTCP _can’t_ implement this draft: this is an RTCP extension.

>> the reason I’m hesitant to say that it always applies is due to draft-ietf-tsvwg-rtcweb-qos-13 allowing different DSCP markings on different RTP flows on a 5-tuple, making me wonder if there could be differential queuing meaning the circuit breaker could trigger for RTP flows but not for others. Aside from that, it’s safe to always behave that way.
> 
> I guess my concern is that "In this case..." can be interpreted to mean the sender should choose or not choose this behavior based on inferences about whether the receiver's RR packets will be too big. Is it reasonable to expect the sender to draw that inference?  (Or would it make sense to chance “In this case..." to "For this reason..."?)

I’d expect an implementation will be able to infer this, but it’s an under-specified and likely poorly implemented part of RTCP. 

> So the point would be that the receiver MAY always treat flows as a group, and SHOULD do so if it thinks the RRs would be too big? I guess that makes sense, depending on your answer about whether the sender can reasonably infer that.

Yes, with the caveat that if different DSCP values are being used for different flows, then it has to restrict the group to flows with the same DSCP.

> Does it make sense to say that in the text?

Sure, can do.

> On reflection I agree for tsvwg-circuit-breaker and RFC 3168. However, section 7 seems to describe an optional feature that depends on RFC 6679. IMO, normative dependences, even in optional features, need a normative reference.

Fine.

Okay.

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