On Tue, Apr 3, 2018 at 4:14 AM, Pascal Thubert (pthubert)
<[email protected]> wrote:
> Hello Warren:
>
> Many thanks for your great review : ) Let's see below:
>

Awesome, thanks.


>> ----------------------------------------------------------------------
>> COMMENT:
>> ----------------------------------------------------------------------
>>
>> Thank you for a very readable document - this is not a simple process, but 
>> the
>> document was clear and understandable. Also, thank you for addressing Jürgen
>> Schönwälder's OpsDir comments (I have not personally checked that each was
>> incorporated, but the authors said that they would, and I'm trusting them).
>>
>> Issues:
>> "In IPv6 ND [RFC4861], a router needs enough storage to hold NCEs for all the
>> addresses to which it can currently forward packets." -- I do not believe 
>> that
>> this is correct and am not quite sure how to fix it. Can you point where in
>> RFC4861 it says this? I'd agree that *to be efficient* a router should have
>> enough space to hold an NCE for all locally attached subnets, but a 
>> constrained
>> device *could* presumably age out and relearn old entries. Apart from that
>> somewhat pathological case, if I'm a router and get a transit packet (not 
>> for a
>> directly connected interface), I don't need to have a neighbor entry for it
>> (otherwise "core" routers would need to build NCEs for every IP on the 
>> Internet
>> :-P). Perhaps "for all directly connected subnets." or "for all directly
>> connected addresses to which it can currently forward packets."?
> [PT>]
> [PT>] Oups, yes, "directly connected" was meant.

Great.

> I wanted to point out that NCEs that are not in use can be flushed, so I also 
> meant to limit the need of memory to IP addresses that are currently in 
> active conversations via this router.
> If there are more conversations than NCEs then caching/flushing becomes 
> ineffective. What about:
>  "for all directly connected addresses to which it is currently forwarding 
> packets , but entries that do not appear to in use may be flushed."

Sounds good, or "for all directly connected addresses to which it is
currently forwarding packets (entries that do not appear to in use may
be flushed)."
I'm fine with either.

> I suggested while RFC 6775 was elaborated that keeping the term "cache" was 
> probably improper for the registration mechanism. I think we kept it because 
> of the high similarity in the content and use.

Yup, fair enough, it is the common term.

>
>>
>> I'm somewhat uncomfortable with the uppercase MUST in:
>> "A network administrator MUST deploy updated 6LR/6LBRs to support the
>> number
>> and type of devices in their network, ..." Having an uppercase MUST driving
>> operators' design decisions stuff feels weird. I'd be perfectly fine with
>> something like "In order to deploy this, network administrators MUST..." or
>> "Network Administrators need to ensure that 6LR/6LBRs support the number
>> and..."
>>
> [PT>] This MUST comes from rev-14, based on Dave Thaler's review 
> (https://www.microsoft.com/en-us/research/uploads/prod/2017/05/draft-ietf-6lo-rfc6775-update-11.pdf
>  )
> I'm not too keen on rolling back the uppercase, cc'ing Dave to validate.
> What about:
> "
>     In order to deploy this, network administrators MUST ensure that 6LR/6LBRs
>     in their network support the number and type of devices that can register 
> to them,
>     based on the  number of IPv6 addresses that those devices require and 
> their
>     address  renewal rate and behavior.
> "

Works for me.

>
>> Nits:
>> Section 4.2.1.  Comparing TID values
>> "In order to keep this document self-contained and yet compatible, the text
>> below is an exact copy from section 7.2.  "Sequence Counter Operation" of
>> [RFC6550]."" I think that it would be helpful to delimit the copied text
>> (perhaps by indenting it) -- it was unclear to me where the copied text 
>> started
>> and ended, and so I had to go read RFC6550 (which kind of defeats the purpose
>> of copying it).
> [PT>]  This text quoted above was discussed at the meeting. The group wanted 
> to detach from RFC 6550 and not give an impression that this spec would 
> inherit RPL's evolution should the referred behavior change. The text was 
> replaced by:
> "
>         As a note to the implementer, the operation of the TID is fully
>         compatible with that of the RPL Path Sequence counter as described in
>         the "Sequence Counter Operation" section of the "IPv6 Routing Protocol
>          for Low-Power and Lossy Networks [RFC6550] specification.
> "
>

Sorry, I guess I was unclear in my note -- I was suggesting something like:
"In order to keep this document self-contained and yet compatible, the
text below (to the end of the section) is an exact copy from section
7.2."
or
"In order to keep this document self-contained and yet compatible, the
text below (from <BEGIN> to <END> is an exact copy from section 7.2."
or something.
But, this was just a suggestion - I'm fine with the original too.


>>
>> Section 4.3.  Registration Ownership Verifier
>> "An RFC6775-only will confuse the name-spaces,"
>> Missing a word - perhaps "device", "6LoWPAN Router" or "implementation"?
> [PT>] What about "An RFC6775-only 6LR or 6LBR"

WFM.

>
>>
>> Section 7.3.  RFC6775-only 6LoWPAN Router
>> "But if RFC6775-only and updated 6LRs
>>    coexist temporarily in a network, then it makes sense for an
>>    administrator to install a policy that allows so, and the capability
>>    to install such a policy should be configurable in a 6LBR though it
>>    is out of scope for this document."
>> s/that allows so/that allows this/ -- readability (purely a nit).
> [PT>] done : )
>

Win!

>>
>> Section Appendix B.  Requirements (Not Normative)
>> "This section lists requirements that were discussed discussed by the 6lo 
>> WG..."
>> I guess that they were discussed at length? :-P
>>
> [PT>] to death as 
> https://tools.ietf.org/html/draft-thubert-6lo-rfc6775-update-reqs-07 but for 
> the section I added based on Juergen's comments for manageability 
> "Requirements Related to Operations and Management". This one came in late 
> and I need your and Juergen's comments on it.
>

Sorry, I was overly terse -- I was just referring to the "discussed discussed".

>
>> Section B.1.  Requirements Related to Mobility
>> " Due to the unstable nature of LLN links, even in an LLN of immobile nodes a
>> 6LN may change" s/of immobile nodes a/of immobile nodes, a/ (add comma --
>> also
>> a nitty nit).
>>
>
> [PT>] done! Please let me know if you are OK with the changes above and I'll 
> publish rev-17

Yup, I'm a happy camper. Thank you very much for such a quick turnaround.
W

>
> Take care,
>
> Pascal
>



-- 
I don't think the execution is relevant when it was obviously a bad
idea in the first place.
This is like putting rabid weasels in your pants, and later expressing
regret at having chosen those particular rabid weasels and that pair
of pants.
   ---maf

_______________________________________________
6lo mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/6lo

Reply via email to