Re: [Last-Call] Tsvart last call review of draft-ietf-sfc-multi-layer-oam-23

[Greg Mirsky <[e-mail]>]

Hi Oliver,
I greatly appreciate your thoughtful questions and comments. Please find my
notes inline below tagged with GIM>>. Also, attached are the diff and the
new working version (please note that updates reflect discussions
with reviewers from RtgDir and TsvArt).

Regards,
Greg

On Mon, May 22, 2023 at 2:27 AM Olivier Bonaventure via Datatracker <
[e-mail]> wrote:

> Reviewer: Olivier Bonaventure
> Review result: Ready with Nits
>
> 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
> [e-mail] if you reply to or forward this review.
>
> This review looked specifically at the transport related issues. The
> reviewer
> did not find any specific transport issue beyond those related to the base
> SFC
> and NSH specification. However, while reading the document, there are some
> parts which could be clearer.
>
> Section 5. It is a bit strange to have a version number in the SFC OAM
> control
> packet and then again a version number in the Echo message. I failed to
> see the
> rationale for these two levels of version numbers.
>
GIM>> Figure 2 displays the SFC Active OAM Header with its versioning. In
fact, this header can be used as SFC NSH Associated Channel Header, similar
to one defined for PW in RFC 5085 <https://www.rfc-editor.org/rfc/rfc5085>.
It is possible that a different format of this header could be defined in
the future (similar to DetNet MPLS d-ACH using ACH's versioning mechanism).
Would re-naming Figure 2 be helpful?
Figure 3 displays the SFC Echo Request/Reply message with its versioning
mechanism that could be helpful if a new format of the message is defined.

>
> In Figure 2, there is a set of 8 bits Flags which is indicated, but no
> flag is
> defined. Why not simply indicating that this field is reserved (must be
> zero
> when transmitted and ignored on reception) ?
>
GIM>> Thank you for the suggestion. Accepted. As a result of this change, I
removed the request for IANA to create in the SFC Active OAM Registry the
sub-registry SFC Active OAM Flags.

>
> In Figure 3, the sequence number is encoded in 32 bits, but the document
> does
> not specify that it contains an unsigned integer that wraps around. This
> should
> be indicated since the initial sequence number must be pseudo randomly
> generated.
>
GIM>>  Thank you for the suggestion. I updated the text as follows:
OLD TEXT:
      The Sequence Number is a four-octet field, and it is assigned by
      the sender and can be, for example, used to detect missed replies.
      The initial Sequence Number MUST be pseudo-randomly generated
      [RFC4086] and then SHOULD be monotonically increasing in the
      course of the test session.
NEW TEXT:
      The Sequence Number is a four-octet field, and it is assigned by
      the sender and can be, for example, used to detect missed replies.
      The initial Sequence Number contains an unsigned integer that
      wraps around.  It MUST be pseudo-randomly generated [RFC4086] and
      then SHOULD be monotonically increasing in the course of the test
      session.
I that acceptable?

>
> In Figure 3, the role of the Echo requests flags is unclear.
>
GIM>> Updated the definition of the Echo Request Flags field as follows:
      The Echo Request Flags is a two-octet bit vector field.
      Section 10.3.1 requests IANA to create a new registry for flags.
      This specification defines all flags for future use.  Flags
      MUST be zeroed on transmission and ignored on receipt.

>
> Figure 4 shows that the SFC Echo Rquest/Reply TLV as a multiple of 32
> bits. Is
> this a requirement (all TLV must end at 32 bits boundaries or not) ? This
> is
> not clear from the text.
>
GIM>> Thank you for pointing that out. Added a note about the four-octet
alignment. I hope it is clearer now:

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 the Source TLV is used, it is not clear from the document how the
> return UDP
> packet will be composed and what information it will contain. An example
> could
> be useful.
>
GIM>> Would the following update make the description more helpful:
OLD TEXT:
    A single Source ID TLV for each address family, i.e., IPv4 and IPv6,
   MAY be present in an SFC Echo Request message.  If the Source TLVs
   for both address families are present in an SFC Echo Request message,
   the SFF MUST NOT replicate an SFC Echo Reply but choose the
   destination IP address for the one SFC Echo Reply it sends based on
   the local policy.  The value of the Port Number field MUST be used as
   the destination UDP port number in the IP/UDP encapsulation of the
   SFC Echo Reply message.  If more than one Source ID TLV per the
   address family is present, the receiver MUST use the first TLV and
   ignore the rest.
NEW TEXT:

   A single Source ID TLV for each address family, i.e., IPv4 and IPv6,
   MAY be present in an SFC Echo Request message.  If the Source TLVs
   for both address families are present in an SFC Echo Request message,
   the SFF MUST NOT replicate an SFC Echo Reply but choose the
   destination IP address for the one SFC Echo Reply it sends based on
   the local policy.  The source IP address used in the IP/UDP
   encapsulation of SFC Echo Reply is one of the IP addresses associated
   with the responder.  The value of the Port Number field MUST be used
   as the destination UDP port number in the IP/UDP encapsulation of the
   SFC Echo Reply message.  The responder selects the source UDP port
   number from the dynamic range of port numbers.  If more than one
   Source ID TLV per the address family is present, the receiver MUST
   use the first TLV and ignore the rest.  The Echo Reply message,
   including relevant TLVs, follows the IP/UDP headers immediately.