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

[Greg Mirsky <[e-mail]>]

Hi Eric,
upon further consideration, the authors decided:

   - to remove the Version field from the SFC Echo Request/Reply message
   - to extend the Version field in the SFC Active OAM reader to a four-bit
   length.

Attached, please find the updated versions of the working document and its
diff with the previous version (-26).

Regards,
Greg

On Tue, Jul 4, 2023 at 6:21 PM Greg Mirsky <[e-mail]> wrote:

> 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.
>