Re: [Roll] Review of draft-ietf-roll-mopex-06

[Rahul Jadhav <[e-mail]>]

Thanks Alvaro for the detailed review and comments. This has greatly
improved the draft. The PR can be found here
<https://github.com/roll-wg/mopex/pull/10>. I haven't updated the draft on
IETF portal as yet since I would like to revisit my changes one more time
but didn't want to further delay this response.

Please find my comments inline.

There are few comments inline that have few questions. Kindly check.

Again, appreciate the rigorous review.

Thanks,
Rahul

On Wed, 3 May 2023 at 02:41, Alvaro Retana <[e-mail]> wrote:

> Hi!
>
> I volunteered to look at this document at the meeting in Japan --
> here's my review.


> In general, I think it looks good.  But there are several
> clarifications and modifications needed.  See comments in-line.  I
> also submitted a PR in the GitHub repository including some editorial
> suggestions:
>
>    https://github.com/roll-wg/mopex/pull/9


[Rahul] This PR is merged. Thanks

>
>
>
> For the Chairs: this document should be marked as replacing
> draft-ietf-roll-mopex-cap (along with the existing tag for
> draft-ietf-roll-capabilities).
>
>
> Thanks!
>
> Alvaro.
>
>
> [Line numbers from idnits.]
>
> ...
> 73      1.  Introduction
> ...
> 91         This document further extends the RPL Control Option syntax to
> handle
> 92         generic flags.  The primary aim of these flags is to define the
> 93         behavior of a node not supporting the given control type.  If a
> node
> 94         does not support a given RPL Control Option, there are following
> 95         possibilities:
>
> [] This extension feels out of place in this document since it is not
> used for anything related to MOPex.  Also, the Capabilities option
> (draft-ietf-roll-capabilities) is not an Extended Option...but that
> draft defines Capabilities TLVs which follow the same format (?!).
>
> What are these extended options used for?  Why do they need to be
> defined in this document?
>

The RPL control options in RFC6550 requires that an implementation silently
ignore the unsupported control option.
We had the need to not just silently ignore the unsupported control option
in context to capabilities. The Capabilities TLV achieved that but then
there was a point to do this at the control option level itself since the
control options would face similar problems. We discussed enabling support
for such extended control options as part of the MOPex i.e., the drafts
supporting MOPex would support this.

If this gets implemented by the MOPex then the capabilities draft will
remove the corresponding flags (J, I, C flag) achieving the same objective.



>
>
> 97            Strip off the option
>
> 99            Copy the option as-is
>
> 101           Ignore the message containing this option
>
> 103           Let the node join in only as a 6LN to this parent
>
> [minor] The specification is done later, so this list doesn't need to
> be included here.
>
>
> [minor] s/6LN/leaf
>

[Rahul] Done

>
> 6LN needs to be expanded, but it also carries the 6LoWPAN-related
> connotation.  If not "leaf" then maybe RUL (?).  Expand and reference
> rfc9010.
>
> [Rahul] It is not really a RUL i.e., it understands RPL but just that it
does not support the MOP/MOPex that is advertised in the DIO and hence
cannot join the network as a router.
I will change from 6LN to leaf (unless someone thinks I should use RAL -
RPL Aware Leaf as the term here).


>
>
> 105     1.1.  Requirements Language and Terminology
>
> 107        The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
> NOT",
> 108        "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in
> this
> 109        document are to be interpreted as described in RFC 2119
> [RFC2119].
>
> [major] Use the rfc8174 template.
>

[Rahul] Changed to RFC 8174 template.

>
>
> [major] I know that later the text says that "For the sake of
> readability, all the known relevant terms are repeated in this
> section."  However, the definitions don't all match what is in
> rfc6550.  To avoid this type of issue, define only new terms and point
> to rfc6550 for the rest.
>
> 111        MOP: Mode of Operation.  Identifies the mode of operation of
> the RPL
> 112        Instance as administratively provisioned at and distributed by
> the
> 113        DODAG root.
>
> 115        MOPex: Extended MOP: This document extends the MOP values over a
> 116        bigger range.  This extension of MOP is called MOPex.
>
> 118        DAO: DODAG Advertisement Object.  An RPL message used to
> advertise
> 119        the target information to establish routing adjacencies.
>
> [major] A DAO is a "Destination Advertisement Object" (not a "DODAG
> Advertisement Object").  Note that (1) §6.4/rfc6550 defines the DAO as
> "used to propagate destination information Upward along the DODAG",
> which is not exactly what is mentioned here, and (2) the Termonology
> section of rfc6550 simply points at §6.4:
>
>    DAO: Destination Advertisement Object (see Section 6.4)
>
> A simple extension + pointer may be a good option here too.
>
>
> 121        DIO: DODAG Information Object.  An RPL message initiated by the
> root
> 122        and used to advertise the network configuration information.
>
> [major] As above, the Terminology section of rfc6550 only says this:
>
>    DIO: DODAG Information Object (see Section 6.3)
>
> Note that §6.3/rfc6550 says that a DIO "carries information that
> allows a node to discover a RPL Instance, learn its configuration
> parameters, select a DODAG parent set, and maintain the DODAG."
> Again, not exactly the same.
>
>
> 124        Current parent: Parent 6LR node before switching to the new
> path.
>
> [minor] This term is not used anywhere in the text.
>

[Rahul] Made changes to all the above points in the terminology section.
Now we are pointed to RFC 6550 where needed, removed unused terms, and
fixed incorrect acronyms.


>
>
> 126        This document uses the terminology described in [RFC6550].  For
> the
> 127        sake of readability, all the known relevant terms are repeated
> in
> 128        this section.
>
> 130     2.  Requirements for this document
>
> 132        Following are the requirements considered for this documents:
>
> 134        REQ1:  MOP extension.  The 3-bits MOP as defined in [RFC6550]
> is fast
> 135               depleting.  An MOP extension needs to extend the
> possibility
> 136               of adding new MOPs in the future.
>
> 138        REQ2:  Backwards compatibility.  The new options and new fields
> in
> 139               the DIO message should be backward compatible i.e. if
> there
> 140               are nodes that support old MOPs they could still operate
> in
> 141               their RPL Instances.
>
> [major] "new options and new fields in the DIO message should be
> backward compatible"
>
> I don't think this requirement was completely met.  Even if the
> MOPex-value can be 0-6, a node that supports old MOPs (and hasn't been
> upgraded) won't be able to operate because it will see MOP 7.  See the
> comment in §3.2 about adding text to point this out.
>

If a node that is not upgraded and sees MOP7, can join as a leaf node as
per RFC 6550. The control options that are not supported will be silently
ignored as per RFC 6550.
This behavior is backward compatible.


>
> 143     3.  Extended MOP Control Message Option
>
> 145        This document reserves the existing MOP value 7 to be used as an
> 146        extender.  DIO messages with an MOP value of 7 MUST refer to the
> 147        Extended MOP (MOPex) option in the DIO message.
>
> [] Later on the term "base DIO MOP" is used.  Take the opportunity
> here to differentiate between the MOP in the DIO and the one in the
> option.  I think that calling the new one MOPex should be enough --
> IOW, no need to introduce "base DIO MOP".
>

[Rahul] Removed "base DIO MOP" and "final MOP" from the draft and used the
MOP and MOPex terms consistently in the document.

>
>
> [major] "MUST refer to"
>
> To "refer" doesn't sound like a good normative action.  I assume you
> mean something like this:
>
>    A DIO message with a MOP value of 7 indicates that the MOP for RPL
>    instance is contained in the Extended MOP (MOPex) option.
>
>
>
> 149                   0                   1                   2
> 150                   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3
> 151                  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+---------------
> 152                  |   Type = TODO |  Opt Length   |     OP-value
> 153                  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+---------------
>
> 155                            Figure 1: Extended MOP Option
>
> 157        The option length value MUST be less than or equal to 2.  An
> option
> 158        length value of zero is invalid and the implementation MUST
> silently
> 159        ignore the DIO on receiving a value of zero.
>
> [major] "OP-value"
>

[Rahul] Thanks for the PR with the fixes.

>
> I assume you meant "MOP-value", but that is not consistent with
> "Extended-MOP-value" as mentioned in §7.3.  Recommendation: use
> "MOPex-value".
>
>
> [major] "length value MUST be less than or equal to 2"
>
> This seems to indicate that the MOPex-value can be anything between
> 0-65535.  However, it is not specified how the representation should
> work.  For example, if the length is 1 only the lower bits are
> represented.  Is that what is intended?
>

[Rahul] Added a reference to Section 6.7.1 that explains how to interpret
the Option Length/Data.

> 161     3.1.  Handling MOPex
>
> 163        The MOPex option MUST be used only if the base DIO MOP is 7.
> If the
> 164        base DIO MOP is 7 and if the MOPex option is not present then
> the DIO
> 165        MUST be silently ignored.  If the base DIO MOP is less than 7
> then
> 166        MOPex MUST NOT be used.  In case the base MOP is 7 and if the
> MOPex
> 167        option is present, then the implementation MUST use the final
> MOP
> 168        value from the MOPex.
>
> [] See my comment above about using "base DIO MOP".
>
>
> [] Similar comment about "final MOP".  It was already established that
> the MOPex is the new MOP.
>

[Rahul] Removed "base DIO MOP" and "final MOP" from the draft and used the
MOP and MOPex terms consistently in the document.

>
>
> [major] What happens if the MOP = 7 but the MOPex option is invalid?
>

[Rahul]  Changed to => "If the DIO MOP is 7 and if the MOPex option is not
present *or invalid* then the DIO MUST be silently ignored."
Earlier it was, "If the DIO MOP is 7 and if the MOPex option is not present
then the DIO MUST be silently ignored."

>
> 170        Note that [RFC6550] allows a node that does not support the
> received
> 171        MOP to still join the network as a leaf node.  This semantics
> 172        continues to be true even in the case of MOPex.
>
> [major] I'm assuming that all other general assumptions about the MOP
> also apply to the MOPex.  Even if it maybe goes without saying, please
> say it. ;-)
>

[Rahul] I did a grep on MOP and checked if there are any other
considerations apart from the one that is mentioned above that applies to
MOPex. I couldn't find any. Regardless I have stated the following:
"All the general assumptions that are applicable in the context of MOP from
RFC6550 are applicable in the context of MOPex as well."

>
> 174     3.2.  Use of values 0-6 in the MOPex option
>
> 176        The MOPex option could also be allowed to re-use the values 0-6,
> 177        which have been used for MOP so far.  The use of current MOPs in
> 178        MOPex indicates that the MOP is supported with an extended set
> of
> 179        semantics e.g., the capability options
> [I-D.ietf-roll-capabilities].
>
> [major] "could also be allowed" -- this sounds like a proposal, an
> idea, but not as a specification.  If this the direction??
>

[Rahul] Updated the sentence to:
"
The MOPex option should also be used for existing MOP values
0-6. The use of current MOPs (values 0 to 6) in MOPex indicates
that the MOP might be supported with an extended set of semantics e.g.,
the capability options <xref target="I-D.ietf-roll-capabilities"/>.
"


>
> [major] "indicates that the MOP is supported with an extended set of
> semantics"
>
> OTOH, this sounds like a firm statement...but it is really an example.
>

[Rahul] Handled in above change

>
> Suggestion>
>    The MOPex option can include values 0-6 to indicate support for the
>    existing MOPs, as specified in [RFC6550] and [RFC6997].  Any extended
>    set of semantics or options for these MOPs is out of scope for this
>    document -- see [I-D.ietf-roll-capabilities] as an example of possible
>    capabilities.
>

[Rahul] The intention of the statement was to clarify the use-case where
MOPex could be used even with MOPs 0 to 6 since the implementation might
want to use the features provided by capabilities that are possible only
with MOPex.

There are three cases:
1. MOP=[0-6], No MOPex: This is supported by RFC 6550 without any
requirement to this draft.
2. MOP=7, MOPex = [0-6]: Requires this draft support. Implementations
supporting just RFC 6550 can join as leaf only. The primary use-case with
this mode is to support old MOPs with new capabilities draft.
3. MOP=7, MOPex >= 7: Requires this draft support. Implementations
supporting just RFC 6550 can join as leaf only.

Points 2 and 3 are almost the same. Do you think I should add this to the
draft itself?



>
>
> [major] Even without additional options, a node that doesn't support
> this specification won't be able to operate if the MOP is 7 and the
> MOPex is 0-6.  Please add some text to indicate that backwards
> compatibility (to nodes wanting support MOPs 0-6) can only be achieve
> if they support this specification.  Otherwise they will only be able
> to join as a leaf.
>

[Rahul] I think this is already stated.
Consider for example, we have a deployment supporting RFC6550 only with the
new spec and it receives MOP7. It won't know how to handle it and thus will
join as a leaf. This point is explicitly stated in the document.


>
> 181     4.  Extending RPL Control Options
> ...
> 186             0                   1                   2
> 187             0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3
> 188            +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-----------
> 189            |X|   OptionType| Option Length |Opt Flags|J|I|C| Option
> Data
> 190            +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-----------
>
> 192                         Figure 2: Extended RPL Option Format
>
> 194        New fields in extended RPL Control Message Option Format:
>
> 196           'X' bit in Option Type: Value 1 indicates that this is an
> extended
> 197           option.  If the 'X' flag is set, a 1-byte Option Flags
> follows the
> 198           Option Length field.
>
> [minor] Because of the space, the figure only says "Opt Flags", not
> "Option Flags".  See comment below about this.
>
>
> 200           Option Length: 8-bit unsigned integer, representing the
> length in
> 201           octets of the option, not including the Option Type and
> Length
> 202           fields.  Option Flags and variable length Option Data fields
> are
> 203           included in the length.
>
> [] You don't need to respecify the Option Length field.  The extra
> text in the second sentence is new, but the definition is not
> different.
>
> If you want to keep this description, then please add also one for Option
> Type.
>

[Rahul] Added Option Type description as well.

>
>
> [minor] Consider changing Figure 2 to only show the Option Flags field
> and then describe its contents here.
>
>
> [major] How should the unassigned flags be treated?  Should they be
> managed by IANA?  How?
>
>
> [nit] The flags are called bits...  For consistency, either call the
> field "Option Bits" or the individual bits "flags".
>
>
> [] The flags only apply if the Option Type is unknown.  Perhaps
> introduce the specification by saying that -- and to avoid
> duplication.
>
> Suggestion>
>
>    The flags specified below apply only when the Option Type is unknown
>    or unsupported.
>

[Rahul] Incorporated this suggestion.


>
> I'm assuming that "not understood" translates to "unknown or unsupported".
>
[Rahul] Yep, that's right. Updated.


>
>
> 205           'J' (Join) bit in Option Flags: A node MUST join only as a
> 6LN if
> 206           the Option Type is not understood.
>
> [] Suggestion>
>
>    'J' (Join) flag: If set, the node MAY only join the network as a leaf.
>

[Rahul] Changed


>
> 208           'C' (Copy) bit in Option Flags: A node that does not
> understand
> 209           the Option Type MUST copy the Option while generating the
> 210           corresponding message.  E.g., if a 6LR receives a DIO
> message with
> 211           an unknown Option with 'C' bit set and if the 6LR chooses to
> 212           accept this node as the preferred parent then the node MUST
> copy
> 213           this option in the subsequent DIO message it generates.
> 214           Alternatively, if the 'C' flag is unset the node MUST strip
> off
> 215           the option and process the message.
>
> [] Suggestion:
>
>    'C' (Copy) flag: If set, the node MUST copy...
>

[Rahul] Changed

>
>
> [minor] 6LR: see the comment above about the 6LN.  s/6LR/RPL router.
>

[Rahul] Changed

>
>
> 217           'I' (Ignore) bit in Option Flags: A node that does not
> understand
> 218           the Option Type MUST ignore this whole message if the 'I'
> bit is
> 219           set.  If the 'I' bit is set then the value of 'J' and 'C'
> bits are
> 220           irrelevant and the message MUST be ignored.
>
> [] Suggestion>
>
>    'I' (Ignore) flag: If set, the node MUST ignore the whole message
>    regardless of the setting of the J and C flags.
>

[Rahul] changed



>
> [major] How should the flags be interpreted if the Option Type is known?
>

[Rahul] If the Option Type is known then that specification will decide how
to handle this Option.


>
> [major] Some considerations should be described related to how/when
> the flags should be set.  For example, not setting the J flag would
> allow a node to join the DODAG even if it potentially doesn't fully
> support whatever the option contains.  Is there a class of information
> that requires the node to only be a leaf vs a RPL router?
>

[Rahul] Added a section for "Implementation Considerations for setting the
Option Flags".


>
>
> ...
> 241        If a node receives an unknown Option without 'X' flag set then
> the
> 242        node MUST ignore the option and process the message.  The
> option MUST
> 243        be treated as if J=0, C=0, I=0.
>
> [major] The first sentence is not a change to what rfc6550 already
> specifies.  No need to specify it again.
>

[Rahul] I have deleted the whole para since I cannot have statement 2
without sentence one.

>
>
>
> ...
> 257     7.  IANA Considerations
>
> 259     7.1.  Mode of operation: MOPex
>
> 261        IANA is requested to assign a new Mode of Operation, named
> "MOPex"
> 262        for MOP extension under the RPL registry.  The value of 7 is to
> be
> 263        assigned from the "Mode of Operation" space [RFC6550]
>
> [minor] Given that the MOP space is being extended, it would be a good
> idea to add "this document" as a reference to the registry.  At the
> same time, the reference to rfc9008 should be removed.
>

[Rahul] The table below the para shows MOP 7 used for MOPex referenced by
"this document".


>
> [major] rfc9008 changed the status of MOP 7 from Unassigned (=
> available) to Reserved ("not available for assignment" [RFC8126]).  To
> assign the value, this document should formally Update rfc9008.
>

> [minor] Also, make it clear that the reference to 7 should be "this
> document" ONLY.  IOW, the current references in the registry to
> RFC9008, RFC9010, and RFC9035 should be removed.
>

> Suggestion>
>
>    This document updated [RFC9008] to remove the reservation on Mode
>    of Operation value 7.
>
>    IANA is requested to assign the Mode of Operation value 7 to MOPex,
>    as shown in Table 2.  As shown there, all other references related
>    to value 7 are to be removed.
>
>    IANA is also requested to replace the reference to [RFC9008] in the
>    overall registry with a reference to this document.
>
>
[Rahul] Many thanks for the text.

> ...
> 286     7.3.  New Registry for Extended-MOP-value
>
> 288        IANA is requested to create a registry for the
> extended-MOP-value
> 289        (MOPex).  This registry should be located in TODO.  New MOPex
> values
> 290        may be allocated only by an IETF review.  Currently no values
> are
> 291        defined by this document.  Each value is tracked with the
> following
> 292        qualities:
>
> [major] "allocated only by an IETF review"
>
> It looks like the space is big!  Should all of it use the same
> registration policy?  Should there be/does it make sense to have a
> Private Use or Experimental range, or maybe a First Come First Served
> range?
>

[Rahul] The range of MOPex values are dynamic i.e., if the Option Length is
1 byte the MOPex range is 256, if it is 2 bytes it is 0-65536.
If we have to allocate few for private/experimental use, we have to mandate
the Option Length to atleast 2 bytes. I am not sure if we should do this.



>
> [major] "Currently no values are defined by this document."
>
> §3.2 says that reusing 0-6 could be allowed -- which means that there
> are values assigned.
>
>
> [major] If 0-6 are reused, how will the two registries be kept in
> sync?  There are two values in the MOP registry that haven't been
> assigned.  Maybe there's a way for IANA to refer to the other
> registry, instead of requiring double registration.  Maybe that could
> be done by indicating that 0-6 are Reserved, and adding a note in the
> Description field pointing at the other registry.  In any case,
> something to talk to IANA about.
>

[Rahul] Great point! I am not sure how to handle this but I have updated
the document to reflect use of 0-6 values in the MOPex field.

>
>
> 294        *  MOPex value
>
> 296        *  Description
>
> 298        *  Defining RFC
>
> [major] Include a table to show how the registry should look like.
>

[Rahul] Included the table to show how the registry will look like.


>
>
> 300     7.4.  Change in RPL Control Option field
>
> [major] The X bit basically changes the meaning/name of the Option
> Types 0x80-0xFF to "Extended Control Options".  Instead of a new
> registry, the existing RPL Control Message Options registry should be
> modified.  Doing it this way avoids changing the definition of Option
> Type and having to formally Update rfc6550.
>
> Suggestion>
>
>    IANA is requested to modify the RPL Control Message Options registry
>    to include an Extended Control Options range as shown in Table A.
>
>    [Include Table A with columns: Range | Option Type | Reference
>
>      0x00-0x7F | Base Options | rfc6550
>      0x80-0xFF | Extended Options | [this document]
>
>    IANA is also requested to add [this document] as a reference for this
>    updated registry.
>

[Rahul] Many thanks for the text. Updated the draft.

>
>
>
> ...
> 321     8.  Security Considerations
> ...
> 328        Capabilities flag can reveal that the node has been upgraded or
> is
> 329        running a old feature set.  This document assumes that the base
> 330        messages that carry these options are protected by RPL security
> 331        mechanisms and thus are not visible to a malicious node.
>
> [major] s/Capabilities flag/The use of MOP 7
>

[Rahul] Updated.

>
> [EoR -06]
>