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

[Tengfei Chang <[e-mail]>]

Hi Yatch,

Thanks a lot for the comments! I will response inline!

On Tue, Nov 26, 2019 at 3:21 PM Yasuyuki Tanaka <[e-mail]>
wrote:

> 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".
>
>
TC: Agree!


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

TC: Agree!  Will update the text accordingly.

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

TC: for the former case,
the length of slotframe 0 could change the bandwidth of the EB to sent,
which could be helpful to be customizable.
the length of slotframe 1 could influence the hash function performance,
such as reducing the collision.
the length of  slotframe 2 changes application traffic bandwidth resolution.

However, those are out-of-scope of MSF.
The same length of all slotframes avoids the collision of cell allocation,
which benefit the performance of MSF. So it's a recommend way to do.

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

TC: I think the reason why we use the keep-alive is because it's already
explained in the standard and we don't need to redefine anything.
Using 6P COUNT sounds better than the keep-alive, but then we need to
define how often the 6P COUNT is going to send to the neighbor, which may
need additional discussion.

TC: I tend more to state the issue and declare some mechanism to resolve is
required but not provide one. (your first solution.)

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


TC: this is the desired design, agreed that it's not clear. Will update the
section.

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

TC: totally agree!

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


TC: Since the MSF installs additional cells for the traffic, which helps
when traffic changes slowly.


> An example is a case when a big sub-tree consisting of many
> nodes moves to you.


TC: The original sentense is saying for periodic traffic type. This is
considered as bursty traffic.


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

TC: 8 is just a value in the example, not recommended. For periodic
traffic, or traffic load changes slowly, a larger MAX_NUMCELLS is preferred.
Some proposal for the recommended value of MAX_NUMCELLS: 64 (power of 2),
or 100.

>
> [question] Section 5.2
>
> After a parent switch, which type of the negotiated cell should be
> scheduled first, TX or RX? Any recommendation?
>

TC: That depends on which has a high priority, the upstream traffic or
downstream traffic.  It should be applications-specific.

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

TC: agreed!

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

TC: thanks for pointing this out!

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

TC: we only mentioned 6P SIGNAL, because that's 6P leaves it to scheduling
function, which must mention how it is used.
TC: For consistency, yes, maybe state COUNT and LIST are not used as well.

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


TC: (personally thought) scheduling function doesn't need to use all the 6P
commands but, when it receives, the node will response still as part of 6P
(not MSF, I agree).
TC: The order of cells in the 6P LIST response, for example, could be
defined by MSF as a certain service. (such as during debugging, I am aware
different SF communicate ends with errors)
TC: I am slightly on keeping it but not strong option.

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

TC: agreed!

>
> * [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)
>

TC: all accepted! Thanks a lot!

>
> Best,
> Yatch
>
> _______________________________________________
> 6tisch mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/6tisch
>


-- 
——————————————————————————————————————

Dr. Tengfei, Chang
Postdoctoral Research Engineer, Inria

www.tchang.org/
——————————————————————————————————————