Re: [Gen-art] Genart last call review of draft-ietf-mmusic-trickle-ice-sip-12

Dale, thanks for your review. Thomas, thanks for your responses. I have entered a No Objection ballot.

Alissa

> On Feb 1, 2018, at 5:47 PM, Thomas Stach <thomass.stach@gmail.com> wrote:
> 
> Dale,
> 
> first of all apologies for the delay in responding to your thorough review.
> 
> Thanks for your effort.
> 
> more inline
> 
> 
> On 2018-01-24 04:45, Dale Worley wrote:
>> Reviewer: Dale Worley
>> Review result: Ready with Issues
>> 
>> I am the assigned Gen-ART reviewer for this draft.  The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>> 
>> Document:  draft-ietf-mmusic-trickle-ice-sip-12
>> Reviewer:  Dale R. Worley
>> Review Date:  2018-01-23
>> IETF LC End Date:  2018-01-26
>> IESG Telechat date:  [unknown]
>> 
>> Summary:
>> 
>>        This draft is on the right track but has open issues, described
>>        in the review.
>> 
>> This draft is technically quite good, and except for various nits,
>> quite well written.  But there are some significant technical issues
>> that need to be properly settled before the protocol is well-defined
>> with good advice on robust usage.  I don't think settling these issues
>> will be difficult.
>> 
>> ** Global/major issues:
>> 
>> * The tokens "end-of-candidate" and "end-of-candidates" seem to be used
>> equivalently in this document, with the following number of uses:
>> 
>>      11 end-of-candidate
>>      19 end-of-candidates
>> 
>> Which is the correct form?
> Hm, as the grammar says "end-of-candidates ", I'll change to the latter.
>> * The definition of application/trickle-ice-sdpfrag describes the use of
>> "a=charset" as an attribute, but the grammar of sdpfrag in section 9
>> does not permit that attribute.
> The grammar allows for using extension attributes.
> "a=charset" would be one of these, but might only be needed if non-ASCII characters are possibly introduced via some potential extension.
>> 
>> * Deployment considerations (section 11)
>> 
>> It might be worth mentioning here what happens if a middlebox deletes
>> Trickle ICE INFO requests because it does not understand them, but
>> does not delete "Supported: trickle-ice" headers.  It seems to me that
>> in order to be robust in this situation, a UA should provide a
>> globally routable address in the m= lines of the initial offer or
>> answer, or if Trickle ICE INFO requests fail, eventually amend the
>> addresses to be globally routable addresses.
> There are so many ways how middle boxes can break any mechanism, that we can't do much to cover everything.
> RFC 5245 has recommendations (e.g. relayed candidates will always work) on how to choose the default destination as provided in the m/c-line.
> I don't think we need to repeat that reasoning here , and after all the user agent might not want to use such a globally routable address.
>> 
>> But the WG may have more knowledge about this situation.
>> 
>> * There are various locations where the language has usages that seem to
>> me to be excessively informal or prolix.  Cases I've noticed are:
>> 
>> 2.  Terminology
>> 
>>    It is assumed that the reader will be
>>    familiar with the terminology from both documents.
>> 
>> Probably s/will be/is/.
> OK
>> 
>> 3.1.  Discovery issues
>> 
>>    Such Offers and Answers can
>>    only be handled meaningfully by agents that actually support
>>    incremental candidate provisioning, which implies the need to confirm
>>    such support before actually using it.
>> 
>> It's probably best to omit "actually" here and elsewhere.  There is
>> also a use of "right from the first Offer".  And in some places
>> "would" is used where it is not needed or could be replaced by "do";
>> "would" should (IMO) be restricted to situations that are
>> counter-factual.  There is also a use of "like for example" instead of
>> "for example".
> I'll have pass through the document and look for these.
>> 
>> I expect the RFC Editor can give guidance on this subject.
>> 
>> ** Detailed/editorial/nits:
>> 
>>        A Session Initiation Protocol (SIP) usage for Trickle ICE
>>                   draft-ietf-mmusic-trickle-ice-sip-12
>> 
>> The current practice is to capitalize "usage" in this situation.
>> (I expect the RFC Editor can give guidance on the current
>> capitalization style for RFCs.)
> We switch to capitalized "Usage" as that  seems correct to me.
>> 
>> Abstract
>> 
>>    This document defines usage semantics for Trickle ICE with the
>>    Session Initiation Protocol (SIP) and defines a new SIP Info Package.
>> 
>> Perhaps /a new SIP Info Package/a new SIP Info Package to support this
>> usage/.
>> 
>> Table of Contents
>> 
>> The capitalization of section titles seems to be inconsistent.
> I'll have a pass through the section titles that are taken over into the ToC
>> 
>> 1.  Introduction
>> 
>>    It describes how ICE candidates
>>    are to be exchanged incrementally with SIP INFO requests [RFC6086]
>> 
>> Probably s/exchanged incrementally with/exchanged incrementally
>> using/, as "with" can be read to mean that the SIP INFO requests are
>> one party which is exchanging.
> ok
>> 
>> 4.  Incremental Signaling of ICE candidates
>> 
>>    The following sections provide further details on how Trickle ICE
>>    Agents perform the initial Offers/Answers exchange (Section 4.1),
>>    perform subsequent Offers/Answers exchanges (Section 4.2) and
>>    establish the INVITE dialog usage (Section 4.3) such that they can
>>    incrementally trickle candidates (Section 4.4).
>> 
>> I think you want to change "Offers/Answers" to "Offer/Answer".
> Yes
>> 
>> 4.3.1.  Establishing dialog state through reliable Offer/Answer delivery
>> 
>> It was stated in section 4.1, "Trickle ICE Agents MUST indicate
>> support for Trickle ICE by including the SIP option-tag 'trickle-ice'
>> in a SIP Supported:  header field within all SIP INVITE requests and
>> responses.", but it would be helpful to remind the reader in figure 3
>> by showing the Supported: headers:
>> 
>>                    Alice                         Bob
>>                      |                            |
>>                      |     INVITE (Offer)         |
>>                      |     Supported: trickle-ice |
>>                      |--------------------------->|
>>                      |     183 (Answer)           |
>>                      |     Supported: trickle-ice |
>>                      |<---------------------------|
>> 
>> and similarly in later figures.
>> 
>> Something similar might be done with the "SRFLX Cand." elements to
>> make the figures less cramped.
>> 
>> 4.3.2.  Establishing dialog state through unreliable Offer/Answer
>> 
>>    These retransmissions MUST
>>    cease on receipt of an INFO request or on transmission of the Answer
>>    in a 2xx response.
>> 
>> Strictly, "a trickle-ice INFO request", as other INFO requests might
>> be received as part of other mechanisms.
> OK
> I'll change to
> "These retransmissions MUST cease on receipt of an INFO request
>             carrying  a 'trickle-ice' Info Package body  or on ..."
>> 
>> 4.3.4.  Considerations for Third Party Call Control
>> 
>>    The peer
>>    Trickle ICE Agent (the UAC) MUST send a Trickle ICE INFO request as
>>    soon as they receive an unreliable provisional response (see
>>    Figure 8).
>> 
>> s/they receive/it receives/
> OK
>> 
>> 4.4.  Delivering candidates in INFO messages
>> 
>>    Agents MUST include a pseudo "m=" line and an identification tag in a
>>    "a=mid:" attribute for every "m=" line whose candidate list they
>>    intend to update.  Such "a=mid:" attributes MUST immediately precede
>>    the list of candidates for that specific "m=" line.  All
>>    "a=candidate" or "a=end-of-candidates" attributes following an
>>    "a=mid:" attribute, up until (and excluding) the next occurrence of a
>>    pseudo "m=" line, pertain to the "m=" line identified by that
>>    identification tag.  An "a=end-of-candidates" attribute, preceding
>>    any pseudo "m=" line, indicates the end of all trickling from that
>>    agent, as opposed to end of trickling for a specific "m=" line, which
>>    would be indicated by a media level "a=end-of-candidates" attribute.
>> 
>> This seems to be the first section that involves the detailed
>> structure of trickle-ice INFO messages.  It would probably be useful
>> here to tell the reader that there is a grammar in section 9.
> OK
>> 
>> This paragraph is very dense and you should see if you can split it in
>> some way.  In particular, I think it is possible to separate the first
>> part, which discusses the structure of the sdpfrag, from the second
>> part, which specifies how the pseudo m= lines in the sdpfrag correlate
>> with the m= lines in the offer/answer.
> I will have a look into this in order to improve
>> 
>> In addition, it seems to me that there is no requirement that the
>> sdpfrag contains as many pseudo m= lines as the offer/answer contains
>> m= lines, nor that the pseudo m= lines be in the same order as the m=
>> lines that they pertain to (the correspondence being made via the mid
>> attributes).  Since this is very different from the semantics of SDP
>> proper, I think it should be pointed out explicitly.
> Makes sense, I'll add a note.
>> 
>> 5.  Initial discovery of Trickle ICE support
>> 
>>    This makes discovery fairly straightforward for Answerers or for
>>    cases where Offers need to be generated within existing dialogs
>>    (i.e., when sending UPDATE or re-INVITE requests).  In both scenarios
>>    prior SDP would have provided the necessary information.
>> 
>> I think you need to expand this to note that INVITE requests are
>> required to include "Supported: trickle-ice" if the UA supports
>> Trickle-ICE.  This is important because a UA may receive an offerless
>> re-INVITE as part of initiating a 3PCC -- the peer UA may be
>> attempting to connect media between the UA and a third UA, and the
>> third UA may not support Trickle ICE, even though the peer UA does.
>> (See, e.g., RFC 3725 section 4.1 and RFC 7088 section 2.3.
>> In this situation, the offerless re-INVITE would not contain
>> "Supported:  trickle-ice" unless the peer UA knows that the third UA
>> supports Trickle-ICE.  The UA, seeing the absence of "Supported:
>> trickle-ice", would send a non-Trickle-ICE offer, even if the previous
>> SDP had indicated support for Trickle-ICE.
> Good point. I'll add an explanation along the lines of your reasoning
>> 
>> 5.1.  Provisioning support for Trickle ICE
>> 
>>    While the exact mechanism for allowing such provisioning is out of
>>    scope here, this specification encourages trickle ICE implementations
>>    to allow the option in the way they find most appropriate.
>> 
>> What is "the option"?  Perhaps "such provisioning".
> Yes. I'll change accordingly
>> 
>> 5.2.  Trickle ICE discovery with Globally Routable User Agent URIs
>> 
>>    This circumvents to use of forking
>>    for that request.
>> 
>> I think you want s/circumvents to/prevents the/.
> OK
>> 
>> 5.3.  Fall-back to Half Trickle
>> 
>> Figure 11 seems to be incorrect -- since Alice is using Half-Trickle
>> ICE, all her candidates are sent in the INVITE, and she shouldn't send
>> any INFO requests.  But an INFO request from Alice is shown.
> No, that is correct. The INFO from Alice acknowledges the receipt of the 183 and stop retransmissions (see section 4.3.2)
>> 
>> 6.  Considerations for RTP and RTCP multiplexing
>> 
>>        INFO sip:alice@example.com SIP/2.0
>>        ...
>>        Info-Package: trickle-ice
>>        Content-type: application/sdp
>>        Content-Disposition: Info-Package
>>        Content-length: 161
>> 
>> Shouldn't this be "Content-type: application/trickle-ice-sdpfrag"?
> Yes. Thanks for spotting this.
>> 
>> 7.  Considerations for Media Multiplexing
>> 
>>        INFO sip:alice@example.com SIP/2.0
>>        ...
>>        Info-Package: trickle-ice
>>        Content-type: application/sdp
>>        Content-Disposition: Info-Package
>>        Content-length: 219
>> 
>> Shouldn't this be "Content-type: application/trickle-ice-sdpfrag"?
> ditto
>> 
>> 9.2.  Grammar
>> 
>>    A sender SHOULD stick to lower-
>>    case for such grammars, but a receiver MUST treat them case-
>>    insensitive.
>> 
>> This is awkward.  Perhaps better is "A sender SHOULD use lower-case
>> for attributes from such earlier grammars, but a receiver MUST treat
>> them case-insensitively."
> OK
>> 
>> I don't know the practice regarding the overall grammar of SDP, but
>> the grammar shown here is very rigid regarding the order of fields in
>> <session-level-fields> and in <trickle-ice-attribute-fields>.  If this
>> rigidity is not intended, there should be some notation on the grammar
>> to show this.
> I don't see any benefit in making it less rigid and I'm somewhat reluctant to change this at this point of the process.
>> 
>> 10.1.  Rationale - Why INFO?
>> 
>>    The decision to use SIP INFO requests as a candidate transport method
>>    is based primarily on their lightweight nature.  Once a dialog has
>>    been established, INFO requests can be exchanged both ways with no
>>    restrictions on timing and frequency and no risk of collision.
>> 
>>    On the other hand, using Offer/Answer and UPDATE requests [RFC3311]
>>    would introduce the following complications:
>> 
>> A critical fact is that the sending of Trickle ICE candidates in one
>> direction is entirely uncoupled from sending candidates in the other
>> direction.  Thus, the sending of candidates in each direction can be
>> done by a stream of INFO requests that is not correlated with the
>> stream of INFO requests in the other direction.  And since each INFO
>> request is cumulatively includes the contents of all previous INFO
>> requests in that direction, ordering between INFO requests need not be
>> preserved.  All of this permits using largely-independent INFO
>> requests.
>> 
>> Contrarily, UPDATE or other offer/answer mechanisms assume that the
>> messages in each direction are tightly coupled with messages in the
>> other direction.
> I like this text and will take it over, unless somebody objects.
>> 
>> 12.2.  application/trickle-ice-sdpfrag Media Type
>> 
>>    This section defines a new Media Type 'application/trickle-ice-
>>    sdpfrag' in accordance with [RFC6838].
>> 
>> s/This section/This document/ -- as in section 12.2.
> OK
>> 
>> This section should refer to the grammar of section 9 as the
>> definition of the media type content.
>> 
>>          SDP files are primarily UTF-8 format text.
>> 
>> The content of application/trickle-ice-sdpfrag is not SDP, strictly
>> speaking.  And as noted later in the document, not all of them are
>> UTF-8.  I think you want something like this:
>> 
>>    The media contents follow the same rules as SDP, except as noted
>>    in this document.  The media contents are text, with the grammar
>>    specified in section 9.
>> 
>> Then continue with "Although the initially ...", which describes the
>> character set implications of "are text".
> OK, will do.
>> 
>>          The "a=charset:"
>>          attribute may be used to signal the presence of other character
>>          sets in certain parts of a trickle-ice-sdpfrag body (see
>>          [RFC4566]).
>> 
>> Except that "a=charset" is not permitted by the grammar of section 9.
> As above, it will fall into the extension-attribute bucket if not understood.
> 
> Thanks again for your thorough review and numerous proposal for improvement.
> Regards
> Thomas
>> 
>> [END]
>> 
>> 
>> 
> 
> _______________________________________________
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art