[sfc] Tsvart last call review of draft-ietf-sfc-multi-layer-oam-23

[Olivier Bonaventure via Datatracker <[e-mail]>]

Reviewer: Olivier Bonaventure
Review result: Ready with Nits

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
[e-mail] if you reply to or forward this review.

This review looked specifically at the transport related issues. The reviewer
did not find any specific transport issue beyond those related to the base SFC
and NSH specification. However, while reading the document, there are some
parts which could be clearer.

Section 5. It is a bit strange to have a version number in the SFC OAM control
packet and then again a version number in the Echo message. I failed to see the
rationale for these two levels of version numbers.

In Figure 2, there is a set of 8 bits Flags which is indicated, but no flag is
defined. Why not simply indicating that this field is reserved (must be zero
when transmitted and ignored on reception) ?

In Figure 3, the sequence number is encoded in 32 bits, but the document does
not specify that it contains an unsigned integer that wraps around. This should
be indicated since the initial sequence number must be pseudo randomly
generated.

In Figure 3, the role of the Echo requests flags is unclear.

Figure 4 shows that the SFC Echo Rquest/Reply TLV as a multiple of 32 bits. Is
this a requirement (all TLV must end at 32 bits boundaries or not) ? This is
not clear from the text.

If the Source TLV is used, it is not clear from the document how the return UDP
packet will be composed and what information it will contain. An example could
be useful.