Re: [Ice] Eric Rescorla's No Objection on draft-ietf-ice-rfc5245bis-17: (with COMMENT) (excluding Security Considerations)

[Christer Holmberg <[e-mail]>]

Hi,

(I have removed the tie-breaker comment, since it is covered in a separate thread)

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Review in context at:
https://mozphab-ietf.devsvcdev.mozaws.net/D3312

>>   Nomination, Regular Nomination:  The process of the controlling agent
>>   indicating to the controlled agent which candidate pair the ICE Given that you have
>>   removed Aggressive Nomination, do you still need to refer to "Regular Nomination"
>
> People asked to keep it in the Terminology section, to make it clear that "nomination" is 5245bis refers to what was called "regular nomination" in RFC 5245.
>
> OK, but then you should explain why you are using "regular" here.

NEW:

Nomination:  The process of the controlling agent
      indicating to the controlled agent which candidate pair the ICE
      agents will use for sending and receiving data. The nomination
      process defined in this specification was referred to "regular nomination"
      in RFC 5245.

---

>>>   candidate gathering, (2) candidate prioritization, (3) redundant
>>>  candidate elimination, and (4) sending of the candidates to the peer.
>>> This is an odd diagram. There's no reason why these have to happen in sequence and in fact in Trickle ICE, they don't, so
>>> this diagram seems misleading., as well as potentially contradicting the beginning of S 5.1.1.
>>>
>>> "An ICE agent gathers candidates when it believes that communication is imminent. "
>>
>> I think the sequence applies to core ICE, and is also how it's done in 5245.
>>
>> The fact that trickle ICE does it differently I think shall be described in the trickle spec.
>>
>> However, it if helps, I could add the following note:
>>
>> "NOTE: This specification assumes that all candidates are gathered before they are sent to the peer. The trickle ICE extensions define procedures
>> where gathered candidates can be sent to the peer as soon as they have been gathered, while additional candidates are still gathered."
>
> The issue here isn't that all candidates are gathered before they are sent to the peer but rather  that the diagram implies that no gathering happens before the peer's 
> candidates are received. That's not a requirement of 5245, and, as I said, contradicts the beginning of S 5.1.1.

I could add the following sentence: "Note that an agent can begin gathering candidates at any point, even if it has not received any candidates (or even been contacted) by its peer."

---

>>>   The agent will receive a Binding or Allocate response.  A successful
>>>   Allocate response will provide the agent with a server reflexive Or nothing or an error.
>>>
>>>              (2^8)*(IP precedence) +
>>>              (2^0)*(256 - component ID)
>>>
>>> Isn't this the same formula as in S 5.1.2.1.
>>
>> Section 5.1.2.1 defines the generic formula. This instance is when it is used by a Lite implementation.
>>
>> Of course, in order to align, I could replace "IP precedence" with "local preference".
>
> My suggestion is to have one copy.
 
I can keep the formula in section 5.1.2.1.

But, in the case of Lite, we still need to say what values are used for the different formula parts - even if we don't show the formula.

---
 
>>      pair priority = 2^32*MIN(G,D) + 2*MAX(G,D) + (G>D?1:0)
>>
>> This was kinda terrible in 5245. Given that you use it once, maybe just have + GT(G, D)
>>
>> And then say GT(G, D) == 1 if G>D and 0 otherwise.
>
> I probably just don't see it, but what is the difference?
>
> It has the same semantics but it's easier to read. I mean, either you think the C syntax is self-explanatory or you
> don't. If you do, then you don't need to explain it. If you don't, it's opaque in the expression,

Personally I think the formula is clear, and we could remove the following sentence:

"where G>D?1:0 is an expression whose value is 1 if G is greater than D, and 0 otherwise."

---

>>>   pair to the remote candidate of the pair, as described in
>>>   Section 7.2.4.
>>>   IMPORTANT: You don't just send a STUN request, you start a STUN transaction,
>>>
>> There are multiple instances in the spec where it talks about sending STUN requests or Binding requests.
>
> I agree, but this statement is still confusing. Is there a reason not to change it?

I could say "by initiating a STUN transaction", but I'd prefer to have common terminology.

---

>>>   The ICE agent constructs a candidate pair whose local candidate
>>>   equals the mapped address of the response, and whose remote candidate
>>>
>>> IMPORTANT: When does this happen?
>>
>> The last sentence of the parent section (7.2.5.3)  says:
>>
>> "If a check is considered a success, the ICE agent performs (in order) the actions described in the following sections."
>
> Please add a reverse reference.

All 7.2.5.3.X. sections happens after the check has been considered a success, so exactly where do you want a reference?

---

>>> 7.3.1.3.  Learning Peer Reflexive Candidates
>>>
>>>This entire section seems to duplicate 7.2.5.3.1
>>
>> Section 7.2.5.3.1 describes how a STUN client detects a peer reflexive candidate then it receives a STUN response.
>>
>> Section 7.3.1.3 describes how a STUN server detects a peer reflexive candidate when it receives a STUN request.
>>
>> Sure, the text is similar, but separate procedures.
>
> This would benefit from consolidation.

We did a choice to keep the client and server procedures separated, so I'd like to keep it. 

Also, there ARE differences in the procedures, so we still need to cover the client- and the server cases separately.
 
---

>>>         in-progress transaction.  Cancellation means that the agent
>>>         will not retransmit the request, will not treat the lack of
>>>         response to be a failure, but will wait the duration of the
>>>
>>> Why are you cancelling In-Progress checks when you receive a peer-reflective check?
>>> If you receive two in a row, then it seems like this delays a successful check.
>>
>> True.
>>
>> So, I will remove the cancel part.
>>
>> Do you still think it should create a new binding request, or is the ongoing In-Progress check enough to check the pair?
>
> My initial reaction is you don't need to do anything extra, but I could easily be wrong here.

I had the same reaction, but I've asked Ari to think about this too.

---

>>> More generally, this document should explain how you end up in this
>>> situation: you only get here when "the source transport address of the request
>>> does not match any existing remote candidate", so how can it be on a check list
>>> unless this is the second observation of a peer reflexive.
>>
>> I am not sure I understand. The source transport sentence you refer to is related to detection of peer reflexive candidates.
>
> My question is what sequence of events can lead to this situation, as it is surprising to the readers.

Are you asking what events can lead to a triggered request?

---

>>>   Session Description Protocol (SDP) [RFC4566] is defined in
>>>   [I-D.ietf-mmusic-ice-sip-sdp].
>>> Presumably you want to cite 5245 S 14, which states:
>>>
>>> Consequently, when a controlling agent is communicating with a peer that supports options it doesn't know about, 
>>> the agent MUST run a regular nomination algorithm.  When regular nomination is used, ICE will converge perfectly 
>>> even when both agents use different pair prioritization algorithms.
>>
>> I am not sure it is related to SDP.
>>
>> I suggest to modify the first paragraph:
>
> OLD:
>
> "For example, the agent will not use the aggressive nomination procedure defined in RFC 5245."
>
> NEW:
>
> "For example, the agent will not use the aggressive nomination procedure defined in RFC 5245. In addition,
>   It will ensure that an RFC 5245-compliant peer does not use aggressive nomination either, as described in
>   Section 14 of RFC 5245."
>
> Perhaps:
> "as required by Section 14 of RFC 5245 for peers which receive unknown ICE options"

Looks good.

---

>> NEW:
>>
>>   One of the complications in achieving interoperability is that ICE
>>   relies on a distributed algorithm running on both agents to converge
>>   on an agreed set of candidate pairs.  If the two agents run different
>>   algorithms, it can be difficult to guarantee convergence on the same
>>   candidate pairs.  The nomination procedure described in
>>   Section 8 eliminates some of the tight coordination by delegating the
>
> some of the need for tight

Looks good.
 
>   selection algorithm completely to the controlling agent, and ICE
>   will converge perfectly even when both agents use different pair
>   prioritization algorithms. One of the keys to such convergence is
>   triggered checks, which ensure that the nominated pair is validated
>   by both agents.  Consequently, any future ICE enhancements MUST
>   preserve triggered checks."

--- 

>>>   there will be four transactions per call (one for RTP and one for
>>>   RTCP, for both caller and callee).  Each transaction is a single
>>>   request and a single response, the former being 20 bytes long, and Is this currently true?
>>>
>>> How many people still don't support RTP-MUX?
>>
>> I don't know. But, unless you use mux exclusive the offerer still has to gather and provide RTCP candidates.
>
> Right, but then there will be three transactions per call in the common case.

Assuming you are using offer/answer, yes. 

Maybe something like:

"In a typical VoIP deployment, where RTP/RTCP multiplexing is used, there will be 2-3 transactions 
per call, depending on whether fallback to non-multiplexing is supported."

Regards,

Christer