Re: [Gen-art] Genart last call review of draft-ietf-ice-rfc5245bis-16

[Christer Holmberg <[e-mail]>]

Hi Stewart,

Thank You for the review! Please see inline.

> Summary: This is a well written document and I am sure it will serve its
>target audience 
> well. However Genart reviews take the perspective of someone new to the
>field, and 
> although I am sure it is probably correct and complete when taken
>together with its
> references the learning curve is perhaps a little steeper than it needs
>to be due to the 
> extent of assumed knowledge. In the nits section of this review I make a
>few simple 
> suggestions that I think would make it easier for the new reader.

Thanks! :)

...

Nits/editorial comments:

>   "in the XOR-RELAYED-ADDRESS attribute. "
>
>SB> As far as I can see this not yet been defined or a reference
>SB> provided in the document.

I will add a reference to RFC 5766.

---

   The table in Figure 8 illustrates an example.

SB> There is something wierd going on here.
SB> Figure 8 seems malformed possibly spread over a page break.

I¹ll check that.

---

> SB> You introduce Ta, but it would be so much kinder to the reader to
> SB> give it a real name.

Ta was defined in RFC 5245, and it's commonly used in ICE-related
discussions, so I think it would cause confusion to change the name at
this point.

---

> SB> DSCP is not well known so needs to defined

I will expand to "DiffServe Codepoint", and add a reference to RFC 2474,
on first occurrence.

---

SB> You introduce FINGERPRINT without a pointer to where it is defined

I will add a reference to RFC 5389 on first occurrence.

---

SB> The 487 error comes out of a hat without a pointer to where it is
SB> defined

It's defined in section 16.2. I can add a reference to that section on
first occurrence.

---

>SB> ICE-CONTROLLED comes out of the same hat without a
>SB> pointer/definition,
>same
>with PRIORITY, MESSAGE-INTEGRITY, ALTERNATE-SERVER, XOR_MAPPED_ADDRESS,
>USE->CANDIDATE, CHECK-LIST

Some are defined in section 16.1. I will a reference to that section on
first occurrence.

Some are defined in RFC 5389 and RFC 5766. I will add references on first
occurrence.

----

>Section 7.3.1.4, the agent sets the nominated flag of the pair to
>SB> should that be nominated or NOMINATED?

"nominated"

---

>In section 8.3.1 it says: " The procedures in Section 8" which is true
>but strangely self referencing

The text actually references the text later in the section :) So, I
suggest to remove the first sentence:

"The procedures in Section 8 require that an ICE agent continue to
   listen for STUN requests and continue to generate triggered checks
   for a data stream, even once processing for that stream completes."


---

>7.3.1.4.  Triggered Checks
>
>   Next, the agent constructs a pair....
>
>SB> Next after what? and a pair of what?

"candidate pair"

I will fix it.

---

>You say "Let HTO" again a user friendly name would be helpful to the new
>reader

The name was provided by transport people that provided text. As it¹s
similar to RTO, I¹d like to keep it.

---

>Appendix B is great, particularly from section B5 onwards. It would be
>great to forward reference this to help the reader understand the
>normative text earlier in the document.

Any particular place where you would like to have the reference? In the
Introduction?

---

Regards,

Christer