Subject: [Ice] Comments on draft-ietf-ice-trickle-04 (Taylor)

From: Taylor Brandstetter <[email-masked]>
Date: Wed, 19 Oct 2016 11:53:17 -0700

I reviewed draft-ietf-ice-trickle-04 and noted various comments/questions
as I went along. Looking back, many of these are the same things Christer
mentioned a week ago, or are purely editorial. But my major takeaways were:

1. Many sections need to be changed in order to not directly prescribe the
SDP offer/answer model. Though I feel it's fine to use SDP offer/answer in
examples.
2. There are still some problems with the freezing/unfreezing rules as I
understand them.

Anyway, here are my comments, grouped by section:

*Section 2 - Terminology*

   - Here and throughout the document, UPnP based port allocation and XMPP
   Jingle Relay Nodes are mentioned, which not even ICEbis references. Should
   these be removed?
   - The concept of a “generation of candidates” is not defined anywhere,
   even though I intuitively know what it means. I recommend defining it in
   ICEbis. The phrase “ICE negotiation session” is also used later, referring
   to (I think) the same thing.
   - It’s stated that Trickle ICE is not defined “in terms of SDP or the
   offer/answer model”. But it is, throughout the entire document. The
   document needs to be changed to refer to “candidate exchanges”, for
   example, instead of offers/answers.

*Section 4 - Sending the Initial Offer*

   - “An agent starts gathering candidates as soon as it has an indication
   that communication is imminent”. This isn’t always true. There are even
   examples later of the opposite happening (candidates being gathered well in
   advance). Maybe just say “an agent *generally* starts gathering…”
   - “A host candidate at a media relay”. Is that really a host candidate?
   Where is this described in ICEbis?
   - There is discussion around hiding host candidates for privacy
   implications. This isn’t specific to Trickle ICE though, so it should go in
   ICEbis or somewhere else.
   - “Methods for calculating priorities and foundations, as well as
   determining redundancy of candidates, work just as with ICEbis.” Not the
   redundancy part, since there are the special prflx rules.

*Section 5.2 - Forming Check Lists and Beginning Connectivity Checks*

   - There is talk about unfreezing candidates. But the concept of freezing
   a candidate doesn't make sense to me; only pairs are unfrozen. Is this
   meant to say “candidate pairs”? I actually notice the same problem in
   ICEbis.
   - The "first checklist with a candidate pair wins" unfreezing rule means
   that the initial unfrozen checklist could be different between the
   initiating and responding agents due to racy trickling, leading to at least
   one of the checklists failing. Suppose for example that checklist 1 has
   only pairs with foundation A and checklist 2 has only pairs with foundation
   B; there's no guarantee on ordering between foundations. Why not just
   "first checklist starts as active, all others frozen"?
   - “With regard to pruning of duplicate candidate pairs, a Trickle ICE
   agent SHOULD follow a policy of ‘first one wins’.” Is this out of date? In
   “Pairing Newly Learned Candidates and Updating Check Lists” there are rules
   for handling duplicate candidates. There are other places where this “first
   one wins” policy is mentioned that should be updated.
   - The concepts of “active” and “frozen” check lists are used here, but
   the definitions are only elaborated upon in “Check List and Timer State
   Updates”.

*Section 7.2 - Check List and Timer State Updates*

   - The definitions of active/frozen are a SHOULD rather than a MUST. If
   the unfreezing algorithm is vital, as stated elsewhere, shouldn’t it be a
   MUST?
   - “A Trickle ICE agent SHOULD consider a check list to be active … when
   the check list is empty.” Wouldn’t that mean that everything begins as
   active, in the common case where you start with 0 pairs? I think the intent
   here is that an empty check list may be considered active or frozen, but
   that’s not exactly what’s written. Also, if that *is* the intent, the
   specification should explain exactly when an empty frozen checklist becomes
   an empty active checklist. I believe those conditions are:
   - Another checklist is completely finished (every pair Successful or
      Failed).
      - Another checklist has a valid pair for all components.

*Section 8 - Discovering and Sending Additional Local Candidates*

   - In my opinion, the rule that you MUST NOT pair a local candidate until
   it's been trickled is not clear enough. Though it's heavily implied.
   - The rule that you must trickle candidates in the order of components,
   to protect the freezing algorithm, is not enough. I believe you'd also need
   to trickle them in order of media stream and priority since this also
   affects which pair is initially unfrozen.

*Section 8.1 - Pairing Newly Learned Candidates and Updating Check Lists*

   - If a checklist is active, the new pair is set to “Waiting” if it’s the
   first pair in that list with that foundation. This puts pairs in the
   Waiting state much more aggressively than ICEbis; if a pair for the first
   media stream/component is still in the Waiting state, it will basically be
   ignored when pairs with that foundation are added for other media streams,
   and they'll all end up as "Waiting". Should we effectively replace “if no
   pair in this checklist has a matching foundation” with “if no pair in *any*
   checklist has a matching foundation”?
   - Even with the above change, the unfreezing rules don’t exactly match
   ICEbis. So a race between candidate signaling and connectivity checks could
   cause a pair to be Frozen on one side and Waiting on the other. Are we ok
   with this?

*Section 8.2 - Announcing End of Candidates*

   - “When an end-of-candidates indication is sent as part of an offer of
   an answer, it can be considered to apply to the session as a whole, which
   is equivalent to having it apply to all media streams.” Again, shouldn’t be
   getting into offer/answer semantics in this document. But even so, this is
   incorrect. “a=end-of-candidates” is media-level.

*Section 11 - Trickle ICE and Peer Reflexive Candidates*

   - This section doesn’t seem to take into account the recent changes,
   where peer reflexive candidates can be replaced by server reflexive
   candidates.

*Section 18 - Acknowledgements*

   - I don’t think I deserve my own paragraph, I hardly did anything. :)

---

From: Emil Ivov <[email-masked]>
Date: Wed, 19 Oct 2016 13:56:23 -0500

Thanks Taylor,

I'll be looking at those this week!

Emil

On Wed, Oct 19, 2016 at 1:53 PM, Taylor Brandstetter
<[email-masked]> wrote:
> I reviewed draft-ietf-ice-trickle-04 and noted various comments/questions as
> I went along. Looking back, many of these are the same things Christer
> Section 2 - Terminology
> Here and throughout the document, UPnP based port allocation and XMPP Jingle
> Relay Nodes are mentioned, which not even ICEbis references. Should these be
> removed?
> The concept of a “generation of candidates” is not defined anywhere, even
> though I intuitively know what it means. I recommend defining it in ICEbis.
> The phrase “ICE negotiation session” is also used later, referring to (I
> think) the same thing.
> It’s stated that Trickle ICE is not defined “in terms of SDP or the
> offer/answer model”. But it is, throughout the entire document. The document
> needs to be changed to refer to “candidate exchanges”, for example, instead
> of offers/answers.
>
>
> Section 4 - Sending the Initial Offer
>
> “An agent starts gathering candidates as soon as it has an indication that
> communication is imminent”. This isn’t always true. There are even examples
> later of the opposite happening (candidates being gathered well in advance).
> Maybe just say “an agent *generally* starts gathering…”
> “A host candidate at a media relay”. Is that really a host candidate? Where
> is this described in ICEbis?
> There is discussion around hiding host candidates for privacy implications.
> This isn’t specific to Trickle ICE though, so it should go in ICEbis or
> somewhere else.
> “Methods for calculating priorities and foundations, as well as determining
> redundancy of candidates, work just as with ICEbis.” Not the redundancy
> part, since there are the special prflx rules.
>
>
> Section 5.2 - Forming Check Lists and Beginning Connectivity Checks
>
> There is talk about unfreezing candidates. But the concept of freezing a
> candidate doesn't make sense to me; only pairs are unfrozen. Is this meant
> to say “candidate pairs”? I actually notice the same problem in ICEbis.
> The "first checklist with a candidate pair wins" unfreezing rule means that
> the initial unfrozen checklist could be different between the initiating and
> responding agents due to racy trickling, leading to at least one of the
> checklists failing. Suppose for example that checklist 1 has only pairs with
> foundation A and checklist 2 has only pairs with foundation B; there's no
> guarantee on ordering between foundations. Why not just "first checklist
> starts as active, all others frozen"?
> “With regard to pruning of duplicate candidate pairs, a Trickle ICE agent
> SHOULD follow a policy of ‘first one wins’.” Is this out of date? In
> The concepts of “active” and “frozen” check lists are used here, but the
> definitions are only elaborated upon in “Check List and Timer State
> Updates”.
>
>
> Section 7.2 - Check List and Timer State Updates
>
> The definitions of active/frozen are a SHOULD rather than a MUST. If the
> unfreezing algorithm is vital, as stated elsewhere, shouldn’t it be a MUST?
> “A Trickle ICE agent SHOULD consider a check list to be active … when the
> check list is empty.” Wouldn’t that mean that everything begins as active,
> in the common case where you start with 0 pairs? I think the intent here is
> that an empty check list may be considered active or frozen, but that’s not
> exactly what’s written. Also, if that *is* the intent, the specification
> should explain exactly when an empty frozen checklist becomes an empty
> active checklist. I believe those conditions are:
>
> Another checklist is completely finished (every pair Successful or Failed).
> Another checklist has a valid pair for all components.
>
>
> Section 8 - Discovering and Sending Additional Local Candidates
>
> In my opinion, the rule that you MUST NOT pair a local candidate until it's
> been trickled is not clear enough. Though it's heavily implied.
> The rule that you must trickle candidates in the order of components, to
> protect the freezing algorithm, is not enough. I believe you'd also need to
> trickle them in order of media stream and priority since this also affects
> which pair is initially unfrozen.
>
>
> Section 8.1 - Pairing Newly Learned Candidates and Updating Check Lists
>
> If a checklist is active, the new pair is set to “Waiting” if it’s the first
> pair in that list with that foundation. This puts pairs in the Waiting state
> much more aggressively than ICEbis; if a pair for the first media
> stream/component is still in the Waiting state, it will basically be ignored
> when pairs with that foundation are added for other media streams, and
> they'll all end up as "Waiting". Should we effectively replace “if no pair
> in this checklist has a matching foundation” with “if no pair in *any*
> Even with the above change, the unfreezing rules don’t exactly match ICEbis.
> So a race between candidate signaling and connectivity checks could cause a
> pair to be Frozen on one side and Waiting on the other. Are we ok with this?
>
>
> Section 8.2 - Announcing End of Candidates
>
> “When an end-of-candidates indication is sent as part of an offer of an
> answer, it can be considered to apply to the session as a whole, which is
> equivalent to having it apply to all media streams.” Again, shouldn’t be
> Section 11 - Trickle ICE and Peer Reflexive Candidates
>
> This section doesn’t seem to take into account the recent changes, where
> peer reflexive candidates can be replaced by server reflexive candidates.
>
>
> Section 18 - Acknowledgements
>
> I don’t think I deserve my own paragraph, I hardly did anything. :)
>
>
>
> _______________________________________________
> Ice mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ice
>

-- 
https://jitsi.org