Last Call Review of draft-ietf-sfc-ioam-nsh-11
review-ietf-sfc-ioam-nsh-11-tsvart-lc-kuehlewind-2023-04-19-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.

Thanks for this well written and clear document. I don't see any
transport-specific issues as this is only a new encapsulation into an existing
protocol. However, I have a few minor general comments below.

Some more general comments:
---
Sec 1:
" The IOAM-Data-Fields are defined in
   [RFC9197].  An implementation of IOAM which leverages NSH to carry
   the IOAM data is available from the FD.io open source software
   project [FD.io]."
Not sure if it is appropriate to refer to a single example implementation in
the intro.

Sec 3:
"The applicability of the IOAM Active and Loopback flags
   [I-D.ietf-ippm-ioam-flags] is outside the scope of this document and
   may be specified in the future."
I don't really understand why this sentence is here. I don't think it gives
anything to the reader.

Also Sec 3:
Maybe I'm misunderstanding something, or maybe this is okay but I would like to
double-check that this is not contradicting: "The operator MUST
   ensure that all nodes along the service path support IOAM."
and
"When a packet with IOAM is received
   at an NSH based forwarding node such as an Service Function Forwarder
   (SFF) that does not understand IOAM header, it SHOULD drop the
   packet."
because if all nodes are IOAM enables, this second condition should never
happen. I not proposing to remove the second sentence but maybe the first one
actually doesn't add much well (or not at least not be normative). Or it should
maybe say something like: "If not all notes a long path are IAOM enables, the
packet might be drop, there an operator needs to take care to configure all
nodes appropriately."...?

Editorial
---
Sec 3: "containing the different IOAM-Data-Fields."
Why different? Different from what? I guess you mean various? Maybe just drop
that word? -> "containing the IOAM-Data-Fields."