Re: [Gen-art] Gen-ART review of draft-ietf-mmusic-ice-tcp-15

[Ari Keränen <[e-mail]>]

Hi Vijay,

Thanks for the review! See inline.

On 10/31/11 10:48 PM, Vijay K. Gurbani wrote:
> Minor issues:
>
> - S1: The draft says, "This specification does so by following the
>    outline of ICE itself, and calling out the additions and changes
>    necessary in each section of ICE to support TCP candidates."
>
>    Does this imply that this specification normatively updates
>    rfc5425 (the ICE RFC)?  If so, this is not reflected in the
>    masthead for the document as "Updates: RFC 5425".

Actually no. These additions are specific to TCP so the (UDP) ICE RFC is 
not updated. In an effort to keep this draft reasonably compact, only 
the delta from the RFC5245 is defined. So, when something is not defined 
here, the behavior defined in RFC5245 is assumed.

> - S4.1, last paragraph.  It says that, "TCP-based STUN transactions
>    are paced out at one every Ta seconds."  However, rfc5245 Section
>    16 says that, "These transactions are paced at a rate of one
>    every Ta millisecond, ..."
>
>    I suspect that in your draft, Ta refers to the same Ta as in
>    rfc5245, so it seems to me that they should be paced one every
>    Ta milliseconds to conform to rfc5245, no?

This one is a bit tricky since even RFC5245 is not consistent with this 
(e.g., section 5.8. of RFC5245 says "Ta seconds later", and section 
16.1. is even more vague). We chose to use "seconds" so that the 
formulas in 16.1. of 5245 would have right units, but actually in the 
text 5245 does use "milliseconds" more often. I'm fine either way but 
since you found this confusing, probably the milliseconds is a better 
choice.

> Nits:
>
> - S3, in the sentence, "Stream-oriented transports introduce another
>      wrinkle, since they require a way to frame the connection so that the
>      application and STUN packets can be extracted in order to determine
>      which is which."
>
>      s/which is which/STUN packets from application layer traffic/

OK.

> - S4.1,
>    s/any assumptions that they make about deployments/any assumptions
>    made about deployments/

OK. We'll fix these in the next revision.


Thanks,
Ari