Re: [Gen-art] Genart last call review of draft-ietf-hip-native-nat-traversal-27

Hi Roni,

sorry, I read your email a bit too late today. I was too trigger happy 
and posted a new version... I thought it would be good to avoid blocking 
IANA with some missing and incorrect details.

On 03/04/2018 09:22 AM, Roni Even (A) wrote:
> Hi Miika,
>   All your responses are OK with me.
> 
> As for posting a new version, I think it will be good to submit one with all the changes that came in the IETF LC
> 
> Roni
> 
> -----Original Message-----
> From: Gen-art [mailto:gen-art-bounces@ietf.org] On Behalf Of Miika Komu
> Sent: Thursday, March 01, 2018 4:13 PM
> To: Roni Even; gen-art@ietf.org
> Cc: hipsec@ietf.org; ietf@ietf.org; draft-ietf-hip-native-nat-traversal.all@ietf.org
> Subject: Re: [Gen-art] Genart last call review of draft-ietf-hip-native-nat-traversal-27
> 
> 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?
> 
> [Roni] Yes
> 
>> 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.
> [Roni] OK
> 
>> 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.
> [Roni] OK
> 
>> 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.
> [Roni]OK
> 
>> 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.
> 
> [Roni]OK
> 
>> 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?
> 
> 
> _______________________________________________
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art
> 