Re: [RTG-DIR] [mpls] Rtgdir early review of draft-ietf-mpls-rfc6374-sr-08

Hi Jeffrey,

I’ve been assigned as the new document shepherd for  draft-ietf-mpls-rfc6374-sr.

AFAICT, Rakesh has responded to your comments and published revision -09.  Does this address your issues?

Thanks,
Tony

> On Feb 20, 2024, at 7:27 AM, Tarek Saad <tsaad.net@gmail.com> wrote:
> 
> Thank you Jeffrey for the review and comments and to the authors for trying to address them.
> Was there other outstanding issues that still need follow-up? If not, can the review result state be updated. Thanks.
>  
> Regards,
> Tarek (as document Shepherd)
>  
>  
> From: Rakesh Gandhi (rgandhi) <rgandhi@cisco.com>
> Date: Friday, February 9, 2024 at 1:20 PM
> To: Zhaohui Zhang <zzhang@juniper.net>, rtg-dir@ietf.org <rtg-dir@ietf.org>
> Cc: mpls@ietf.org <mpls@ietf.org>, Tarek Saad <tsaad.net@gmail.com>
> Subject: Re: Rtgdir early review of draft-ietf-mpls-rfc6374-sr-08
> 
> Thanks, Jeffrey, for the detailed review. Apology for the delay in replying.
>  
> We have posted a revised draft with suggested changes.
> 
> HTML:     https://www.ietf.org/archive/id/draft-ietf-mpls-rfc6374-sr-09.html
> Diff:     https://author-tools.ietf.org/iddiff?url2=draft-ietf-mpls-rfc6374-sr-09
>  
> Please see in line with replies tagged with <RG>….
>  
>  
> From: Zhaohui Zhang via Datatracker <noreply@ietf.org>
> Date: Tuesday, August 29, 2023 at 4:50 PM
> To: rtg-dir@ietf.org <rtg-dir@ietf.org>
> Cc: draft-ietf-mpls-rfc6374-sr.all@ietf.org <draft-ietf-mpls-rfc6374-sr.all@ietf.org>, mpls@ietf.org <mpls@ietf.org>
> Subject: Rtgdir early review of draft-ietf-mpls-rfc6374-sr-08
> 
> Reviewer: Zhaohui Zhang
> Review result: Has Issues
> 
> General comment:
> 
> The document name and title/subject (including the short title at each
> pager header) should have RFC6374 removed. It's better to be named as
> "Performance Measurement for SR-MPLS", because it is not only using
> RFC6374 but also 7876 and 9341.
> 
> <RG> Updated.
> 
> Some specific commnets:
> 
>    [RFC6374] specifies protocol mechanisms to enable the efficient and
>    accurate measurement of performance metrics in MPLS networks using
>    query and response messages.  [RFC7876] specifies mechanisms for
>    sending and processing out-of-band responses over an UDP return path
>    when receiving RFC 6374 based query messages.  These mechanisms are
>    also well-suited in SR-MPLS networks.
> 
>    This document utilizes the mechanisms defined in [RFC6374] for
>    Performance Delay and Loss Measurements in SR-MPLS networks, for both
>    SR-MPLS links and end-to-end SR-MPLS paths including Policies.  In
>    addition, this document defines Return Path TLV and Block Number TLV
>    extensions for [RFC6374].
> 
> Should mention RFC7876 and RFC9341 as well.
> 
> <RG> Updated.
> 
>    For delay and loss measurement in SR-MPLS networks, the procedures
>    defined in [RFC6374] are used in this document.  Note that the one-
>    way, two-way and round-trip delay measurements are defined in
>    Section 2.4 of [RFC6374] and are further described in this document
>    for SR-MPLS networks.  Similarly, the packet loss measurement is
>    defined in Section 2.2 of [RFC6374] and is further described in this
>    document for SR-MPLS networks.
> 
> My personal preference is not to repeat what is already described/specified
> in RFC6374. An implementation can't solely rely on this document but need
> to refer to RFC6374 and other RFCs anyway, so this document can just refer
> to those and point out differences. This will highlight
> the differences and reduces the chance of copy and paste errors - a good
> thing for authors, reviewers and implementors.
> 
> <RG> Updated to remove various repetitions.
> 
> I did not try to match the description between this document and the "base"
> RFCs. So far I have not seen many differences besides the following:
> 
> - TTL
> - Return Path TLV, presumably only applicable to SR-MPLS
> - Block Number TLV, presumably not specific to SR-MPLS
> - SR-P2MP
> 
> Am I missing anything?
> 
> <RG> FYI, the draft also describes:
> 
> the one-way/two-way/LB modes as applied to SR-MPLS.
> destination and source TLVs as applied to SR-MPLS.
> Path segment for loss.
> 
>    SR is enabled with MPLS data plane on nodes Q1 and R1.  The nodes Q1
>    and R1 may be directly connected via a link enabled with MPLS
>    (Section 2.9.1 of [RFC6374]) or a Point-to-Point (P2P) SR-MPLS path
>    [RFC8402].  The link may be a physical interface, virtual link, or
>    Link Aggregation Group (LAG) [IEEE802.1AX], or LAG member link.  The
>    SR-MPLS path may be an SR-MPLS Policy [RFC9256] on node Q1 (called
>    head-end) with destination to node R1 (called tail-end).
> 
> The paragraph mentions "link". Is that the same as the "SR-MPLS link" used
> throughout the document? Is there a difference between "MPLS link" and
> "SR-MPLS" link when it comes to performance measurement?
> 
> <RG> Updated to say link.
> 
> 
>    It may be desired in SR-MPLS networks that the same path (same set of
>    links and nodes) between the querier and responder be used in both
>    directions of the measurement.  This is achieved by using the SR-MPLS
>    Return Path TLV extension defined in this document.
> 
> Should "It may be ..." be changed to "If it is ..."? I don't see justification
> for "it may be desired".
> 
> <RG> Updated.
> 
> 
>    The packet loss measurement using Alternate-Marking Method defined in
>    [RFC9341] requires collecting Block Number of the traffic counters.
>    This is achieved by using the Block Number TLV extension defined in
>    this document.
> 
> In RFC9341, the wording is MAY not MUST or "required":
> 
> <RG> Updated.
> 
> 
>    The Alternate-Marking Method described in this document literally
>    splits the packets of the measured flow into different measurement
>    blocks.  An implementation MAY use a Block Number (BN) for data
>    correlation.
> 
> Is it that Block Number TLV extension defined here is not SR-MPLS specific?
> Better to clarify.
> 
> <RG> Updated for MPLS.
> 
>    A query message as shown in Figure 2 is sent over the SR-MPLS links
>    for both delay and loss measurement using the procedures described in
>    [RFC6374].  For SR-MPLS links, the TTL value MUST be set to 255 in
>    the SR-MPLS header.  SR-MPLS encapsulation (e.g., using adjacency SID
>    of the link) can be added for transmitting the query messages for SR-
>    MPLS links.
> 
> Can you elaborate how adjacency SID can/should be used? If Q1 and R1 are
> "directly" connected via an (SR-)MPLS link, do you need any MPLS
> encapsulation, and in particular what adjacency SID does Q1 use?
> 
> <RG> Updated to remove it.
> 
> 
>    Note that in addition to the P2P SR-MPLS paths, the SR-MPLS Segment
>    List Sub-TLV is also applicable to the P2MP SR-MPLS paths.  For
>    example, for P2MP SR-MPLS paths, it may only carry the Node Segment
>    Identifier of the querier in order for the response to follow an SR-
>    MPLS path back to the querier.
> 
> Shojld the "may only carry" be "MUST only carry"?
> 
> <RG> Updated to remove it.
> 
> 
> SR-P2MP policy documents are not mature yet. draft-ietf-pim-sr-p2mp-policy
> is only about general concept and  draft-ietf-idr-sr-p2mp-policy is only at
> -00 revision - not ready for WGLC. It's best to leave P2MP outside the scope
> of this document.
> 
> <RG> Updated to remove it.
> 
> Thanks,
> 
> Rakesh
> 
>  
> 
> _______________________________________________
> mpls mailing list
> mpls@ietf.org
> https://www.ietf.org/mailman/listinfo/mpls