Hi Magnus,

On 27 Feb 2014, at 14:58, Magnus Westerlund <magnus.westerlund@ericsson.com> wrote:
> I have reviewed draft-ietf-avtcore-rtp-circuit-breakers-05 and have some comments:

Thanks for the feedback. I have some responses inline.

> 1) Section 3:
> This modifies the RTCP timing rules to allow RTCP
>      reports to be sent early, in some cases immediately, provided the
>      average RTCP reporting interval remains unchanged.
> 
> I would suggest changing "provided the average RTCP reporting interval
> remains unchanged." is changed to:
> 
> 	provided the RTCP transmission keeps within its bandwidth
>        allocation.
> 
> The reason is that AVPF does not necessarily keep within the average
> reporting interval. As that is commonly under using the RTCP bandwidth.

Ok.

> 2) Section 3:
>      The use of the RTP/AVPF profile is dependent
>      on signalling, but is otherwise generally backwards compatible
>      with the RTP/AVP profile, as it keeps the same average RTCP
>      reporting interval as the base RTP specification.
> 
> I wouldn't claim that it is generally backwards compatible. In many
> configurations it would not, and even with the timeout clarification
> being proposed in draft-ietf-avtcore-rtp-multi-stream-03 there will
> still be cases where interoperability would be questionable.
> 
> I suggest some other formulations, maybe saying that one can configure
> AVPF to be backwards compatible.

I think we can just remove the sentence in question without affecting the meaning of the draft.

> 3) Section 4:
> 
> TCP congestion control intentionally tries to fill
>   the router queues, and uses the resulting packet loss as congestion
>   feedback.
> 
> I think a reference would be appropriate after “TCP congestion control".

I can add a reference to RFC 5681.

> 4) Section 4.1:
> 
>   Accordingly, if a sender of RTP data packets receives two or more
>   consecutive RTCP SR or RR packets from the same receiver, and those
>   packets correspond to its transmission and have a non-increasing
>   extended highest sequence number received field (i.e., the sender
>   receivers at least three RTCP SR or RR packets that report the same
>   value in the extended highest sequence number received field for an
>   SSRC, but the sender has sent RTP data packets for that SSRC that
>   would have caused an increase in the reported value of the extended
>   highest sequence number received if they had reached the receiver),
>   then that sender SHOULD cease transmission (see Section 4.5).
> 
> 
> It is a long sentence,

Agree; we should try to rephrase that sentence. 

> and my comment relate to "has sent RTP data
> packets for that SSRC that would have caused an increase in the
> reported value of the extended highest sequence number"
> 
> If the sender sends very few packets, this effects the probability for
> this CB to trigger. So if I send only 1 packet per reporting interval,
> and each packet is lost with a probability of 10%, then I have 1%
> (0.1^2) probability of this being triggered. If I send 10 packets per
> interval, then I have 0.1^20 = 1E-20. Thus, it might be worth making
> some lower threshold recommendation here. This example assumes equal
> distribution of loss also, but if we take into burst a short packet
> burst may be likely to be completely lost, while the same amount of
> packets may be likelier to have at least one arrive if distributed
> over the reporting interval.

I agree that this is a potential issue, although I expect the practical impact will be limited (the most likely case I can see that would trigger this is a voice call, where one party is sending only comfort noise for several reporting intervals, and there is an asymmetric path failure). I can certainly add a note highlighting the issue, so application developers can be aware in those cases where it might affect their application. In terms of a threshold though, what value would you suggest?

> 5) Section 4.2:
> 
> Accordingly, an RTP
>   sender that has not received any RTCP SR or RTCP RR packets reporting
>   on the SSRC it is using for three or more RTCP reporting intervals
>   SHOULD cease transmission (see Section 4.5).
> 
> Maybe one should clarify that the reporting intervals used in this
> calculation is the interval calculated for the SSRC sending the media.
> 
> This is in comparision with 4.1 where the trigger is calculated on a
> per remote basis for the received reports from that particular remote
> SSRC.

Okay.

> Is there an issue if one gets reports from multiple remote SSRCs on
> ones transmission. From a media timeout in a point to point case,
> these reports although from many SSRCs should not skew the result as
> they would be based on the same underlying data. But, using any two
> consecutive reports, does not ensure that one is measuring over an
> sufficient long interval.

I’m not sure I see the problem here. Can you clarify?

> 6) Section 4.2:
> 
> When calculating the
>   timeout, the fixed minimum RTCP reporting interval SHOULD be used
>   (based on the rationale in Section 6.2 of RFC 3550 [RFC3550]).
> 
> I think this sentence provides an minor issue if one tries to match
> this with the RFC 3550 terminology. fixed minimum (interval) is used
> in section 6.2. I think the added "RTCP reporting" is confusing here,
> I would suggest to use another formulation here.
> 
> I think you may have to define what fixed minimum RTCP reporting
> interval means, i.e. the RTCP reporting interval (T or Td?, i.e. with
> or without randomization) (and in case of randomization, two random
> draws or simple 2*T?) calculated with the current set of RTCP stack
> parameters and using the fixed minimum interval (SHOULD be 5 sec).

Okay. I meant without randomisation, but will clarify.

> 7) Section 5:
> 
> As there is a difference between CB #1 and #3 and #2 that has to do
> with that the first (#1 and #3) uses incoming RTCP reports and that #2
> uses the CB implementers estimate of what the regular reporting
> interval is. Thus, I wonder if there needs to exist include a
> reflection over t_rr_int as well as the RTCP SSRC timeout calculation
> that we recommend to use 5 seconds as fixed minimal value in those
> calculations (if 5 sec larger than t_rr_int).

Maybe, I’m not sure. Can you give a concrete suggestion for the recommendation? You’re more familiar with the impact of T_rr_int on the timing than me.

> 8) Section 5:
> 
>   Reports of ECN-CE packets sent as reduced-size RTCP ECN feedback
>   packets without an RTCP SR/RR packet MUST be ignored.
> 
> Shouldn't then be accounted as loss event in CB #3? Or are you
> assuming that this will anyway be counted in the next compound packets
> ECN XR summary?

The motivation is in the following paragraph of the draft. This is to allow the congestion control algorithm to use early feedback without worrying about accidental interactions with the circuit breaker. The information will be sent in the regular RTCP reports anyway, which the circuit breaker will then consider.

> 9) Section 9:
> 
> This attack can be avoided if RTCP packets are
>   authenticated, for example using the Secure RTP profile [RFC3711].
> 
> Maybe this should reference draft-ietf-rtp-security-options for other
> potential solutions?

Sure.


Cheers,
Colin



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