Last Call Review of draft-ietf-mmusic-trickle-ice-sip-12
review-ietf-mmusic-trickle-ice-sip-12-tsvart-lc-ott-2018-01-26-00

Hi,

Reviewer: Jörg Ott
Review result: Largely ready with Issues

I've reviewed this document as part of TSV-ART's ongoing effort to 
review key IETF documents. These comments were written primarily for the
transport area directors, but are copied to the document's authors for
their information and to allow them to address any issues raised.  When
done at the time of IETF Last Call, the authors should consider this
review together with any other last-call comments they receive. Please
always CC tsv-art@ietf.org if you reply to or forward this review.

The draft defines a how the Session Initiation Protocol (SIP) shall
make use the incremental discovery and exchange of IP addresses as
provided by tricke ICE; the main purpose is reducing call setup latency.

The draft defines address the SIP aspects comprehensively with all
necessary features.  From a transport perspective relevant is primarily
its use the SIP INFO method for carrying updates to the collected
addresses to notify the respective peer that further ones can now
be tried and inform when when the address gathering is complete.

Before proceeding, I note that SIP as defined in RFC 3261 and
referenced in the draft can use UDP as a transport (which I thought
was deprecated at some point, but couldn't find evidence to this
end).

This means that SIP message generation may lead straight to packet
generation on the network and thus uncontrolled generation of SIP
INFO frames will lead to uncontrolled, potentially bursty, network
traffic.

As far as I recall, this has always been an issue with SIP INFO
for which no pacing or congestion control was defined (this is
in contrast to SUBCRIBE/NOTIFY, for which packages need to specify
how to rate control notifications).

The document authors are aware of this but provide, IMHO, insufficient
guidance when they write in section 10.9:

10.9.  Rate of INFO Requests

    A Trickle ICE Agent with many network interfaces might create a high
    rate of INFO requests if every newly detected candidate is trickled
    individually without aggregation.  Implementors that are concerned
    about loss of packets in such a case might consider aggregating ICE
    candidates and sending INFOs only at some configurable intervals.

Given that IP addresses may be gathered rapidly and poor implementations
may send them one by one, implementers MUST be concerned with this and
MUST rate limit the transmission of INFO messages.  There are examples
in other SIP specs (see SUB/NOT, for example) that provide clearer
guidance from the authors may borrow.  I acknowledge that SIP INFO
messages are strictly unidirectional and hence one cannot map them
naturally to determine when one was received.  So the simplest way may
be a careful pacing.  But the group has probably thought more about
this.

If SIP runs on top of TCP, which is probably the standard way, this is
not an issue for the network anymore, but it may remain one for SIP
proxies and other intermediaries forwarding the SIP INFO messages.
Also, an endpoint may not be able to tell that it has congestion
controlled transport all the way.

Minor notes:

I found two cases, where the correct standards keywords (MUST) may
be missing:

Section 4.1.3, first paragraph on page 10 reads:

    If the Answerer accepts to use RTCP multiplexing [RFC5761] and/or
    exclusive RTCP multiplexing [I-D.ietf-mmusic-mux-exclusive], it will
    include the "a=rtcp-mux" attribute in the initial Answer.

will -> MUST?

Section 4.4, bottom of page 19:

    When receiving INFO requests carrying any candidates, agents will
    therefore first identify and discard the attribute lines containing
    candidates they have already received in previous INFO requests or in
    the Offer/Answer exchange preceding them.  Two candidates are

Will -> ???


Best,
Jörg