Hi Mirja, Thanks a lot for your review. We've just posted an updated revision (12): https://www.ietf.org/archive/id/draft-ietf-sfc-ioam-nsh-12.txt Please see inline below: > -----Original Message----- > From: Mirja Kühlewind via Datatracker <noreply@xxxxxxxx> > Sent: Wednesday, 19 April 2023 16:00 > To: tsv-art@xxxxxxxx > Cc: draft-ietf-sfc-ioam-nsh.all@xxxxxxxx; last-call@xxxxxxxx; sfc@xxxxxxxx > Subject: Tsvart last call review of draft-ietf-sfc-ioam-nsh-11 > > Reviewer: Mirja Kühlewind > Review result: Ready > > 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@xxxxxxxx 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. ...FB: John had a similar comment. We removed the reference to the open source implementation in revision -12. > > 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. ...FB: There has been quite a bit of debate in the working group about the use of IOAM active flags, the O-bit in NSH etc. The above sentence was a consequence of this discussion. IMHO it is worthwhile to keep it for those who participated in the discussions. > > 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."...? ...FB: True that it is a bit of belts and suspenders. The drop behavior is due to RFC 8300, section 2.2 - which the document now explicitly refers to (and even reproduces the associated text). In order to have this drop behavior not kick-in, the logic is that the operator MUST configure the devices accordingly. > > 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." ...FB: Fixed. Thanks again for your review. Cheers, Frank > -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
