Re: [Tsv-art] TSV-ART Last Call review for draft-ietf-mmusic-trickle-ice-sip

[Thomas Stach <[e-mail]>]

Joerg,

first of all apologies for the delay in responding to your review.

Thanks for your effort.

Responses inline


On 2018-01-26 12:09, Joerg Ott wrote:
> 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 [e-mail] 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.
Ok, we will come up with stricter text and e.g. change to MUST.
With respect to pacing we would stick to our recommendation to send
INFOs only at some configurable intervals without specifying a concrete 
value,
since an appropriate value would differ dependent on the specific network.
>
> 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?
We removed normative language at various places where we referred to 
other specs based on the review of our AD Ben Campbell.
So, I will stick to "will"
>
> 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 -> ???
needs to be a MUST.

Thanks again for your review.

>
>
> Best,
> Jörg
>