Re: [sfc] Éric Vyncke's Discuss on draft-ietf-sfc-multi-layer-oam-26: (with DISCUSS and COMMENT)

[Greg Mirsky <[e-mail]>]

Hi Eric,
thank you for your thorough review and thoughtful comments. Please find my
responses below tagged GIM>>.

Regards,
Greg

On Sun, Jul 2, 2023 at 11:20 PM Éric Vyncke via Datatracker <
[e-mail]> wrote:

> Éric Vyncke has entered the following ballot position for
> draft-ietf-sfc-multi-layer-oam-26: 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-multi-layer-oam/
>
>
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
>
> # Éric Vyncke, INT AD, comments for draft-ietf-sfc-multi-layer-oam-26
>
> Thank you for the work put into this document.
>
> Please find below one blocking DISCUSS points (very easy to address), some
> non-blocking COMMENT points (but replies would be appreciated even if only
> for
> my own education).
>
> Other thanks to Carlos Bernardos, the Internet directorate reviewer (at my
> request), please consider this int-dir review:
>
> https://datatracker.ietf.org/doc/review-ietf-sfc-multi-layer-oam-26-intdir-telechat-bernardos-2023-06-28/
> (and I have read Greg's reply, still waiting for a revised I-D though)
>
GIM>> I'll be glad to upload the next version. Perhaps that can be done
right after your concerns get addressed.

>
> Special thanks to Don Eastlake for the shepherd's detailed write-up
> including
> the WG consensus and the justification of the intended status and the
> author
> counts.
>
> I hope that this review helps to improve the document,
>
> Regards,
>
> -éric
>
> # DISCUSS
>
> As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
> DISCUSS ballot is a request to have a discussion on the following topics:
>
> ## Section 6
>
> As there two V header fields defined (one for Echo message and one for all
> OAM
> messages), it is unclear whether the Echo V-field is set back to 0 when
> SFC OAM
> header Version is incremented? Or in other words, should Echo with V=0 but
> with
> OAM V != 0 be interpreted as specified in this document ?
>
GIM>> A very interesting scenario. The intention is to have independent
versioning of the SFC OAM Header and SFC Echo Request/Reply (as well as any
other active OAM protocol applied to SFC). Based on that, in the scenario
you describe, SFC Echo with V == 0 just be interpreted as defined in this
document. Would you suggest that this scenario be added to the description
of the Version field of SFC Echo Request/Reply as an example of the
relationship?
NEW TEXT:
      Versioning of SFC Echo Request/
      Reply is independent of the versioning of the SFC Active OAM
      header (Section 5).  For example, if a new SFC Active OAM header
      format with V = 1 is defined, an SFC Echo Request/Reply packet
      with V = 0 MUST be handled as described in this document.

>
> Are the TLV type depending on the Version being 0 ?
>
GIM>> It seems unnecessary. Do you think that that should be explicitly
noted in the document?

>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
>
> # COMMENTS
>
> ## Multi-layer ?
>
> I wonder what is "multi-layer" (per the I-D title) aspect in the
> specification.
> Suggest to either explain the "multi-layer" aspect or drop it from the
> draft
> title.
>
GIM>> At the very early stage, the title of the draft did include
"multi-layer". Later, it was changed to "Active OAM for Service Function
Chaining (SFC)". Would you suggest that we change from
draft-ietf-sfc-multi-layer-oam to, for example, draft-ietf-sfc-active-oam,
at this stage?

>
> ## Section 3
>
> Suggest to write that SFFI stands for SFF Instance in Figure 1.
>
GIM>> A good point, thank you. Added to the Acronyms:
SFI:        Service Function Instance
Updated the second para in Section 3 as follows:
OLD TEXT:
   The architecture example depicted in Figure 1 considers a service
   function chain that includes three distinct service functions.  In
   this example, the SFP traverses SFF1, SFF2, and SFF3.  Each SFF is
   connected to two instances of the same service function.
NEW TEXT:
    The architecture example depicted in Figure 1 considers a service
   function chain that includes three distinct service functions.  In
   this example, the SFP traverses SFF1, SFF2, and SFF3.  Each SFF is
   connected to two service function instances (SFIs) of the same
   service function.

>
> Isn't `FM SFC OAM` a pleonasm ? I.e., either "FM" or "OAM" are redundant.
>
GIM>> OAM is a more general term that includes Fault Management and
Performance Monitoring protocols.

>
> `(e.g., NSH)` why `e.g.` when section 1 states the focus of this I-D to
> NSH ?
>
GIM>>  Our intention was to define an active OAM that is applicable to
different SFC mechanisms and use SFC NSH as an example to illustrate the
realization of SFC Echo Request/Reply.

>
> About REQ#1, should this be a "MUST" rather than a "SHOULD" ?
>
GIM>> We need to consider that active SFC OAM excludes SFIs along the SFP1.
On the other hand, the application of the SFI to a user packet can be
re-classified and switched to another SFP2. To examine that SFP2, a
different active SFC OAM packet can be transmitted from that Re-classifier.
that is different from, for example, tracing in an ECMP environment and is
why we feel that "SHOULD" is appropriate rather than MUST.

>
> In REQ#8, `the initiator of such a request` should probably be more
> specific.
>
GIM>> Would the following update make the requirement more clear:
OLD TEXT:
      REQ#8: SFC OAM MUST be able to trigger on-demand FM with responses
      being directed towards the initiator of such a request.
NEW TEXT:
       REQ#8: SFC OAM MUST be able to trigger on-demand FM remotely with
      responses being directed toward the initiator of the remote
      request.


> ## Section 4
>
> Isn't RFC 8300 a better reference for the O bit rather than an IETF draft ?
>
GIM>> Med helped me answer this question.

>
> ## Section 5
>
> Please add a justification / actual data about `But extra IP/UDP headers,
> especially in an IPv6 network, add noticeable overhead.`
>
GIM>> I propose the following update of that sentence:
OLD TEXT:
   But extra IP/
   UDP headers, especially in an IPv6 network, add noticeable overhead.
NEW TEXT:
   But an extra
   IP/UDP header, especially in an IPv6 network, adds overhead compared
   to the length of an active OAM control packet (e.g., BFD Control
   packet [RFC5880]).  In some environments, for example, when measuring
   performance metrics, it is beneficial to transmit OAM packets in a
   broad range of lengths to emulate application traffic closer.

Is the updated text acceptable?

>
> I am really concerned by having only 2 bits to indicate the version while
> there
> are 8 bits reserved on the side. This does not look too good for revisions.
>
GIM>> A very valid and practical point, thank you. Would you agree to
increase the Version field to a four-bit length?

>
> When can an implementation deviate from the SHOULD in Sender's Handle: `The
> sender of the Echo Request SHOULD use a pseudo-random number generator ` ?
> While I understand that this is an opaque value, and using a random number
> prevents fingerprinting, I would assume that some implementations would
> like to
> add some private semantics to this number.
>
GIM>> Yes, an implementation may use the Sender's Handle for some
proprietary signaling as long as the system that receives SFC Echo Request
doesn't alter the value of the Sender's Handle field but copies it into SFC
Echo Reply. Do you feel that a note with that example needs to be added?

>
> ## Section 6
>
> Suggest adding some text about the absence of TLVs based on the length of
> the
> SFC OAM header.
>
GIM>> Would the following update match your expectation:
OLD TEXT:
    TLV is a variable-length construct whose length is multiple of four-
   octet words.  Multiple TLVs MAY be placed in an SFC Echo Request/
   Reply packet.  None, one or more sub-TLVs may be enclosed in the
   value part of a TLV, subject to the semantics of the (outer) TLV.
NEW TEXT:
   TLV is a variable-length construct whose length is multiple of four-
   octet words.  Multiple TLVs MAY be placed in an SFC Echo Request/
   Reply packet.  None, one or more sub-TLVs may be enclosed in the
   value part of a TLV, subject to the semantics of the (outer) TLV.  If
   no TLVs are included in an SFC Echo Request/Reply, the value of the
   Length field in the SFC Active OAM header MUST be 16 octets.

>
> ## Section 6.1
>
> To avoid confusion s/TTL Exceeded/NSH TTL Exceeded/ (even if explained
> later in
> the text).
>
GIM>> A good point. True, the proposed SFC Echo Request/Reply mechanism can
be used in SFC NSH, we believe that it can also be applied in, for example,
SFC based on RFC 8595 <https://www.rfc-editor.org/rfc/rfc8595.html>, would
s/TTL Exceeded/SFC TTL Exceeded/ be acceptable?

>
> In `The receiver of said Echo Request can set it to one of the values`
> should
> the "can" be replaced by a "MUST" ?
>
GIM>> Indeed, thank you for pointing this out. Done.

>
> ## Section 6.3
>
> `If the NSH is used,` but section 1 specifies that this I-D is only about
> NSH.
>
GIM>> Our intention was to define an Echo Request/Reply mechanism for any
technology that can realize SFC. NSH is one such technology. RFC 8595
defines how MPLS can be used to realize SFC. What would you suggest?

>
> `Reply via an IPv4/IPv6 UDP Packet` is unclear when reading it. Please add
> a
> forward reference to section 6.3.1 (I would also prefer to have 6.3.1
> earlier
> in the flow
>
GIM>> Added the reference as you suggested:
   *  Reply via an IPv4/IPv6 UDP Packet (2).  If an SFC Echo Request is
      not encapsulated in IP/UDP, then this value requests the use of
      the Source ID TLV Section 6.3.1.


> ## Section 6.5.2
>
> How `If the NSH of the received SFC Echo Request includes the MAC Context
> Header, the packet's authentication MUST be verified before using any
> data. `is
> different to the text of section 6.4 about the MAC ?
>
GIM>> Yes, it appears as unnecessary duplication. Would backreference to
Section 4 as follows to address your concern:

   If the NSH of the received SFC Echo Request includes the MAC Context
   Header, the packet's authentication MUST be verified before using any
   data as defined in Section 6.4.

>
> `Sequence Number in the Echo Request sent matches to the Sequence Number
> in the
> Echo Reply received` should it rather be a window of sequence number ?
> I.e.,
> pipelined requests could be sent.
>
GIM>> A very good point, thank you. Update as follows:
OLD TEXT:
      the Sequence Number in the Echo
      Request sent matches to the Sequence Number in the Echo Reply
      received.
NEW TEXT:
      the Sequence Number in the Echo Reply
      received matches the Sequence Number of one of outstanding
      transmitted Echo Requests.

>
> ## Section 6.6
>
> `Consistency Verification Request` this message is not defined before this
> section and not in RFC 8300. If specified in this section, may I suggest to
> rename the section to be about CVReq ?
>
GIM>> Thank you for the suggestion. Renamed the section to "The Use of
Consistency Verification Request Message"

>
> ## Section 7
>
> `if the underlay is an IPv6 network` what about IPv4 ? I would assume that
> IPv[46] underlays can support AH/ESP.
>
GIM>> IPv6 is used as an example. Of course, AH and ESP are equally
applicable in IPv4. Would you suggest re-wording like:
   If the underlay is an IPv4 or IPv6 network, IP Authentication Header
   [RFC4302] or IP Encapsulating Security Payload Header [RFC4303] can
   be used to provide integrity protection.

>
> Should this be a SHOULD in `an implementation MAY check that the source of
> the
> Echo Request is indeed part of the SFP`?
>
GIM>> In the course of the earlier discussion, that text got changed to the
following:
   To protect against unauthorized sources trying to obtain information
   about the overlay and/or underlay, an implementation MUST have means
   to check that the source of the Echo Request is part of the SFP.
I hope that this change addresses your concern.