[6tisch] Yatch's review on draft-ietf-6tisch-msf-08 - Re: MSF review

[Yasuyuki Tanaka <[e-mail]>]

Hi all,

I reviewed draft-ietf-6tisch-msf-08. Here are my comments;

* [clarification] Section 1

draft> the 6 steps described in Section 4.  The end state of the join
draft> process is that the node is synchronized to the network, has
draft> mutually authenticated to the network, has identified a
draft> preferred routing parent, and has scheduled one default
draft> negotiated cell (defined in Section 5.1) to/from its preferred
draft> routing parent. After the join

Here, saying "one negotiated TX cell" would be clearer instead of "one
default negotiated cell".

* [clarification] Section 2

draft> ..., while Slotframe 0 is used for the bootstrap traffic as
draft> defined in the Minimal 6TiSCH Configuration.

I think this part is misleading. Slotframe 0 has the minimal shared
cell, which is basically used for all broadcast frames including EBs
and DIOs. These frames are needed not only for the bootstrap phase.

Given that, the following part seems a little bit strange as well.

draft> MSF uses the minimal cell to exchange the following packets:
draft>
draft> 1.  Enhanced Beacons (EBs), defined by [IEEE802154-2015].
draft>     These are broadcast frames.
draft> 2.  Broadcast DODAG Information Objects (DIOs), defined by
draft>     [RFC6550].  Unicast DIOs SHOULD NOT be sent on minimal cell.

Why don't we say just, "the minimal cell is used for broadcast frames
such as Enhanced Beacons (EBs) and broadcasted DODAG Information
Objects (DIOs)"? I would add, "Cells scheduled by MSF are meant to be
used only for unicast frames."

* [clarification] Section 3

draft> The default slotframe length (SLOTFRAME_LENGTH) is RECOMMENDED
draft> for Slotframe 0, 1 and 2, although any value can be advertised
draft> in the EBs.

Does this mean, these three slotframes can have different lengths? Or,
the slotframes have the same length, which may be different from
SLOTFRAME_LENGTH?

I don't know why we want to accept the former case, which makes MSF
more complex.

A related description is found in the latter part of the section:

draft> stated in [IEEE802154-2015].  However, when the Slotframe 0, 1
draft> and 2 use the same length value, it is possible for negotiated
draft> cell to avoid the collision with AutoRxCell.

* [technical] Section 4.8

The whole part of this section doesn't seem to be part of the
bootstrapping process. This describes how to handle unused cells,
which is part of the "housekeeping".

More importantly, the idea mentioned here doesn't look good, which is
not a "SHOULD" thing from my view point.

What if your child switches its preferred parent from you to another
node, and a CLEAR request sent by the child doesn't reach you because
of bad luck? The child loses all the cells scheduled with you, but
it's still in your communication range. The child will respond to your
keep-alive message, which prevents you from removing cells which will
no longer used. Another example is a case where your child reboots and
selects another node as its preferred parent.

A relevant discussion on ML:
https://mailarchive.ietf.org/arch/msg/6tisch/D2rvB3w5uKfhFQYzztqCu-h1fW4

So, I would suggest:

- removing Section 4.8 entirely
- adding a subsection to Section 5, mentioning a need for a mechanism
   removing up unused negotiated cells, without proposing a concrete
   idea

draft> When a neighbor is declared unreachable, the node MUST remove
draft> all negotiated cells with that neighbor from its own schedule.
draft> In addition, it MAY issue a 6P CLEAR to that neighbor (which
draft> can fail at the link-layer).  The node MAY be removed from the
draft> neighbor table.

In addition, the behavior described above, CLEAR from a parent to its
old child, contradicts what Section 8 says:

draft> MSF uses 2-step 6P Transactions exclusively.  6P transactions
draft> are only initiated by a node towards its preferred parent.

If we want to keep the idea in the text, then I would propose sending
a COUNT request instead of a (some form of?) keep-alive frame. This
would cause RC_ERR_SEQNUM in the examples given above, and a CLEAR
transaction will follows. And, I wouldn't use SHOULD or MUST there,
leaving it to implementers.

[clarification] Section 5.1

It is unclear whether we should have separate pairs of
NumCellsElapsed/NumCellsUsed for negotiated TX cells and negotiated RX
cells or not.

If we have NumCellsUsed dedicated for RX, the following text is not
correct:

draft> NumCellsUsed: Counts the number of negotiated cells that have
draft>     been used.This counter is initialized at 0.  NumCellsUsed
draft>     is incremented by exactly 1 when, during a negotiated cell
draft>     to the preferred parent, either of the following happens:

We need to increment NumCellsUsed when receiving a frame on the
*autonomous* RX cell while there is no negotiated RX cells scheduled
with the preferred parent. In this case, NumCellsUsed counts how many
times the *autonomous* RX cell is used. This is not aligned with what
is described in Section 5.1.

The same thing is seen in the definition of NumCellsElapse.

An expected process I believe is:

   if we have negotiated RX cells scheduled with the preferred parent:
     - NumCellsElapse: the number of times negotiated RX cells are
       visited
     - NumCellsUsed: the number of times negotiated RX cells are used
   else
     - NumCellsElapse: the number of times the autonomous RX cell is
       visited
     - NumCellsUsed: the number of times the autonomous RX cell is used

This is not well described in the current version.

Another comment to this section is that, the counters shouldn't be
updated by invalid frames, because we cannot guarantee the source
address of an invalid frame is intact. Or, an attacker can trigger
unnecessary 6P transactions by sending garbage frames over the
autonomous RX cell of a victim node, although the expression of
"invalid frame" is ambiguous.

draft> * The node receives a frame from its preferred parent.  The
draft>   counter increments regardless of whether the frame is a valid
draft>   IEEE802.15.4 frame or not.

[question] Section 5.1

draft> Meanwhile, the latency won't increase much by using a larger
draft> value of MAX_NUMCELLS for periodic traffic type.

Why...? With a larger MAX_NUMCELLS, MSF cannot react traffic increase
quickly, which could results in many frames wauting in the TX
queue. An example is a case when a big sub-tree consisting of many
nodes moves to you. Incoming traffic increases a lot, but you don't
have enough cells to handle. Negotiated TX cells are added one by one
with an interval determined by MAX_NUMCELLS. Then, latency from the
point of the topological change will get high.

By the way, the default or recommended value of MAX_NUMCELLS is 8?
MAX_NUMCELLS is not listed in Figure 3.

[question] Section 5.2

After a parent switch, which type of the negotiated cell should be
scheduled first, TX or RX? Any recommendation?

draft> 1.  the node counts the number of negotiated cells it has per
draft>     slotframe to the old preferred parent

draft> 2.  the node triggers one or more 6P ADD commands to schedule
draft>     the same number of negotiated cells with same cell options
draft>     to the new preferred parent

[clarification] Section 5.3

It would be better to mention explicitly RELOCATION for negotiated RX
cells are not supported. If MSF supports it, we need text describing
how.

[possible error?] Section 5.3

draft> preferred parent.  When NumTx reaches 256, both NumTx and
draft> NumTxAck

The recommended size of NumTx is 1 byte, the maximum number of which
is 255. See Section 15.

* [question] Section 6

draft> 6.  6P SIGNAL command
draft>
draft>    The 6P SIGNAL command is not used by MSF.

Why do we need this section? How about other commands, COUNT and LIST,
which are not mentioned in the draft at all?

If MSF doesn't support LIST, we don't need Section 8 and the rule for
RC_EOL. Section 8 is provided for LIST, according to Xavi:

 > >> What is Section 10, "Rule for Ordering Cells", for...? Why do we
 > >> need this section?
 > >>
 > >
 > > I'll let other co-authors answer
 > >
 >

 > XV> I think this was though to handle pagination when the LIST
 > command is received. This is, define what are the cells to return
 > when a list command is requesting cells from a particular offset.

See https://mailarchive.ietf.org/arch/msg/6tisch/3SrQj7LXzfFZUSEYasWcwGSrFRM

[technical] Section 12

draft> clear: Abort the 6P Transaction.  Issue a 6P CLEAR command to
draft>     that neighbor (this command may fail at the link layer).
draft>     Remove all cells scheduled with that neighbor from the
draft>     local schedule.  Keep that node in the neighbor and routing
draft>     tables.

Manipulating the routing table is not a job of MSF nor 6top. I'm not
sure what exactly the neighbor table is here, but I'd suggest removing
the last sentence.

* [nits/trivial suggestions]
- Abstract: s/MSF builds/MSF is built/ ?
- Section 1: "the root" --> "the DODAG root" or "the RPL DODAG root"
- Section 3: "time offset" --> "slot offset"
- Section 3: "the frame to be transmitted" --> a unicast frame ..."
- Section 5.1: dwonstream --> downstream
- Section 5.3: remove "(and need to be retransmitted)", it's redundant.
- WAITDURATION_MIN --> WAIT_DURATION_MIN (for readability)
- MAX_NUMCELLS --> MAX_NUM_CELLS (for readability)

Best,
Yatch