Subject: [MMUSIC] Re: review of ICE-13

From: Jonathan Rosenberg <[email-masked]>
Date: Wed, 28 Feb 2007 21:59:49 -0500

Sorry for the long delay in responding to this and Ekr's reviews. 
Responses inline on just the main comments. I'll reply privately to Dan 
on the nits, unless there is anything that requires discussion, in which 
case I'll bring it out.

Dan Wing wrote:

> An impressive document, Jonathan.
> 

The most impressive thing will be to FINISH it.

> 
> Primary comment:
> 
> * After doing a binding discovery, the agent needs to keep those
>   bindings open until it's sure they're no longer used.  Consider the
>   case of a little old lady from Pasadena who takes 35 seconds to type
>   in a phone number; if you obtained the binding as soon as she went
>   offhook, the binding will have timed out by the time the Invite is
>   sent.  Similarly, SIP retransmissions could cause a healthy delay in
>   the Invite reaching the called party or all of the called parties
>   (when forking) or the final target (if retargeting).
> 
>   I suggest this keepalive should be a STUN Binding Request, so that 
>   if an implementation keeps this binding open for a long time it 
>   is aware of a reassignment of public transport address by the NAT
>   (due to NAT crashing, new DHCP lease for the NAT, whatever).

Yes, that makes sense.

I added the following to the end of section 4.1.1:

<t>
Once server reflexive and relayed candidates are allocated, they MUST
be kept alive until ICE processing has completed. For server reflexive
candidates learned through the Binding Discovery usage, this MUST be
another Binding Request from the Binding Discovery usage. For relayed
candidates learned through the Relay Usage, this MUST be a new
Allocate request. The Allocate request will also refresh the server
reflexive candidate.
</t>

> 
> General comments:  
> 
> * As Eric said, more headings and lists would be useful; some of these
>   sections run two pages and the text is dense.

OK, I'll need to get specific comments on where it gets bad.

> 
> * I noticed both "lite" and "passive" are used almost interchangably
>   in the document, with the intention they mean the same thing.  It
>   would be useful to eliminate one of those terms.

Oops, bug. Passive should have been completely removed. I removed the 
stragglers.

> 
> * I found using "L" and "R" to refer to the agents a bit confusing
>   because it's difficult to say a sentence like "R takes its local
>   candidate and sends it its remote peer, L".  I did notice some of
>   the examples use A and B, and others use L and R.  It would be
>   useful to stick with one unless there is a reason to switch between
>   A/B and L/R.

I liked L and R since it allowed you to visualize. We've been using that 
for some time now. I agree it should be consistent, though. I've changed 
to L/R.

> 
> * Run http://tools.ietf.org/tools/idnits/ against the document;
>   boilerplate is wrong, a few lines exceed the maximum.

Will do before submission.

> 
> * In several places different terminology was used which appears to
>   mean the same thing:
>   - Triggered / tentative

actually they're not the same; in the paragraphs where it is described, 
the triggered check is called tentative since you need to figure out 
whether you'll actually do it based on current states. Anyway it was 
possible to describe this without the word tentative, so I removed it.

>   - Ignore duplicates / prune / eliminate redundant candidates

pruning is different; thats the process of removing PAIRS. Ignore 
duplicants/eliminate redundant were the same. I consolidated that to 
eliminate redundant.

>   - Intermediary / media relay / (and "gateway", when talking about
>     the SIP option tag in 12.2).

intermediary is the most general term. Media relay is a form of an 
intermediary - a TURN server. Gateway in 12.2 can be eliminated. I 
substituted that for intermediary.

>   - Active / in-use

This one is really messed up. in-use actually had two different definitions.

I've cleaned this up. Now, 'default' refers to the candidates in the 
m/c-line, a much more natural term for what their role is now. in-use 
refers to a media stream that has a non-zero port. Active is gone.

>   - Lite / passive / passive-only

Per above, passive and passive-only are gone.

>   - Callee / answerer (in section 12)

These are well defined. I don't see the issue here.

> 
> * Is the plan for TURN, or for ICE, to explain that TURN is only
>   needed if the two agents are behind NATs that perform Address and
>   Port Dependent Mapping (previously called "symmetric NAT")?  Right
>   now there isn't a discussion of this in ICE, but it might be useful
>   to provide guidance on when the 'provider variance' is permissable.

I added words on that. It now says:

<t>
Agents SHOULD obtain relayed candidates and MUST obtain server
reflexive candidates. The requirement to obtain relayed candidates is
at SHOULD strength to allow for provider variation. Use of relays is
expensive, and when ICE is being used, relays will only be required
when both endpoints are behind NATs that perform address and port
dependent mapping. Consequently, some deployments might consider this
use case to be marginal, and elect not to use relays. If they are not
used, it is RECOMMENDED that it be implemented and just disabled
through configuration, so that it can re-enabled through configuration
if conditions change in the future.
</t>

> 
> * There isn't a discussion of what happens when a lite implementation
>   encounters a lite implementation.  I assume there aren't any
>   connectivity checks sent by either side.  (Should be added to 5.2)

Its at the end of 5.1.

-Jonathan R.
-- 
Jonathan D. Rosenberg, Ph.D.                   600 Lanidex Plaza
Cisco Fellow                                   Parsippany, NJ 07054-2711
Cisco Systems
[email-masked]                              FAX:   (973) 952-5050
http://www.jdrosen.net                         PHONE: (973) 952-5000
http://www.cisco.com

_______________________________________________
mmusic mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/mmusic