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 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 r
Re: [Gen-art] Genart last call review of draft-ietf-mmusic-trickle-ice-sip-12
Thomas Stach writes: >> * 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. Yes. >> 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) Yes, I was wrong there. >> 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 >> and in . 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. I see no objection to making the field order rigid. I just wanted to make sure that the given grammar matched your understanding/intention. Dale ___ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
Re: [Gen-art] Genart last call review of draft-ietf-mmusic-trickle-ice-sip-12
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
[Gen-art] Genart last call review of draft-ietf-mmusic-trickle-ice-sip-12
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 figure
