Last Call Review of draft-ietf-6lo-minimal-fragment-07
review-ietf-6lo-minimal-fragment-07-tsvart-lc-ott-2020-01-30-00

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@ietf.org if you reply to or forward this review.


This document defines a mechanism that allows stateful forwarding of 6LoWPAN
fragments at intermediate nodes without prior reassembly. One purpose is to reduce
the need for reassembly buffers in resource-constrained nodes. To this end, a 
forwarding node will create forwarding state upon receipt of the first fragment, 
using the pair (previous hop's MAC address, datagram_tag) as a forwarding label
against which fragments are matched. The routing decision is taken based upon
the first fragment and then applied to all subsequent ones using the above "label".

Generally, the document is in good state and almost ready to publish. A few nits,
one on protocol state management below:

Nits:

The text uses both "6lo fragments" and "6LoWPAN fragments".

Section 5, end of 1st para:
"Since the datagram_tag is uniquely associated to the source Link-Layer address
of the fragment, the forwarding node MUST assign a new datagram_tag from its
own namespace for the next hop and rewrite the fragment header of each
fragment with that datagram_tag."

This sentence is correct but it comes after the description of handling subsequent
fragments, rather than the first one, and subsequent ones should, of course, not
receive a new datagram_tag. Maybe move the sentence further up or make explicit
reference to the first fragment.

Sect. 5, bullet list, "* a datagram_size"
Just as a remark, this does not seem to be used. It *could* be used to check if a
fragment beyond the end of the packet arrives, but has otherwise no (documented)
meaning. The draft should spell out its purpose.

Sect. 5, bullet list, "a timer that allows discarding the stale FF state after some timeout"
Surely needed, but no advice is given. There is generally no explicit statement when to
discard the state. What should an implementation do to interoperate, given that
upstream multiplexing of packet fragments from multiple sources can yield diverse
intervals between consecutive fragments? Would this be the same timer previously
used for reassembly? If so, maybe just state this.

Sect. 7, 2nd bullet: "attck" -> "attack"