Subject: [Ice] Review of draft-ietf-ice-trickle-12

From: Nils Ohlmeier <[email-masked]>
Date: Tue, 11 Jul 2017 22:28:28 -0700

Hello,

I reviewed draft-ietf-ice-trickle-12 and have the following comments.

Section 5.2:

"After the initiator and responder exchange ICE descriptions,
…"

This sounds to me like both agents can only start after ICE descriptions have been exchanged.
But in fact especially the initiator can start receiving check requests without having received the
responders ICE description, learn remote candidates from these requests. I guess 5245-bis
covers that the ICE initiator needs to ready to receive incoming check requests any time.
But it might help to make this sound a little less strict.

"Under Trickle ICE, check lists can be empty until candidate pairs are
conveyed or received."
I thought that Trickle ICE conveys or receives candidates, which then get turned into candidate
pairs once they are paired up. So I think this “candidates pairs” should be replaced with
“candidates” here.

Section 8.1.1, 4th paragraph below figure 2:
"Then, as the checks proceed (see Section 6.2.5.4 of [rfc5245bis]),
   for each pair that enters the Succeeded state (denoted here by "S"),
   the agent will unfreeze all pairs for all media streams with the same
   foundation (e.g., if the pair in column 1, row 1 succeeds then the
   agent will unfreeze the pair in column 1, row 2)."
This sentence, up to the last brackets with the example, got me to expect that the agent will unfreeze
rows 2-4. So I was surprised that only row 2 in the example gets unfrozen.

"ICE also specifies
   that, if all the pairs in a media stream for one foundation are
   unfrozen (e.g., column 1, rows 1 and 2 representing both components
   for the audio stream), then all of the candidate pairs in the entire
   column are unfrozen (e.g., column 1, rows 3 and 4)."

After this next sentences I think either of these two sentences needs to be updated. Based on the example
data in the brackets I guess the first sentence here should be “Then, as the checks proceed, for each pair
that enters the Succeeded state, the agent will unfreeze all pairs with the same foundation for the same
media stream as the succeeded pair.”
Or is the first sentence right, and the example data is wrong and the second sentence not needed?

In all there paragraphs for Case 1-3 I would suggest to replace “in this column” with “in it’s column”.

Section 8.2

I think the end-of-candidates indication is missing an indication for the generation it applies to, for
example by adding the ufrag. Because without it I’m not sure how the receiving ICE agent knows
to which ICE session to apply the end-of-candidates in case of an ICE restart.

Appendix A

The numeric list for the first example should state explicitly what kind of ICE agents Alice and Bob
are having/using here, similar to the second example where 1. and 2. explicitly describe the assumptions.

---

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 12 Jul 2017 20:53:50 -0600

Hi Nils,

Thanks for the review. Comments inline.

> But it might help to make this sound a little less strict.

I suggest that we remove the first clause, so that the paragraph starts
like this:

   As soon as the agents have obtained local and remote candidates,...

> pairs once they are paired up. So I think this “candidates pairs” should

Agreed.

> "Then, as the checks proceed (see Section 6.2.5.4 of [rfc5245bis]), for
> each pair that enters the Succeeded state (denoted here by "S"), the
> agent will unfreeze all pairs for all media streams with the same
> foundation (e.g., if the pair in column 1, row 1 succeeds then the agent
> will unfreeze the pair in column 1, row 2)."

Yes, Taylor pointed this out, too. It's already fixed in source control:

https://github.com/ice-wg/trickle/commit/d122941c1079be45013e1721cdbb07657f98a186#diff-f7ced034e08b14404c7832d8f5f19567

> "ICE also specifies that, if all the pairs in a media stream for one
> foundation are unfrozen (e.g., column 1, rows 1 and 2 representing both
> components for the audio stream), then all of the candidate pairs in the
> entire column are unfrozen (e.g., column 1, rows 3 and 4)."

We determined that the second sentence is not needed:

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

> column” with “in it’s column”.

That is indeed clearer.

> to which ICE session to apply the end-of-candidates in case of an ICE

I think that's right. For example, in Section 8 the spec states:

   Also, candidate trickling needs to be correlated to a specific ICE
   session, so that if there is an ICE restart, any delayed updates for
   a previous session can be recognized as such and ignored by the
   receiving party.

Thus we need to specify this "binding" in Secton 8.2, as well. I suggest
the following text change.

OLD

   Once all candidate gathering is completed or expires for a specific
   media stream, the agent will generate an "end-of-candidates"
   indication for that stream and convey it to the remote agent via the
   signaling channel.  The exact form of the indication depends on the
   application protocol.

NEW

   Once all candidate gathering is completed or expires for an ICE
   session associated with a specific media stream, the agent will
   generate an "end-of-candidates" indication for that session and
   convey it to the remote agent via the signaling channel.  Although
   the exact form of the indication depends on the application protocol,
   the indication MUST specify the generation (ufrag/pwd combination) so
   that an agent can correlate the end-of-candidates indication with a
   particular ICE session.

> explicitly describe the assumptions.

I don't think the spec is explaining anything specific to trickle in the
first example (e.g., in step 2 of the first example, Alice could convey
the candidate to Bob in an initial ICE description or in a trickled
candidate - it doesn't make any difference.

However, I suggest that we might want to add a clause to the end of the
following paragraph:

OLD

   At this point the check list only contains Failed candidates and the
   valid list is empty.  This causes the media stream and potentially
   all ICE processing to fail.

NEW

   At this point the check list only contains Failed candidates and the
   valid list is empty.  This causes the media stream and potentially
   all ICE processing to fail, even though if trickle agents could
   subsequently convey candidates that would cause previously empty
   check lists to become non-empty.

Peter