Re: [sfc] Tsvart last call review of draft-ietf-sfc-ioam-nsh-11

["Frank Brockners (fbrockne)" <[e-mail]>]

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 <[e-mail]>
> Sent: Wednesday, 19 April 2023 16:00
> To: [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]
> 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-
> [e-mail] 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
>