Re: [RTG-DIR] Rtg Dir review draft-ietf-6lo-rfc6775-update-13.txt

["Pascal Thubert (pthubert)" <[e-mail]>]

Hello Adrian:

Many, many thanks for this incredible review : )

I'm attaching the tentative new draft 15. I'll publish by cutoff as a starting point and incorporate what we can based on your responses on the below till then.
Please erase in your response everything for which an agreement is already found.

Let's see below:
> 
> Comments:
> 
> The document is quite hard work. More cross-references would help, and
> bringing the abbreviations to the top would also make things easier. But the
> main problem was the trickle feed of how everything hangs together - it's all
> there, but you have to read the entire document to get the whole picture.
> 
> ==Major==
> 
> 4.3
> 
>    With this specification, the Registration Unique ID is allowed to be
>    extended to different types of identifier, as long as the type is
>    clearly indicated.  For instance, the type can be a cryptographic
>    string and used to prove the ownership of the registration as
>    discussed in "Address Protected Neighbor Discovery for Low-power and
>    Lossy Networks" [I-D.ietf-6lo-ap-nd].  In order to support the flows
>    related to the proof of ownership, this specification introduces new
>    status codes "Validation Requested" and "Validation Failed" in the
>    EARO.
> 
> This seems fine, but I don't see how "the type is clearly indicated".
> 
[PT>] changed to  

    With this specification, the ROVR is allowed to be of different types, as
    long as the type is signaled in the message that carries the new type.

Note: The new drafts such as AP-ND will have to do that in a fashion that can be backward compatible. This and following comments seem to indicate that the reader will believe that the "RUID" is used beyond the scope of a registration, to index the registration or its owner. 
This is not so. So I changed the term ROVR which seems to imply "Index" to "Registration Ownership Verifier". ROVR is now defined as follows:

Registration Ownership Verifier (ROVR):
       Enables to correlate multiple registrations for a same IPv6 Address. 
        This can be a unique ID of the Registering Node, such as the EUI-64 
        address of an interface. This can also be a token obtained 
        with cryptographic methods and used as proof of ownership of the 
        registration. The scope of a ROVR is one registration and it cannot be
        used to correlate different registrations.




In more details for your question above: that AP-ND introduces a new flag in an MBZ field to indicate its type. The length of the ID is the same as EUI-64 so an RFC6775-only implementation will miss that the type is different. Collisions will be extremely rare and will not lead to problematic situation since the ID is only used as a match to recognize that the node that updates a registration is the node that made the registration. It is NOT as a key for looking up that node. 2 nodes could use the same ROVR without a damaging effect, unless they try to also form the same address. Which make the situation even more exceedingly rare since AP-ND does not use the crypto ID as IID for forming addresses. So it would be a double collision of 2 independent 64bits identifiers. It makes more sense to me to worry whether 64bits is enough robustness for AP ND and define how to work backward compatibility if the ROVR can be more than 64bits. I'll take that Q to the WG in London.


> I think you also have to restate the uniqueness assumption in this new
> context. Probably that uniqueness is required across {type, value} (unless, of
> course, your answer to my first question is that type is embedded in value).
> This possibly cuts into backward compatibility as well?

[PT>] Added the text below:

Note on ROVR collision: different techniques for forming the
    ROVR will operate in different name-spaces. < RFC6775> operates on EUI-64 addresses. <xref target="I-D.ietf-6lo-ap-nd"/> generates
    cryptographic tokens. While collisions are not expected in the EUI-64
    name-space only, they may still happen in the case of <draft-ietf-6lo-ap-nd> 
    and in a mixed situation.
    An implementation that understands the name-space 
    MUST consider that ROVRs from different name-spaces are different even if 
    they have the same value. An RFC6775-only will confuse the name-spaces, which
    slightly increases the risk of an ROVR collision. A collision of ROVR has no
    effect if the two Registering Nodes register different addresses, since the 
    ROVR is only significant within the context of one registration. An ROVR is
    not expected to be unique to one registration, and this specification allows
    that a node use the same ROVR to register multiple IPv6 addresses.
    This is why the ROVR MUST NOT be used as a key to identify the Registering
    Node, or as an index to the registration. It is only used as a match to
    recognize that the node that updates a registration for an IPv6 address is
    the node that made the original registration for that IPv6 address.
    This is done to protect the ownership of the address.  
   Also, when the ROVR is not an EUI-64 address, then it SHOULD NOT be used as
    the interface ID of the Registered Address. This way, a registration that
    uses that ROVR will not collision with that of an IPv6 Address derived from
    EUI-64 and using the EUI-64 as ROVR per <xref target="RFC6775"/>.    

> See also 6.1/6.2
> 
[PT>] 
Changed the definition of ROVR as mentioned above
 

> Furthermore, 7.1.2 says...
> 
>    A node that supports this specification MUST always use an EARO as a
>    replacement to an ARO in its registration to a router.  This is
>    harmless since the "T" flag and TID field are reserved in [RFC6775],
>    and are ignored by a RFC6775-only router.  A router that supports
>    this specification answers an ARO with an ARO and answers an EARO
>    with an EARO.
> 
> ...but this doesn't mention the "variation" of the ROVR that might also arise
> with the EARO. How will the RFC6775-only router handle these new ROVRs
> and their "clearly indicated" types?

[PT>] added the last sentence in

       A router that supports this specification answers an
       ARO with an ARO and answers an EARO with an EARO. A router that
        does not will consider the ROVR as an EUI-64 and treat it the same,
        which has no consequence if the Registered Addresses are different.

You'll note that at least one of the addresses are formed in a fashion that is not related to the ROVR, the chances that both the IID and the ROVR collision are so low that we can safely ignore them.

> 
> ---
> 
> 7.1.2
> 
>    One alternate way for a 6LN to discover the router's capabilities is
>    to start by registering...
> 
> You went to a lot of trouble to define the E-flag. You then made the use of the
> 6CIO (and hence the E-flag) only a SHOULD, and you defined an alternate
> mechanism. (Note: you say "one alternate" implying there are
> more!)
> 
> Choice is not good. It complicates the specification and the implementation.
> Why go there? Can't you settle on one mechanism and make it necessary and
> sufficient?
[
[PT>] We chatted on the ML. Seems I can drop the alternate method altogether and MUST the 6CIO in RS/RA. The 6CIO section now includes:

    The 6CIO MUST be present in both Router Solicitation (RS) and Router 
    Advertisement (RA) messages, unless the capabilities of this node are
    already known by the peer. This can be determined by a 6LR if there is
    an existing registration in place for the 6LN that is based on EARO. This
    can also be implicit, or configured in all nodes in a network.

Note that discovering the capabilities in RS RA introduces a capability that is much needed for AP ND, that is a longer ROVR, which makes the security of the crypto token stronger.
I'll propose text that allows for that change and discuss with the WG.

> 
> ---
> 
> You create new registries in 10.1 and 10.2. You must tell the IANA what
> allocation policies to use
> 
> 
[PT>] 10.1 has 
    'The policy is "IETF Review" or "IESG Approval" [RFC8126].'

10.2 does not create a registry, not sure I need to say anything?

> ==Minor==
> 
> You have a number of cases of "SHOULD" and "RECOMMENDED" without
> corresponding "MAY" clauses to describe variations. I think that I could
> deduce the reasons (implementation decided to not do it) and the results
> (function is not as rich), but you really should spell these things out.
> 
> ---
[PT>] Did some cleaning, please review diffs in -15

> 
> Would you consider folding Appendix C into the top of Section 2 so that it is
> possible to find the expansion of abbreviations more easily?
> 
[PT>] Done

> ---
> 
> RFC 6775 updates RFC 4944. Does this document also update RFC 4944?
[PT>] It only updates what RFC 6775 introduced. So I'd say now, unless we need to list all ancestry?


> 
> ---
> 
> I missed a discussion of Manageability and especially diagnosing faults.
> It's not mandatory, but it would have been good. (There is a trickle of info
> about policy configuration in 7.3).
> 
[PT>] It is in appendix, and happened as the result of Juergen's review.

> ---
> 
> The 2119 boilerplate should be updated to the most recent, viz.
> 
>    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>    "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
> "MAY", and
>    "OPTIONAL" in this document are to be interpreted as described in BCP
>    14 [RFC2119] [RFC8174] when, and only when, they appear in all
>    capitals, as shown here.
> 
> ...which will require you add 8174 as a reference
[PT>] done, thanks!

> 
> ---
> 
> Sect 2 expects the reader to be familiar with terms and concepts in a long list
> of other documents. Doesn't that make them normative refs?
> 
> [RFC7102]
> [RFC7228]
> [RFC4861]
> [RFC4862]
> [RFC6606]
> [RFC4919]
> [RFC6775]
> [I-D.ietf-ipv6-multilink-subnets]
[PT>] 
[PT>] Done, I removed the latter draft upon Dave's review.

> 
> ---
> 
> Sect 2 has
> 
>    Extended LLN:  The aggregation of multiple LLNs as defined in
>          [RFC4919], interconnected by a Backbone Link via Backbone
>          Routers, and forming a single IPv6 MultiLink Subnet.
> 
> I'm not super-familiar with 4919, but a quick scan did not make it obvious
> what you are referencing in that document.  "LLN" is not mentioned. Nor is
> "aggregation" or "interconnect" (in this context).
> There is some mention in 4.2 of "seamless integration": is that what you are
> referencing?
[PT>] Actually, RFC 6550 should have been referenced there. I removed the term aggregation.


	Multiple LLNs as defined < RFC6550> interconnected
	by a Backbone Link via Backbone Routers, and forming a single IPv6
	MultiLink Subnet.
> 
> ---
> 
> Sect 2 has
> 
>    Registration:  The process during which a 6LN registers its
>          address(es) with the Border Router so the 6BBR can serve as
>          proxy for ND operations over the Backbone.
> 
> Do you mean s/Border/Backbone/ ?
> Otherwise it seems strange that the 6LN registers with the 6LBR so that the
> 6BBR can do something.
[PT>] The backbone router draft explains this further. The text was really missing. Reworded to:
	The process during which a 6LN registers an IPv6 Address with
	a 6LR in order to obtain services such as DAD and routing back.
    Duding that flow, the 6LBR may serve as proxy for the registration
    of the 6LN to the 6BBR so the 6BBR can provide IPv6 ND proxy services over
    the Backbone.
> 
> ---
> 
> Sect 2
> 
>    Binding:  The association between an IP address with a MAC address, a
>          port and/or other information about the node that owns the IP
>          address.
> 
> This was hard to parse. Possibly you mean...
> 
>    Binding:  The association between an IP address, a MAC address, a
>          port, and other information about the node that owns the IP
>          address.
> 
[PT>] Great, used that text

> ---
> 
> Sect 2
> 
>    Registering Node:  The node that performs the registration to the
>          6BBR, which may proxy for the registered node.
> 
> Confusing!
> The 6BBR is defined as the "proxy for the registration".
> Now we appear to have:
> - a node that needs to be registered
> - a node that does the registration to the 6BBR
> - the 6BBR (the proxy)
> The final subclause of your text ("which may proxy for the registered
> node") could be applied to the node that performs the registration or to the
> 6BBR.
> 
[PT>] Indeed! Changed as follows
 Registered Node:
	The 6LN for which the registration is performed, 
	and which owns the fields in the Extended ARO option.
 Registering Node:
                The node that performs the registration; this may be the Registered Node,
                or a proxy such as a 6LBR performing a registration to a 6BBR on behalf of 
                the Registered Node.




> ---
> 
> 4.1
> 
>    The Extended ARO (EARO) deprecates the ARO and is backward compatible
>    with it.  More details on backward compatibility can be found in
>    Section 7.
> 
>    The semantics of the ARO are modified as follows:
> 
> Given the "deprecates" statement, you probably want...
> 
[PT>] Changed to "replaces"

>    The semantics of the EARO are identical to the ARO with the following
>    modifications:
> 
> ---
> 
> 4.2
> 
>    The Transaction ID (TID) is a sequence number that is incremented
>    with each re-registration.
> 
> Who increments?
[PT>] 
Changed to 	

    The TID is a sequence number that is incremented by the 6LN with each
    re-registration to a 6LR.
> 
> ---
> 
> 4.2
> 
>    The TID may also be used by the
>    network to track the sequence of movements of a node in order to
>    route to the current (freshest known) location of a moving node.
> 
> You don't need to track the sequence of movements in order to identify the
> freshest known location. You only have to spot the most recent TID.
> This makes a big difference to implementations: is there a need to track the
> sequence or can an implementation just look for the most recent TID?
> 
[PT>] True; the text may also lean towards privacy issues. Changed to:

	The TID is a sequence number that is incremented by the 6LN with each
              re-registration to a 6LR.
	The TID is used to detect the freshness of the registration request and
	to detect one single registration by multiple 6LoWPAN border
	routers (e.g., 6LBRs and 6BBRs) supporting the same 6LoWPAN.
	The TID may also be used by the network to route to the current
              (freshest known) location of a moving node by spotting the most recent TID.

> ---
> 
> 4.7
> 
>    If the registry in the 6LBR is saturated, then the LBR cannot decide
>    whether a new address is a duplicate.  In that case, the 6LBR replies
>    to a EDAR message with a EDAC message that carries a new Status Code
>    indicating "6LBR Registry saturated" Table 1.  Note: this code is
>    used by 6LBRs instead of Status 2 when responding to a Duplicate
>    Address message exchange and passed on to the Registering Node by the
>    6LR.  There is no point for the node to retry this registration
>    immediately via another 6LR, since the problem is global to the
>    network.  The node may either abandon that address, de-register other
>    addresses first to make room, or keep the address in TENTATIVE state
>    and retry later.
> 
> Three points:
> 
> "cannot" seems strong since the first occurrence of the duplicate might
> already be in the registry.
[PT>] The problem indicated above is confined to a "new" address. I'm not sure what to do beyond that.

> 
> de-registering an address to make room seems a risky business since some
> other 6LR might grab the space.
[PT>] 
[PT>] True, we do not have signaling for doing the de registration and the new registration transactionally. Unsure what to do with this.

> 
> Shouldn't the actions be at the 6LBR
> - to notify the operator
> - to clear out "old" entries from the registry (although that may require
>   magic beyond housekeeping after Registration Lifetime expiration:
>   perhaps it could curtail the Delay state?)
> 
[PT>] yes, recommendations can be made to take actions ahead of time and provide management  alerts; automatically setting a shorter lifetime may result in additional traffic and no space saving is all the addresses are used so I'm not sure we can recommend that.
Part of the OPSDIR review was about this. there's new test in section " Requirements Related to Operations and Management" that I can tune to address your concern:


	    Req7.1: A management model SHOULD be provided providing access to the
        6LBR, monitor its usage vs. capacity, and alert in case of congestion. 

> ---
> 
> 5.
> 
>    The 6CIO is typically sent in a Router Solicitation (RS) message.
>    When used to signal capabilities per this specification, the 6CIO is
>    typically present in Router Advertisement (RA) messages but can also
>    be present in RS, Neighbor Solicitation (NS) and Neighbor
>    Advertisement (NA) messages.
> 
> I didn't find the two uses of "typically" gave me confidence to know what to
> code.
[PT>] yes, I removed that text already.

> 
> ---
> 
> 6.1
> 
>    |   3   | Moved: The registration failed because it is not the      |
>    |       | freshest.  This Status indicates that the registration is |
>    |       | rejected because another more recent registration was     |
>    |       | done, as indicated by a same ROVR and a more recent TID.  |
>    |       | One possible cause is a stale registration that has       |
>    |       | progressed slowly in the network and was passed by a more |
>    |       | recent one.  It could also indicate a ROVR collision.     |
> 
> Do you think "Moved" is the best name to cover all of these possibilities?
[PT>] I'm open to a better term : )

> 
> But, anyway, how can you have a RUID collision by definition of a RUID?
[PT>] Say I generate a crypto token and the 64 bits are the exact same as the EUI 64 of another node that uses RFC 6775. As discussed above this would be extremely rare. 
Since one of  the IPv6 addresses at least is not derived from the ROVR, having the harmful collision of both ROVR and IPv6 address is quasi impossible, like winning the lottery every week for a whole lifetime.

> 
> ---
> 
> 6.1
> 
>                    The node SHOULD maintain the TID in a persistent
>                    storage.
> 
> Unfortunate to push persistent storage requirements. But, since this is a
> SHOULD, all processes are in place to support not storing across restarts. So
> why make anyone do it? Surely you could fall back to the default handling
> without persistent storage.
> 
[PT>] Agreed; that text was a bad duplicate of the ROVR section. It is removed.

The text in the ROVR section now says:

	The Registering Node SHOULD store the ROVR,	or enough information to 
              regenerate it, in persistent memory.
	Otherwise, if a reboot causes a loss of memory, re-registering the
	same address could be impossible until the 6LRs and the 6LBR time out the
	previous registration, or a management action is taken to clear the relevant
              state in the network.

> ---
> 
> 6.2
> 
>    Code:           The ICMP Code as defined in [RFC4443].  The ICMP Code
>                    MUST be set to 1 with this specification.  An odd
>                    value of the ICMP Code indicates that the TID field
>                    is present and obeys this specification.
> 
> You're overloading the ICMP Code in an ugly way. But you knew that :-) It
> would probably be more normal to steal the top bit so that allocation of new
> codes can continue more normally. But failing that, I think you would be wise
> to make it so the IANA registry clearly shows that the odd values must not be
> assigned in future (section 10.2).
[PT>] Yes! the EARO should have been transported in DAR DAC. I read that only odd values should be assigned in the future since the future will always extend this. 
New proposed text:
     Also, all even values of the Code field are reserved for the 
     ICMP types above, and should not be assigned in the future.

> 
> ---
> 
> It is not clear whether anything in App A and B is intended to be normative.
> A clear statement would be helpful.
> 
> ==Nits==
> 
> idnits shows three problems
> 
>  ** There is 1 instance of too long lines in the document, the longest one
>      being 3 characters in excess of 72.
> 
>   == Missing Reference: 'Perlman83' is mentioned on line 1392, but not
> defined
> 
>   == Missing Reference: 'IEEEstd802154' is mentioned on line 1615, but not
>      defined
> 
> The references are caused by you having a third references section. I've not
> seen that before. Why not merge the sections as normal?
> 
> ---
> 
> Document header
> 
> I suspect "cisco" should have a capital "C"
> 
[PT>] Did not in the old days : ) 

> ---
> 
> I think the document title should spell out "Neighbor Discovery" and
> "IPv6 over Low-Power Wireless Personal Area Networks"
[PT>] Per a previous review, the title is now:
"Registration Extensions for 6LoWPAN Neighbor Discovery"
This seems to work with your comment


> 
> ---
> 
> Sect 1
> 
> s/an IPv6 Low Power Networks/any IPv6 Low Power Network/
> 
> ---
> 
> Sect 1
> 
>    to enable additional capabilities and enhancements such as:
> 
> s/such as/including/
> 
[PT>] done
> ---
> 
> You use "NS" in section 4, but don't expand it until 4.1
> 
[PT>] Fixed with the glossary in section 2.
> ---
> 
> 4.4 could very usefully point at 6.2 to help the reader parse the text.
[PT>] one for both EARO and DAM.

> 
> ---
> 
> Shouldn't the figure in 6.2 show the optional ND options as described in 4.4?
> 
[PT>] Actually that text is not compatible with a variable length ROVR so it might be better to omit it. I need to check with the group is supporting a larger ROVR is desirable.

> ---
> 
> 4.7 has "LBR" which should be "6LBR"
> 
> ---
[PT>] done

> 
> 6.1
> 
> s/SLLAO option/SLLA option/ (twice)
[PT>] Three times : )

> s/The EARO option/The EARO/  (three times)
[PT>] even more
> 
> ---
> 
> 6.3
> 
>    This specification defines new capability bits for use in the 6CIO,
>    which was introduced by [RFC7400] for use in IPv6 ND RA messages.
> 
> Say "...defines four new..." to save me having to work out which bits are new.
> 
[PT>] Done.
> ---
> 
> 6.3
>    Routers that support this specification MUST set the "E" flag and 6LN
>    SHOULD favor 6LR routers that support this specification over those
>    that do not.
> 
> s/6LR routers that support/a 6LR that supports/
> 
> ---
[PT>] done
> 
> 8.
> 
>    This specification extends [RFC6775], and the security section of
>    that standard also applies to this as well.
> 
> Don't think 6775 is an Internet Standard.
> Suggest s/standard/document/.
[PT>] done