Re: [MMUSIC] AD Evaluation of draft-ietf-mmusic-trickle-ice-sip-11

[Thomas Stach <[e-mail]>]

Ben,

thanks a lot for your thorough review.

We appreciate your comments.

Response inline


On 2017-12-07 00:24, Ben Campbell wrote:
> Hi,
>
> These are my AD Evaluation comments for draft-ietf-mmusic-trickle-ice-sip-11. I have quite a few comments, and would like to resolve at least the substantive comments prior to IETF Last Call.
>
> Thanks!
>
> Ben.
>
>
> *** Substantive:
>
> - The draft needs a "deployment considerations" section relating to the
> presence of middleboxes that may not be aware of trickle ice. I recognize
> that b2bUAs may simply be endpoints that don't do trickle, but I wonder about
> monitoring devices, etc. Even if the answer is "there is no impact", some
> substantiating text would be helpful.
We will mention section 19 of draft-ietf-ice-rfc5245bis and will add 
something that the  newly introduced INFO messages
need to considered as well if the middle boxes needs to see the whole 
picture.
> -3.2: There seem to be some inconsistencies on how the text talks about the
> architectural model. For example, section 3.2 separates the "ICE agent" from
> the "offer/answer" module, but 3.3 talks about a "trickle ICE agent" doing
> offer/answer. (I suspect this is a terminilogy issue where "trickle ICE
> agent" as the user agent and "ICE agent" as a software module are too easily
> confused.)
Good point. The "ICE Agent" in Fig. 2 is actually meant to be a software 
module. We'll change to "ICE module" in order to make that clear.
> 4.1.1: What is the motivation for requiring "audio" in the M-line when one
> does not know any candidates in advance? What if the intended media is not
> audio? I don't understand why the lack of advance candidates would change
> that. (Maybe this is an artifact of the pseudo-M-lines from the INFO bodies?)
Well spotted. This is actually an artifact. Your comment applies as well 
to the the proto field
where we could also have something different from RTP/AVP.
I propose to remove the bullet list and change from

"If the Offerer wants to send its initial Offer before knowing any
    candidate of one or more media descriptions, it MUST include the
    following default values in the corresponding "m=" line.

    o  The media field is set to 'audio'.

    o  The port value is set to '9'.

    o  The proto value is set to 'RTP/AVP'."

to

"If the Offerer wants to send its initial Offer before knowing any
candidate of one or more media descriptions, it MUST include a port value set to '9'
in the corresponding "m=" line. "

> -4.1.1, 2nd to last paragraph : "... it still MUST include the "a=rtcp-mux"
> and/or "a=rctp-mux-only" attribute in the initial Offer."
> IIUC, that's an already existing requirement from the referenced spec. If so,
> please do not use 2118/8174 keywords to describe it here, unless in the form
> of directly quoted text.
OK
Would changing from "... it still MUST include  ..." to "... it will 
include ... " address your comment?
> -4.2.2, RFC editor note:
> This askes the RFC Editor to do research. I don't think that's a reasonable
> expectation. They may notice that sort of thing anyway, but the authors
> should plan to recheck references during AUTH48. (This comment applies to
> several other similar RFC editor notes.)
It was also meant as a reminder to the authors to cross check during 
AUTH48. Would the following text be better?

"RFC EDITOR NOTE: The section XXX in above sentence is correct for
    version XX of said I-D.  The authors need to cross-check during Auth48 since it could have have
    changed in the meantime."

> -4.2.2, third paragraph after Fig 4: "or when new candidates have not been
> learned since then) and/or they MAY also deliver newly learned candidates (if
> available).  The Offerer MAY include an end-of-candidates attribute in case
> candidate discovery has ended in the mean time."
>
> Are those MAYs really correct when the conditions are true? E.g. if you have
> newly learned candidates, there's only a MAY requirement to add them?
> Likewise, if discovery has ended, there's only a MAY requirement to add the
> end-of-candidates attribute?
The motivation for the "MAY"s  was to allow for pre-building of that 
initial INFO only from information that is available  when the Offer was 
sent.
Of course new candidates need to be trickled eventually.
Changing the "MAY"s to "SHOULD"s would still allow for such 
pre-building. We can do that if you prefer.
> -4.2.2, 4th paragraph after fig 4: " and MAY begin trickling"
> that seems like a statement of fact rather than a normative
> offering-of-permission.
You're  correct. We will  s/MAY/can/
> -4.2.2, last paragraph: "... Answerer MUST repeat exactly the same Answer..."
> Is that an external requirement, or a new normative one?
It is required by RFC3264 as stated at the end of that sentence.
We will change "... Answerer MUST repeat ... " to "... Answerer needs to 
repeat ...".
> -4.2.3, first paragraph: "Trickle ICE Agents MAY therefore respond to an
> INVITE request with provisional responses without an SDP answer"
> That's only if the offerer indicated support for trickle ICE, right?
Well, that "MAY" is standard RFC3261 behavior, but, yes, in the context 
of this document, the Offerer would have indicated support for trickle ICE.
Based on your comments above, we    s/MAY/can/
> -4.3: It seems like the use of SDP syntax in the INFO messages creates quite
> a bit of complexity due to the use of SDP in (even more) ways not
> contemplated by it's design. Did the WG discuss the possibility of using a
> designed-for-purpose syntax in the INFO bodies?  (This is just my curiosity;
> I don't expect to make that sort of fundamental change this late in the
> process.)
No, there wasn't discussion about an alternative syntax.
There was discussion about using some generic 'application/sdpfrag' 
media-type, but the consensus was to define a SDP subset specifically 
taylored for trickle-ICE.
By using "SDP-like" syntax we also assumed that corresponding code could 
still be re-used for building the INFO bodies.
> -4.3: Discussion of "pseudo-M-lines":
> A complete separation of the trickle-ice from offer/answer seems unlikely to
> me, since both must send and receive SIP messages in the same dialog. Is it that much harder to remember the media type than it is to remember dialog state?  More to the point, are there known implementations that require this?
I'm, not sure if I  get your point.
But yes, a complete separation seems unlikely since we can have 
candidates already in the Offer, which need to be delivered to the ICE 
module.
The concept of the pseudo m-lines and a=mid attributes was introduced in 
order to be able to determine to which m-line a candidate belongs.

Having said that, I just recognized - sort of embarrassingly - that we 
omitted to explicitly specify that a-mid attributes need to go into the 
offers and answers although we have them in all the SDP examples. I'll 
add that into the next revision.
> -4.3, last bullet: "The fmt SHOULD appear only once..."
> Why not MUST? Would it every be reasonable to include more than one?
I'm OK with MUST
> -4.3, paragraph starting with : "Note that [I-D.ietf-ice-trickle] requires
> that when candidates are trickled, each candidate MUST be delivered..."
> Don't use 2119/8174 keywords to describe external requrements, except in
> directly quoted text.
will change as above
> -4.3: 2nd paragraph prior to Fig 9:
> Are the discussions about removing previously exchanged candidates still in
> process? IMO, the "MAY" and "RECOMMENDED" in this paragraph refer to
> requirements that are too vague to state in normative terms.
These discussions stalled in the meantime, which is partly the reason 
for the vagueness.
We could
s/it MAY treat that as an exceptional case/ it would have to treat that 
as an exceptional case/
and
s/is RECOMMENDED for this situation/is advisable for this situation/
> - figure 9 (and other figures including INFO bodies): Please include a real
> content-length value.
OK
> -5, first paragraph: Please don't use normative terms to describe external
> requirements.
OK
> -5.1, first paragraph:
> I'm confused by the reference to WebRTC clients. These are not assumed to use
> SIP at all. How are they relevant examples?
Well spotted. Will remove
> -5.2: IIUC, using the GRUU means the INVITE can go only to that particular
> device. This seems to circumvent the recipient's ability to use forking for
> their own purposes. That's not a showstopper, but it deserves mention.
Agreed
> -5.3: The comparisons to XMPP don't seem to add much, and seems like
> editorializing. (Also in 3.1). The fact that XMPP has more advanced discovery
> mechanisms is not particularly relevant unless you want to use those as an
> example of how to solve the problems in SIP.
Agreed. Can remove the mentioning of XMPP.
> -6, 6th paragraph: "The Trickle Answerer MUST follow the guidance
>     on the usage of the "a=rtcp" attribute as given in
>     [I-D.ietf-mmusic-ice-sip-sdp] and [RFC3605]"
> External requirement...
Agreed
> -8.2, first paragraph: External requirement.
Here, I think MAY/MUST is correct, since draft-ietf-ice-trickle does not 
provide the attributes. They are only defined in this document
> -9: Please comment about whether this media type is reasonable to use for
> other SDP related applications? I gather the answer is "no" given that it's
> called "trickle-ice-sdpfrag".
The answer is indeed  "No" per the reasons given already by Paul Kyzivat.
Do you want us to explicit add into the text that other SDP related 
applicationsneed to define their own media type?
> -9.2, first paragraph: "A sender SHOULD stick to lower-
>     case for such grammars, but a receiver SHOULD treat them case-
>     insensitive."
> Why is the second SHOULD not a MUST?
MUST is correct
> - 10.7: The requirement to include the payload only applies to INFO requests
> for this particular package, right? That is, not necessarily "all INFO
> requests".
Sure
> - 10.9: You used the need to pool candidates as a argument against using
> update. Doesn’t this create the same requirement?
The argument was about using offer/answer in UPDATE request/response 
which would introduce blocking/risk of glare.
INFO does not have this problem, but could still pool candidates if they 
like, but they don't have to.
> Also, do you think it
> reasonable that an implementation might not be concerned about this?
I can't know that. 10.9 is just meant as a reminder to implementors to 
think about congestion.
Do you want us to change anything in the text?
> -11.2: Please use the IESG ([e-mail]) for the change control authority.
> The mmusic mailing list could be closed some day.
Will do
> -12: The registered media type refers to this document for security
> considerations, but I don't see any considerations specific to it here.
Ok, I will explicitly mention the new media type does not introduce 
additional security considerations
>
> *** Editorial and Nits:
>
> - General: IDNits shows some outdated references. Please fix before final
> published version.
maybe due to updates in the meantime.
I spotted however that we still still refer to 
I-D.ietf-mmusic-rfc5245bis  instead of I-D.ietf-ice-rfc5245bis
> - section 1, paragraph 1:
>
> The sentence structure for listing the 3 phases is odd. The first two are in
> a comma separated list, but the third is in a separate sentence. I suggest
> breaking all 3 into separate sentences, with the needed connecting words.
>
> Missing "and" before "the second phase"
>
> I think the word "latency" would be better expressed as "delay" or "setup
> delay", etc. Too many readers are going to think of "latency" in terms of
> network latency (i.e packet delivery).
OK
> - 1, paragraph 2: "simultaneously"
> I think the operating idea is that they can happen in "parallel" or "be
> interleaved".
OK
> -2: Please use the new boilerplate in RFC 8174, unless you specifically want
> lower case versions of the RFC 2119 keywords to be interpreted in their 2119
> sense.
OK
> -2, 2nd paragraph: "This specification makes use of all terminology..."
> I suggest removing "all".
OK
> -3.2, first paragraph:
> I think "the exception of the actual INFO messages," is too big of an
> exception for this paragraph to be meaningul. For example, this is a huge
> change for middleboxes that pay attention to the SDP (e.g. SBCs); I think
> saying that things "would look the same" is an overstatement.
Would the following be better?

"From the perspective of all SIP middle boxes and proxies
Offer/Answer exchanges look partly similar for
Trickle ICE as they would for ICE for SIP [I-D.ietf-mmusic-ice-sip-sdp 
<https://tools.ietf.org/html/draft-ietf-mmusic-trickle-ice-sip-11#ref-I-D.ietf-mmusic-ice-sip-sdp>].
However, in order to have the full picture of the candidate exchange, the newly introduced INFO messages need to
be considered as well."
   

> -4, item 5: "Note that in case of forking multiple early dialogs will exist."
> s/will/may (or "might" if you want to avoid "may")
OK
> -4.1.1, 2nd paragraph: "...before knowing any candidate of one or more media
> descriptions..."
> s/of/for
OK
> -4.1.1, last paragraph: This seems redundant to the similar statement in
> section 4, item 2.
Probably, but people tend to read only snippets of a document.
> -4.1.2, last paragraph:
> s/neither/none (unless you expect there to be exactly 2 trickled candidates.)
OK
> -4.2.1, title (and several related titles):
> I think that usage of "asserting" will trip up some users. Consider
> "establishing" instead.
OK
> - Figure 3: This figure needs a comment about the meaning of "+SRFLX Cand.",
> similar to those for later figures.
OK
> -4.2.3, first paragraph:
> s/possibility/ability
OK
> -4.2.3, paragraph after Fig 6: "When sending the Answer, the agent MUST
> repeat all currently known and used candidates, if any, and MAY include all
> newly gathered candidates since the last INFO request was sent.  If that
> Answer was sent in a unreliable provisional response, the Answerers MUST
> repeat exactly the same Answer in the 200 OK response to the INVITE request
> in order to fulfill the corresponding requirements in [RFC3264]."
>
> This seems self contradictory, but could be fixed with some connecting
> language like "However", or "An exception..."
OK
> -4.2.4, title: Spell out 3PCC on first mention. Also, an informational
> citation would be helpful.
OK
> -4.3, 3 paragraphs prior to Fig 9: "is an indication of the peer agent that
> it will not send any further candidates"
> s/of/from
OK
> -4.3, last paragraph before Fig 9: Please refer to the figure by number
> rather than relative position.
OK
> -5.2, title: Expand "GRUU" on first mention.
> First paragraph: s/"SIP US"/"SIP UA"
> "... require SIP support" - Should that say "trickle ICE support"?
> 2nd paragraph: Please define "targeted trickling"
OK
> -7, 4th paragraph: I don't understand the meaning of "latest on..." in this
> context.
It might know already e.g. from configuration. Will remove,  in order to 
avoid confusion
> -7, paragraph between "offer" and "INFO" examples: "Once the dialog is
> established as described in section Section 4.2 the Answerer sends the
> following INFO request."
> The word "section" is repeated.
OK
> -9.2, first paragraph: "It specifies the subset of existing SDP attributes,
> that are needed..."
> s/are/is  (the phrase constrains "subset", not "attributes").
OK
> "The grammar uses the indicator for case-sensitivity %s is defined in
> [RFC7405], but also imports grammars for other SDP attributes that precede
> the production of that RFC. "
> I can't parse that sentence.
Maybe due to the typo? It should read "... the indicator for 
case-sensitivity %s _as_ defined in [RFC7405], ..."
> - 10.1, 2nd paragraph: s/introduces/"would introduce"
> 3rd paragraph: s/"one exchange"/"one active exchange"
OK
> - 11.2: Weird line spacing.
> "Applications which use this Media Type:
> The listed applications only use this media type when using trickle-ice,
> right? Would it make sense to just include "trickle-ice"?
OK

Regards
Thomas