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

[Peter Saint-Andre <[e-mail]>]

On 4/2/18 4:14 PM, Eric Rescorla wrote:
> Eric Rescorla has entered the following ballot position for
> draft-ietf-ice-trickle-18: No Objection

Should this be "Recuse"?

> 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.

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

> 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?

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

> Also, why are you proposing prioritizing other candidates over prflx
> 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.

>    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.

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

>    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?

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.

> 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.

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

>    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

Agreed.

>    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

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

>    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?

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.

>    (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"

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

>    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"

Will change these to "using protocol" too.

>    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"

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

Peter