Subject: [Ice] Eric Rescorla's No Objection on draft-ietf-ice-trickle-18: (with COMMENT)

From: Eric Rescorla <[email-masked]>
Date: Mon, 02 Apr 2018 15:14:39 -0700

Eric Rescorla has entered the following ballot position for
draft-ietf-ice-trickle-18: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ice-trickle/

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

Comments in context: https://mozphab-ietf.devsvcdev.mozaws.net/D3650

I'm a listed author on this document, but I haven't worked on it in quite
some time, so this is read with mostly fresh eyes.

Important comments:
   agent SHOULD follow a policy of keeping the higher priority candidate
   unless it is peer reflexive.

IMPORTANT: This section is confusing wrt how pruning works. AIUI, you
follow ordinary 5245 pruning procedures at the beginning of the
connection for whatever candidates you have. Is that incorrect?

Also, why are you proposing prioritizing other candidates over prflx
candidates? That's not the procedure in 5245.

   maximum number of candidate pairs (100 by default as per
   [rfc5245bis]), the new pair is discarded.

IMPORTANT: Why are you discarding before you check for redundancy?
This seems like it evicts the wrong pair.

   new candidate to the priority of the pre-existing candidate and then
   re-sorting the check list.
IMPORTANT: This seems to slow down checks if the existing check is
already running. What is the rationale for this?

Other comments:

   Generation:  All of the candidates conveyed within an ICE session (as
      defined in [rfc5245bis]).

Note that "generation" isn't a 5245 term, so this text could be clearer.

   ICE Description:  Any session-related (as opposed to candidate-
      related) attributes required to configure an ICE agent.  These

It might be helpful to say "ICE session" to distinguish from session-level
attributes in SDP

   for instance, the encoding for the Session Description Protocol (SDP)
   [RFC4566] is defined in [I-D.ietf-mmusic-trickle-ice-sip].

Note that this has the side effect of precluding agressive nomination

   consider the overall session to be under active negotiation as soon
   as possible.)
   As noted in Section 3, in application protocols that use SDP the

Why is this a benefit, actually? I see why it is in the offer so that you can
parallelize the gathering on both sides, but why here?

   (rather than as a part of the initial ICE description or a response
   thereto), it is the responsibility of the using protocol to define
   methods for associating the indication with one or more specific data

I see here you say "using protocol" as opposed to "application"

   o  A signaling protocol SHOULD provide a way for parties to advertise
      and discover support for Trickle ICE before an ICE session begins

And here you use "signaling protocol" rather than "using protocl"

   To achieve this, when trickling candidates, agents MUST respect the
   order of components as reflected by their component IDs; that is,

I'm surprised to see a MUST paired with "While not crucial"

---

From: Peter Saint-Andre <[email-masked]>
Date: Mon, 2 Apr 2018 18:15:40 -0600

> draft-ietf-ice-trickle-18: No Objection

Should this be "Recuse"?

> https://datatracker.ietf.org/doc/draft-ietf-ice-trickle/

You left early and I joined late. Hopefully we have enough overlap...

> connection for whatever candidates you have. Is that incorrect?

Right, there's no reason to override normal pruning procedures.

> candidates? That's not the procedure in 5245.

This text was in the spec when I joined the author team. I am not sure
of the rationale. The only difference with Trickle ICE is handling
candidate pairs that come in later. Section 13 notes that it's possible
for a server reflexive candidate to be perceived as peer reflexive if
the STUN connectivity check arrives before signaling of the trickled
candidate, however it also notes that this "does not require any change
in processing". See also below.

> This seems like it evicts the wrong pair.

Hmm, it could. Checking the max-pair rule after the redundancy check
would indeed make more sense.

> already running. What is the rationale for this?

Good point. This is the same issue with prflx in another guise. I'm not
sure of the rationale. The only thing I can come up with is that we're
trying to work around the fact that some server reflexive candidates
could be perceived initially as peer reflexive. Throwing out the earlier
"peer reflexive" candidate in favor of the later candidate (which might
be server reflexive) might be a workaround.

I'll ask Emil to clarify this for us.

> Note that "generation" isn't a 5245 term, so this text could be clearer.

Let's remove "(as defined in [rfc5245bis])".

> It might be helpful to say "ICE session" to distinguish from session-level

Agreed.

> Note that this has the side effect of precluding agressive nomination

IIRC that was a feature. Should we call it out?

> parallelize the gathering on both sides, but why here?

It seems this could help the initiator learn whether the responder
supports trickle if no discovery mechanism exist in the using protocol.
Although in that case the initiator probably would have sent a full or
at least adequate generation of candidates with the offer (half
trickle), as mentioned in Section 12 the initiator could trickle more
candidates after receiving the response.

> I see here you say "using protocol" as opposed to "application"

5245bis uses the terms "using protocol" and "ICE usage", which seem
preferable (to me an application sounds like a user agent). Will fix
throughout.

> And here you use "signaling protocol" rather than "using protocl"

Will change these to "using protocol" too.

> I'm surprised to see a MUST paired with "While not crucial"

That's been bothering me, too. At the least, I'd change MUST to SHOULD.

Peter

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 3 Apr 2018 05:13:24 +0000

Hi,

> Right, there's no reason to override normal pruning procedures.
...
>> Why are you discarding before you check for redundancy?
> Hmm, it could. Checking the max-pair rule after the redundancy check would indeed make more sense.

What if the pruning would result in a pair that e.g., is already in the Succeeded state to be removed? Is that what you want to do?

---

> Let's remove "(as defined in [rfc5245bis])".

I am not sure whether that is the issue. The 5245bis reference is for "ICE session", and I think Ekr's comment was that the definition of "Generation" needs to be clearer - no matter if there is a reference to 5245bis or not.

Regards,

Christer

> It might be helpful to say "ICE session" to distinguish from 
> session-level attributes in SDP

Agreed.

> Why is this a benefit, actually? I see why it is in the offer so that 
> you can parallelize the gathering on both sides, but why here?

It seems this could help the initiator learn whether the responder supports trickle if no discovery mechanism exist in the using protocol.
Although in that case the initiator probably would have sent a full or at least adequate generation of candidates with the offer (half trickle), as mentioned in Section 12 the initiator could trickle more candidates after receiving the response.

> I see here you say "using protocol" as opposed to "application"

5245bis uses the terms "using protocol" and "ICE usage", which seem preferable (to me an application sounds like a user agent). Will fix throughout.

Peter

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 3 Apr 2018 09:38:32 -0600

> What if the pruning would result in a pair that e.g., is already in the Succeeded state to be removed? Is that what you want to do?

I don't think so. I'm on a call right now but will reply again.

Peter

---

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 4 Apr 2018 10:45:02 -0600

> I don't think so. I'm on a call right now but will reply again.

That was a long call. ;-)

It seems to me that when a new candidate pair is received, we'd want to
remove a duplicate pair only if it is in the Waiting or Frozen state.

Peter

---

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 4 Apr 2018 10:57:59 -0600

> remove a duplicate pair only if it is in the Waiting or Frozen state.

Specifically, I suggest the following revised text for Section 8.1:

###

8.1.  Pairing Newly Learned Candidates and Updating Check Lists

   Forming candidate pairs works as described in the ICE specification
   [rfc5245bis].  However, adding the new pair to a check list happens
   according to the following rules:

   1.  If the new pair's local candidate is server reflexive, the agent
       MUST replace the candidate with its base before completing the
       redundancy check in step 2.

   2.  The agent eliminates redundant pairs by following the rules in
       Section 5.1.3 of [rfc5245bis], but only if the old pair has a
       state of Waiting or Frozen (thus avoiding removal of pairs for
       which connectivity checks are in flight or for which connectivity
       checks have already yielded a definitive result).

   3.  If after the foregoing redundancy test the check list where the
       pair is to be added already contains the maximum number of
       candidate pairs (100 by default as per [rfc5245bis]), the agent
       SHOULD discard any pairs in the Failed state to make room for the
       new pair.  If there are no such pairs, the agent SHOULD discard
       the new pair.

   4.  Otherwise, add the new pair to the check list.

###

Peter

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 4 Apr 2018 19:17:47 -0500

>        Section 5.1.3 of [rfc5245bis], but only if the old pair has a

I wonder if "old" is ambiguous here.

-Benjamin

---

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 4 Apr 2018 18:26:20 -0600

Right, “existing pair” would be better.

Sent from mobile, might be terse 

> On Apr 4, 2018, at 6:17 PM, Benjamin Kaduk <[email-masked]> wrote:
>

---

From: Peter Saint-Andre <[email-masked]>
Date: Fri, 6 Apr 2018 10:56:14 -0600

A follow-up on peer reflexive candidates...

>> draft-ietf-ice-trickle-18: No Objection

<snip/>

> I'll ask Emil to clarify this for us.

Emil and I synced up just now.

Right after IETF 95, Taylor Brandstetter raised an issue about the
handling of peer reflexive candidates during de-duplication, quoted here
for convenience from the mail archive (see
https://www.ietf.org/mail-archive/web/ice/current/msg00304.html):

###

It's possible for a remote peer reflexive candidate to be created simply
because a binding request was received before the candidate was
signaled. Then, according to this rule, the peer reflexive candidate
would not be replaced by the server reflexive candidate when it's
signaled, because the peer reflexive candidate is guaranteed to have a
higher priority.

As a result, the endpoints will prioritize the same candidate pair
differently, because one endpoint sees the candidate as peer reflexive
while the other sees it as something else. This means the endpoints
could have different ideas of which candidate pair is the highest
priority, which could result in sub-optimal selection of a candidate
pair, or asymmetric selection if the controlling agent is an RFC 5245
endpoint that uses aggressive nomination.

So, I suggest a minor modification to the rule: peer reflexive
candidates are always replaced by other types of candidates, regardless
of priority.

###

Looking back at the text we recently removed in -19, I'm not convinced
that we ever captured Taylor's feedback properly. I will look into it
more carefully later today or over the weekend and propose a further
text adjustment.

Peter

---

From: Christer Holmberg <[email-masked]>
Date: Sun, 8 Apr 2018 19:58:01 +0000

Hi,

>> candidate pairs that come in later. Section 13 notes that it's 
>> possible for a server reflexive candidate to be perceived as peer 
>> reflexive if the STUN connectivity check arrives before signaling of 
>> the trickled candidate, however it also notes that this "does not 
>> require any change in processing". See also below.
>> 
>> Good point. This is the same issue with prflx in another guise. I'm 
>> not sure of the rationale. The only thing I can come up with is that 
>> we're trying to work around the fact that some server reflexive 
>> candidates could be perceived initially as peer reflexive. Throwing 
>> out the earlier "peer reflexive" candidate in favor of the later 
>> candidate (which might be server reflexive) might be a workaround.
>> 
> Right after IETF 95, Taylor Brandstetter raised an issue about the handling of peer reflexive candidates 
> during de-duplication, quoted here for convenience from the mail archive (see
> It's possible for a remote peer reflexive candidate to be created simply because a binding request was received 
> before the candidate was signaled. Then, according to this rule, the peer reflexive candidate would not be replaced 
> by the server reflexive candidate when it's signaled, because the peer reflexive candidate is guaranteed to have a higher priority.
>
> As a result, the endpoints will prioritize the same candidate pair differently, because one endpoint sees the candidate 
> as peer reflexive while the other sees it as something else. This means the endpoints could have different ideas of which 
> candidate pair is the highest priority, which could result in sub-optimal selection of a candidate pair, or asymmetric selection 
> if the controlling agent is an RFC 5245 endpoint that uses aggressive nomination.
>
> So, I suggest a minor modification to the rule: peer reflexive candidates are always replaced by other types of candidates, regardless of priority.
>
> ###
>
> Looking back at the text we recently removed in -19, I'm not convinced that we ever captured Taylor's feedback properly. I will 
> look into it more carefully later today or over the weekend and propose a further text adjustment.

Not sure I follow. Is this a binding-request-was-received-before-the-candidate-was-signaled case specific to trickle? Are we talking about trickled candidates?

If so, does it below to section 7? In another email I asked for clarification whether section 7 only applies to the initial check list (as section 8 seems to cover the case when candidates are later trickled).

Regards,

Christer

---

From: Peter Saint-Andre <[email-masked]>
Date: Mon, 9 Apr 2018 11:58:47 -0600

> Not sure I follow. Is this a binding-request-was-received-before-the-candidate-was-signaled case specific to trickle? Are we talking about trickled candidates?

My reading of Taylor's original email is that this is about receiving
duplicate candidates through the trickling process:

   At IETF 95, the decision was made to choose the candidate with the
   higher priority if duplicate candidates are received (see:
   https://github.com/ice-wg/trickle/issues/6). However, I see a
   potential issue with this, specifically regarding peer reflexive
   candidates.

And specifically about receiving a binding request before receiving any
signaling of the candidate:

   It's possible for a remote peer reflexive candidate to be created
   simply because a binding request was received before the candidate
   was signaled. Then, according to this rule, the peer reflexive
   candidate would not be replaced by the server reflexive candidate
   when it's signaled, because the peer reflexive candidate is
   guaranteed to have a higher priority.

https://www.ietf.org/mail-archive/web/ice/current/msg00304.html

> If so, does it below to section 7? In another email I asked for clarification whether section 7 only applies to the initial check list (as section 8 seems to cover the case when candidates are late…

Yes, I read all of your email messages. :-)

I agree that the exact roles of Section 7 and Section are slightly
murky, and will propose better text today.

Peter

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 9 Apr 2018 18:22:22 +0000

Hi,

...

>>> Looking back at the text we recently removed in -19, I'm not 
>>> convinced that we ever captured Taylor's feedback properly. I will look into it more carefully later today or over the weekend and propose a further text adjustment.
>> 
> My reading of Taylor's original email is that this is about receiving duplicate candidates through the trickling process:
>
> And specifically about receiving a binding request before receiving any signaling of the candidate:
>

I just try to figure out how it is specific to trickle, and/or why normal 5245bis procedures don't apply.

>> If so, does it below to section 7? In another email I asked for clarification whether section 7 only applies to 
>> the initial check list (as section 8 seems to cover the case when candidates are later trickled).
>
> I agree that the exact roles of Section 7 and Section are slightly murky, and will propose better text today.

My suggestion would be to place sections 7.1 and 7.3 into a separate main section, and call it "Forming Initial Check List", or something like that. Sections 7.2 and 7.4 would then be part of a "Performing Connectivity Checks" section.

Regards,

Christer

---

From: Peter Saint-Andre <[email-masked]>
Date: Mon, 9 Apr 2018 16:52:24 -0600

> My suggestion would be to place sections 7.1 and 7.3 into a separate main section, and call it "Forming Initial Check List", or something like that. Sections 7.2 and 7.4 would then be part of a "Pe…

Yes, I've made a bunch of changes like that. I'll publish -20 shortly
(whose diff appears larger than it really is because I've also renamed
some sections, made some sections no longer sub-sections, etc.).

Peter