Alvaro Review - Major and Minor Issues

[inesrob]

Source: https://mailarchive.ietf.org/arch/msg/roll/_3m1pKzut7k5hl3LLo7ZvH82Juc/

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:

#9

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?

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

6LN needs to be expanded, but it also carries the 6LoWPAN-related
connotation. If not "leaf" then maybe RUL (?). Expand and reference
rfc9010.

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.

[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.

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.

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".

[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"

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?

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.

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

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. ;-)

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??

[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.

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.

[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.

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.

[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.

I'm assuming that "not understood" translates to "unknown or unsupported".

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.

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...

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

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.

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

[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?

...
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.

...
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.

[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.

...
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?

[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.

294 * MOPex value

296 * Description

298 * Defining RFC

[major] Include a table to show how the registry should 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.

...
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

[EoR -06]

[inesrob]

Reply from Rahul: https://mailarchive.ietf.org/arch/msg/roll/n1UkGoi-Wg7QwQi8hFS7nJv6xPg/

[inesrob]

Reply from Alvaro: https://mailarchive.ietf.org/arch/msg/roll/J3GRhVvJ97aay9kzINMT2cxPcUM/