Subject: Re: [AVTCORE] Benjamin Kaduk's Yes on draft-ietf-avtcore-cc-feedback-message-08: (with COMMENT)

From: Colin Perkins <[email-masked]>
Date: Mon, 2 Nov 2020 13:26:31 +0000

> On 24 Sep 2020, at 00:25, Benjamin Kaduk via Datatracker <[email-masked]> wrote:
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-avtcore-cc-feedback-message-08: 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-avtcore-cc-feedback-message/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Section 1
> 
>   control designs are not interoperable.  To enable algorithm evolution
>   as well as interoperability across designs (e.g., different rate
>   adaptation algorithms), it is highly desirable to have generic
>   congestion control feedback format.
> 
> nit: singular/plural mismatch "format"/"to have generic".
> 
>   To help achieve interoperability for unicast RTP congestion control,
>   this memo proposes a common RTCP feedback packet format that can be
> 
> (side note) is there work underway for non-unicast RTP congestion
> control?

Not that I know of.

> Section 2
> 
>   In addition the terminology defined in [RFC3550], [RFC3551],
>   [RFC3611], [RFC4585], and [RFC5506] applies.
> 
> [3551 and 3611 don't seem to have prominent dedicated "definitions"
> sections.]

Removed these references.

> Section 3.1
> 
>   o  L (1 bit): is a boolean to indicate if the packet was received. 0
>      represents that the packet was not yet received and all the
>      subsequent bits (ECN and ATO) are also set to 0.  1 represents
>      that the packet was received and the subsequent bits in the block
>      need to be parsed.
> 
> (side note) it's tempting to parse this as the "loss bit", but then a
> value of true means not-lost and false means lost, which feels somewhat
> unnatural.  That said, preserving all-bits-zero as "no data" is probably
> worth the tradeoff…

Renamed the bit to “R” to better match its use.

>   sent previous reports for RTP packets included in both reports.  If
>   an RTP packet was reported as received in one report, that packet
>   MUST also be reported as received in any overlapping reports sent
>   later that cover its sequence number range.
> 
> What should a recipient do if this invariant is violated?  Tear down the
> RTP session as a protocol violation?

Treat the later report as an update, as in the previous sentence. I reordered the text to clarify. (It makes no sense for a packet to be reported as received then lost, but it’s not going to cause any great harm)

> Section 4
> 
>   feedback, using a feedback interval range of 50-200ms.  Applications
>   need to negotiate an appropriate congestion control feedback interval
>   at session setup time, based on the choice of congestion control
>   algorithm, the expected media bit rate, and the acceptable feedback
>   overhead.
> 
> Are there protocol mechanisms standardized that can perform this
> negotiation?  (I note that though we provide SDP signalling for the use
> of CCFB overall, we do not define SDP signalling for the feedback
> interval.)

SDP has various mechanisms, such as b= lines amongst others, that allow this negotiation.

> Section 7
> 
>   provides duplicate information.  Accordingly, when congestion control
>   feedback is to be used with RTP and ECN, the SDP offer generated MUST
>   include an "a=ecn-capable-rtp:" attribute to negotiate ECN support,
>   along with an "a=rtcp-fb:" attribute with the "ack" parameter "ccfb"
>   to indicate that the RTP Congestion Control Feedback Packet is to be
>   used for feedback.  The "a=rtcp-fb:" attribute MUST NOT include the
>   "nack" parameter "ecn", so the RTCP ECN Feedback Packet will not be
>   used.
> 
> Am I reading this correctly that a "mixed" deployment with Offerer that
> supports both mechanisms and an Answerer that only supports RFC 6679
> will result in ECN not being used for the RTP session at all, even
> though 6679 is supported by both parties?  Why does it not suffice to
> only constrain the Answer behavior?

Good catch – I’ve changed this to allow the receiver to choose.

> Section 8
> 
>      control.  TMMBR could, however, be viewed a complementary
>      mechanism that can inform the sender of the receiver's current
>      view of acceptable maximum bit rate.  The Received Estimated
>      Maximum Bit-rate (REMB) mechanism [I-D.alvestrand-rmcat-remb]
>      provides similar feedback.
> 
> The REMB I-D expired in 2014; is it still useful to reference it by name
> (as opposed to a representative of a class)?  Contrast to
> draft-holmer-rmcat-transport-wide-cc-extensions, which expired in 2016,
> but we reference only as an example of the class of schemes that adds a
> transport-wide sequence number to each RTP packet.

Fixed.

>   RTCP Extended Reports (XR):  Numerous RTCP extended report (XR)
>      blocks have been defined to report details of packet loss, arrival
>      times [RFC3611], delay [RFC6843], and ECN marking [RFC6679].  It
>      is possible to combine several such XR blocks into a compound RTCP
>      packet, to report the detailed loss, arrival time, and ECN marking
>      marking information needed for effective sender-based congestion
>      control.  However, the result has high overhead both in terms of
>      bandwidth and complexity, due to the need to stack multiple
>      reports.
> 
> If I am reading correctly, RFC 6679 only provides ECN counters, not
> necessarily at packet granularity, so it may not actually provide as
> much information as this mechanism.

Right.

> Section 9
> 
> Are the named individuals the members of the design team?  If not, where
> can the membership of the design team be determined?

I think so…. it’s been a long time.

> Section 11
> 
> I look forward to the clarification regarding off-path attacks produced
> in response to the tsv-art review.
> 
> In theory the exposure of (somewhat) fine-grained timing information at
> per-packet granularity could open up new attacks that look for side
> channels in processing of other protocols operating between the same
> endpoints, but I am not really convinced that millisecond-scale
> information presents enough exposure to merit mentioning here.
> 
>   congestion on the path.  This will negatively impact the quality of
>   experience of that receiver.  Since RTP is an unreliable transport, a
> 
> And potentially other entities using the bottleneck segment of that
> path? Also could potentially cause excessive resource consumption on the
> sender?

Yes, updated.

>   sender can intentionally leave a gap in the RTP sequence number space
>   without causing harm, to check that the receiver is correctly
>   reporting losses.
> 
> [I assume that recommended remediation measures in the face of such a
> lying peer are covered in the referenced documents already.]

I’m not sure they are, but such guidance would relate to RTP implementations in general, not just to this feedback mechanism, so this isn’t the right place to add it.

>  An on-path attacker that can modify RTCP congestion control feedback
>   packets can change the reports to trick the sender into sending at
>   either an excessively high or excessively low rate, leading to denial
>   of service.  The secure RTCP profile [RFC3711] can be used to
>   authenticate RTCP packets to protect against this attack.
> 
> I guess to try to do this off-path you'd need to guess the SSRCs and
> sequence-numbers (mod 2**16) in addition to transport ports, which
> quickly becomes infeasible if there's more than one stream, so it really
> is just an off-path threat.

Right – it’s not feasible as an off-path attack, because you need to also guess the SSRC, sequence number, and ports.

> Section 12.1
> 
> It seems that we only list 3611 as normative for the terminology
> reference, but I didn't see any terminology usage that we clearly needed
> 3611 for.

It should be informative, yes.

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