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

From: Christer Holmberg <[email-masked]>
Date: Wed, 5 Oct 2016 13:43:31 +0000

Hi,

I took a look at draft-trickle-ice.

In general, I think the document could use some editorial clean ups.
However, I will only comments on the parts which I think are essential to
correct:

Q1:

In general, you should use common terminology, e.g, when referring to the
ICE spec. Sometimes you say ³Vanilla ICE specifies², sometimes "The ICE
specification [rfc5245bis] specifies², etc.

Q2:

In section 2, the definition of Trickled Candidate says:

   "Candidates that a Trickle ICE agent sends after
    an offer or answer but within the same context.²

1)	What is the scope of ³context²?

Q3: 

In section 2, the definition of Half Trickle says:

      "The mechanism is mostly meant for use in cases where
      support for Trickle ICE cannot be confirmed prior to sending an
      initial offer.²

1)	I guess it should be support for Trickle ICE by the answerer.

2)	Does this mean one MUST use half trickle in cases where the support is
not known?

Q4:

Section 2 talks about ³first generation of candidates². It is unclear what
is meant by ³first generation².

Q5:

In section 3, why do we talk about SDP, and we do we use RFC 2119
terminology for SDP endpoints? That belongs to the ice-sip-sdp draft.

Q6:

In section 5, the text says:

	"If this is not the case, the agent MUST process the offer
	according to Vanilla ICE procedures [rfc5245bis] or offer/answer
	processing rules [RFC3264] if no ICE support is detected at all.²

1)	Why do you now suddenly reference RFC 3264 for offer/answer, and even
mandate agents to use it? In the introduction you said that ³offer/answer²
does NOT mean RFC 3364 within the scope of this document.

Q7:

In section 5.2, the text says:

	"With regard to pruning of duplicate candidate pairs, a Trickle ICE
	agent SHOULD follow a policy of "first one wins" and not re-apply the
	pruning procedure if a higher-priority candidate pair is received
	from the remote agent.²

1)	Is there a specific reason for not ³re-pruning²?

Q8:

In section 8, the text says:

	"After an offer or an answer has been sent, agents will most likely
	continue discovering new local candidatesŠ²

1)	I assume it should say ³Trickle ICE agents will most likelyŠ²

Q9:

In section 8, the text says:

	"The new candidate is then checked for redundancy against the existing
	list of local candidates.  If its transport address and base match
	those of an existing candidate,Š²

1)	In order to align with the 5245bis terminology, I assume ³transport
address² and ³base² should be in uppercase?

Q10:

In section 8, the text says:

 	"When trickle updates are sent, each candidate MUST be delivered to
	the receiving Trickle ICE implementation not more than once and in
	the same order that they were sent.²

1)	It¹s difficult for me to parse this, especially the ³same order² part.
I think you should simply say that the signalling protocol must ensure
that re-transmitted candidates are not treated as two separate candidates.

Q11:

In section 8, the text says:

	"Also, candidate trickling needs to be correlated to a specific ICE
negotiation session,²

1)	What is an ³ICE negotiation session²?

Q12:

In section 9, the text says:

 	"At any time during ICE processing, a Trickle ICE agent may receive
	new candidates from the remote agent.  When this happens and no local
	candidates are currently known for this same stream,²

1)	What is meant by ³this same stream²? I assume you refer to the stream
with which the candidate is associated. If so, please make it more clear.

Q13:

In section 13, you again reference to the rules in RFC 3264 regarding when
an offer or answer can be sent.

Q14:

In section 13, I assume the second bullet also apply to ICE restart. If
so, please make it clear.

Regards,

Christer

---

From: Peter Saint-Andre <[email-masked]>
Date: Mon, 10 Oct 2016 21:11:21 -0600

Thanks for the review. Comments inline.

> specification [rfc5245bis] specifies², etc.

In general I feel that the term "Vanilla ICE" is cute but not all that 
helpful. We could say "ICE core" instead. (BTW, as far as I can see, 
there is no instance of "Vanilla ICE specifies" in this document and all 
references are to the actual specification.)

> 1)	What is the scope of ³context²?

Perhaps "session" or even "ICE negotiation session" would be clearer.

> 1)	I guess it should be support for Trickle ICE by the answerer.

Yes, that's better.

> 2)	Does this mean one MUST use half trickle in cases where the support is

That is discussed in section 14. I see no reason to use MUST here or 
even SHOULD - it's up to the implementation.

> is meant by ³first generation².

I suggest "set" instead of "generation". We can also clarify that this 
is the set of candidates that is sent with the initial offer.

> terminology for SDP endpoints? That belongs to the ice-sip-sdp draft.

Yes, that paragraph belongs in draft-ietf-mmusic-trickle-ice-sip.

> does NOT mean RFC 3364 within the scope of this document.

I suggest that we remove the clause "or offer/answer processing rules 
[RFC3264]".

> 1)	Is there a specific reason for not ³re-pruning²?

Hmm, it seems that this is perhaps not consistent with some text that we 
recently modified in section 8.1:

###

    Once this is done, the agent examines the check list looking for
    another pair that would be redundant with the new one.  If such a
    pair exists and its type is not peer reflexive, the pair with the
    higher priority is kept and the one with the lower priority is
    discarded.  If, on the other hand, the type of the pre-existing pair
    is peer reflexive, the agent MUST replace it with the new candidate
    it received (regardless of their respective priorities); this is done
    by setting the priority of the new candidate to the priority of the
    pre-existing candidate and then re-sorting the check list.

       Note: Replacing pre-existing pairs with seemingly equivalent
       higher-priority ones helps guarantee that both agents will have
       the same view of candidate priorities.  This is particularly
       important during aggressive nomination, when priority is sometimes
       the only way a controlled agent can determine the selected pair.
       It is for that same reason that peer-reflexive candidates need to
       always be updated if equivalent alternatives are received through
       signaling.

###

> 1)	I assume it should say ³Trickle ICE agents will most likelyŠ²

Correct. Will fix.

> address² and ³base² should be in uppercase?

5245bis does not seem to be consistent in this regard, e.g.:

                                                               We call
    the host candidate associated with a given server reflexive candidate
    the BASE.

       Note: "Base" refers to the address an agent sends from for a
       particular candidate.  Thus, as a degenerate case host candidates
       also have a base, but it's the same as the host candidate.

Most instances are lowercase, so I'd leave it as-is for now.

> that re-transmitted candidates are not treated as two separate candidates.

It strikes me as sensible to specify that:

(a) the signalling protocol must provide in-order delivery

(b) the Trickle ICE agent must not treat re-transmitted candidates as 
distinct candidates (i.e., it must effectively de-dupe candidates)

(I'm not sure what it would mean for something like (b) to apply to the 
protocol, not the agent.)

> 1)	What is an ³ICE negotiation session²?

That should be defined. As I understand it, an ICE negotiation session 
consists of all the exchanges that occur before an ICE restart (if any) 
is sent.

> with which the candidate is associated. If so, please make it more clear.

Will do.

> an offer or answer can be sent.

I think this is better phrase along the lines of "If a Trickle ICE agent 
supports offer/answer semantics [RFC3264], it MAY generate a subsequent 
offer at any time that is allowed by [RFC3264]."

> so, please make it clear.

Correct.

Thanks again for the review.

Peter

---

From: Emil Ivov <[email-masked]>
Date: Thu, 27 Oct 2016 17:33:28 -0500

> references are to the actual specification.)

I don't share the concern about vanilla ICE but I don't feel strongly 
about it.
> Perhaps "session" or even "ICE negotiation session" would be clearer.

session sounds good.

>> 1)    I guess it should be support for Trickle ICE by the answerer.

SGTM.

> is the set of candidates that is sent with the initial offer.

I think "set" can be ambiguous here and mistakenly perceived as the 
first batch of candidates. Generation is a term that was initially used 
in Jingle which is why I felt it was a good way to refer to "all 
candidates within the same ICE session"
> Yes, that paragraph belongs in draft-ietf-mmusic-trickle-ice-sip.

Actually that one is intentionally there. There was a decision taken 
very early on that this draft will use jsep style SDP to facilitate 
understanding.

At this point of the document's life cycle I'd be reluctant to revise 
this decision especially as I don't see what problem it would be solving.
> I suggest that we remove the clause "or offer/answer processing rules

The text is there for completeness. 5245 bis says that you fall back to 
3264 if ICE is not supported.

What we are saying here is: if trickle is not supported you can fallback 
to vanilla ICE or (in case even vanilla ICE is not supported) fallback 
to 3264 as per 5245.

I still think this adds a marginal amount of clarity to implementers but 
I don't feel strongly about it.

> recently modified in section 8.1:

That's actually part of the text we added. This is what we talked about 
in the past two meetings and in between.

>> 1)    I assume it should say ³Trickle ICE agents will most likelyŠ²

This is the trickle ICE spec ... Trickle ICE is assumed by default. But 
fine.

> (a) the signalling protocol must provide in-order delivery

I like this wording!

> distinct candidates (i.e., it must effectively de-dupe candidates)

I agree but I don't think we need to mention that. I'd rather go with 
UNDEFINED for "what happens if an agent gets a candidate twice" becaue 
we don't have an use case for why this would happen.

> protocol, not the agent.)

It would just mean: if I am a Trickle ICE stack and you (the signalling 
protocol stack or the app) feed me the same candidate twice, I am going 
to throw an exception or behave in an unpredictable way.

> consists of all the exchanges that occur before an ICE restart (if any)

Yes.

Thanks for the review Christer and for the edits to Peter!

Emil

> _______________________________________________
> Ice mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ice
>

-- 
https://jitsi.org

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 28 Oct 2016 07:43:17 +0000

Hi,

>> Perhaps "session" or even "ICE negotiation session" would be clearer.

What is the definition of “session”?

>candidates within the same ICE session"

So, what exactly do you want to say?

>very early on that this draft will use jsep style SDP to facilitate

What is “jsep style SDP”???

>this decision especially as I don't see what problem it would be solving.

I was very skeptical to splitting ICE and SDP to begin with. Now when it’s
done we should stick to it.

>3264 if ICE is not supported.

You only fall back if you support 3264 to begin with - which should not be
a requirement in 5245bis as it’s supposed to be “protocol independent”.

>to vanilla ICE or (in case even vanilla ICE is not supported) fallback

That is wrong. We can not mandate 5245bis endpoints to support SDP. In
that case we should make 5245bis SDP dependent again.

>This is the trickle ICE spec ... Trickle ICE is assumed by default. But

“Agents”, “ICE agents”… whatever. As long as the draft uses consistent
terminology, and it is clear what is meant.

>> Most instances are lowercase, so I'd leave it as-is for now.

I want to be consistent. If something needs to be changed in 5245bis,
we’ll change it.

>we don't have an use case for why this would happen.

It can happen when you use trickle-sip-sep.

>> consists of all the exchanges that occur before an ICE restart (if any)

You should then add a definition for it, in addition to “session”, “ICE
session”, and whatever other sessions you may refer to.

Regards,

Christer