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

["Eric Vyncke (evyncke)" <[e-mail]>]

Hello Greg

Thank you for your prompt reply, very much appreciated.

As you will see below, the proposed changes (and the one in your other email -- even better) will address my blocking DISCUSS point. I.e., as soon as a revised I-D with the changes is uploaded, I am clearing my DISCUSS into a NOOBJ.

See below for EV>

Regards

-éric

From: iesg <[e-mail]> on behalf of Greg Mirsky <[e-mail]>
Date: Wednesday, 5 July 2023 at 03:33
To: Eric Vyncke <[e-mail]>
Cc: The IESG <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>
Subject: Re: Éric Vyncke's Discuss on draft-ietf-sfc-multi-layer-oam-26: (with DISCUSS and COMMENT)

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]<mailto:[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.

EV> nothing urgent, let's indeed focus on the DISCUSS, then the COMMENTs


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.

EV> this new text, or even the more radical change in your other email, are enough to address the above DISCUSS.




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?

EV> this would be nicer I think, when rereading myself, this should have been a non-blocking COMMENT though.


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

EV> thanks for the explanation, let's no change the filename now as it is too late


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

EV> understood based on your previous reply above, still suggest to remove '(e.g., NSH)' as the I-D is only about NSH


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.

EV> fair enough

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.

EV> perfect, thanks


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

EV> fair enough as well and thanks to Med ;-)


## 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?

EV> I would have prefer to put byte counts and compare IPv4/UDP and IPv6/UDP wrt the actual OAM packets


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?

EV> Much better, it even matches the 4-bit version field in IPv[46] header


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?

EV> it was mainly for my own education to be honest. I.e., up to the authors to add some text.


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

EV> thanks


## 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?

EV> better indeed


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?

EV> suggest removing 'IF the NSH is used' completely as it is redundant with the abstract/intro


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

EV> perfect


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

EV> perfect


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

EV> better


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.

EV> yes, thank you