Make a large number of small editorial changes (a sub set of PR 35 that is easier to merge) #38

pthatcherg · 2017-07-12T06:07:25Z

No description provided.

Lots of removals of duplicate or unneeded text.

Don't change: - sending checks, timing, triggered checks, etc - handling responses and how to determine success or failure or errors - the valid lists and how they work - check list states and how they change

akeranen

Reviewed about half of the changes. Overall good stuff, but some changes I'd reconsider / tweak a bit.

akeranen · 2017-07-12T14:15:33Z

draft-ietf-ice-rfc5245bis.xml

@@ -304,7 +304,7 @@ the TURN request will create a binding on each NAT, but only the
 outermost server reflexive candidate (the one nearest the TURN server)
 will be discovered by the agent. If the agent is not behind a NAT,
 then the base candidate will be the same as the server reflexive
-candidate and the server reflexive candidate is redundant and will be
+candidate and the server reflexive candidate is redundant will be


I think this was (more) correct in the previous version

akeranen · 2017-07-12T14:23:04Z

draft-ietf-ice-rfc5245bis.xml

-obtained through Virtual Private Networks (VPNs) and Realm Specific IP
-(RSIP) <xref target="RFC3102"/> (which lives at the operating system
-level).
+specific port from an IP address on the host.


I think the VPN part is good to mention, but would leave RSIP away.

Added VPN back

akeranen · 2017-07-12T14:24:29Z

draft-ietf-ice-rfc5245bis.xml

 candidates the base is the same as the host candidate. For relayed
-candidates the base is the same as the relayed candidate (i.e., the
-transport address used by the TURN server to send from).
+candidates the base is the same as the relayed candidate.


I'd keep this clarification

Added back.

akeranen · 2017-07-12T14:25:22Z

draft-ietf-ice-rfc5245bis.xml

-different, then the foundation will be different. Two candidate pairs
-with the same foundation pairs are likely to have similar network
-characteristics.  Foundations are used in the frozen algorithm.
+different, then the foundation will be different.


Good clarification. But I would keep the last sentence "Foundations are used in the frozen algorithm."

I added it back, but to the first sentence.

akeranen · 2017-07-12T14:26:02Z

draft-ietf-ice-rfc5245bis.xml

 component is one whose transport address matches the default
 destination for that component.
 </t>

-<t hangText="Candidate Pair:"> A pairing containing a local candidate
+<t hangText="Candidate Pair:"> The pair of a local candidate


akeranen · 2017-07-12T15:36:18Z

draft-ietf-ice-rfc5245bis.xml

-<t>A CHECK LIST with at least one pair that is
-Waiting is called an active CHECK LIST, and a CHECK LIST with all
-pairs Frozen is called a frozen CHECK LIST.
+<t>Additionally, a check list with at least one pair in the waiting


s/waiting/Waiting/ (for consistency)

akeranen · 2017-07-12T15:36:48Z

draft-ietf-ice-rfc5245bis.xml

-pairs Frozen is called a frozen CHECK LIST.
+<t>Additionally, a check list with at least one pair in the waiting
+state is called "active", while a check list with all pairs in the
+frozen state is called "frozen".


s/frozen/Frozen/

At one point I asked whether we really need this check list property. Is it used for anything. Unless there is a usage, I would suggest discussing whether we can remove it.

I couldn't find it referenced anywhere when I searched the document for "active" and "frozen".

At IETF#99 it was decided to remove "active/frozen" check list, as it is not used anywhere.

We do need to scan the trickle specs, though, to make sure some usage has not been defined for trickle.

The trickle spec does talk about active and frozen check lists. For example:

A Trickle ICE agent initially considers all check lists to be frozen.
It then inspects the first check list and attempts to unfreeze all
candidate pairs it has received so far that belong to the first
component on the first media stream (i.e., the first media stream
that was reported to the ICE implementation from the using
application). If that first component of the first media stream does
not contain candidates for one or more of the currently known pair
foundations, and if candidate pairs already exist for that foundation
in one of the following components or media streams, then the agent
unfreezes the first of those candidate pairs.

And:

The ICE specification [rfc5245bis], Section 5.1.4, requires that an
agent will terminate the timer for a triggered check in relation to
an active check list once the agent has exhausted all frozen pairs in
the check list. This will not work with Trickle ICE, because more
pairs will be added to the check list incrementally.

And:

When a check list is set to Failed as described above, regular ICE
requires the agent to update all other check lists, placing one pair
from each check list into the Waiting state - effectively unfreezing
all remaining check lists. However, under Trickle ICE other check
lists might still be empty at this point (because candidates have not
yet been received), and following only the rules from regular ICE
would prevent the agent from unfreezing those check lists (because
the state of a check list depends on the state of the candidate pairs
in that check list, but there are none yet). Therefore a Trickle ICE
agent needs to monitor whether a check list is active or frozen
independently of the state of the candidate pairs in the check list
(since there might not be any pairs yet). With regard to empty check
lists, by default a Trickle ICE agent MAY consider an empty check
list to be either active or frozen. When a Trickle ICE agent
considers an empty check list to be frozen, during the candidate
checking process it SHOULD change the check list to active if
checking of another check list is completely finished (i.e., if every
pair in the other check list is either Successful or Failed), if
another check list has a valid candidate pair for all components, or
if it adds a candidate pair to the check list (because, in accordance
with Section 8.1.1, when inserting a new candidate pair into an empty
check list, the agent sets the pair to a state of Waiting).

I'll take this to the list.

akeranen · 2017-07-12T15:38:08Z

draft-ietf-ice-rfc5245bis.xml

 </section>

 <section title="Forming Candidate Pairs">

 <t>
-First, the agent takes each of its candidates for a media stream


I think original was better here

I re-added some of the text, but not exactly the original. Take another look.

for IETF 99 session

Sorry, I thought I re-added text here. I guess not. I just did so again. It's almost the same as the original, so I'm not sure it's worth discussing.

IETF99: One comment see below, otherwise OK

akeranen · 2017-07-12T15:41:32Z

draft-ietf-ice-rfc5245bis.xml

@@ -1886,59 +1692,49 @@ target="fig-state-fsm"/>.
   +-----------+         +-----------+
 ]]></artwork></figure>

-<t>


I'd keep this

akeranen · 2017-07-12T15:43:38Z

draft-ietf-ice-rfc5245bis.xml

 <t><list style="numbers">

-<t>The CHECK LISTs are placed in an ordered list (the order is determined
+<t>The check lists are placed in an ordered list (the order is determined
 by each ICE usage), called the CHECK LIST SET.


s/CHECK LIST SET/check list set/

And fix some typos

pthatcherg

I put back all the things Ari wanted, except for one (the "Component ID is Component ID" thing).

pthatcherg · 2017-07-12T22:48:54Z

draft-ietf-ice-rfc5245bis.xml

@@ -304,7 +304,7 @@ the TURN request will create a binding on each NAT, but only the
 outermost server reflexive candidate (the one nearest the TURN server)
 will be discovered by the agent. If the agent is not behind a NAT,
 then the base candidate will be the same as the server reflexive
-candidate and the server reflexive candidate is redundant and will be
+candidate and the server reflexive candidate is redundant will be


pthatcherg · 2017-07-12T22:49:34Z

draft-ietf-ice-rfc5245bis.xml

-obtained through Virtual Private Networks (VPNs) and Realm Specific IP
-(RSIP) <xref target="RFC3102"/> (which lives at the operating system
-level).
+specific port from an IP address on the host.


Added VPN back

pthatcherg · 2017-07-12T22:50:17Z

draft-ietf-ice-rfc5245bis.xml

 candidates the base is the same as the host candidate. For relayed
-candidates the base is the same as the relayed candidate (i.e., the
-transport address used by the TURN server to send from).
+candidates the base is the same as the relayed candidate.


Added back.

pthatcherg · 2017-07-12T22:51:21Z

draft-ietf-ice-rfc5245bis.xml

-different, then the foundation will be different. Two candidate pairs
-with the same foundation pairs are likely to have similar network
-characteristics.  Foundations are used in the frozen algorithm.
+different, then the foundation will be different.


I added it back, but to the first sentence.

pthatcherg · 2017-07-12T22:51:33Z

draft-ietf-ice-rfc5245bis.xml

 component is one whose transport address matches the default
 destination for that component.
 </t>

-<t hangText="Candidate Pair:"> A pairing containing a local candidate
+<t hangText="Candidate Pair:"> The pair of a local candidate


pthatcherg · 2017-07-12T23:15:17Z

draft-ietf-ice-rfc5245bis.xml

-<t>A CHECK LIST with at least one pair that is
-Waiting is called an active CHECK LIST, and a CHECK LIST with all
-pairs Frozen is called a frozen CHECK LIST.
+<t>Additionally, a check list with at least one pair in the waiting


pthatcherg · 2017-07-12T23:15:20Z

draft-ietf-ice-rfc5245bis.xml

-pairs Frozen is called a frozen CHECK LIST.
+<t>Additionally, a check list with at least one pair in the waiting
+state is called "active", while a check list with all pairs in the
+frozen state is called "frozen".


pthatcherg · 2017-07-12T23:19:26Z

draft-ietf-ice-rfc5245bis.xml

 </section>

 <section title="Forming Candidate Pairs">

 <t>
-First, the agent takes each of its candidates for a media stream


I re-added some of the text, but not exactly the original. Take another look.

pthatcherg · 2017-07-12T23:20:26Z

draft-ietf-ice-rfc5245bis.xml

 <t><list style="numbers">

-<t>The CHECK LISTs are placed in an ordered list (the order is determined
+<t>The check lists are placed in an ordered list (the order is determined
 by each ICE usage), called the CHECK LIST SET.


pthatcherg · 2017-07-12T23:20:28Z

draft-ietf-ice-rfc5245bis.xml

@@ -1886,59 +1692,49 @@ target="fig-state-fsm"/>.
   +-----------+         +-----------+
 ]]></artwork></figure>

-<t>


stpeter · 2017-07-17T15:18:12Z

draft-ietf-ice-rfc5245bis.xml

-the same type and MUST be different for candidates of different
-types. The type preference for peer reflexive candidates MUST be
-higher than that of server reflexive candidates. Note that candidates
+The type preference MUST be an integer from 0 (lowest prefereence) to


"prefereence" should be "preference"

stpeter · 2017-07-17T15:30:36Z

draft-ietf-ice-rfc5245bis.xml

-mechanism is needed.
+In certain usages of ICE, both agents may end up choosing the same
+role, resulting in a role conflict.  The section describes a mechanism
+for detetecing and repairing role conflicts.  The usage document


s/detetecing/detecting/

stpeter · 2017-07-17T15:34:58Z

draft-ietf-ice-rfc5245bis.xml

-</t>
-</list>
-</t>
+<t>If an agent want to change its implmentation level, it MAY restart


s/implmentation/implementation/

akeranen

Added flags for issues to be discussed in the IETF 99 session

akeranen · 2017-07-19T13:03:57Z

draft-ietf-ice-rfc5245bis.xml

-pair, and each of its candidates is called the selected candidate.
-Before a  pair has been selected, any valid candidate pair
-can be used for sending and receiving media (only one candidate pair
-at any given time).


for IETF 99 session
just "sending media" instead of "sending and receiving media"

My think is that any valid candidate pair can be used for receiving any time, not just before a pair has been selected. Why would you throw away media that you receive on a valid candidate pair just because it comes out of order with the candidate pair selection (or for any other reason)?

But it's true that's not just editorial. Sorry about that. I can make it a separate PR/issue/thread if you'd like.

IETF99 Christer & Peter: Needs to be checked rest of the text and how this aligns.

In the list, we decided to put this back to sending and receiving.

If we go for sending and receiving, there are other places that need to be aligned too. For example, in green line 567, should it say send and receive? Similarly, in the definition of Base (line 718), should it say send and receive?

I updated line 567 as such.

akeranen · 2017-07-19T13:10:39Z

draft-ietf-ice-rfc5245bis.xml

@@ -999,53 +964,23 @@ following exceptions:
 <section title="Server Reflexive and Relayed Candidates">

 <t>
-Agents SHOULD obtain relayed candidates and SHOULD obtain server
-reflexive candidates. These requirements are at SHOULD strength to


for IETF 99 session

akeranen · 2017-07-19T13:11:05Z

draft-ietf-ice-rfc5245bis.xml

 configured, and the DNS procedures in <xref target="RFC5389"/> (using
 SRV records with the "stun" service) be used to discover the STUN
 server, and the DNS procedures in <xref target="RFC5766"/> (using SRV
 records with the "turn" service) be used to discover the TURN server.
 </t>

-<t>This specification only considers usage of a single STUN or TURN


for IETF 99 session

akeranen · 2017-07-19T13:13:32Z

draft-ietf-ice-rfc5245bis.xml

-component ID is the component ID for the candidate, and MUST be
-between 1 and 256 inclusive.
-</t>
+<t>The component ID MUST be an integer between 1 and 256 inclusive.</t>


The first "component ID" is the attribute name whereas the second is the concept. Unfortunately they have the same spelling. But no strong opinion on this.

akeranen · 2017-07-19T13:13:53Z

draft-ietf-ice-rfc5245bis.xml



 </section>

 <section anchor="sec-guidelines" title="Guidelines for Choosing Type and Local Preferences">

-<t>One criterion for selection of the type and local preference values


for IETF 99 session

akeranen · 2017-07-19T13:46:32Z

draft-ietf-ice-rfc5245bis.xml

-if the state of the CHECK LIST for that media stream is Running, and
-there was a previous selected pair for that component due to an ICE
-restart
-</t>


for IETF 99 session
Remove details

Yeah, I guess you're right. I re-added it.

akeranen · 2017-07-19T13:46:45Z

draft-ietf-ice-rfc5245bis.xml

 Unless an agent is able to produce a selected pair for all components
 associated with a media stream, the agent MUST NOT continue
 sending media for any component associated with that media stream.
 </t>
-
+****


Yeah, sorry. I removed it.

akeranen · 2017-07-19T13:47:48Z

draft-ietf-ice-rfc5245bis.xml

-codecs, it is RECOMMENDED that the sender set the marker bit <xref
-target="RFC3550"/> when an agent switches transmission of media from
-one candidate pair to another.  </t>
-


for IETF 99 session
Removed media-specific details. Perhaps better in the ICE-SIP draft?

I don't think such RTP-specific things should be a part of the ICE spec. The ICE spec should be about how to make ICE interop, not about what to do with RTP when using ICE. But, sure let the WG decide at IETF 99.

And, yes, this is another non-editorial. Sorry about that. Do you want me to make a separate PR/issue/thread?

Perhaps this could be re-written but we have to have this advice in some RFC but unless we can point at this advice in other specs, I think we need to keep it here.

IETF99: Jonathan: shorter generic text
Cullen: very important. Need to be here or referenced here. Need to get this right.
Will keep this as such.

akeranen · 2017-07-19T13:48:04Z

draft-ietf-ice-rfc5245bis.xml

-RECOMMENDED that, when an agent receives an RTP packet with a new
-source or destination IP address for a particular media stream, that
-the agent re-adjust its jitter buffers.
-</t>


for IETF 99 session
Like above

IETF99: Same as above. will keep this.

akeranen · 2017-07-19T13:50:36Z

draft-ietf-ice-rfc5245bis.xml

@@ -4059,7 +3742,7 @@ role. The content of the attribute is a 64-bit unsigned integer in
 network byte order, which contains a random number. The number is used
 for solving role conflicts, when it is referred to as the tie-breaker
 value. An ICE agent MUST use the same number for all Binding requests,
-for all streams, within an ICE session. The ICE agent MAY change


for IETF 99 session

This is just fixing it to make it consistent with sec-recv-response, which says " The ICE agent MUST NOT change the tie-breaker value." I think this was just an oversight when making that change.

IETF99: This seems to conflict with the spec. Let's not change.

I changed it to "MAY change the tie-breaker" in all 3 places.

fandreas · 2017-07-19T14:02:37Z

draft-ietf-ice-rfc5245bis.xml

@@ -571,7 +571,8 @@ Internet and have a public IP address at which it can receive packets
 from any correspondent. To make it easier for these devices to support
 ICE, ICE defines a special type of implementation called LITE (in
 contrast to the normal FULL implementation). 
-Lite agents do not generate connectivity checks or run
+Lite agents only uses host candidates 
+and does not generate connectivity checks or run


s/only uses only/use only
s/does not/do not

I think I already fixed that.

akeranen · 2017-07-19T16:10:59Z

draft-ietf-ice-rfc5245bis.xml

+The agent pairs each local candidate with each remote candidate for
+the same component of the same media stream with the same IP
+address 
+


missing closing t tag

martinthomson

I don't have time to go through this in detail, but here are a couple of comments.

martinthomson · 2017-07-20T11:16:27Z

draft-ietf-ice-rfc5245bis.xml

-RTP/RTCP using contents of the packets, rather than the port on which
-they are received. Fortunately, this demultiplexing is easy to do,
-especially for RTP and RTCP.
+media using the contents of the packets, rather than the port on which


This misses data channels.

"media" here includes data channels. But I agree it could be more explicit. Should I throw in "SCTP" with the "e.g, RTP and RTCP" above? Then we would need to add a reference.

martinthomson · 2017-07-20T11:19:30Z

draft-ietf-ice-rfc5245bis.xml

-</t>
+<t>Note that the responding agent, when obtaining its candidates, will		
+typically know if the other agent supports RTP/RTCP multiplexing, in		
+which case it will not need to obtain a separate candidate for		


all this trailing whitespace!

fluffy · 2017-07-20T12:18:38Z

draft-ietf-ice-rfc5245bis.xml

-server are not authenticated, and any ALTERNATE-SERVER attribute in a
-response is ignored. Agents MUST support the backwards compatibility
-mode for the Binding request defined in <xref
-target="RFC5389"/>. Allocate requests SHOULD be authenticated using a


It seems still need auth text

Auth text re-added.

fluffy · 2017-07-20T12:19:37Z

draft-ietf-ice-rfc5245bis.xml

 configured, and the DNS procedures in <xref target="RFC5389"/> (using
 SRV records with the "stun" service) be used to discover the STUN
 server, and the DNS procedures in <xref target="RFC5766"/> (using SRV
 records with the "turn" service) be used to discover the TURN server.
 </t>

-<t>This specification only considers usage of a single STUN or TURN
-server. When there are multiple choices for that single STUN or TURN


clearly we have to change the first sentence that one can only have a single server

That agrees with the IETF 99 minutes.

fluffy · 2017-07-20T12:20:06Z

draft-ietf-ice-rfc5245bis.xml

-<t>This specification only considers usage of a single STUN or TURN
-server. When there are multiple choices for that single STUN or TURN
-server (when, for example, they are learned through DNS records and
-multiple results are returned), an agent SHOULD use a single STUN or


I think we still want to cover the text about multiple IPs from one DNS name

fluffy · 2017-07-20T12:28:24Z

draft-ietf-ice-rfc5245bis.xml

-of the maximum component ID provided by each agent across all
+The agent pairs each local candidate with each remote candidate for
+the same component of the same media stream with the same IP
+address.  It is	possible that some of the local candidates won't get paired with		


"IP Address" -> "IP Address Family" ?

fluffy · 2017-07-20T12:32:47Z

draft-ietf-ice-rfc5245bis.xml

-USE-CANDIDATE attribute in the Binding request, the valid pair
-generated from that check has its nominated flag value set to true. This
-flag indicates that this valid pair SHOULD be used for media, unless
-the sending agent detects that the candidate pair does not work.


Removing the "unless ... does not work" seems like a normative change.

fluffy · 2017-07-20T12:48:10Z

draft-ietf-ice-rfc5245bis.xml

+component, transport protocol, and IP address family.  For each
+component of each media stream, if there is only one candidate pair,
+that pair is added to the valid list.  If there is more than one pair,
+it is RECOMMENDED that an agent follow the procedures of RFC 6724


It seems that this 6724 might need more thought on how it is used. THis does seem like a non editorial change from old text.

No, it specified 6724 before, Peter just reorganized it.

fluffy · 2017-07-20T12:57:46Z

draft-ietf-ice-rfc5245bis.xml


 <t>
-Agents always send media using a candidate pair, using candidate pairs
-in the VALID LIST. Once a candidate pair has been selected only that


Hmm,do we need to keep the VALID LIST concept ?

Two paragraphs above, it says agents may send media on valid candidate pairs, so I believe this is redundant.

fluffy · 2017-07-20T13:00:43Z

draft-ietf-ice-rfc5245bis.xml

 </t>

-<t>An agent MUST restart ICE for a media stream if:</t>
+<t>If an agent wants to change the destinations of media streams, it
+MAY restart ICE to do so.</t>


should both the MAY actually be a MUST?

fluffy · 2017-07-20T13:14:15Z

draft-ietf-ice-rfc5245bis.xml

-codecs, it is RECOMMENDED that the sender set the marker bit <xref
-target="RFC3550"/> when an agent switches transmission of media from
-one candidate pair to another.  </t>
-


Perhaps this could be re-written but we have to have this advice in some RFC but unless we can point at this advice in other specs, I think we need to keep it here.

fluffy · 2017-07-20T13:17:51Z

draft-ietf-ice-rfc5245bis.xml

@@ -4059,7 +3762,7 @@ role. The content of the attribute is a 64-bit unsigned integer in
 network byte order, which contains a random number. The number is used
 for solving role conflicts, when it is referred to as the tie-breaker
 value. An ICE agent MUST use the same number for all Binding requests,
-for all streams, within an ICE session. The ICE agent MAY change
+for all streams, within an ICE session. The ICE agent MUST NOT change


It seems that if some people implement changing this, then a new thing that says MUST NOT is going to fail when talking to them

This seems like it will not work well with 5245 things. 5245 says "An ICE restart (Section 9.1) causes a new selection of roles and tie-breakers."

akeranen · 2017-07-20T11:57:36Z

draft-ietf-ice-rfc5245bis.xml

-pair, and each of its candidates is called the selected candidate.
-Before a  pair has been selected, any valid candidate pair
-can be used for sending and receiving media (only one candidate pair
-at any given time).


IETF99 Christer & Peter: Needs to be checked rest of the text and how this aligns.

akeranen · 2017-07-20T12:08:28Z

draft-ietf-ice-rfc5245bis.xml

@@ -999,53 +964,23 @@ following exceptions:
 <section title="Server Reflexive and Relayed Candidates">

 <t>
-Agents SHOULD obtain relayed candidates and SHOULD obtain server
-reflexive candidates. These requirements are at SHOULD strength to


IETF99: Naked SHOULDs questioned in IESG rev.

akeranen · 2017-07-20T12:12:07Z

draft-ietf-ice-rfc5245bis.xml

+<t>
+An agent MUST support the backwards compatibility mode for the Binding
+request defined in <xref target="RFC5389"/> and MUST ignore any
+ALTERNATE-SERVER attribute in a STUN Binding response.
 </t>


This was also OK (part of TURN spec)

akeranen · 2017-07-20T12:19:32Z

draft-ietf-ice-rfc5245bis.xml

-<t>This specification only considers usage of a single STUN or TURN
-server. When there are multiple choices for that single STUN or TURN
-server (when, for example, they are learned through DNS records and
-multiple results are returned), an agent SHOULD use a single STUN or


IETF99: needs clarification for TURN servers with multiple IP addresses. Should recommend advice to use single/limited amount of TURN servers with only ones that are working / with credentials. Only if different network characteristics. Use same set across check lists.
First sentence of the limitation is OK. For the rest need clarification.

OK, I added some text that says the agent SHOULD use at least one, but MAY use more than one. Is that good?

akeranen · 2017-07-20T12:20:39Z

draft-ietf-ice-rfc5245bis.xml



 </section>

 <section anchor="sec-guidelines" title="Guidelines for Choosing Type and Local Preferences">

-<t>One criterion for selection of the type and local preference values


IETF99: does DS fairness cover this? If yes, just ref.

akeranen · 2017-07-20T13:06:52Z

draft-ietf-ice-rfc5245bis.xml

-candidate pair used for sending media. In case of a relayed
-candidate, media is sent from the agent and forwarded through
-the base (located in the TURN server), using the procedures defined
-in <xref target="RFC5766"/>.


IETF99: Cullen will think more. Others think OK.

akeranen · 2017-07-20T13:08:07Z

draft-ietf-ice-rfc5245bis.xml

-if the state of the CHECK LIST for that media stream is Running, and
-there was a previous selected pair for that component due to an ICE
-restart
-</t>


akeranen · 2017-07-20T13:14:43Z

draft-ietf-ice-rfc5245bis.xml

-codecs, it is RECOMMENDED that the sender set the marker bit <xref
-target="RFC3550"/> when an agent switches transmission of media from
-one candidate pair to another.  </t>
-


IETF99: Jonathan: shorter generic text
Cullen: very important. Need to be here or referenced here. Need to get this right.
Will keep this as such.

akeranen · 2017-07-20T13:15:15Z

draft-ietf-ice-rfc5245bis.xml

-RECOMMENDED that, when an agent receives an RTP packet with a new
-source or destination IP address for a particular media stream, that
-the agent re-adjust its jitter buffers.
-</t>


IETF99: Same as above. will keep this.

akeranen · 2017-07-20T13:20:39Z

draft-ietf-ice-rfc5245bis.xml

@@ -4059,7 +3742,7 @@ role. The content of the attribute is a 64-bit unsigned integer in
 network byte order, which contains a random number. The number is used
 for solving role conflicts, when it is referred to as the tie-breaker
 value. An ICE agent MUST use the same number for all Binding requests,
-for all streams, within an ICE session. The ICE agent MAY change


IETF99: This seems to conflict with the spec. Let's not change.

JonathanLennox · 2017-07-20T15:37:07Z

draft-ietf-ice-rfc5245bis.xml

-stream as part of subsequent candidate exchange process.
+If the peer is a lite agent, the agent pairs local candidates with
+remote candidates that are for the same media stream and have the same
+component, transport protocol, and IP address family.  For each


Do we want to worry about NAT64 (RFC 7050) for Lite - Lite connections? (Probably not?)

Unless we have some easy-to-agree and ready-to-merge text on that, I think it's better to leave it out.

akeranen

One missing comment from IETF 99 session

pthatcherg

OK, I think I addressed everything from IETF 99.

The only things pending are:

Cullen had 2 things he wanted to look more closely at. I looked more closely, and am convinced they are OK.
The paragraph around candidate freeing in the face of call forking: I took at stab at making it better, but the new thing needs review. It's a complex thing to express.

pthatcherg · 2017-09-06T14:30:50Z

draft-ietf-ice-rfc5245bis.xml

-pair, and each of its candidates is called the selected candidate.
-Before a  pair has been selected, any valid candidate pair
-can be used for sending and receiving media (only one candidate pair
-at any given time).


In the list, we decided to put this back to sending and receiving.

pthatcherg · 2017-09-06T14:33:19Z

draft-ietf-ice-rfc5245bis.xml

-</t>
+<t>Note that the responding agent, when obtaining its candidates, will		
+typically know if the other agent supports RTP/RTCP multiplexing, in		
+which case it will not need to obtain a separate candidate for		


pthatcherg · 2017-09-06T14:35:31Z

draft-ietf-ice-rfc5245bis.xml

@@ -999,53 +964,23 @@ following exceptions:
 <section title="Server Reflexive and Relayed Candidates">

 <t>
-Agents SHOULD obtain relayed candidates and SHOULD obtain server
-reflexive candidates. These requirements are at SHOULD strength to


Added back, but briefer (per IETF 99 minutes), but in a shorter version (per IETF 99 minutes).

pthatcherg · 2017-09-06T14:37:24Z

draft-ietf-ice-rfc5245bis.xml

 configured, and the DNS procedures in <xref target="RFC5389"/> (using
 SRV records with the "stun" service) be used to discover the STUN
 server, and the DNS procedures in <xref target="RFC5766"/> (using SRV
 records with the "turn" service) be used to discover the TURN server.
 </t>

-<t>This specification only considers usage of a single STUN or TURN


Note: in the IETF 99 session, we agreed this should be removed. The minutes also say " Try to come up with better text how to handle multiple servers."

pthatcherg · 2017-09-06T14:37:33Z

draft-ietf-ice-rfc5245bis.xml

 configured, and the DNS procedures in <xref target="RFC5389"/> (using
 SRV records with the "stun" service) be used to discover the STUN
 server, and the DNS procedures in <xref target="RFC5766"/> (using SRV
 records with the "turn" service) be used to discover the TURN server.
 </t>

-<t>This specification only considers usage of a single STUN or TURN
-server. When there are multiple choices for that single STUN or TURN


That agrees with the IETF 99 minutes.

pthatcherg · 2017-09-06T23:43:08Z

draft-ietf-ice-rfc5245bis.xml

 </t>

-<t>An agent MUST restart ICE for a media stream if:</t>
+<t>If an agent wants to change the destinations of media streams, it
+MAY restart ICE to do so.</t>


pthatcherg · 2017-09-06T23:46:32Z

draft-ietf-ice-rfc5245bis.xml

+the agents to be flushed.  The only difference between an ICE restart
+and a brand new media session is that during the restart media can
+continue to be sent, and that a new media session always requires the
+roles to be determined.
 </t>


I went with "using existing media sessions". The text already refers to "new media session", so "existing media sessions" is clear, I believe.

pthatcherg · 2017-09-06T23:58:57Z

draft-ietf-ice-rfc5245bis.xml

-flag indicates that this valid pair SHOULD be used for media, unless
-the sending agent detects that the candidate pair does not work.
-This concludes the ICE processing for this media stream or all
-media streams; see <xref target="sec-conclude"/>.


The behavior of the nominated flag is defined in "sec-up-fav", and so this was redundant. If we want behavior around what the nominated flag means, it should be in that section, not this one.

Actually, that section just ends up point to sec-choose-favor. So I added the line (if the controlled agent detects that the
candidate pair does not work, it SHOULD NOT select the candidate pair).

pthatcherg · 2017-09-07T00:00:34Z

draft-ietf-ice-rfc5245bis.xml

-same role. This section describes procedures for checking for this case
-and repairing it. These procedures apply only to usages of ICE that
-require conflict resolution. The usage document MUST specify whether this
-mechanism is needed.


pthatcherg · 2017-09-07T00:22:56Z

draft-ietf-ice-rfc5245bis.xml

-can quickly change after ICE has completed.
+A full agent SHOULD NOT stop sending checks and responses from a
+candidate until three seconds after all media streams using that
+candidate are Completed, after which the agent MAY free the candidate


The Completed state for a check list is well defined, so I updated the text to make it more clear if references that.

I reworded the whole thing to be as explicit as I could make it. Hopefully I got it right and it is more clear now.

taylor-b · 2017-09-12T19:47:29Z

draft-ietf-ice-rfc5245bis.xml

+mechanism that allows dual-stack hosts to prefer connectivity over
+IPv6, but to fall back to IPv4 in case the v6 networks are
+disconnected. Implementation should follow the guidelines from <xref
+target= "I-D.ietf-ice-dualstack-fairness"/> to avoid excessively delays


s/excessively/excessive

cdh4u · 2017-09-29T09:23:22Z

draft-ietf-ice-rfc5245bis.xml

+pair selected by an ICE agent for sending media.
+Each of its candidates is called the selected candidate.
+Before a pair has been selected, any valid candidate pair
+can be used for sending and receiving media.
 </t>



If I understand the difference between "nominated pair" and "selected pair", the selected pair is a nominated pair that is accepted by the controlled node? If so, I think it should be more clear.

I agree that we should make it clear that the controlling agent nominates and then selects (and sends on the nominate but receives on any in the time in between) and the controlled agent selects.

cdh4u · 2017-09-29T09:24:31Z

draft-ietf-ice-rfc5245bis.xml

-<t hangText="Controlled Agent:"> An ICE agent that waits for the
-controlling agent to select the final choice of candidate pairs.
+<t hangText="Controlled Agent:"> The ICE agent that waits for the 
+controlling agent to nominate a candidate pair.
 </t>



Should we say that the controlled agent is responsible for selecting a candidate nominated by the controlling agent?

I added text that says so.

cdh4u · 2017-09-29T09:25:14Z

draft-ietf-ice-rfc5245bis.xml

-a whole to work. For media streams based on RTP, unless RTP and RTCP
-are multiplexed in the same port, there are two components per media
-stream -- one for RTP, and one for RTCP. </t>
+a whole to work.</t>



What is the reason for removing the RTP/RTCP clarification?

I added it back.

cdh4u · 2017-09-29T09:26:19Z

draft-ietf-ice-rfc5245bis.xml

-flag set to tell the peer that this pair has been nominated for use.
+found and then selects a candidate pair from the valid candidate pairs
+and sends a STUN request on the selected pair with a flag set to indicate
+to the controlled peer that it has nominated the selected pair.
 This is shown in <xref target="fig-regular-select"/>.
 </t>



I am not sure using "select" terminology is good here, because it confuses with the "selected pair" concept. I would like to "picks", or use some other terminology (e.g., "chooses").

I changed to "chooses".

pthatcherg and others added 12 commits May 17, 2017 08:45

Checkpoint with review comments

fb4eeec

More editorial changes

62b9ffc

Lots of removals of duplicate or unneeded text.

More editorial updates

5d7df63

Simplify text around valid list

eed26b7

Undo some over-aggressive changes

912ccf8

Cleanup some changes I made

2527720

More fixes

776b04b

More clean ups

2bec4f2

Update lite-lite

e71cef3

More changes

e518844

Remove a lot of the more complex changes

1cb559f

Don't change: - sending checks, timing, triggered checks, etc - handling responses and how to determine success or failure or errors - the valid lists and how they work - check list states and how they change

Merge branch 'master' into peter-review-easy

b610377

akeranen requested changes Jul 12, 2017

View reviewed changes

pthatcherg changed the title ~~Make a sub set of https://github.com/ice-wg/rfc5245bis/pull/35 that is easy to merge~~ Make a large number of small editorial changes (a sub set of PR 35 that is easier to merge) Jul 12, 2017

Add back the things Ari said to add back

2fa7547

And fix some typos

pthatcherg commented Jul 12, 2017

View reviewed changes

stpeter reviewed Jul 17, 2017

View reviewed changes

Fix spelling issues

22c9285

akeranen reviewed Jul 19, 2017

View reviewed changes

fandreas reviewed Jul 19, 2017

View reviewed changes

akeranen force-pushed the master branch 2 times, most recently from c902152 to 2d5a3e0 Compare July 19, 2017 15:15

akeranen reviewed Jul 19, 2017

View reviewed changes

Fix things from Ari's review

aaeba56

martinthomson reviewed Jul 20, 2017

View reviewed changes

fluffy reviewed Jul 20, 2017

View reviewed changes

fluffy suggested changes Jul 20, 2017

View reviewed changes

akeranen reviewed Jul 20, 2017

View reviewed changes

JonathanLennox reviewed Jul 20, 2017

View reviewed changes

akeranen reviewed Jul 20, 2017

View reviewed changes

pthatcherg added 4 commits September 6, 2017 08:24

Address several comments from IETF 99 meeting minutes.

921d1bf

Fix some things messed up while rebasing.

fe15d99

Fix the rest of the comments from IETF 99.

ce1e46d

Fix build

3277f10

pthatcherg commented Sep 7, 2017

View reviewed changes

Further tweak when local candidates can be freed

e1852c2

taylor-b reviewed Sep 12, 2017

View reviewed changes

pthatcherg and others added 4 commits September 27, 2017 08:06

Fix a typo

5d92d9e

Merge with master.

2b92781

Put back more detailed candidate attributes.

a6214f1

Re-add some deleted text requested during review.

6e6247a

cdh4u reviewed Sep 29, 2017

View reviewed changes

akeranen approved these changes Sep 29, 2017

View reviewed changes

pthatcherg added 2 commits September 29, 2017 08:03

Resolve comments from PR review

fff96a6

Merge

53fcc8d

akeranen merged commit e738297 into master Sep 29, 2017

Make a large number of small editorial changes (a sub set of PR 35 that is easier to merge) #38

Make a large number of small editorial changes (a sub set of PR 35 that is easier to merge) #38

Conversation

pthatcherg commented Jul 12, 2017

akeranen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pthatcherg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akeranen left a comment

Choose a reason for hiding this comment

akeranen Jul 19, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akeranen Jul 19, 2017 •

edited