6TiSCH Operation Sublayer (6top) Protocol (6P)
draft-ietf-6tisch-6top-protocol-12

Thanks to everyone who worked on this document. I have a handful of minor,
editorial suggestions.

---------------------------------------------------------------------------

Please expand TSCH and 2TiSCH in the Abstract.

---------------------------------------------------------------------------

§3.1.1:

>  5.  The SF running on node B selects 2 out of the 3 cells from the
>      CellList of the 6P ADD Request.  Node B locks those cells in it

nit: "its"

>      schedule until the transmission is successfull (i.e. node B

nit: "successful"

---------------------------------------------------------------------------

§3.3.1:

>  be used), or partially succeed (less than NumCells cells from the

Nit: "fewer than" rather than "less than."

---------------------------------------------------------------------------

§3.3.2:

>  o  The CellList in a 6P Request (2-step transaction) or 6P Response
>     (3-step transaction) MUST either be empty, contain exactly
>     NumCells cells, or more than NumCells cells.  The case where the
>     CellList is not empty but contains less than NumCells cells is not
>     supported.

nit: "...fewer than..."

It would be a good idea to clearly indicate what a recipient of a message with a
non-empty CellList with fewer entries than NumCells requires should do. This is
also applicable for the "Candidate CellList" for RELOCATE. I can't find
equivalent language for ADD -- is it okay for a message to (for example) have a
NumCells value of 3, but include only 2 cells in its CellList?

---------------------------------------------------------------------------

§3.3.3:

>  verification on Candidate CellList can succeed (NumCells cells from
>  the Candidate CellList can be used), fail (none of the cells from the
>  Candidate CellList can be used) or partially succeed (less than
>  NumCells cells from the Candidate CellList can be used).  In all


nit: "...fewer than..."

---------------------------------------------------------------------------

§3.3.5:

>  MaxNumCells:  The maximum number of cells to be listed.  Node B MAY
>        return less than MaxNumCells cells, for example if MaxNumCells
>        cells do not fit in the frame.

nit: "...fewer than..."

>  If node B has less than Offset cells that match the request, node B
>  returns an empty CellList and a Code field set to RC_EOL.

nit: "...fewer than..."

Thank you for addressing my DISCUSS and some of the comments.

(1) S3.2.2: "The value of SeqNum MUST be different at each new 6P Request issued to the same neighbor." Should that be "...different...to the same neighbor and SF"?  S3.4.6 talks about maintaining independent SeqNums per neighbor, per SF.

(2) S3.2.3: "The SF MAY redefine the format and meaning of the CellOptions field."  If the "RECOMMENDED" values are expected to be the default, how are changes to the formatting/meaning communicated between A and B?

(3) S3.2.4: "The CellList is an opaque set of bytes, sent unmodified to the SF."  Given that A and B don't really know what the contents of the CellList are, or even if the "RECOMMENDED format" is followed, I don't see the need to Normatively define the Cell Format.  IOW, s/RECOMMENDED/recommended

Substantive Comments:

Requirements Language: The document contains at least a few instances of lower case "must". Please consider using the boilerplate from RFC 8174.

§3.1.1, figure 4: Did I miss an explanation of why A sends 3 candidate cells when it needs 2 new cells? I can guess why, but it would be best to explicitly explain it.

§6.2.2 and §6.2.3: Do you really expect new message types and commands to be added routinely enough to justify
 a registry?

Editorial Comments and Nits:

Abstract: Please expand 6top on first mention.  You do so later in the paragraph, but not the first time. (This pattern repeats in the document body)

§1: Please expand 6TiSCH on first mention.

§3.1.1, Figure 4, step 2: "Node A locks the candidate cells in it schedule until..." doesn't parse. Should "it" be "its"?

§4: This section seems to be about 6top scheduling function _specification_, not the functions themselves. Is this correct?

Appendix A: Seems like this should be part of the IANA considerations.

Thank you for resolving my DISCUSS; original COMMENT section preserved below.

The CellList and CellOptions fields can effectively be redefined by the SF, so the document only gives
interpretations for them that are referred to as "RECOMMENDED", which is a bit jarring since they seem
to be expected to be the default values.  Perhaps it would be better to say that the descriptions in this
document are the "default interpretation unless redefined by the SF in use".


In several places we see:

   CellList:  A list of 0, 1 or multiple candidate cells.

How about "0 or more"?  Also, the "Its length is [...]" is not
universally present as the next sentence.


As another general note, it might be useful to say a bit more about the fields in the Response and
Confirmation messages, i.e., give a list of fields which must match the ones in the corresponding Request.

Section 3.3.3

   Upon receiving the request, Node B checks if the length of the
   Candidate CellList is larger or equal to NumCells.  Node B's SF
   verifies that all the cells in the Relocation CellList are indeed
   scheduled with node A, and are associate the options specified in the
   CellOptions field.  If that check fails, node B MUST send a 6P
   Response to node A with return code RC_ERR_CELLLIST.  If that check
   passes,

I think this should be "if either check fails" and "if both checks
pass".


Section 3.3.6

If link-level access message protection is not in use, the ability to inject a CLEAR message
with no SeqNum checking seems a fairly powerful capability to give to an attacker.  I don't
immediately see a way around it, but this would be a candidate for inclusion in an expanded
Security Considerations.

Section 3.3.7

Do we need to explicitly say that the SIGNAL payload's length is determined by the length
of the IE header?

Section 3.4.3 is perhaps a bit underspecified.
In particular, does "consider it never happened" include reusing
that SeqNum for the next transaction?

Section 3.4.6
Is there value in using the term "lollipop counter" when the
behavior needs to be defined here anyway?

Section 3.4.6.2

I'd be interested to see an example where the node that has
power-cycled is the one sending the request that triggers
inconsistency detection.

Section 5

   [...] These cases
   SHOULD be handled by an appropriate policy, such as blacklisting the
   attacker after several attempts.

It's not clear that pure blacklisting is always an appropriate
policy.  Rate-limiting or time-limited blacklisting might be a more
appropriate example.

Though key management is out of scope, it may be worth explicitly mentioning
that forgery and misattribution attacks become more damaging when a single
key is shared amongst a group of more than 2 participants.


In the IANA considerations, should the new subregistries (e.g., message types and CellOptions
bitmap) be scoped down to just 6P version 0 or are they intended to be version-agnostic?

I support Ben's DISCUSS

As I read this document, this only works if two nodes are running the same SF. Do you think you could make that point earlier in the document?

Have read through the document but did not perform a detailed review.

Thanks for the well-written and easy to read document. Only section 3.2.3. is a bot confusing as TX, RX, and S show up in the table but are not really explained at all (besides in the IANA considerations at the very end).

Some more, mostly editorial comment:

1) sec 3.2.3: "The CellOptions is an opaque set of bits, sent unmodified to the SF.
   The SF MAY redefine the format and meaning of the CellOptions field."
   Not sure you can say that the celloptions bits are opaque given you define them in this section...

2) In sec 3.3.1:
"NumCandidate MUST be larger or equal to NumCells."
What happens if that is not the case. Should node B assign the requested cells anyway, or send an error, or both?

and also in section 3.3.2:
"The case where the CellList is not empty but contains less than NumCells cells is not supported."
What does that mean? Should an error be sent or the message just ignored?

3) Not sure if the "6P Version Numbers" registry is really needed at this point of time. I guess if many new versions show up, it would still be time to create this registry with the next version.