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

[Dale Worley <[e-mail]>]

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?

* 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.

* 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.

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/.

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 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.)

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.

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.

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".

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.

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/

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.

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.

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.

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.

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".

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/.

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.

6.  Considerations for RTP and RTCP multiplexing

       INFO sip:[e-mail] 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"?

7.  Considerations for Media Multiplexing

       INFO sip:[e-mail] 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"?

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."

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.

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.

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.

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".

         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.

[END]