Re: Genart last call review of draft-ietf-hip-native-nat-traversal-27

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Roni,

thanks for the detailed review! My comments are below.

On 02/26/2018 03:21 PM, Roni Even wrote:
Reviewer: Roni Even
Review result: Almost Ready

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-hip-native-nat-traversal-??
Reviewer: Roni Even
Review Date: 2018-02-26
IETF LC End Date: 2018-02-26
IESG Telechat date: Not scheduled for a telechat

Summary:
The document is almost ready for publication as a standard track RFC

Major issues:

Minor issues:

1. in section 4.2 "Gathering of candidates MAY also be performed by other means
than described in this section.  For example, the candidates could be
    gathered as specified in Section 4.2 of [RFC5770] if STUN servers are
    available, or if the host has just a single interface and no STUN orData
    Relay Server are available." I did not see this a different ways since
    section 3 says "The hosts use either Control Relay Servers or Data Relay
    Servers (or other infrastructure including STUN or TURN servers) for
    gathering the candidates." so STUN is mentioned also here.

I suggest to remove the remark in parenthesis (or other infrastructure including STUN or TURN servers). Does this solve the issue?

2. In section 4.6.2 "The connectivity check messages MUST be paced by the Ta
value negotiated during the base exchange as described in Section 4.4.  If
neither one of the hosts announced a minimum pacing value, a value of  20 ms
SHOULD be used." in section 4.4 the default value is 50 ms?

Good catch! I double checked this from the ICE spec, which defaults also to 50 ms. So, I change the value to 50 ms also in section 4.6.2.

3. in section 5.4 what about "ICE-STUN-UDP         2" ;  I assume it is not
relevant but this is also the IANA registeration

I think it makes sense to add the missing one as you suggest, but omit it from the IANA registration since it is already registered for RFC5770.

4. In section 5.5 "The TRANSACTION_PACING is a new parameter" it is not new it
is in RFC5770

You're right, I'll change this.

5. In section 5.10 "SERVER_REFLEXIVE_CANDIDATE_ALLOCATION_FAILED  63" is the
only new one. this also relates to section 7 that says that all error values in
section 5.10 are new while the rest are in RFC5770. Also there is no mention in
section 7 of which registry is used for the error values.

Good catch, I'll correct these and add the IANA registry.

Nits/editorial comments:
1. Expand SPI and LSI when first appear in the document

2. in section 2 "the base of an candidate" should be "a candidate"

3. In section 3 "so it is the Initiator may also have registered to a Control
and/or Data Relay Server" maybe "so  the Initiator may also need to register to
a Control and/or Data Relay Server"

4. In section 4.2 "However, it is RECOMMENDED that a Data Relay Client
registers a new server reflexive candidate for each its peer for the reasons
described" maybe "for each of its..."

Thanks for spotting these, will fix as suggested.

5. In section 4.2 I could not parse the sentence "where Ta is the value used
for Ta is the value used for the"

Should be "where Ta is the value used for the"...

6. in section 4.6 "as defined in section in 6.7 in [RFC7401]:"  change to "as
defined in section 6.7 in [RFC7401]:"

Will fix this too.

Should I post a new version with the suggested changes?




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux