Subject: [clue] Rob's WGLC of the protocols draft

From: "Rob Hansen (rohanse2)" <[email-masked]>
Date: Sat, 18 Jul 2015 02:19:20 +0000

Hi all, Simon, Roberta,

I've now reviewed the protocols document, and think the schemas and protocol look solid and workable; I have a couple of minor quibbles with the state machines that are probably worth discussion that I'll highlight here:

1)      We need a timeout for the 'Wait for ADV' state (see comments on page 22); at present there's no way for an MC that doesn't receive an ACK to give up.

2)      Generally I don't see the utility of the retry count mechanism when receiving NACKs for its ADV/CONF  messages (see page 19). I don't see a reason to limit the sender from sending as many new ADV/CONFs that may be more acceptable as they want, mandating it must be different from any that have been NACKed since the last ACK, is a better way to do it. Since the error codes generally mean the sender has done something invalid sending a new message probably won't help and most implementations will move straight to the exit state on receiving a NACK.

Those were my only issues of note with the protocol. However, I did have concerns with the level of normative direction that's provided as part of the document; I think it needs to be more rigorous throughout in defining what a sender and receive should do when they receive or construct messages; reading it through it was obvious to me what everything was *meant* to do based on the names of the fields and concepts common with other protocols, but RFCs are generally meant to be written so that a  hypothetical robot with no prior experience could read it and its references and go ahead and implement a working system. I've called specific instances out below where I could identify areas I thought needed more rigor. Am happy to review it again for new language or maybe even to go through myself and revise the document with more normative language if you two didn't have the time or inclination.

The remaining issues are minor schema niggles, spelling or grammar notes and a note that an overall security section is probably required. Specific issues are listed at the end of the email.

As per not attending the previous IETF my role at work has transitioned to the point where I'm much less involved with the design of new standards so I'm also not attending the current IETF in person and won't be in the future until and unless my role shifts again. I'll be attending the CLUE session virtually, though, and I'm still committed to finishing the CLUE work (As soon as the WGLC for the protocol document finishes and the initial discussions shake out I'll be updating the signalling document ready for WGLC). Have fun in Prague, all; I enjoyed all the IETFs attended and will definitely miss them!

Rob

Specific issues:

Page 3:
"enabling a MP" - should be "enabling an MP"
"enabling a MC" - should be "enabling an MC"
"protocol messages are detailed in Section 4 The CLUE Participant" - should have a full stop afer '4'
MP and MC are used without bhaving been previously defined

Page 4:
List of endpoint types is almost but not quite alphabetically ordered.
"a synonymous of Capture Encoding." - should be "a synonym of Capture Encoding."
"flow on a realiable and ordered" - should be "flow on a reliable and ordered"

Page 5:
"the CPs are able" - "CP" is used without having been defined
"A mechanism for the negotiation of the CLUE protocol version and extensions is envisioned in the initiation phase" - such a mechanism has been defined elsewhere in the document; I don't think 'envisioned' is the right word; "is part of the initial phase", maybe?
"Indeed, being CPs A and B both transmitting and receiving, it is possible to distinguish between two dialogues:" - I find this sentence difficult to parse. This is also the first reference to A and B, so they require introduction. Something like "In many cases CPs will seek to both transmit and receive media. Hence in a call between two CPs, A and B, there would be two seperate dialogs, as follows:"

Page 7:
"sequenceNr" - I cannot find where in the document the rules for using the sequence number are made explicit (eg, the sender must increment the sequence numbers by one for each new message sent, the receiver must remember the most recent sequence number received and send a 401 error if it receives a message with an unexpected sequence number, etc).
I think "sequenceNr" should be of type positiveInteger, not unsignedInt; it's not a hard requirement, but generally sequence numbers must be non-zero in most protocols and I think we should stick to the convention.
"clueId" - Similarly, I cannot find any rules for constructing the clueId or what should be done with it.
"Each CP should manage up to three streams of sequence numbers: (i) one for the messages exchanged in the initiation phase, (ii) one for the messages exchanged as MP, and (iii) one for the messages exchanged as MC." - this is the only guidance currently on using the sequence numbers; when more details are provided this should be moved there. The fact that these sequence number streams are independent is important and quite easy for an implementation to get wrong, so it should probably be repeated in the sections on sending the OPTIONS, ADV and CONF messages where these independent streams are used.

Page 10:
"Only one <version> element SHOULD be provided for each major version supported" - is there a reason to not make this a MUST?
At present there's no explicit normative language mandating including key elements for non-error cases. I think this must be stated normatively, something like "If the responseCode is of the type 2xx the response MUST include <mediaProvider>,  <mediaConsumer>, <version> and <commonOptions> elements; it MAY include them for any other response code.". The sentence ""These two elements are optional in the OPTIONS RESPONSE since, in case of error response code, the CR might not want to add further information besides the response code and the reason string." then becomes unnecesary.
Whether the OPTIONS handling behaviour is defined (here on in the state machine) the sections on versioning and extensions should be referenced.

Page 11:
"During the telepresence session, the ADV can be sent from the MP both periodically and on a per-event basis, i.e., each time there are changes in the MP's CLUE telepresence capabilities." - This seems slightly unclear to me at present; something like "During a single CLUE session, an MP may send new ADV messages to replace the previously advertised options, if, for instance, its media CLUE telepresence capabilities change mid-call."
Either here, in the MC state machine, or both, we need to make clear that a new ADV completely invalidates the previous ADV.

Page 13:
"The <ack> element SHOULD not be present at all if an ACK message has been already sent back to the MP." - I can't see a reason not to make this a MUST. The words "at all" are also probably unnecessary.
"a piggibacking mechanism" - this should be "a piggybacking mechanism"

Page 15:
"Response codes beginning with "1" could be associated to temporary responses." - we don't have any 1xx codes yet; I think until we do this should either be removed or be made more definitive: "will be", not "could be" (though I'd phrase it "represent a delayed or incomplete response").

Page 16:
"a DTLS/SCTP channel established as depicted in [I-D.ietf-clue-signaling]" - draft-ietf-clue-datachannel, not draft-ietf-clue-signaling
"Over such a channel there are typically two CLUE streams between the channel terminations flowing in opposite directions.  In other words, typically, both channel terminations act simultaneously as a MP and as a MC." - I'd move this to after "and with the MP process.", and rephrase it as something like "Endpoints often wish to both send a receiver media, ie, act as both MP and MC. As such there will often be two sets of messages flowing in opposite directions; the state machines of these two flows do not interact with each other."
"When in the OPTIONS state..." - this paragraph describes the flow of the options state well, but I think we need two small explicit state machines drawn out for it.
"The initiation phase might fail because of one of the following reasons..." - This list doesn't include failing because of a major version mismatch. Generally I think this part needs to be made more normative: the behaviour of the OPTIONS receivers should be defined with success and error cases for both the OPTIONS sender and receiver. This should also reference the rules for constructing the version numbers in the OPTIONS RESPONSE section
as a reminder, or else the definition should be moved here.

Page 17:
Similarly, the rules on acting as an MP/MC based on the OPTIONS negotiation should be made explicit (eg, only if A has advertised as MP and B has responded as MC)

Page 19:
Defaults/guidance on timeout and retry thresholds are required
" if a CONF+ACK message arrives" - CONF+ACK should be defined here as a CONFIGURE message with the <ack> element set to TRUE.
"CONF+ACK messages referring to out-of-date ADVs MUST be ignored" - one of the sections that needs to have more rigerous sequence number language - "out of date" must be defined as having a sequence number equal to or less than the highest seen so far.
"the number of NACKs for the issued ADV is under the retry threshold ("NACK received && retry not expired"), the MP moves back to the ADV state for preparing a new ADV" - I'm not sure the retry threshold concept refelcts how implementations can successfully overcome NACKS. I think we may do better with a concept like "the MP MAY return to the ADV state and send a new ADV (with an incremented sequence number) that it judges may be more acceptable to the MC. The MP MUST NOT return to the ADV state if it will send an ADVERTISEMENT with values (excepting the sequence number) identical to one that has received a NACK response subsequent to the most recent ACK received. If the MP does not have a new ADVERTISEMENT candidate that may be more acceptable it MUST move to the MP TERMINATED state."
"If the MP is fine with the MC's configuration" - "is fine with" should be something like "successfully processes".
I would also remove the retry threshold on the Conf processing section and just wait to die based on the timeout or the far end giving up.
Explicit behaviour needs to be defined for what actually happens when the Established or MP TERMINATED states are reached (reference the signaling doc for the former and the data channel doc for the latter).

Page 22:
"Wait for ADV" does not have a timeout and currently has no way to reach the MC TERMINATED state except by receiving the requisite number of bad ADVs; there needs to be a defined way for the MC to give up if the far end never sends it an ADV.
We currently have two paths once an ADV is received; either send an ACK, move to the CONF state and later send a CONF or send a CONF+ACK. If we feel we need both then we need to provide guidance as to which to do; the only reason I can see to keep both is something like "If the MC can send a CONF+ACK immediately it MAY do so <details here of how in state machine terms>. If it cannot send the CONF+ACK immediately it MUST immediately send an ACK and move to the CONF state <details here of how in state machine terms>." We should also be explicit about the CONF/CONF+ACK messages: a CONF+ACK MUST have the <ack> element set to true, while a CONF message MUST NOT have the <ack> element set to true.
Again, would suggest removing the the "retry expired" concept and mandate that MCs must only send a new CONF if they can change to be more likely to be accepted and have a timeout.
Again, need explicit guidance on what actually happens when the Established or MC TERMINATED states are reached.

Page 23:
"error CONF RESPONSE received && retry not expired, timeout && retry not expired" - "retry not expired" is specified twice here
"timeout && retry expired" - "retry expired" is covered under "error CONF RESPONSE received && retry expired", this should just be timeout.

Page 24:
"CLUE protocol messages are XML messages compliant to the CLUE protocol XML schema" - may be worth referencing the data model document again here.
"Such a schema is the one included in the yet to come RFC" - this is no longer true, and should be covered by the reference above.
"The version of the XML schema contained in the standard document deriving from this draft will be 1.0" - again, this is covered by the data model doc and no longer needed here.
Do we still need the section on pre-standardisation interop?

Page 26:
Somewhere we need normative language on how two endpoints actually negotiate and use an extension.

Pages 31-43
I have not gone through and reviewed the examples against the schemas; if these examples were auto-generated from up-to-date versions of the schemas then there should be no need. On the other hand if they were written by hand then let me know and I'll go through and review them.

The document seems to be missing an overall Security Considerations section, which I believe is mandatory.