Re: [clue] [Tsv-art] Tsvart telechat review of draft-ietf-clue-datachannel-15

[Joerg Ott <[e-mail]>]

Hi Christer,

thanks, also for the updated draft.  See inline.

On 10.04.19 20:07, Christer Holmberg wrote:
> Hi Joerg,
> 
> Thank You for the comments! Please see inline.
>     
>>     Reviewing this document for tsv-art, I don't see any any transport-specific
>>     issues in the document, but a bunch of other (smaller) issues and nits came up.
>>     
>>     I am not sure that that the reference to a "SIP User Agent" as an entity is
>>     strictly correct here. SIP User Agents are defined in RFC 3261 to carry out
>>     SIP transactions. Just speaking about "User Agents" may be more accurate.
>>     But this is just a side note.
> 
> I think it is good to have "SIP" there for clarity.

I don't think this is correct but it is marginal and I see your point.

> ----
> 
>>     Section 3.2.3 defines behaviour in a negative way, i.e., via MUST NOT
>>     statements, excluding all but one possibility. This could be problematic in
>>     the future if another option gets added to SCTP usage, which would then
>>     implicitly be allowed. It would be better if the behaviour was defined i
>>      a positive way, i.e., using MUST.
>    
> In general I agree with you. But, in the case the base data channel spec mandates a set of SCTP extensions, and the text says that a couple of them must not be used for CLUE. I think that is the clearest way.
> 
> (And, if additional SCTP extensions are added to rtcweb-data-channel in the future, that could cause problems no matter if we use MUST or MUST NOT, depending on whether such extensions can be used with CLUE or not.)

Well, if you have 4 features A, B, C, D and you state that B and C MUST
NOT be used, this leaves A and D.  If somebody adds E to SCTP, then E is
implicitly allowed.  I think it is just easier to spell out what
implementers are supposed to do.

> ----
> 
>>     Sect. 3.2.5, 2nd para: use "MUST" instead of "must" since this appears to
>>     be normative, so RFC 2119 wording should be used.
> 
> The reason I use "must" is because the text is referencing requirements in another specification, i.e., the requirement is not defined or introduced by this draft. That is based on guidance I have received on earlier drafts.

Ok.  Maybe make this explicit in the text.  "According to RFCxxxx, ..."

> ----
> 
>>     Sect. 3.2.7, 1st para: this appears t be normative language and thus
>>     should use the RFC 2119 keywords.
> 
> As for your comment on 3.2.5, the text is simply referencing requirements defined elsewhere.

See above.

> ----
>      
>>     Sect. 3.3.1.1, table 1 + 2nd para after table 1: the text in the 2nd para
>>     defines the value for the "application usage"; this should also be reflected
>>     in table 1 since it seems that only one application usage is defined.
>    
> I don't know how I would get it into the table, as it is a generic description of an m- line for SCTP over DTLS.
> 
> I could of course copy/paste the table, indicate that it shows the m- line for CLUE, and replace "application usage" with "webrtc-datachannel". But, I am not sure that would make things more clear.

All I am saying is that all other table entries refer to explicit values
while this one points indirectly to the text.  There is nothing wrong
technically, rather a matter of presentation.

> ----
>    
>>     Sect. 3.3.1.2.: this again appears to be normative, so RFC 2119 language
>>     should be used.
>>     What does the sentence "A CLUE entity can choose any valid SCTP port
>>     value." mean in this context?  Is a "valid SCTP port" value that of a
>>     previously established SCTP connection within the context of the
>>     SIP session? If such a match is desired it should be specified or a
>>     reference to a peer document (draft-ietf-mmusic-sctp-sdp-26?)
>>     should be provided.
>    
> draft-ietf-mmusic-sctp-sdp specifies how the port is used, and the port range, so suggest adding a reference to draft-ietf-mmusic-sctp-sdp.

Sounds good.

> ---
> 
>>     Sect. 3.3.2: NOTE statement: It is ok to have a note but this still is
>>     somewhat normative. It should also be specified what happens if
>>     the values _are_ set by the peer. What is the error handling?
>>     Ignore? Reject the connection setup?
>    
> I guess we could allow two options: either discard the parameters, or reject the SDP and take proper actions to release the connection.
> 
> I am also ok "de-noting" the text.

Ok

> ----
> 
>>     Figure 1 is a nice example. But it would be even better if a complete
>>     example with the SDP for the data channel setup (in the previous
>>     or the same offer/answer exchange) be given. Makes life easier
>>     for readers and implementers.
>    
> draft-ietf-clue-signaling contains more complete SDP examples, so I would suggest to add a reference there.

Yepp, this works for me.

> --------
> 
>      Editorial:
> 
>>     Don't just copy the first paragraph(s) from the introduction to create an abstract.
> 
> I will see if I can make the Abstract shorter.

I want you to make it different.  An abstract is _not_ the introduction
even though the first bits may overlap.

> ----
> 
>>     3.2.2, 2nd para, 3rd line: "what" -> "which"
>    
> Will fix.
> 
> ----
> 
>>     Sect. 3.3.1: the ref to the clue-signalling draft is missing a link (all other refs have one).
>    
> Not sure I understand. What do you mean by "missing a link"?

It's missing a hyperlink.  No idea why but all other citations seem to
have hyperlinks.

> ----
>    
>>     Fix the formatting of table 2 to avoid letter breaking from words.
>    
> Will do.

As you said in a different email, this may be hard.  Hyphenation is the
only thing that comes to mind.  Or abbreviation.  The present way looks
just broken.

But essentially, this is close to done.

Best,
Jörg