Subject: Re: [Ice] Mirja Kühlewind's No Objection on draft-ietf-ice-rfc5245bis-17: (with COMMENT) - Pull Request

From: Christer Holmberg <[email-masked]>
Date: Sat, 17 Feb 2018 09:35:22 +0000

Hi,

I have created a pull request for the changes based on the IESG reviews:

https://github.com/ice-wg/rfc5245bis/pull/59

Regards,

Christer

-----Original Message-----
From: Christer Holmberg [mailto:[email-masked]] 
Sent: 17 February 2018 10:37
To: Mirja Kühlewind <[email-masked]>; The IESG <[email-masked]>
Cc: [email-masked]; Peter Thatcher <[email-masked]>; [email-masked]; [email-masked]; [email-masked]
Subject: RE: Mirja Kühlewind's No Objection on draft-ietf-ice-rfc5245bis-17: (with COMMENT)

Hi Mirja,

Thank You for your review and comments! Please see inline.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Important nits; please address:

>1) Sec 14.2: RFC 6928, RFC 5389, and RFC 6298 should be actual references and listed in the reference section.

Will be fixed as suggested.

---

> 2) Sec 14.2: Maybe you should really not use normative language in the reasoning part of this section?!
>    I find especially this SHOULD in the following sentence confusing (as it
>    doesn't give a reference to another RFC that defines this):
>   "Let MinPacing be the minimum pacing interval between
>          transactions, which SHOULD be 5ms."
>   Given that the previous text says:
>  "all transactions from all agents [...] MUST NOT be sent more often than once
>   every 5ms"
>
>  My recommendation: please use lower cases "shoulds" and "musts" in 
> the  reasoning part!

I think it would be good to keep upper cases in most instances, but I do agree that the usage of SHOULD in the MinPacing sentence is confusing, so what about the following change:

OLD:

"Let MinPacing be the minimum pacing interval between transactions, which SHOULD be 5ms."

NEW:

"Let MinPacing be the minimum pacing interval between transactions, which is 5 ms (see above)."

---

> 3) Sec 19.4.1: "(say, Ta=20ms)"
> Maybe use also a value of 50ms here to avoid confusion.

Will be fixed as suggested.

Regards,

Christer