From nobody Thu Jun 1 17:48:41 2017 Return-Path: X-Original-To: clue@ietfa.amsl.com Delivered-To: clue@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B1760129B65; Thu, 1 Jun 2017 17:48:39 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.88 X-Spam-Level: X-Spam-Status: No, score=-1.88 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RP_MATCHES_RCVD=-0.001, T_SPF_HELO_PERMERROR=0.01, T_SPF_PERMERROR=0.01] autolearn=ham autolearn_force=no Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IuZ8MWBFz4vf; Thu, 1 Jun 2017 17:48:36 -0700 (PDT) Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 15C2B12945A; Thu, 1 Jun 2017 17:48:35 -0700 (PDT) Received: from Svantevit.roach.at (cpe-70-122-154-80.tx.res.rr.com [70.122.154.80]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id v520mWCl039437 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Thu, 1 Jun 2017 19:48:33 -0500 (CDT) (envelope-from adam@nostrum.com) X-Authentication-Warning: raven.nostrum.com: Host cpe-70-122-154-80.tx.res.rr.com [70.122.154.80] claimed to be Svantevit.roach.at From: Adam Roach To: clue@ietf.org Cc: clue-chairs@ietf.org, draft-ietf-clue-protocol@tools.ietf.org Message-ID: Date: Thu, 1 Jun 2017 19:48:30 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="------------4531E6A4C5A1DCEBF2B7F286" Content-Language: en-US Archived-At: Subject: [clue] AD Review: draft-ietf-clue-protocol-13 X-BeenThere: clue@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: CLUE - ControLling mUltiple streams for TElepresence List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Jun 2017 00:48:40 -0000 This is a multi-part message in MIME format. --------------4531E6A4C5A1DCEBF2B7F286 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit CLUE working group -- I have completed my AD review for the CLUE protocol document. Based on my reading, I do not think it is yet ready for IETF last call. Most of my comments below are feedback that the document authors should treat as normal last call comments. The feedback that I consider to block progressing the document, in my role as AD, is explicitly marked with the prefix "BLOCKER", and these will need to be resolved in a new version of the document before progressing it further. Note that it is entirely possible that something I have marked "BLOCKER" may stem from an error on my part; so recognize that these are not demands for change, as much as a need to have things either fixed in the document or explained to me. Title: The rule of thumb is that all but a small handful of well-known acronyms need to be expanded in titles and abstracts. I recognize that "CLUE" is a bit tortured, as acronyms go, but the title of this document is, broadly speaking, opaque. Please change it to something meaningful, such as "Protocol for Controlling Multiple Streams for Telepresence (CLUE)" General: There are three instances of excessively long lines in the document. General: Please number and caption the figures in this document. Also, please refer to the figure numbers when pointing to them (e.g., the first paragraph of section 6.1 should read something like: "As soon as the sub-state machine of the MP (Figure 1) is activated..."." BLOCKER: General: There are several mentions of timeouts and retry thresholds in the text and its corresponding state machines; however, the document neither defines nor cites a document as defining what these timeout and retry values are. These need to be defined and described. If the timer and retry scheme allows the two ends of the connection to have different values for timeouts and number of retries, then there need to be additional error procedures that allow the MC and MP state machines to stay in sync (if the timer/retry values can be different, it's possible for one state machine to transition to "terminated," while the other is still active, and you need messaging to clean this up). The remainder of this comment is non-blocking: Related to this, the document frequently refers to retries as "expiring" (e.g., "retry expired" on the state diagrams). That doesn't really make sense unless "retry" is the name of a timer rather than a counter; I think you mean to say "exhausted" or something similar. General: This document defines six message types, all of which have at least two names, many of which have more. It would be a lot easier to keep track of what is being described if these were kept consistent. I suggest choosing one term for each concept and sticking with it. In other words, please pick only one name from each of the following lines, and do not use any of the others: 1. OPTIONS, options 2. OPTIONS RESPONSE, optionsResponse 3. ADVERTISEMENT, ADV, advertisement 4. ADVERTISEMENT ACKNOWLEDGEMENT, ADV ACK, ACK, NACK, ack 5. CONFIGURE, CONF, CONF+ACK, configure 6. CONFIGURE RESPONSE, CONF RESPONSE, configureResponse The terminology section appears to be in alphabetical order, except for "Capture Encoding" (which was presumably "Media Capture Encoding" in some earlier version). Please fix. The definitions for "Endpoint", "MCU", and "Media Stream/Stream" vary from the definition given in draft-ietf-clue-framework. Is this intentional? The first paragraph of section 4 mentions the framework and data model documents without citations. There should probably be citations to the corresponding documents. Section 4 contains the following text: The CLUE protocol represents the mechanism for the exchange of CLUE information between CLUE Participants This is sufficiently circular as to be basically meaningless. Suggest: "...for the exchange of telepresence information between..." Section 5: The versioning scheme in here is rather perplexing. Is there some technical reason the protocol restricts major version numbers to a single digit and minor versions to a single digit? Strongly suggest that this should be expanded to allow multiple digits for both the major and minor versions. Section 5: It should be made clear that the XML Schema excerpts in section 5 are non-normative. In particular, I recommend adding the following text to the end of the first paragraph of section 5: "This section includes non-normative excerpts of the schema to aid in describing it." Section 5: If the single-digit version numbers are maintained (and, again, I strongly recommend against this), the definition of versionType appears to be wrong: it allows a major version digit of "0", which would seem to be precluded by the way versions are currently defined. Section 5: The XML definition allows zero or more elements to appear in a message. If more than one is allowed, the document should explain how multiple IDs are handled. If they are not, then the schema and/or text needs to prohibit having more than one. Section 5: The description for says that a 402 will be sent in the case of an "unexpected" sequence number. This needs clarification: is this for case where a sequence number gap is detected? A repeated sequence number? A number that is too small? All of the above? At least describe this with the 402 error code, and point to that description from here. Section 5: The description for would benefit from the addition of "This document describes version 1.0". Section 5.1: "The OPTIONS message is sent by the CP which is the CI to the CP which is the CR as soon as the CLUE data channel is ready." Although it's a problem in other parts of the document too, the dense use of nearly identical two-letter acronyms in here makes it quite hard to read. I had to keep consulting the definitions of those acronyms to decode this sentence (as well as several similar ones). Consider just expanding these terms (as well as "MP" and "MC") instead of using them in prose. Section 5.1 starts to wobble back and forth between referring to elements with and without angle brackets (e.g., it uses both "supportedVersions" and ""). Please pick one and stick with it. BLOCKER: Section 5.1: The description of describes a scheme in which multiple supported versions can be listed; and, if the list is omitted, it implies that only the version described in is supported. This text does not define (nor does any other text that I can find) what should be set to when is used. Intuitively, it seems that should be set to the largest minor version of the smallest major version advertised in , but that (or whatever the correct answer is) needs to be clearly spelled out. BLOCKER: Section 5.2: "If the responseCode is of the type 2xx the response MUST also include..." -- you can't use "2xx" in a normative statement without first defining what it means. As you don't use the "#xx" format anywhere else, I suggest rephrasing: "If the is between 200 and 299 inclusive, the response MUST also include..." Sections 5.2, 5.4, 5.6: It seems really odd that the document defines a base clueMessageType that all messages derive from, but then leaves all the response types (optionsResponse, ack, configureResponse) to repeatedly and independently add ,, and the sequence number of the corresponding message over and over again. I would strongly suggest adding something like: ...and then defining those three response types as being extensions of "clueResponse" instead of "clueMessage", like this: Regardless of how you do this, any section that adds "responseCode" and "reasonString" needs to point to section 5.7 in its description to explain how those fields are populated and interpreted. Section 5.5: This section allows a boolean flag in a message to acknowledge an . Is this intended to always be handled like a 200? Consider: if you define a 201 response code in the future, will implementations be unable to convey its meaning in a CONF + ACK? Given that the document defines a class of codes for success rather than a simple success flag in general, it seems that this element should carry a success response code rather than just a boolean. If you decide to keep the boolean, be very clear that it is to be treated as a 200 rather than any other potential success code; and that conveying any other kind of success requires a separate message. Section 5.5: the final paragraph mentions the element -- it would be helpful to add something like "see [I-D.ietf-clue-datamodel] for the definition of ." BLOCKER: Section 5.7 indicates that there is a class of response codes, starting with "1", which are used to indicate "delayed or incomplete" responses. The document does not describe any protocol behavior for this class of response. The description of the meaning of this class (and the obvious parallels to HTTP and SIP) imply that some subsequent response associated with the same request will be arriving at some point in the future. If that's the intention, this document needs a *lot* more text (and corresponding adjustments to the state machines) to explain how these 100-class codes are handled. In practice, since the current version of the document does not define nor make use of 100-class codes, I suggest that the most reasonable path forward is to remove discussion of codes starting with "1" from the first paragraph of section 5.7, instead adding a paragraph immediately following it that says something like: This document does not define response codes starting with "1", and such response codes are not allowed to appear in major version 1 of the CLUE protocol. The range from 100 to 199 inclusive is reserved for future major versions of the protocol to define response codes for delayed or incomplete operations if necessary. Response codes starting with "5" through "9" are reserved for future major versions of the protocol to define new classes of response, and are not allowed in major version 1 of the CLUE protocol. Response codes starting with "0" are not allowed. Section 5.7: "The response codes and strings defined for use with CLUE are as follows" - this strongly implies that the descriptions given in this table are the only ones that are allowed in CLUE element, and that any other messages should presumably be treated as an error. Surely that's not what you mean. Suggest rephrasing to indicate that the "Description" text can be sent in the , but that implementations can (and are encouraged to) include more specific descriptions of the error condition, if possible. Section 5.7: I can't figure out how an implementation could ever send a "300" response code. If the XML syntax is incorrect, that's a 301. If the message contains an invalid value, that's a 302. And those are the only two conditions that are described for 300. I think you need to give "300" a bit more thought -- if you can't come up with a more generic description (e.g., "low-level request error"), you probably want to remove it. Section 5.7: for 403, be clear which identifier is meant. Do you mean "The used in the message is not valid..."? Section 5.7: for 404, please be clear that you're talking about the sequence number rather than just using "number" without qualification. Section 5.7: The description for 405 uses the acronym "MCC" without expanding it. Please expand it. BLOCKING: The state machines in section 6 and its subsections don't have transitions for all possible messages that could arrive in a state. This can cause interop issues. Please add text that clearly indicates whether such messages do or do not cause a transition. (This might be as simple as "messages not shown for a state do not cause the state to change," but only if you carefully check that this is true -- for example, what should an MP state machine do do if it gets a "CONF + ACK" in the state "WAIT FOR CONF"?) Section 6: The fifth paragraph says "CLUE channel" where it means "CLUE data channel." Section 6: The eighth paragraph says: The CP moves from the ACTIVE state to the IDLE one when the sub-state machines that have been activated are (both) in the relative TERMINATED state (see sections Section 6.1 and Section 6.2). The "both" in this paragraph is confusing, since it's possible to have only one or the other machine running. Please rephrase. Section 6.2 describes a state machine that starts in a state called "WAIT FOR ADV." This state does not appear to be timer-supervised, meaning that implementations of this state machine can stay in this state literally forever. Is that the intention? Section 6.2 also describes the possibility of sending a and separately or as a combined message. I would have expected to see some discussion here about why implementations might choose one behavior over the other. Section 7: The second sentence of the second paragraph needs a verb. Section 7, paragraph 3: this claims that versions are a non-negative *integer* rather than a *digit*. As I mention above, this seems to be the right way to handle versions, but it is decidedly at odds with text elsewhere in the document and in the schema. Regardless of how you choose to treat versions, the text needs to be consistent. Section 7, final paragraph: replace "Clue" with "CLUE." Section 8 contains a paragraph starting with "In that case, the new information..." but it's not clear which case it's referring to. Please replace "In that case" with a description of the case under consideration. Section 8 contains the phrase "Similarly to what said before..." -- this is ungrammatical and needs to be rephrased. Section 8 indicates that extensions need to indicate "the standard version of the protocol the extension refers to." Given that there is compatibility within a major version of a protocol, I think this means to say "the major standard version of the protocol that the extension refers to." Section 8 has a paragraph starting "For that reason..." -- it's not clear what reason is being referred to here. Please clarify. Section 8 contains schema that contains a element. It should be clarified that this is the *protocol* version, not the *extension* version (or, if it's the extension version, that needs to be spelled out too, but I think you'll need a new namespace for that...?) BLOCKING: Section 8.1 is an example section, which are non-normative in IETF documents. It contains 2119-style normative language, however. These normative statements need to be moved out of the example section (and probably into section 8). The remainder of this comment is non-blocking: I also find the "SHOULD" in this section to be highly perplexing. Can you explain the rationale behind requiring schema, but not requiring any description of what the schema *means?) BLOCKING: The example in section 8.1 includes the following: xmlns="clue-info-extension-myVideoExtensions" I'm pretty certain that namespaces are required to be identified by URIs rather than arbitrary strings. Section 8.1: The final paragraph also has normative language in it, although it appears to be reiterating requirements from elsewhere in the document. I suggest lowercasing "MUST" in this paragraph. Section 8.1: The final paragraph mentions the use of and to negotiate the extension. An example demonstrating this negotiation would be extremely useful. The schema in section 9 contains: If you do not intend to bake this "-17" into the document (and I can't imagine you do), please add an RFC editor note to change it to something else upon publication. Also, the schema in section 9 is (with rare exception) unindented. This makes it *very* hard to read. Please consider formatting it with conventional XML indentation. (This also applies to the schema excerpts earlier in the document.) Has there been any automated tool-based checking that the examples in section 10 to verify that they conform to the schema in section 9 (and the schemata it imports)? Section 10.2: Please expand the acronym "MCCs" in the section title (keep in mind that this appears in the table of contents, where it needs to makes sense). Section 10 in general: While it does consume a lot of space, I don't think that defining six rather different message types and then showing only *one* type in the examples is very illustrative of the protocol. I would *STRONGLY* suggest adding at least a response message, and ideally the example section should contain at least one example for each of the six message types. Section 11, paragraph 4: Replace "Clue" with "CLUE." Section 12.1 has a strange double-double quote around the URN name, and section 12.3 repeats this for the MIME type. All subsections of section 12: please update all registrant contact information to point to the IESG (iesg@ietf.org) rather than the CLUE working group and one of the authors. Section 12.4.1: These descriptions will appear in an IANA registry, where the phrase "in this document" will have no context and be rather nonsensical. Please rephrase. Sections 13 through 23 should include an RFC editor note asking for removal before publication. /a --------------4531E6A4C5A1DCEBF2B7F286 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit

CLUE working group --

I have completed my AD review for the CLUE protocol document. Based on my reading, I do not think it is yet ready for IETF last call.

Most of my comments below are feedback that the document authors should treat as normal last call comments. The feedback that I consider to block progressing the document, in my role as AD, is explicitly marked with the prefix "BLOCKER", and these will need to be resolved in a new version of the document before progressing it further. Note that it is entirely possible that something I have marked "BLOCKER" may stem from an error on my part; so recognize that these are not demands for change, as much as a need to have things either fixed in the document or explained to me.

Title: The rule of thumb is that all but a small handful of well-known acronyms need to be expanded in titles and abstracts. I recognize that "CLUE" is a bit tortured, as acronyms go, but the title of this document is, broadly speaking, opaque. Please change it to something meaningful, such as "Protocol for Controlling Multiple Streams for Telepresence (CLUE)"

General: There are three instances of excessively long lines in the document.

General: Please number and caption the figures in this document. Also, please refer to the figure numbers when pointing to them (e.g., the first paragraph of section 6.1 should read something like: "As soon as the sub-state machine of the MP (Figure 1) is activated..."."

BLOCKER: General: There are several mentions of timeouts and retry thresholds in the text and its corresponding state machines; however, the document neither defines nor cites a document as defining what these timeout and retry values are. These need to be defined and described. If the timer and retry scheme allows the two ends of the connection to have different values for timeouts and number of retries, then there need to be additional error procedures that allow the MC and MP state machines to stay in sync (if the timer/retry values can be different, it's possible for one state machine to transition to "terminated," while the other is still active, and you need messaging to clean this up). The remainder of this comment is non-blocking: Related to this, the document frequently refers to retries as "expiring" (e.g., "retry expired" on the state diagrams). That doesn't really make sense unless "retry" is the name of a timer rather than a counter; I think you mean to say "exhausted" or something similar.

General: This document defines six message types, all of which have at least two names, many of which have more. It would be a lot easier to keep track of what is being described if these were kept consistent. I suggest choosing one term for each concept and sticking with it. In other words, please pick only one name from each of the following lines, and do not use any of the others:

  1. OPTIONS, options
  2. OPTIONS RESPONSE, optionsResponse
  3. ADVERTISEMENT, ADV, advertisement
  4. ADVERTISEMENT ACKNOWLEDGEMENT, ADV ACK, ACK, NACK, ack
  5. CONFIGURE, CONF, CONF+ACK, configure
  6. CONFIGURE RESPONSE, CONF RESPONSE, configureResponse

The terminology section appears to be in alphabetical order, except for "Capture Encoding" (which was presumably "Media Capture Encoding" in some earlier version). Please fix.

The definitions for "Endpoint", "MCU", and "Media Stream/Stream" vary from the definition given in draft-ietf-clue-framework. Is this intentional?

The first paragraph of section 4 mentions the framework and data model documents without citations. There should probably be citations to the corresponding documents.

Section 4 contains the following text:

   The CLUE protocol represents the mechanism
   for the exchange of CLUE information between CLUE Participants

This is sufficiently circular as to be basically meaningless. Suggest: "...for the exchange of telepresence information between..."

Section 5: The versioning scheme in here is rather perplexing. Is there some technical reason the protocol restricts major version numbers to a single digit and minor versions to a single digit? Strongly suggest that this should be expanded to allow multiple digits for both the major and minor versions.

Section 5: It should be made clear that the XML Schema excerpts in section 5 are non-normative. In particular, I recommend adding the following text to the end of the first paragraph of section 5: "This section includes non-normative excerpts of the schema to aid in describing it."

Section 5: If the single-digit version numbers are maintained (and, again, I strongly recommend against this), the definition of versionType appears to be wrong: it allows a major version digit of "0", which would seem to be precluded by the way versions are currently defined.

Section 5: The XML definition allows zero or more <clueId> elements to appear in a message. If more than one is allowed, the document should explain how multiple IDs are handled. If they are not, then the schema and/or text needs to prohibit having more than one.

Section 5: The description for <sequenceNr> says that a 402 will be sent in the case of an "unexpected" sequence number. This needs clarification: is this for case where a sequence number gap is detected? A repeated sequence number? A number that is too small? All of the above? At least describe this with the 402 error code, and point to that description from here.

Section 5: The description for <v> would benefit from the addition of "This document describes version 1.0".

Section 5.1: "The OPTIONS message is sent by the CP which is the CI to the CP which is the CR as soon as the CLUE data channel is ready." Although it's a problem in other parts of the document too, the dense use of nearly identical two-letter acronyms in here makes it quite hard to read. I had to keep consulting the definitions of those acronyms to decode this sentence (as well as several similar ones). Consider just expanding these terms (as well as "MP" and "MC") instead of using them in prose.

Section 5.1 starts to wobble back and forth between referring to elements with and without angle brackets (e.g., it uses both "supportedVersions" and "<supportedVersions>"). Please pick one and stick with it.

BLOCKER: Section 5.1: The description of <supportedVersions> describes a scheme in which multiple supported versions can be listed; and, if the list is omitted, it implies that only the version described in <v> is supported. This text does not define (nor does any other text that I can find) what <v> should be set to when <supportedVersions> is used. Intuitively, it seems that <v> should be set to the largest minor version of the smallest major version advertised in <supportedVersions>, but that (or whatever the correct answer is) needs to be clearly spelled out.

BLOCKER: Section 5.2: "If the responseCode is of the type 2xx the response MUST also include..." -- you can't use "2xx" in a normative statement without first defining what it means. As you don't use the "#xx" format anywhere else, I suggest rephrasing: "If the <responseCode> is between 200 and 299 inclusive, the response MUST also include..."

Sections 5.2, 5.4, 5.6: It seems really odd that the document defines a base clueMessageType that all messages derive from, but then leaves all the response types (optionsResponse, ack, configureResponse) to repeatedly and independently add <responseCode>,<responseString>, and the sequence number of the corresponding message over and over again. I would strongly suggest adding something like:

   <!-- CLUE RESPONSE TYPE -->
   <xs:complexType name="clueResponseType">
   <xs:complexContent>
   <xs:extension base="clueMessageType">
   <xs:sequence>
   <xs:element name="responseCode" type="responseCodeType"/>
   <xs:element name="reasonString" type="xs:string" minOccurs="0"/>
   <xs:element name="requestSequenceNr" type="xs:positiveInteger"/>
   <xs:any namespace="##other" processContents="lax" minOccurs="0"/>
   </xs:sequence>
   </xs:extension>
   </xs:complexContent>
   </xs:complexType>

...and then defining those three response types as being extensions of "clueResponse" instead of "clueMessage", like this:

   <!-- ADV ACK MESSAGE TYPE -->
   <xs:complexType name="advAcknowledgementMessageType">
   <xs:complexContent>
   <xs:extension base="clueResponseType">
   <xs:anyAttribute namespace="##other" processContents="lax"/>
   </xs:extension>
   </xs:complexContent>
   </xs:complexType>

Regardless of how you do this, any section that adds "responseCode" and "reasonString" needs to point to section 5.7 in its description to explain how those fields are populated and interpreted.

Section 5.5: This section allows a boolean flag in a <configure> message to acknowledge an <advertisement>. Is this intended to always be handled like a 200? Consider: if you define a 201 response code in the future, will implementations be unable to convey its meaning in a CONF + ACK? Given that the document defines a class of codes for success rather than a simple success flag in general, it seems that this <ack> element should carry a success response code rather than just a boolean. If you decide to keep the boolean, be very clear that it is to be treated as a 200 rather than any other potential success code; and that conveying any other kind of success requires a separate <ack> message.

Section 5.5: the final paragraph mentions the <captureEncodings> element -- it would be helpful to add something like "see [I-D.ietf-clue-datamodel] for the definition of <captureEncodings>."

BLOCKER: Section 5.7 indicates that there is a class of response codes, starting with "1", which are used to indicate "delayed or incomplete" responses. The document does not describe any protocol behavior for this class of response. The description of the meaning of this class (and the obvious parallels to HTTP and SIP) imply that some subsequent response associated with the same request will be arriving at some point in the future. If that's the intention, this document needs a *lot* more text (and corresponding adjustments to the state machines) to explain how these 100-class codes are handled. In practice, since the current version of the document does not define nor make use of 100-class codes, I suggest that the most reasonable path forward is to remove discussion of codes starting with "1" from the first paragraph of section 5.7, instead adding a paragraph immediately following it that says something like:

  This document does not define response codes starting with "1", and such
  response codes are not allowed to appear in major version 1 of the CLUE
  protocol. The range from 100 to 199 inclusive is reserved for future major
  versions of the protocol to define response codes for delayed or incomplete
  operations if necessary. Response codes starting with "5" through "9" are
  reserved for future major versions of the protocol to define new classes of
  response, and are not allowed in major version 1 of the CLUE protocol.
  Response codes starting with "0" are not allowed.

Section 5.7: "The response codes and strings defined for use with CLUE are as follows" - this strongly implies that the descriptions given in this table are the only ones that are allowed in CLUE <reasonString> element, and that any other messages should presumably be treated as an error. Surely that's not what you mean. Suggest rephrasing to indicate that the "Description" text can be sent in the <reasonString>, but that implementations can (and are encouraged to) include more specific descriptions of the error condition, if possible.

Section 5.7: I can't figure out how an implementation could ever send a "300" response code. If the XML syntax is incorrect, that's a 301. If the message contains an invalid value, that's a 302. And those are the only two conditions that are described for 300. I think you need to give "300" a bit more thought -- if you can't come up with a more generic description (e.g., "low-level request error"), you probably want to remove it.

Section 5.7: for 403, be clear which identifier is meant. Do you mean "The <clueId> used in the message is not valid..."?

Section 5.7: for 404, please be clear that you're talking about the sequence number rather than just using "number" without qualification.

Section 5.7: The description for 405 uses the acronym "MCC" without expanding it. Please expand it.

BLOCKING: The state machines in section 6 and its subsections don't have transitions for all possible messages that could arrive in a state. This can cause interop issues. Please add text that clearly indicates whether such messages do or do not cause a transition. (This might be as simple as "messages not shown for a state do not cause the state to change," but only if you carefully check that this is true -- for example, what should an MP state machine do do if it gets a "CONF + ACK" in the state "WAIT FOR CONF"?)

Section 6: The fifth paragraph says "CLUE channel" where it means "CLUE data channel."

Section 6: The eighth paragraph says:

   The CP moves from the ACTIVE state to the IDLE one when the sub-state
   machines that have been activated are (both) in the relative
   TERMINATED state (see sections Section 6.1 and Section 6.2).

The "both" in this paragraph is confusing, since it's possible to have only one or the other machine running. Please rephrase.

Section 6.2 describes a state machine that starts in a state called "WAIT FOR ADV." This state does not appear to be timer-supervised, meaning that implementations of this state machine can stay in this state literally forever. Is that the intention?

Section 6.2 also describes the possibility of sending a <ack> and <configure> separately or as a combined message. I would have expected to see some discussion here about why implementations might choose one behavior over the other.

Section 7: The second sentence of the second paragraph needs a verb.

Section 7, paragraph 3: this claims that versions are a non-negative *integer* rather than a *digit*. As I mention above, this seems to be the right way to handle versions, but it is decidedly at odds with text elsewhere in the document and in the schema. Regardless of how you choose to treat versions, the text needs to be consistent.

Section 7, final paragraph: replace "Clue" with "CLUE."

Section 8 contains a paragraph starting with "In that case, the new information..." but it's not clear which case it's referring to. Please replace "In that case" with a description of the case under consideration.

Section 8 contains the phrase "Similarly to what said before..." -- this is ungrammatical and needs to be rephrased.

Section 8 indicates that extensions need to indicate "the standard version of the protocol the extension refers to." Given that there is compatibility within a major version of a protocol, I think this means to say "the major standard version of the protocol that the extension refers to."

Section 8 has a paragraph starting "For that reason..." -- it's not clear what reason is being referred to here. Please clarify.

Section 8 contains schema that contains a <version> element. It should be clarified that this is the *protocol* version, not the *extension* version (or, if it's the extension version, that needs to be spelled out too, but I think you'll need a new namespace for that...?)

BLOCKING: Section 8.1 is an example section, which are non-normative in IETF documents. It contains 2119-style normative language, however. These normative statements need to be moved out of the example section (and probably into section 8). The remainder of this comment is non-blocking: I also find the "SHOULD" in this section to be highly perplexing. Can you explain the rationale behind requiring schema, but not requiring any description of what the schema *means?)

BLOCKING: The example in section 8.1 includes the following:

   xmlns="clue-info-extension-myVideoExtensions"

I'm pretty certain that namespaces are required to be identified by URIs rather than arbitrary strings.

Section 8.1: The final paragraph also has normative language in it, although it appears to be reiterating requirements from elsewhere in the document. I suggest lowercasing "MUST" in this paragraph.

Section 8.1: The final paragraph mentions the use of <options> and <optionsResponse> to negotiate the extension. An example demonstrating this negotiation would be extremely useful.

The schema in section 9 contains:

<xs:import namespace="urn:ietf:params:xml:ns:clue-info"
schemaLocation="data-model-schema-17.xsd"/>

If you do not intend to bake this "-17" into the document (and I can't imagine you do), please add an RFC editor note to change it to something else upon publication.

Also, the schema in section 9 is (with rare exception) unindented. This makes it *very* hard to read. Please consider formatting it with conventional XML indentation. (This also applies to the schema excerpts earlier in the document.)

Has there been any automated tool-based checking that the examples in section 10 to verify that they conform to the schema in section 9 (and the schemata it imports)?

Section 10.2: Please expand the acronym "MCCs" in the section title (keep in mind that this appears in the table of contents, where it needs to makes sense).

Section 10 in general: While it does consume a lot of space, I don't think that defining six rather different message types and then showing only *one* type in the examples is very illustrative of the protocol. I would *STRONGLY* suggest adding at least a response message, and ideally the example section should contain at least one example for each of the six message types.

Section 11, paragraph 4: Replace "Clue" with "CLUE."

Section 12.1 has a strange double-double quote around the URN name, and section 12.3 repeats this for the MIME type.

All subsections of section 12: please update all registrant contact information to point to the IESG (iesg@ietf.org) rather than the CLUE working group and one of the authors.

Section 12.4.1: These descriptions will appear in an IANA registry, where the phrase "in this document" will have no context and be rather nonsensical. Please rephrase.

Sections 13 through 23 should include an RFC editor note asking for removal before publication.

/a

--------------4531E6A4C5A1DCEBF2B7F286-- From nobody Fri Jun 2 17:14:58 2017 Return-Path: X-Original-To: clue@ietfa.amsl.com Delivered-To: clue@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5363B12EA8C; Fri, 2 Jun 2017 17:14:56 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.881 X-Spam-Level: X-Spam-Status: No, score=-1.881 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.001, T_SPF_HELO_PERMERROR=0.01, T_SPF_PERMERROR=0.01] autolearn=ham autolearn_force=no Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fQ6cjItbAKQ0; Fri, 2 Jun 2017 17:14:54 -0700 (PDT) Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id E727B129BFF; Fri, 2 Jun 2017 17:14:53 -0700 (PDT) Received: from Svantevit.roach.at (cpe-70-122-154-80.tx.res.rr.com [70.122.154.80]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id v530EqsG079113 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Fri, 2 Jun 2017 19:14:53 -0500 (CDT) (envelope-from adam@nostrum.com) X-Authentication-Warning: raven.nostrum.com: Host cpe-70-122-154-80.tx.res.rr.com [70.122.154.80] claimed to be Svantevit.roach.at From: Adam Roach To: clue@ietf.org Cc: clue-chairs@ietf.org, draft-ietf-clue-signaling@tools.ietf.org Message-ID: <0b69d2f1-11e1-8fd1-d4a1-2faacc0a8528@nostrum.com> Date: Fri, 2 Jun 2017 19:14:51 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Archived-At: Subject: [clue] AD Review: draft-ietf-clue-signaling-11 X-BeenThere: clue@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: CLUE - ControLling mUltiple streams for TElepresence List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Jun 2017 00:14:56 -0000 CLUE working group -- I have completed my AD review for the CLUE signaling document. The document is generally in good shape, but I think we'll need another revision before putting it in front of the IETF for last call (especially due to the apparent incomplete removal of support for specifying multiple CLUE groups per session). Most of my comments below are feedback that the document authors should treat as normal last call comments. The feedback that I consider to block progressing the document, in my role as AD, is explicitly marked with the prefix "BLOCKER", and these will need to be resolved in a new version of the document before progressing it further. Note that it is entirely possible that something I have marked "BLOCKER" may stem from an error on my part; so recognize that these are not demands for change, as much as a need to have things either fixed in the document or explained to me. Title: The rule of thumb is that all but a small handful of well-known acronyms need to be expanded in titles and abstracts. I recognize that "CLUE" is a bit tortured, as acronyms go, but the title of this document is, broadly speaking, opaque. Please change it to something meaningful, such as "Session Signaling for Controlling Multiple Streams for Telepresence (CLUE)" BLOCKER: General: It is clear from reading this version of the document that earlier versions contained the notion of multiple "a=group:CLUE" lines in a single SDP description. This version appears to have tried to remove all related text, but there are still enough mentions that talk about CLUE groups in a way that implies that there can be multiples so as to cause confusion. These need to be cleaned up. I call the specific instances out on a section-by-section basis below. I'm mentioning it up here since it's really only one blocker issue with a bunch of instances. General: The _protocol_ document has a host of different terms for each kind of CLUE message (e.g., "ADVERTISEMENT," "ADV," and "advertisement" for the same operation). This document exacerbates the situation by introducing yet more variations, such as "Advertisement" and "Configure". Please coordinate with draft-ietf-clue-protocol to use a consistent set of names for these operations between the two documents. Introduction: The convention I see in this document is to write defined terms with initial caps; please replace "encoding group" with "Encoding Group." Section 3: Please add a reference to RFC3840 (e.g.: 'The "sip.clue" media feature tag [RFC3840] indicates...") Section 4.2: "Presence of the data channel in a CLUE group..." implies there can be more than one group. Replace "a" with "the." Section 4.3: ...its "mid" value MUST be included in a CLUE group' implies there can be more than one group. Replace "a" with "the." Section 4.4.1: "...in a CLUE group as defined above." implies there can be more than one group. Replace "a" with "the." Section 4.4.1: '..."m=" lines in the same CLUE group in the SDP message...' very strongly implies there can be more than one group. Rephrase, perhaps along the lines of "...CLUE-controlled "m=" lines in the SDP message..." 4.4.2: 'These "m=" lines are CLUE-controlled and hence MUST include their "mid" in the CLUE group corresponding to the CLUE group of the Encoding they wish to receive.' is getting pretty explicit about the presence of multiple CLUE groups. Fix. 4.5.2.1, first sentence: Replace "If the recipient is a CLUE-capable..." with "If the recipient of an offer is a CLUE-capable..." BLOCKER: Section 4.5.2.2: For avoidance of doubt, this section should clearly indicate what the answer should do with CLUE-controlled lines that it has no intention of receiving (for sendonly) or sending (for recvonly). I believe the expectation here is to set the port to zero (rather than, e.g., setting the direction to inactive). The document should explicitly state this behavior: if implementations make different choices between port-zero and inactive and don't expect the other behavior, you can end up with incompatibilities. Section 4.5.3.1, paragraph 2: My recollection is that telling implementors not to send media is frequently misinterpreted to mean that they don't have to send/receive RTCP either. This causes all kinds of grief. It will probably head off issues if this section is phrased more like "...MAY choose not to send RTP on the non-CLUE-controlled channels (although RTCP is still sent and received as normal) during the period..." Section 4.5.4.1: 'Subsequent offer/answer exchanges MAY add additional "m=" lines...' -- this should probably also mention "and activate inactive ones." Section 4.5.4.1: 'Subsequent offer/answer exchanges MAY also deactivate "m=" lines for CLUE-controlled media.' -- again, the interpretation of "deactivate" may be different between implementors. Please be clear about whether this means "a=inactive", port=0, or both. Section 4.5.4.1: The final paragraph talks about "deactivating" non-CLUE media. Again, this should be explicit about what is meant. Section 4.5.4.2: 'If, in an ongoing non-CLUE call, an SDP offer/answer exchange completes with both sides having included a data channel "m=" line in their SDP and with the "mid" for that channel in corresponding CLUE groups..." implies that there can be more than one CLUE group. Fix. Section 4.5.4.3: "...include the data channel in a matching CLUE group..." implies there can be more than one group. Replace "a matching" with "the." Section 4.5.4.3: "Any active "m=" lines still included in a CLUE group..." implies there can be more than one group. Replace "a" with "the." Section 4.5.4.3: "Note that this is distinct from cases where the CLUE protocol negotiation fails, or an error occurs in the CLUE protocol; see [I-D.ietf-clue-protocol] for details of media and state preservation in this circumstance." -- I carefully scrubbed the CLUE protocol document to try to determine what this is referring to. Please change it to "see [I-D.ietf-clue-protocol] section X.Y.Z", but replacing "X.Y.Z" with the section that provides the details you allude to. BLOCKER: Compare the normative statements in paragraph 2 of Section 5.3: Generally, implementations that receive messages for which they have incomplete information SHOULD wait until they have the corresponding information they lack before sending messages to make changes related to that information. For example, an answerer that receives a new SDP offer with three new "a=sendonly" CLUE "m=" lines for which it has received no CLUE Advertisement providing the corresponding capture information SHOULD include corresponding "a=inactive" lines in its answer, and SHOULD make a new SDP offer with "a=recvonly" when and if a new Advertisement arrives with Captures relevant to those Encodings. With the normative statements in section 4.5.2.2: If the initial offer contained "a=recvonly" CLUE-controlled media lines the recipient SHOULD include corresponding "a=sendonly" CLUE- controlled media lines for accepted Encodings ... If the initial offer contained "a=sendonly" CLUE-controlled media lines the recipient MAY include corresponding "a=recvonly" CLUE- controlled media lines 5.3 says "SHOULD set a=inactive" in the exact same circumstances 4.5.2.2 says "SHOULD set a=sendonly". Please pick one expected behavior and make sure both sections agree. Ideally, you would refactor this so that the normative statement is made in only one location. Section 7 appears to be oddly silent on the use of RTCP MUX. I suspect that the intention is that CLUE will generally multiplex RTCP when using BUNDLE? I would expect this to be called out. Section 7 offers that the use of BUNDLE has the advantage of reducing the number of ICE candidates that need to be collected. This is true only in the case that some m= section are marked as bundle-only; this, in turn, implies that a bundled offer is going to contain one or more m= sections that will fail if the remote endpoint does not implement BUNDLE. This interacts particularly badly with the text in section 4.5.1 that allows, under certain circumstances, that 'the initial offer MAY contain "m=" lines for CLUE-controlled media.', since you can end up with the whole CLUE session falling apart if the bundle-only lines are not chosen carefully. This is a relatively complicated issue that I wouldn't expect impementors to get correct without some guidance on the use of bundle-only, and in particular its interaction with CLUE-controlled lines. Please add text to Section 7 that discusses these issues. Section 8: Please insert a page break before the ladder diagram so that the entity names don't appear on a page by themselves. Section 8: "In this case Bob is the Channel Initiator..." this isn't clear (and, in fact, it's counterintuitive to me) -- perhaps there should be some text indicating *why* Bob is the Channel Initiator. General, but surfaced in section 8: The procedures described in this document virtually guarantee that every CLUE call that is established will result in glare (response code 491) behavior. This might cause the operations folks some heartburn, as it means that their error counts will spike once CLUE is deployed. Further, without fairly advanced analysis of the callflow, this will make it impossible to distinguish "expected" CLUE-induced 491s from the oddball actual glare conditions usually signaled by 491. Has any consideration been given to avoiding this situation (e.g., by having the called party wait on the order of one second before attempting to negotiate its encodings)? Section 8 contains the following text: Bob also sends his SDP answer as part of SIP 200 OK 2. Alongside his original audio, video and CLUE "m=" lines he includes two active recvonly "m= "lines and a zeroed "m=" line for the third. This should probably be qualified to indicate that these are the same m-sections as were present in the offer (as opposed to creating new sections). Section 8: Replace "Having received this Alice..." with "Having received this offer, Alice..." Section 9: "From the lack of the data channel and grouping framework..." -- it should be noted that implementations that end up in communication with normal non-CLUE WebRTC implementations might get a datachannel but no CLUE group. The quoted text implies that the presence or absence of a datachannel can be used to determine CLUE support, which might cause implementors to rely on that exclusively. Please change the text to indicate that the CLUE group should be used as the sole determinant of CLUE support (at least, as far as SDP signaling is concerned). Section 10: It is rather unusual to include authors in the acknowledgements section. For each of Rob Hansen, Paul Kyzivat, and Christian Groves, I suggest removing the individual's name from either the Acknowledgements section or from the authors list. Section 11.2: "This specification registers a new media feature tag in the SIP [RFC3264] tree..." This should cite RFC3261 for SIP rather than RFC3264. Section 11.2: Please indicate "iesg@ietf.org" as the contact for this registration. BLOCKING: Section 12 indicates that no security mechanisms are mandatory to use "due to the issues addressed in [RFC7202]." This vastly misconstrues the intent and text of RFC7202, which (roughly speaking) says "we don't mandate security for RTP in general because it is used in a vast and varied array of applications." CLUE is a very specific, very narrow application of RTP that does not suffer from the generality that makes a one-size-fits-all solution for RTP infeasible. In fact, RFC7202 is quite clear that its guidance applies exclusively to RTP, and not to protocols that *use* RTP: "Documents that define an interoperable class of applications using RTP are subject to [RFC3365], and thus need to specify MTI security mechanisms." If there are specific arguments put forth in RFC7202 that the working group believes also apply to this document, please reiterate them here. The current citation to RFC7202 is problematic, though, since it says the opposite of what this security section implies. If I understand it correctly, though, the argument to put forth here is that implementations MAY choose not to secure legacy (non-CLUE-controlled) media lines because SIP was specified prior to RFC3365, and was therefore not bound by its requirements; and, as a consequence, many existing SIP endpoints are incapable of using DTLS-SRTP. This rationale is orthogonal to the guidance in RFC7202, and should not cite it. The use of "DTLS" to refer to "DTLS-SRTP" is also confusing, as they are different things. I propose changing the paragraph as follows: This attack can be prevented by ensuring that the media recipient intends to receive the media packets. As such, all CLUE-capable devices MUST support key negotiation and receiver intent assurance via DTLS-SRTP [RFC5763] on CLUE-controlled RTP "m=" lines. As specified in [I-D.ietf-clue-framework], all CLUE-controlled RTP streams must be secured and implemented using mechanisms such as SRTP [RFC3711]. CLUE implementations MAY choose not to require the use of SRTP to secure legacy (non-CLUE-controlled) media for backwards compatibility with older SIP clients that are incapable of supporting SRTP. Section 12: "To prevent this, SIP signaling SHOULD always be encrypted using TLS..." While I agree with the statement, I think it's a bit outside the purview of this document to specify this. Suggest: "...SIP signaling used to set up CLUE sessions SHOULD..." /a From nobody Wed Jun 7 10:22:43 2017 Return-Path: X-Original-To: clue@ietfa.amsl.com Delivered-To: clue@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id ABE9C127A91; Wed, 7 Jun 2017 10:22:41 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -14.522 X-Spam-Level: X-Spam-Status: No, score=-14.522 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ztPaOl8HqORW; Wed, 7 Jun 2017 10:22:37 -0700 (PDT) Received: from aer-iport-3.cisco.com (aer-iport-3.cisco.com [173.38.203.53]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 3B492128ACA; Wed, 7 Jun 2017 10:22:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=14743; q=dns/txt; s=iport; t=1496856156; x=1498065756; h=subject:to:references:cc:from:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=HtpC2Jb1+Xf3ZPm6cTCQbUImym3NhqqDYgeNGDjKknE=; b=TsexxMnIc0WAHcaLSWGXnWP6byWTfI32E0Y586BkLYDvxVtJ15weOk3l Q1BK1YTZQzNOYFmbeXbDj5Yw4hRyFqto6FS+Hb3Qj9J5H6pKRg/u/vxhv JW8lfrDsXG1QLfLQ9qWFIjvIpwWKJrr2oqM3IlAlDPzK1cRsQQG3cBb2Y M=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0DcAAARNjhZ/xbLJq1YBhkBAQEBAQEBA?= =?us-ascii?q?QEBAQcBAQEBAYVHg3OKGHOPRQMGgSdylQ6CEIYkAoMzGAECAQEBAQEBAWsohRg?= =?us-ascii?q?BAQEBAgEjFUEFCwsYAgImAgJXBgEMBgIBAReKAwUIrkWCJot/AQEBAQEBAQEBA?= =?us-ascii?q?QEBAQEBAQEBIIELg0WBS4ImK4IkHTSEXIMggmEBBIEsAY8GjgQCil2IW4IGhT6?= =?us-ascii?q?DS4ZxlGcfOIEKMCEjMSpzhBQcgWQCPzaHEwYBgjgBAQE?= X-IronPort-AV: E=Sophos;i="5.39,311,1493683200"; d="scan'208";a="653437226" Received: from aer-iport-nat.cisco.com (HELO aer-core-1.cisco.com) ([173.38.203.22]) by aer-iport-3.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Jun 2017 17:22:21 +0000 Received: from [10.47.200.21] ([10.47.200.21]) (authenticated bits=0) by aer-core-1.cisco.com (8.14.5/8.14.5) with ESMTP id v57HMKmV020220 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 7 Jun 2017 17:22:21 GMT To: Adam Roach , clue@ietf.org References: <0b69d2f1-11e1-8fd1-d4a1-2faacc0a8528@nostrum.com> Cc: clue-chairs@ietf.org, draft-ietf-clue-signaling@tools.ietf.org From: Robert Hansen Message-ID: <5168f94a-811e-897a-3c83-f44b17a11568@cisco.com> Date: Wed, 7 Jun 2017 18:22:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <0b69d2f1-11e1-8fd1-d4a1-2faacc0a8528@nostrum.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated-User: rohanse2 Archived-At: Subject: Re: [clue] AD Review: draft-ietf-clue-signaling-11 X-BeenThere: clue@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: CLUE - ControLling mUltiple streams for TElepresence List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jun 2017 17:22:41 -0000 Hi Adam, Thank you for the very detailed review - I'll find the time to review it, respond and post a revised draft ASAP. Rob On 03/06/17 01:14, Adam Roach wrote: > CLUE working group -- > > I have completed my AD review for the CLUE signaling document. The > document is generally in good shape, but I think we'll need another > revision before putting it in front of the IETF for last call > (especially due to the apparent incomplete removal of support for > specifying multiple CLUE groups per session). > > Most of my comments below are feedback that the document authors > should treat as normal last call comments. The feedback that I > consider to block progressing the document, in my role as AD, is > explicitly marked with the prefix "BLOCKER", and these will need to be > resolved in a new version of the document before progressing it > further. Note that it is entirely possible that something I have > marked "BLOCKER" may stem from an error on my part; so recognize that > these are not demands for change, as much as a need to have things > either fixed in the document or explained to me. > > Title: The rule of thumb is that all but a small handful of well-known > acronyms need to be expanded in titles and abstracts. I recognize that > "CLUE" is a bit tortured, as acronyms go, but the title of this > document is, broadly speaking, opaque. Please change it to something > meaningful, such as "Session Signaling for Controlling Multiple > Streams for Telepresence (CLUE)" > > BLOCKER: General: It is clear from reading this version of the > document that earlier versions contained the notion of multiple > "a=group:CLUE" lines in a single SDP description. This version appears > to have tried to remove all related text, but there are still enough > mentions that talk about CLUE groups in a way that implies that there > can be multiples so as to cause confusion. These need to be cleaned > up. I call the specific instances out on a section-by-section basis > below. I'm mentioning it up here since it's really only one blocker > issue with a bunch of instances. > > General: The _protocol_ document has a host of different terms for > each kind of CLUE message (e.g., "ADVERTISEMENT," "ADV," and > "advertisement" for the same operation). This document exacerbates the > situation by introducing yet more variations, such as "Advertisement" > and "Configure". Please coordinate with draft-ietf-clue-protocol to > use a consistent set of names for these operations between the two > documents. > > Introduction: The convention I see in this document is to write > defined terms with initial caps; please replace "encoding group" with > "Encoding Group." > > Section 3: Please add a reference to RFC3840 (e.g.: 'The "sip.clue" > media feature tag [RFC3840] indicates...") > > Section 4.2: "Presence of the data channel in a CLUE group..." implies > there can be more than one group. Replace "a" with "the." > > Section 4.3: ...its "mid" value MUST be included in a CLUE group' > implies there can be more than one group. Replace "a" with "the." > > Section 4.4.1: "...in a CLUE group as defined above." implies there > can be more than one group. Replace "a" with "the." > > Section 4.4.1: '..."m=" lines in the same CLUE group in the SDP > message...' very strongly implies there can be more than one group. > Rephrase, perhaps along the lines of "...CLUE-controlled "m=" lines in > the SDP message..." > > 4.4.2: 'These "m=" lines are CLUE-controlled and hence MUST include > their "mid" in the CLUE group corresponding to the CLUE group of the > Encoding they wish to receive.' is getting pretty explicit about the > presence of multiple CLUE groups. Fix. > > 4.5.2.1, first sentence: Replace "If the recipient is a > CLUE-capable..." with "If the recipient of an offer is a CLUE-capable..." > > BLOCKER: Section 4.5.2.2: For avoidance of doubt, this section should > clearly indicate what the answer should do with CLUE-controlled lines > that it has no intention of receiving (for sendonly) or sending (for > recvonly). I believe the expectation here is to set the port to zero > (rather than, e.g., setting the direction to inactive). The document > should explicitly state this behavior: if implementations make > different choices between port-zero and inactive and don't expect the > other behavior, you can end up with incompatibilities. > > Section 4.5.3.1, paragraph 2: My recollection is that telling > implementors not to send media is frequently misinterpreted to mean > that they don't have to send/receive RTCP either. This causes all > kinds of grief. It will probably head off issues if this section is > phrased more like "...MAY choose not to send RTP on the > non-CLUE-controlled channels (although RTCP is still sent and received > as normal) during the period..." > > Section 4.5.4.1: 'Subsequent offer/answer exchanges MAY add additional > "m=" lines...' -- this should probably also mention "and activate > inactive ones." > > Section 4.5.4.1: 'Subsequent offer/answer exchanges MAY also > deactivate "m=" lines for CLUE-controlled media.' -- again, the > interpretation of "deactivate" may be different between implementors. > Please be clear about whether this means "a=inactive", port=0, or both. > > Section 4.5.4.1: The final paragraph talks about "deactivating" > non-CLUE media. Again, this should be explicit about what is meant. > > Section 4.5.4.2: 'If, in an ongoing non-CLUE call, an SDP offer/answer > exchange completes with both sides having included a data channel "m=" > line in their SDP and with the "mid" for that channel in corresponding > CLUE groups..." implies that there can be more than one CLUE group. Fix. > > Section 4.5.4.3: "...include the data channel in a matching CLUE > group..." implies there can be more than one group. Replace "a > matching" with "the." > > Section 4.5.4.3: "Any active "m=" lines still included in a CLUE > group..." implies there can be more than one group. Replace "a" with > "the." > > Section 4.5.4.3: "Note that this is distinct from cases where the CLUE > protocol negotiation fails, or an error occurs in the CLUE protocol; > see [I-D.ietf-clue-protocol] for details of media and state > preservation in this circumstance." -- I carefully scrubbed the CLUE > protocol document to try to determine what this is referring to. > Please change it to "see [I-D.ietf-clue-protocol] section X.Y.Z", but > replacing "X.Y.Z" with the section that provides the details you > allude to. > > > BLOCKER: Compare the normative statements in paragraph 2 of Section 5.3: > > Generally, implementations that receive messages for which they have > incomplete information SHOULD wait until they have the corresponding > information they lack before sending messages to make changes related > to that information. For example, an answerer that receives a new > SDP offer with three new "a=sendonly" CLUE "m=" lines for which it > has received no CLUE Advertisement providing the corresponding > capture information SHOULD include corresponding "a=inactive" lines > in its answer, and SHOULD make a new SDP offer with "a=recvonly" when > and if a new Advertisement arrives with Captures relevant to those > Encodings. > > With the normative statements in section 4.5.2.2: > > If the initial offer contained "a=recvonly" CLUE-controlled media > lines the recipient SHOULD include corresponding "a=sendonly" CLUE- > controlled media lines for accepted Encodings > ... > If the initial offer contained "a=sendonly" CLUE-controlled media > lines the recipient MAY include corresponding "a=recvonly" CLUE- > controlled media lines > > 5.3 says "SHOULD set a=inactive" in the exact same circumstances > 4.5.2.2 says "SHOULD set a=sendonly". Please pick one expected > behavior and make sure both sections agree. Ideally, you would > refactor this so that the normative statement is made in only one > location. > > > Section 7 appears to be oddly silent on the use of RTCP MUX. I suspect > that the intention is that CLUE will generally multiplex RTCP when > using BUNDLE? I would expect this to be called out. > > Section 7 offers that the use of BUNDLE has the advantage of reducing > the number of ICE candidates that need to be collected. This is true > only in the case that some m= section are marked as bundle-only; this, > in turn, implies that a bundled offer is going to contain one or more > m= sections that will fail if the remote endpoint does not implement > BUNDLE. This interacts particularly badly with the text in section > 4.5.1 that allows, under certain circumstances, that 'the initial > offer MAY contain "m=" lines for CLUE-controlled media.', since you > can end up with the whole CLUE session falling apart if the > bundle-only lines are not chosen carefully. This is a relatively > complicated issue that I wouldn't expect impementors to get correct > without some guidance on the use of bundle-only, and in particular its > interaction with CLUE-controlled lines. Please add text to Section 7 > that discusses these issues. > > Section 8: Please insert a page break before the ladder diagram so > that the entity names don't appear on a page by themselves. > > Section 8: "In this case Bob is the Channel Initiator..." this isn't > clear (and, in fact, it's counterintuitive to me) -- perhaps there > should be some text indicating *why* Bob is the Channel Initiator. > > General, but surfaced in section 8: The procedures described in this > document virtually guarantee that every CLUE call that is established > will result in glare (response code 491) behavior. This might cause > the operations folks some heartburn, as it means that their error > counts will spike once CLUE is deployed. Further, without fairly > advanced analysis of the callflow, this will make it impossible to > distinguish "expected" CLUE-induced 491s from the oddball actual glare > conditions usually signaled by 491. Has any consideration been given > to avoiding this situation (e.g., by having the called party wait on > the order of one second before attempting to negotiate its encodings)? > > > Section 8 contains the following text: > > Bob also sends his SDP answer as part of SIP 200 OK 2. Alongside his > original audio, video and CLUE "m=" lines he includes two active > recvonly "m= "lines and a zeroed "m=" line for the third. > > This should probably be qualified to indicate that these are the same > m-sections as were present in the offer (as opposed to creating new > sections). > > > Section 8: Replace "Having received this Alice..." with "Having > received this offer, Alice..." > > Section 9: "From the lack of the data channel and grouping > framework..." -- it should be noted that implementations that end up > in communication with normal non-CLUE WebRTC implementations might get > a datachannel but no CLUE group. The quoted text implies that the > presence or absence of a datachannel can be used to determine CLUE > support, which might cause implementors to rely on that exclusively. > Please change the text to indicate that the CLUE group should be used > as the sole determinant of CLUE support (at least, as far as SDP > signaling is concerned). > > Section 10: It is rather unusual to include authors in the > acknowledgements section. For each of Rob Hansen, Paul Kyzivat, and > Christian Groves, I suggest removing the individual's name from either > the Acknowledgements section or from the authors list. > > Section 11.2: "This specification registers a new media feature tag in > the SIP [RFC3264] tree..." This should cite RFC3261 for SIP rather > than RFC3264. > > Section 11.2: Please indicate "iesg@ietf.org" as the contact for this > registration. > > > BLOCKING: Section 12 indicates that no security mechanisms are > mandatory to use "due to the issues addressed in [RFC7202]." This > vastly misconstrues the intent and text of RFC7202, which (roughly > speaking) says "we don't mandate security for RTP in general because > it is used in a vast and varied array of applications." CLUE is a very > specific, very narrow application of RTP that does not suffer from the > generality that makes a one-size-fits-all solution for RTP infeasible. > In fact, RFC7202 is quite clear that its guidance applies exclusively > to RTP, and not to protocols that *use* RTP: "Documents that define an > interoperable class of applications using RTP are subject to > [RFC3365], and thus need to specify MTI security mechanisms." If there > are specific arguments put forth in RFC7202 that the working group > believes also apply to this document, please reiterate them here. The > current citation to RFC7202 is problematic, though, since it says the > opposite of what this security section implies. > > If I understand it correctly, though, the argument to put forth here > is that implementations MAY choose not to secure legacy > (non-CLUE-controlled) media lines because SIP was specified prior to > RFC3365, and was therefore not bound by its requirements; and, as a > consequence, many existing SIP endpoints are incapable of using > DTLS-SRTP. This rationale is orthogonal to the guidance in RFC7202, > and should not cite it. > > The use of "DTLS" to refer to "DTLS-SRTP" is also confusing, as they > are different things. > > I propose changing the paragraph as follows: > > This attack can be prevented by ensuring that the media recipient > intends > to receive the media packets. As such, all CLUE-capable devices MUST > support key negotiation and receiver intent assurance via DTLS-SRTP > [RFC5763] on CLUE-controlled RTP "m=" lines. As specified in > [I-D.ietf-clue-framework], all CLUE-controlled RTP streams must be > secured and implemented using mechanisms such as SRTP [RFC3711]. CLUE > implementations MAY choose not to require the use of SRTP to secure > legacy > (non-CLUE-controlled) media for backwards compatibility with older > SIP clients that are incapable of supporting SRTP. > > Section 12: "To prevent this, SIP signaling SHOULD always be encrypted > using TLS..." While I agree with the statement, I think it's a bit > outside the purview of this document to specify this. Suggest: "...SIP > signaling used to set up CLUE sessions SHOULD..." > > /a > From nobody Mon Jun 12 06:15:25 2017 Return-Path: X-Original-To: clue@ietfa.amsl.com Delivered-To: clue@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id DA98D129515 for ; Mon, 12 Jun 2017 06:15:23 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.901 X-Spam-Level: X-Spam-Status: No, score=-1.901 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001] autolearn=unavailable autolearn_force=no Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3FtvnA5mkGFC for ; Mon, 12 Jun 2017 06:15:20 -0700 (PDT) Received: from brc1.unina.it (antispam.unina.it [192.132.34.50]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C024912869B for ; Mon, 12 Jun 2017 06:15:10 -0700 (PDT) X-ASG-Debug-ID: 1497273305-05ce377288eed8c0001-dOUo1C Received: from smtp1.unina.it (smtp1.unina.it [192.132.34.61]) by brc1.unina.it with ESMTP id nv6cAdMAZLZC074D (version=TLSv1 cipher=AES256-SHA bits=256 verify=NO); Mon, 12 Jun 2017 15:15:05 +0200 (CEST) X-Barracuda-Envelope-From: spromano@unina.it X-Barracuda-Apparent-Source-IP: 192.132.34.61 Received: from [10.154.106.172] (sessfw99-sesbfw99-92.ericsson.net [192.176.1.92]) (authenticated bits=0) by smtp1.unina.it (8.14.4/8.14.4) with ESMTP id v5CDF09P011679 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 12 Jun 2017 15:15:03 +0200 Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) X-ASG-Orig-Subj: Re: AD Review: draft-ietf-clue-protocol-13 Content-Type: multipart/alternative; boundary="Apple-Mail=_98F84C8D-5934-4692-AFD3-3E0BEFE50FBF" From: Simon Pietro Romano In-Reply-To: Date: Mon, 12 Jun 2017 15:14:58 +0200 Cc: clue@ietf.org, clue-chairs@ietf.org, draft-ietf-clue-protocol@tools.ietf.org Message-Id: <4523923C-254E-45B1-AC8E-B301834E0876@unina.it> References: To: Adam Roach X-Mailer: Apple Mail (2.3124) X-Barracuda-Connect: smtp1.unina.it[192.132.34.61] X-Barracuda-Start-Time: 1497273305 X-Barracuda-Encrypted: AES256-SHA X-Barracuda-URL: http://192.132.34.50:8000/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at unina.it X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=6.0 tests=BSF_SC0_MISMATCH_TO, HTML_MESSAGE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.35465 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.00 BSF_SC0_MISMATCH_TO Envelope rcpt doesn't match header 0.00 HTML_MESSAGE BODY: HTML included in message Archived-At: Subject: Re: [clue] AD Review: draft-ietf-clue-protocol-13 X-BeenThere: clue@ietf.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: CLUE - ControLling mUltiple streams for TElepresence List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Jun 2017 13:15:24 -0000 --Apple-Mail=_98F84C8D-5934-4692-AFD3-3E0BEFE50FBF Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hello Adam, thanks a lot for your thorough review and sorry for answering that late. = I am currently abroad for a conference. Together with Roberta, we=E2=80=99= ll work on your comments and send back our answers in the next few days. Cheers, Simon _\\|//_ ( O-O ) ~~~~~~~~~~~~~~~~~~~~~~o00~~(_)~~00o~~~~~~~~~~~~~~~~~~~~~~~~ Simon Pietro Romano Universita' di Napoli Federico = II Computer Engineering Department=20 Phone: +39 081 7683823 -- Fax: +39 081 7683816 e-mail: spromano@unina.it = <>. = Magritte. oooO ~~~~~~~~~~~~~~~~~~~~~~~( )~~~ Oooo~~~~~~~~~~~~~~~~~~~~~~~~~ \ ( = ( ) \_) ) = / = (_/ > On 02 Jun 2017, at 02:48, Adam Roach wrote: >=20 > CLUE working group -- >=20 > I have completed my AD review for the CLUE protocol document. Based on = my reading, I do not think it is yet ready for IETF last call. > Most of my comments below are feedback that the document authors = should treat as normal last call comments. The feedback that I consider = to block progressing the document, in my role as AD, is explicitly = marked with the prefix "BLOCKER", and these will need to be resolved in = a new version of the document before progressing it further. Note that = it is entirely possible that something I have marked "BLOCKER" may stem = from an error on my part; so recognize that these are not demands for = change, as much as a need to have things either fixed in the document or = explained to me. >=20 > Title: The rule of thumb is that all but a small handful of well-known = acronyms need to be expanded in titles and abstracts. I recognize that = "CLUE" is a bit tortured, as acronyms go, but the title of this document = is, broadly speaking, opaque. Please change it to something meaningful, = such as "Protocol for Controlling Multiple Streams for Telepresence = (CLUE)" >=20 > General: There are three instances of excessively long lines in the = document. >=20 > General: Please number and caption the figures in this document. Also, = please refer to the figure numbers when pointing to them (e.g., the = first paragraph of section 6.1 should read something like: "As soon as = the sub-state machine of the MP (Figure 1) is activated..."." > BLOCKER: General: There are several mentions of timeouts and retry = thresholds in the text and its corresponding state machines; however, = the document neither defines nor cites a document as defining what these = timeout and retry values are. These need to be defined and described. If = the timer and retry scheme allows the two ends of the connection to have = different values for timeouts and number of retries, then there need to = be additional error procedures that allow the MC and MP state machines = to stay in sync (if the timer/retry values can be different, it's = possible for one state machine to transition to "terminated," while the = other is still active, and you need messaging to clean this up). The = remainder of this comment is non-blocking: Related to this, the document = frequently refers to retries as "expiring" (e.g., "retry expired" on the = state diagrams). That doesn't really make sense unless "retry" is = the name of a timer rather than a counter; I think you mean to say = "exhausted" or something similar. > General: This document defines six message types, all of which have at = least two names, many of which have more. It would be a lot easier to = keep track of what is being described if these were kept consistent. I = suggest choosing one term for each concept and sticking with it. In = other words, please pick only one name from each of the following lines, = and do not use any of the others: > OPTIONS, options > OPTIONS RESPONSE, optionsResponse > ADVERTISEMENT, ADV, advertisement > ADVERTISEMENT ACKNOWLEDGEMENT, ADV ACK, ACK, NACK, ack > CONFIGURE, CONF, CONF+ACK, configure > CONFIGURE RESPONSE, CONF RESPONSE, configureResponse > The terminology section appears to be in alphabetical order, except = for "Capture Encoding" (which was presumably "Media Capture Encoding" in = some earlier version). Please fix. >=20 > The definitions for "Endpoint", "MCU", and "Media Stream/Stream" vary = from the definition given in draft-ietf-clue-framework. Is this = intentional? >=20 > The first paragraph of section 4 mentions the framework and data model = documents without citations. There should probably be citations to the = corresponding documents. >=20 > Section 4 contains the following text: >=20 > The CLUE protocol represents the mechanism > for the exchange of CLUE information between CLUE Participants > This is sufficiently circular as to be basically meaningless. Suggest: = "...for the exchange of telepresence information between..." >=20 > Section 5: The versioning scheme in here is rather perplexing. Is = there some technical reason the protocol restricts major version numbers = to a single digit and minor versions to a single digit? Strongly suggest = that this should be expanded to allow multiple digits for both the major = and minor versions. >=20 > Section 5: It should be made clear that the XML Schema excerpts in = section 5 are non-normative. In particular, I recommend adding the = following text to the end of the first paragraph of section 5: "This = section includes non-normative excerpts of the schema to aid in = describing it." >=20 > Section 5: If the single-digit version numbers are maintained (and, = again, I strongly recommend against this), the definition of versionType = appears to be wrong: it allows a major version digit of "0", which would = seem to be precluded by the way versions are currently defined. >=20 > Section 5: The XML definition allows zero or more elements to = appear in a message. If more than one is allowed, the document should = explain how multiple IDs are handled. If they are not, then the schema = and/or text needs to prohibit having more than one. >=20 > Section 5: The description for says that a 402 will be = sent in the case of an "unexpected" sequence number. This needs = clarification: is this for case where a sequence number gap is detected? = A repeated sequence number? A number that is too small? All of the = above? At least describe this with the 402 error code, and point to that = description from here. >=20 > Section 5: The description for would benefit from the addition of = "This document describes version 1.0". >=20 > Section 5.1: "The OPTIONS message is sent by the CP which is the CI to = the CP which is the CR as soon as the CLUE data channel is ready." = Although it's a problem in other parts of the document too, the dense = use of nearly identical two-letter acronyms in here makes it quite hard = to read. I had to keep consulting the definitions of those acronyms to = decode this sentence (as well as several similar ones). Consider just = expanding these terms (as well as "MP" and "MC") instead of using them = in prose. > Section 5.1 starts to wobble back and forth between referring to = elements with and without angle brackets (e.g., it uses both = "supportedVersions" and ""). Please pick one and = stick with it. >=20 > BLOCKER: Section 5.1: The description of describes = a scheme in which multiple supported versions can be listed; and, if the = list is omitted, it implies that only the version described in is = supported. This text does not define (nor does any other text that = I can find) what should be set to when is used. = Intuitively, it seems that should be set to the largest minor = version of the smallest major version advertised in , = but that (or whatever the correct answer is) needs to be clearly spelled = out. >=20 > BLOCKER: Section 5.2: "If the responseCode is of the type 2xx the = response MUST also include..." -- you can't use "2xx" in a normative = statement without first defining what it means. As you don't use the = "#xx" format anywhere else, I suggest rephrasing: "If the = is between 200 and 299 inclusive, the response MUST also include..." >=20 > Sections 5.2, 5.4, 5.6: It seems really odd that the document defines = a base clueMessageType that all messages derive from, but then leaves = all the response types (optionsResponse, ack, configureResponse) to = repeatedly and independently add ,, and = the sequence number of the corresponding message over and over again. I = would strongly suggest adding something like: >=20 > > > > > > > > > > > > > > ...and then defining those three response types as being extensions of = "clueResponse" instead of "clueMessage", like this: > > > > > > > > >=20 > Regardless of how you do this, any section that adds "responseCode" = and "reasonString" needs to point to section 5.7 in its description to = explain how those fields are populated and interpreted. >=20 > Section 5.5: This section allows a boolean flag in a = message to acknowledge an . Is this intended to always be = handled like a 200? Consider: if you define a 201 response code in the = future, will implementations be unable to convey its meaning in a CONF + = ACK? Given that the document defines a class of codes for success rather = than a simple success flag in general, it seems that this element = should carry a success response code rather than just a boolean. If you = decide to keep the boolean, be very clear that it is to be treated as a = 200 rather than any other potential success code; and that conveying any = other kind of success requires a separate message. >=20 > Section 5.5: the final paragraph mentions the = element -- it would be helpful to add something like "see = [I-D.ietf-clue-datamodel] for the definition of ." >=20 > BLOCKER: Section 5.7 indicates that there is a class of response = codes, starting with "1", which are used to indicate "delayed or = incomplete" responses. The document does not describe any protocol = behavior for this class of response. The description of the meaning of = this class (and the obvious parallels to HTTP and SIP) imply that some = subsequent response associated with the same request will be arriving at = some point in the future. If that's the intention, this document needs a = *lot* more text (and corresponding adjustments to the state machines) to = explain how these 100-class codes are handled. In practice, since the = current version of the document does not define nor make use of = 100-class codes, I suggest that the most reasonable path forward is to = remove discussion of codes starting with "1" from the first paragraph of = section 5.7, instead adding a paragraph immediately following it that = says something like:=20 > This document does not define response codes starting with "1", and = such > response codes are not allowed to appear in major version 1 of the = CLUE > protocol. The range from 100 to 199 inclusive is reserved for future = major > versions of the protocol to define response codes for delayed or = incomplete > operations if necessary. Response codes starting with "5" through = "9" are > reserved for future major versions of the protocol to define new = classes of > response, and are not allowed in major version 1 of the CLUE = protocol. > Response codes starting with "0" are not allowed. > Section 5.7: "The response codes and strings defined for use with CLUE = are as follows" - this strongly implies that the descriptions given in = this table are the only ones that are allowed in CLUE = element, and that any other messages should presumably be treated as an = error. Surely that's not what you mean. Suggest rephrasing to indicate = that the "Description" text can be sent in the , but that = implementations can (and are encouraged to) include more specific = descriptions of the error condition, if possible. >=20 > Section 5.7: I can't figure out how an implementation could ever send = a "300" response code. If the XML syntax is incorrect, that's a 301. If = the message contains an invalid value, that's a 302. And those are the = only two conditions that are described for 300. I think you need to give = "300" a bit more thought -- if you can't come up with a more generic = description (e.g., "low-level request error"), you probably want to = remove it. >=20 > Section 5.7: for 403, be clear which identifier is meant. Do you mean = "The used in the message is not valid..."? >=20 > Section 5.7: for 404, please be clear that you're talking about the = sequence number rather than just using "number" without qualification. >=20 > Section 5.7: The description for 405 uses the acronym "MCC" without = expanding it. Please expand it. >=20 > BLOCKING: The state machines in section 6 and its subsections don't = have transitions for all possible messages that could arrive in a state. = This can cause interop issues. Please add text that clearly indicates = whether such messages do or do not cause a transition. (This might be as = simple as "messages not shown for a state do not cause the state to = change," but only if you carefully check that this is true -- for = example, what should an MP state machine do do if it gets a "CONF + ACK" = in the state "WAIT FOR CONF"?) >=20 > Section 6: The fifth paragraph says "CLUE channel" where it means = "CLUE data channel." >=20 > Section 6: The eighth paragraph says: >=20 > The CP moves from the ACTIVE state to the IDLE one when the = sub-state > machines that have been activated are (both) in the relative > TERMINATED state (see sections Section 6.1 and Section 6.2). > The "both" in this paragraph is confusing, since it's possible to have = only one or the other machine running. Please rephrase. >=20 > Section 6.2 describes a state machine that starts in a state called = "WAIT FOR ADV." This state does not appear to be timer-supervised, = meaning that implementations of this state machine can stay in this = state literally forever. Is that the intention? >=20 > Section 6.2 also describes the possibility of sending a and = separately or as a combined message. I would have expected = to see some discussion here about why implementations might choose one = behavior over the other. >=20 > Section 7: The second sentence of the second paragraph needs a verb. >=20 > Section 7, paragraph 3: this claims that versions are a non-negative = *integer* rather than a *digit*. As I mention above, this seems to be = the right way to handle versions, but it is decidedly at odds with text = elsewhere in the document and in the schema. Regardless of how you = choose to treat versions, the text needs to be consistent. > Section 7, final paragraph: replace "Clue" with "CLUE." >=20 > Section 8 contains a paragraph starting with "In that case, the new = information..." but it's not clear which case it's referring to. Please = replace "In that case" with a description of the case under = consideration. >=20 > Section 8 contains the phrase "Similarly to what said before..." -- = this is ungrammatical and needs to be rephrased. >=20 > Section 8 indicates that extensions need to indicate "the standard = version of the protocol the extension refers to." Given that there is = compatibility within a major version of a protocol, I think this means = to say "the major standard version of the protocol that the extension = refers to." >=20 > Section 8 has a paragraph starting "For that reason..." -- it's not = clear what reason is being referred to here. Please clarify. >=20 > Section 8 contains schema that contains a element. It should = be clarified that this is the *protocol* version, not the *extension* = version (or, if it's the extension version, that needs to be spelled out = too, but I think you'll need a new namespace for that...?) >=20 > BLOCKING: Section 8.1 is an example section, which are non-normative = in IETF documents. It contains 2119-style normative language, however. = These normative statements need to be moved out of the example section = (and probably into section 8). The remainder of this comment is = non-blocking: I also find the "SHOULD" in this section to be highly = perplexing. Can you explain the rationale behind requiring schema, but = not requiring any description of what the schema *means?) >=20 > BLOCKING: The example in section 8.1 includes the following: >=20 > xmlns=3D"clue-info-extension-myVideoExtensions" > I'm pretty certain that namespaces are required to be identified by = URIs rather than arbitrary strings. >=20 > Section 8.1: The final paragraph also has normative language in it, = although it appears to be reiterating requirements from elsewhere in the = document. I suggest lowercasing "MUST" in this paragraph. >=20 > Section 8.1: The final paragraph mentions the use of and = to negotiate the extension. An example demonstrating = this negotiation would be extremely useful. >=20 > The schema in section 9 contains: >=20 > schemaLocation=3D"data-model-schema-17.xsd"/> > If you do not intend to bake this "-17" into the document (and I can't = imagine you do), please add an RFC editor note to change it to something = else upon publication. >=20 > Also, the schema in section 9 is (with rare exception) unindented. = This makes it *very* hard to read. Please consider formatting it with = conventional XML indentation. (This also applies to the schema excerpts = earlier in the document.) > Has there been any automated tool-based checking that the examples in = section 10 to verify that they conform to the schema in section 9 (and = the schemata it imports)? >=20 > Section 10.2: Please expand the acronym "MCCs" in the section title = (keep in mind that this appears in the table of contents, where it needs = to makes sense). >=20 > Section 10 in general: While it does consume a lot of space, I don't = think that defining six rather different message types and then showing = only *one* type in the examples is very illustrative of the protocol. I = would *STRONGLY* suggest adding at least a response message, and ideally = the example section should contain at least one example for each of the = six message types. >=20 > Section 11, paragraph 4: Replace "Clue" with "CLUE." >=20 > Section 12.1 has a strange double-double quote around the URN name, = and section 12.3 repeats this for the MIME type. > All subsections of section 12: please update all registrant contact = information to point to the IESG (iesg@ietf.org ) = rather than the CLUE working group and one of the authors. >=20 > Section 12.4.1: These descriptions will appear in an IANA registry, = where the phrase "in this document" will have no context and be rather = nonsensical. Please rephrase. >=20 > Sections 13 through 23 should include an RFC editor note asking for = removal before publication. >=20 > /a --Apple-Mail=_98F84C8D-5934-4692-AFD3-3E0BEFE50FBF Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hello Adam,

thanks a lot for your thorough review and sorry for answering = that late. I am currently abroad for a conference. Together with = Roberta, we=E2=80=99ll work on your comments and send back our answers = in the next few days.

Cheers,

Simon
          =             =   _\\|//_
      =                     =   = ( O-O )
      = ~~~~~~~~~~~~~~~~~~~~~~o00~~(_)~~00o~~~~~~~~~~~~~~~~~~~~~~~~
  =                 =    = Simon Pietro Romano
    =            Universita' di = Napoli Federico II
        =               Computer Engineering = Department 
           =   Phone: +39 081 7683823 -- Fax: +39 081 7683816
                =                     =        e-mail: spromano@unina.it

    <<Molti mi = dicono che lo scoraggiamento =C3=A8 l'alibi degli 
=     idioti. Ci rifletto un istante; e mi = scoraggio>>. Magritte.
      =                 =                oooO
       ~~~~~~~~~~~~~~~~~~~~~~~( =   )~~~ Oooo~~~~~~~~~~~~~~~~~~~~~~~~~
=               =    \ (            (   = )
=               =                   =   \_)          ) /
                =                     =                     =               =  (_/



On 02 Jun 2017, at 02:48, Adam Roach <adam@nostrum.com> = wrote:

=20 =20

= CLUE working group --

I have completed my AD review for the CLUE protocol document. Based on my reading, I do not think it is yet ready for IETF last call.

Most of my comments below are feedback that the = document authors should treat as normal last call comments. The feedback that I consider to block progressing the document, in my role as AD, is explicitly marked with the prefix "BLOCKER", and these will need to be resolved in a new version of the document before progressing it further. Note that it is entirely possible that something I have marked "BLOCKER" may stem from an error on my part; so recognize that these are not demands for change, as much as a need to have things either fixed in the document or explained to = me.

Title: The rule of thumb is that all but a small = handful of well-known acronyms need to be expanded in titles and abstracts. I recognize that "CLUE" is a bit tortured, as acronyms go, but the title of this document is, broadly speaking, opaque. Please change it to something meaningful, such as "Protocol for Controlling Multiple Streams for Telepresence (CLUE)"

General: = There are three instances of excessively long lines in the document.

General: Please number and caption = the figures in this document. Also, please refer to the figure numbers when pointing to them (e.g., the first paragraph of section 6.1 should read something like: "As soon as the sub-state machine of the MP (Figure 1) is activated..."."

BLOCKER: General: There are several mentions of = timeouts and retry thresholds in the text and its corresponding state machines; however, the document neither defines nor cites a document as defining what these timeout and retry values are. These need to be defined and described. If the timer and retry scheme allows the two ends of the connection to have different values for timeouts and number of retries, then there need to be additional error procedures that allow the MC and MP state machines to stay in sync (if the timer/retry values can be different, it's possible for one state machine to transition to "terminated," while the other is still active, and you need messaging to clean this up). The remainder of this comment is non-blocking: Related to this, the document frequently refers to retries as "expiring" (e.g., "retry expired" on the state diagrams). That doesn't really make sense unless "retry" is the name of a timer rather than a counter; I think you mean to say "exhausted" or something similar.

General: This document defines six message types, = all of which have at least two names, many of which have more. It would be a lot easier to keep track of what is being described if these were kept consistent. I suggest choosing one term for each concept and sticking with it. In other words, please pick only one name from each of the following lines, and do not use any of the others:

  1. OPTIONS, options
  2. OPTIONS RESPONSE, optionsResponse
  3. ADVERTISEMENT, ADV, advertisement
  4. ADVERTISEMENT ACKNOWLEDGEMENT, ADV ACK, ACK, NACK, = ack
  5. CONFIGURE, CONF, CONF+ACK, configure
  6. CONFIGURE RESPONSE, CONF RESPONSE, = configureResponse

The terminology section appears to be in = alphabetical order, except for "Capture Encoding" (which was presumably "Media Capture Encoding" in some earlier version). Please fix.

The= definitions for "Endpoint", "MCU", and "Media Stream/Stream" vary from the definition given in draft-ietf-clue-framework. Is this intentional?

The first paragraph of section = 4 mentions the framework and data model documents without citations. There should probably be citations to the corresponding documents.

Section = 4 contains the following text:

   The CLUE protocol represents the mechanism
   for the exchange of CLUE information between CLUE =
Participants

This is sufficiently circular as to be = basically meaningless. Suggest: "...for the exchange of telepresence information between..."

Section 5: The versioning scheme in = here is rather perplexing. Is there some technical reason the protocol restricts major version numbers to a single digit and minor versions to a single digit? Strongly suggest that this should be expanded to allow multiple digits for both the major and minor versions.

Section 5: It should be made clear that the XML Schema = excerpts in section 5 are non-normative. In particular, I recommend adding the following text to the end of the first paragraph of section 5: "This section includes non-normative excerpts of the schema to aid in describing it."

Section 5: If the single-digit = version numbers are maintained (and, again, I strongly recommend against this), the definition of versionType appears to be wrong: it allows a major version digit of "0", which would seem to be precluded by the way versions are currently defined.

Section 5: The XML definition = allows zero or more <clueId> elements to appear in a message. If more than one is allowed, the document should explain how multiple IDs are handled. If they are not, then the schema and/or text needs to prohibit having more than one.

Section 5: The description for = <sequenceNr> says that a 402 will be sent in the case of an "unexpected" sequence number. This needs clarification: is this for case where a sequence number gap is detected? A repeated sequence number? A number that is too small? All of the above? At least describe this with the 402 error code, and point to that description from here.

Section 5: The description for <v> would benefit from = the addition of "This document describes version 1.0".

Section 5.1: "The OPTIONS message is sent by the CP which is = the CI to the CP which is the CR as soon as the CLUE data channel is ready." Although it's a problem in other parts of the document too, the dense use of nearly identical two-letter acronyms in here makes it quite hard to read. I had to keep consulting the definitions of those acronyms to decode this sentence (as well as several similar ones). Consider just expanding these terms (as well as "MP" and "MC") instead of using them in prose.

Section 5.1 starts to wobble back and forth = between referring to elements with and without angle brackets (e.g., it uses both "supportedVersions" and "<supportedVersions>"). Please pick one and stick with it.

BLOCKER: Section 5.1: The = description of <supportedVersions> describes a scheme in which multiple supported versions can be listed; and, if the list is omitted, it implies that only the version described in <v> is supported. This text does not define (nor does any other text that I can find) what <v> should be set to when <supportedVersions> is used. Intuitively, it seems that <v> should be set to the largest minor version of the smallest major version advertised in <supportedVersions>, but that (or whatever the correct answer is) needs to be clearly spelled out.

BLOCKER: Section 5.2: "If the = responseCode is of the type 2xx the response MUST also include..." -- you can't use "2xx" in a normative statement without first defining what it means. As you don't use the "#xx" format anywhere else, I suggest rephrasing: "If the <responseCode> is between 200 and 299 inclusive, the response MUST also include..."

Sections 5.2, 5.4, = 5.6: It seems really odd that the document defines a base clueMessageType that all messages derive from, but then leaves all the response types (optionsResponse, ack, configureResponse) to repeatedly and independently add <responseCode>,<responseString>, and the sequence number of the corresponding message over and over again. I would strongly suggest adding something like:

   <!-- CLUE RESPONSE TYPE -->
   <xs:complexType name=3D"clueResponseType">
   <xs:complexContent>
   <xs:extension base=3D"clueMessageType">
   <xs:sequence>
   <xs:element name=3D"responseCode" type=3D"responseCodeType"/>
   <xs:element name=3D"reasonString" type=3D"xs:string" =
minOccurs=3D"0"/>
   <xs:element name=3D"requestSequenceNr" =
type=3D"xs:positiveInteger"/>
   <xs:any namespace=3D"##other" processContents=3D"lax" =
minOccurs=3D"0"/>
   </xs:sequence>
   </xs:extension>
   </xs:complexContent>
   </xs:complexType>

...and then defining those = three response types as being extensions of "clueResponse" instead of "clueMessage", like = this:

   <!-- ADV ACK MESSAGE TYPE -->
   <xs:complexType name=3D"advAcknowledgementMessageType">
   <xs:complexContent>
   <xs:extension base=3D"clueResponseType">
   <xs:anyAttribute namespace=3D"##other" processContents=3D"lax"/>
   </xs:extension>
   </xs:complexContent>
   </xs:complexType>

Regardless of how you do this, any section that adds "responseCode" and "reasonString" needs to point to section 5.7 in its description to explain how those fields are populated and interpreted.

Section 5.5: This section allows a = boolean flag in a <configure> message to acknowledge an <advertisement>. Is this intended to always be handled like a 200? Consider: if you define a 201 response code in the future, will implementations be unable to convey its meaning in a CONF + ACK? Given that the document defines a class of codes for success rather than a simple success flag in general, it seems that this <ack> element should carry a success response code rather than just a boolean. If you decide to keep the boolean, be very clear that it is to be treated as a 200 rather than any other potential success code; and that conveying any other kind of success requires a separate <ack> message.

Section 5.5: the final = paragraph mentions the <captureEncodings> element -- it would be helpful to add something like "see [I-D.ietf-clue-datamodel] for the definition of <captureEncodings>."

BLOCKER: Section = 5.7 indicates that there is a class of response codes, starting with "1", which are used to indicate "delayed or incomplete" responses. The document does not describe any protocol behavior for this class of response. The description of the meaning of this class (and the obvious parallels to HTTP and SIP) imply that some subsequent response associated with the same request will be arriving at some point in the future. If that's the intention, this document needs a *lot* more text (and corresponding adjustments to the state machines) to explain how these 100-class codes are handled. In practice, since the current version of the document does not define nor make use of 100-class codes, I suggest that the most reasonable path forward is to remove discussion of codes starting with "1" from the first paragraph of section 5.7, instead adding a paragraph immediately following it that says something like:

  This document does not = define response codes starting with "1", and such
  response codes are not allowed to = appear in major version 1 of the CLUE
  protocol. The range from 100 to 199 = inclusive is reserved for future major
  versions of the protocol to define = response codes for delayed or incomplete
  operations if necessary. Response codes = starting with "5" through "9" are
  reserved for future major versions of = the protocol to define new classes of
  response, and are not allowed in major = version 1 of the CLUE protocol.
  Response codes starting with "0" are = not allowed.

Section 5.7: "The response codes and strings = defined for use with CLUE are as follows" - this strongly implies that the descriptions given in this table are the only ones that are allowed in CLUE <reasonString> element, and that any other messages should presumably be treated as an error. Surely that's not what you mean. Suggest rephrasing to indicate that the "Description" text can be sent in the <reasonString>, but that implementations can (and are encouraged to) include more specific descriptions of the error condition, if possible.

Section 5.7: I = can't figure out how an implementation could ever send a "300" response code. If the XML syntax is incorrect, that's a 301. If the message contains an invalid value, that's a 302. And those are the only two conditions that are described for 300. I think you need to give "300" a bit more thought -- if you can't come up with a more generic description (e.g., "low-level request error"), you probably want to remove it.

Section = 5.7: for 403, be clear which identifier is meant. Do you mean "The <clueId> used in the message is not = valid..."?

Section 5.7: for 404, please be clear that = you're talking about the sequence number rather than just using "number" without qualification.

Section 5.7: The description for = 405 uses the acronym "MCC" without expanding it. Please expand it.

BLOCKING: = The state machines in section 6 and its subsections don't have transitions for all possible messages that could arrive in a state. This can cause interop issues. Please add text that clearly indicates whether such messages do or do not cause a transition. (This might be as simple as "messages not shown for a state do not cause the state to change," but only if you carefully check that this is true -- for example, what should an MP state machine do do if it gets a "CONF + ACK" in the state "WAIT FOR CONF"?)

Section 6: The fifth paragraph says "CLUE = channel" where it means "CLUE data channel."

Section 6: The eighth = paragraph says:

   The CP moves from the ACTIVE state to the IDLE =
one when the sub-state
   machines that have been activated are (both) in the relative
   TERMINATED state (see sections Section 6.1 and Section 6.2).

The "both" in this paragraph is confusing, since it's = possible to have only one or the other machine running. Please rephrase.

Section 6.2 describes a state machine that starts in a state called "WAIT FOR ADV." This state does not appear to be timer-supervised, meaning that implementations of this state machine can stay in this state literally forever. Is that the intention?

Section 6.2 also describes the = possibility of sending a <ack> and <configure> separately or as a combined message. I would have expected to see some discussion here about why implementations might choose one behavior over the = other.

Section 7: The second sentence of the second = paragraph needs a verb.

Section 7, paragraph 3: this claims that = versions are a non-negative *integer* rather than a *digit*. As I mention above, this seems to be the right way to handle versions, but it is decidedly at odds with text elsewhere in the document and in the schema. Regardless of how you choose to treat versions, the text needs to be consistent.

Section 7, final paragraph: replace "Clue" with = "CLUE."

Section 8 contains a paragraph starting with = "In that case, the new information..." but it's not clear which case it's referring to. Please replace "In that case" with a description of the case under consideration.

Section 8 contains the = phrase "Similarly to what said before..." -- this is ungrammatical and needs to be rephrased.

Section 8 indicates that extensions need to indicate "the standard version of the protocol the extension refers to." Given that there is compatibility within a major version of a protocol, I think this means to say "the major standard version of the protocol that the extension refers to."

Section 8 = has a paragraph starting "For that reason..." -- it's not clear what reason is being referred to here. Please = clarify.

Section 8 contains schema that contains a = <version> element. It should be clarified that this is the *protocol* version, not the *extension* version (or, if it's the extension version, that needs to be spelled out too, but I think you'll need a new namespace for that...?)

BLOCKING: Section = 8.1 is an example section, which are non-normative in IETF documents. It contains 2119-style normative language, however. These normative statements need to be moved out of the example section (and probably into section 8). The remainder of this comment is non-blocking: I also find the "SHOULD" in this section to be highly perplexing. Can you explain the rationale behind requiring schema, but not requiring any description of what the schema *means?)

BLOCKING: = The example in section 8.1 includes the following:

   =
xmlns=3D"clue-info-extension-myVideoExtensions"

I'm = pretty certain that namespaces are required to be identified by URIs rather than arbitrary strings.

Section = 8.1: The final paragraph also has normative language in it, although it appears to be reiterating requirements from elsewhere in the document. I suggest lowercasing "MUST" in this paragraph.

Section 8.1: The final paragraph = mentions the use of <options> and <optionsResponse> to negotiate the extension. An example demonstrating this negotiation would be extremely useful.

The schema in section 9 = contains:

<xs:import =
namespace=3D"urn:ietf:params:xml:ns:clue-info"
schemaLocation=3D"data-model-schema-17.xsd"/>

If = you do not intend to bake this "-17" into the document (and I can't imagine you do), please add an RFC editor note to change it to something else upon publication.

Also, the = schema in section 9 is (with rare exception) unindented. This makes it *very* hard to read. Please consider formatting it with conventional XML indentation. (This also applies to the schema excerpts earlier in the document.)

Has there been any automated tool-based checking = that the examples in section 10 to verify that they conform to the schema in section 9 (and the schemata it imports)?

Section= 10.2: Please expand the acronym "MCCs" in the section title (keep in mind that this appears in the table of contents, where it needs to makes sense).

Section 10 in = general: While it does consume a lot of space, I don't think that defining six rather different message types and then showing only *one* type in the examples is very illustrative of the protocol. I would *STRONGLY* suggest adding at least a response message, and ideally the example section should contain at least one example for each of the six message types.

Section 11, paragraph 4: Replace "Clue" with "CLUE."

Section 12.1 has a strange double-double quote around the URN name, and section 12.3 repeats this for the MIME type.

All subsections of section 12: please update all = registrant contact information to point to the IESG (iesg@ietf.org) rather than the CLUE working group and one of the authors.

Section 12.4.1: These descriptions will appear in an IANA registry, where the phrase "in this document" will have no context and be rather nonsensical. Please rephrase.

Sections 13 through 23 should include an RFC editor note = asking for removal before publication.

/a


= --Apple-Mail=_98F84C8D-5934-4692-AFD3-3E0BEFE50FBF--