Re: [MMUSIC] Mirja Kühlewind's No Objection on draft-ietf-mmusic-ice-sip-sdp-37: (with COMMENT)

[Mirja Kuehlewind <[e-mail]>]

Yes, sounds all good. You could eventually consider to add some explanation about why port 9 is used in the text but that’s optional… Thanks!


> On 5. Aug 2019, at 20:44, Christer Holmberg <[e-mail]> wrote:
> 
> Hi Mirja,
> 
> Thank You for the review! Please see inline.
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
>> 1) First I have a processing question for the IESG (and maybe the RFC editor) but it might be just me not knowing this: 
>> As I understand it, RFC5245 was spilt up into RFC8445 and this document, however, I find it a bot odd that both documenst 
>> obsolete RFC5245. Is that what we usually do? Did we have this case before? Is that the right thing to do?
> 
> That's a good question. I hope the chairs and/or the AD can give some guidance.
> 
> ---
> 
>> 2) One quick question: Why is a port value of "9" used to signal use of the default destination, instead of e.g. "0"? Is that
>> because port "0" is used to reset the data stream? 
> 
> Correct.
> 
>> However, couldn't this combination of address and port "0" not be treated differently? Or is that to avoid any potential false connections? 
>> How could that happen? Isn't there a better way to do that? I mainly would like to understand what the reason is and maybe request to also explain this in the document.
> 
> The usage of port '9' comes from the SDP rules for negotiating a TCP stream (RFC 4145), where port '9' means that the port value can be discarded.
> 
> For ICE, port '9' will be used with the ICE Trickle extension, where you don't yet provide any candidate when you send the SDP, having the same the-port-can-be-discarded meaning.
> 
> ---
> 
>> 3) A minor editorial comment
>> Sec 4: "This specification defines eight new SDP attributes"
>> Given these attributes have already been specified in RFC5245, I wouldn't call them "new".
> 
> I guess we could remove "new".
> 
> ---
> 
>> 4) Question on sec 4.1:
>> "   <transport>:  indicates the transport protocol for the candidate.
>>     This specification only defines UDP.  However, extensibility is
>>     provided to allow for future transport protocols to be used with
>>     ICE by extending the sub-registry "ICE Transport Protocols" under
>>     "Interactive Connectivity Establishment (ICE)" registry."
>> The registry also contain an entry for TCP (see RFC6544). However, I also wonder a bit why a new registry was created initially instead 
>> of just using the protocol numbers or keyword in the IANA Protocol Numbers Registry...?
> 
> Unfortunately I don't know. The registry was created in RFC 6544, and I was not much involved in the work on that draft.
> 
> Anyone else know? Jonathan?
> 
> ---
> 
>> 5) A request in section 5.4:
>> "If absent in an offer and answer the default value of the attribute
>>  is 50 ms, which is the recommended value specified in [RFC8445]."
>> RFC8445 also specifies a minimum of 5ms (MUST). It would be good to also indicate here that this minimum exists without relying on the user to look up RFC8445.
> 
> 5ms is not only the minimum value for an agent, it is the minimum combined value for all agents - if you e.g., have a browser with multiple tabs, each using ICE for WebRTC.
> 
> So, we need to say that. Something like:
> 
>   "As defined in [RFC8455], regardless of the Ta value chosen for each agent, the combination of
>    all transactions from all agents (if a given implementation runs several concurrent agents) must not
>    be sent more often than once every 5 ms." 
> 
> (I assume you mean section 4.5)
> 
> ---
> 
>> 6) Also further on in section 5.4:
>>  "Once both agents have indicated the pacing value they with to use, both agents MUST use the larger of the indicated values."
>> Given this in normatively specified in RFC8445, maybe you should not use normative language in this document but provide in addition again a reference to RFC8445.
> 
> I suggest:
> 
>   "As defined in [RFC8455], once both agents have indicated the pacing value they want to use, both agents will use the larger of the indicated values."
> 
> (I assume you mean section 4.5)
> 
> ---
> 
> 7) And similar on the use of MUST in section 5:
> "The keepalives MUST be sent
>   regardless of whether the data stream is currently inactive, .."
> This is specified in RFC8445, so maybe consider not using normative language here as well... however, this case is maybe less clear.
> 
> I suggest:
> 
>   "As defined in [RFC8455], the keepalives will be sent..."
> 
> ---
> 
> 8) You probably should explicitly instruct IANA in the IANA consideration section to update the references to this RFC instead of RFC5245 in the respective registry.
> 
> Good point. Will do that.
> 
> ---
> 
> Regards,
> 
> Christer
> 
>