Re: [sfc] Éric Vyncke's Discuss on draft-ietf-sfc-ioam-nsh-11: (with DISCUSS and COMMENT)

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

Hi Eric,

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: Éric Vyncke via Datatracker <[e-mail]>
> Sent: Tuesday, 2 May 2023 08:47
> To: The IESG <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: Éric Vyncke's Discuss on draft-ietf-sfc-ioam-nsh-11: (with DISCUSS
> and COMMENT)
> 
> Éric Vyncke has entered the following ballot position for
> draft-ietf-sfc-ioam-nsh-11: Discuss
> 
> When responding, please keep the subject line intact and reply to all email
> addresses included in the To and CC lines. (Feel free to cut this introductory
> paragraph, however.)
> 
> 
> Please refer to
> https://www.ietf.org/about/groups/iesg/statements/handling-ballot-
> positions/
> for more information about how to handle DISCUSS and COMMENT
> positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-sfc-ioam-nsh/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thank you for the work put into this document.
> 
> Please find below one blocking DISCUSS points (easy to address), some
> non-blocking COMMENT points (but replies would be appreciated even if
> only for
> my own education), and some nits.
> 
> Special thanks to Greg Mirsky for the shepherd's detailed write-up including
> the WG consensus ***and*** the justification of the intended status.
> 
> I hope that this review helps to improve the document,
> 
> Regards,
> 
> -éric
> 
> # DISCUSS (blocking)
> 
> As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
> DISCUSS ballot is just a request to have a discussion on the following topics:
> 
> ## Section 3
> 
> This is probably due to my lack of knowledge about NSH... So, a simple
> discussion over email will probably be enough to clear my DISCUSS points.
> 
> RFC 9197 has an incremental tracing (section 4.4.1), how does it impact the
> length of the IOAM header in this case? Assuming that this header size is
> increased, I suggest to add some text about increasing the length field of
> IOAM
> header.

...FB: We added a sentence to section 3 to address this point:

   In case the IOAM Incremental Trace Option-Type is used, an SFC-aware
   node that serves as an IOAM transit node, needs to adjust the "IOAM
   HDR Len" field accordingly, see Section 4.4 in [RFC9197].

> 
> `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.` is actually a copy of RFC 8300 ```   Packets with
> Next
> Protocol values not supported SHOULD be silently dropped
>       by default, although an implementation MAY provide a configuration
>       parameter to forward them. ```
> and not a new behaviour. So, let's rather be clear and use a structure like
> "Per section 2.2 of RFC 8300, ..." followed by the RFC 8300 text. While very
> similar to Jim Guichard's DISCUSS point, it is related to another part of the
> document.
> 
...FB: We now make explicit reference to RFC 8300, Section 2.2 - in both cases:

The operator MUST ensure that SFC-aware
   nodes along the Service Function Path support IOAM, otherwise packets
   might be dropped (see Section 3 further below, as well as [RFC8300]
   Section 2.2).

.... and further below:

Per Section 2.2 of [RFC8300], packets with Next Protocol values not
   supported SHOULD be silently dropped by default.  Thus, when a packet
   with IOAM is received at an NSH based forwarding node such as an
   Service Function Forwarder (SFF) that does not support the IOAM
   header, it SHOULD drop the packet.



> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 
> # COMMENTS (non-blocking)
> 
> ## Section 1
> 
> Please expand SFF at first use.

...FB: Done.

> 
> ## Section 3
> 
> Please expand ESP (is it IPsec ESP ?) in the packet format.

...FB: The mention of IPsec ESP is a bit artificial in any case. We kept things generic with just using L2/L3 moving forward.

> 
> Should there be text about the absence of padding in the "IOAM Option and
> Optional Data Space" field (assuming that all IOAM options are always in
> 32-bit
> units)?

..:FB: The section got expanded with a note that there is no need for padding:

      IOAM Option and Data Space:  IOAM-Data-Fields as specified by the
         IOAM-Type field.  IOAM-Data-Fields are defined corresponding to
         the IOAM-Option-Type (e.g., see Section 4 of [RFC9197] and
         Section 3.2 of [RFC9326]) and are always aligned by 4 octets,
         thus there is no padding field.

> 
> ## Section 4
> 
> `IANA is requested to allocate protocol numbers for the following "NSH
> Next
> Protocol" related to IOAM` is underspecified. If it is about
> https://www.iana.org/assignments/nsh/nsh.xhtml#next-protocol, then
> let's be
> clear.

...FB: Updated accordingly.

> 
> Note: I intended to DISCUSS this point, but as Jim is already holding a
> DISCUSS
> on the same point, I will let him to be the owner.
> 
> # NITS (non-blocking / cosmetic)
> 
> A couple of missing hypens in "open source" or "8 bit field".

...FB: Those should be fixed. I hope I caught all of them.

Thanks again for your review.

Cheers, Frank

> 
>