Re: [mpls] Requested review of draft-xp-mpls (draft-xp-mpls-spring-lsp-ping-path-sid)

[Tarek Saad <[e-mail]>]

Thanks Loa and authors. I’m following up to ensure the comments raised by Loa as blockers to WG adoption were fully addressed.
Can you please confirm?

Regards,
Tarek (as MPLS WG chair)

From: [e-mail] <[e-mail]>
Date: Thursday, January 25, 2024 at 7:18 AM
To: "[e-mail], [e-mail], cpignata"@gmail.com <"[e-mail], [e-mail], cpignata"@gmail.com>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Requested review of draft-xp-mpls
Authors,

The working group chairs (via Tarek) asked me to review this draft prior
to the WGAP.

The chairs asked me to consider two questions:

Is this in the scope of the WG?
-------------------------------

I think it is "the charter says "Evolve key MPLS protocols, including LDP,
tLDP, mLDP, RSVP-TE for packet networks, and LSP Ping to meet new
requirements." This is clearly evolving LSP Ping for new requirements.

Should the working group take this work on?
-------------------------------------------
Yes - his is clearly needed and may be viewed as part of the cooperation
with the SPRING working group mentioned in the MPLS charter.

General
-------

The document is clear and well-written, especially the description of the
sub-TLVs in section 4.

I think we can go ahead and start the WGAP, after a slight update.


I have grouped the rest of my comments into three groups:

1. To be fixed before the WGAP.
===============================

Note: Sometimes I put a comment in this group for no other reason than
that it is easy to fix, comments in this group are also not necessarily
blocking a working group adoption, if I think they are I say so.

1.1 Abstract
------------
SR is not a well-known abbreviation (see
https://www.rfc-editor.org/materials/abbrev.expansion.txt), you need to
expand it in the Abstract. Maybe we should tell the SPRING chairs to do
something about it. However, there is a problem SR may be expanded in more
than one way.

1.2 Introduction and Section 3
------------------------------

There are some inconsistency:

Target Forwarding Equivalence Class (FEC) stack TLV (Introduction)
                                          ^
Target FEC Stack TLV (Section 3)
           ^

1.3 Return Subcode
------------------

You use RSC for return subcode, but does never expand it.

1.4 Section 4.
--------------

In the first paragraph you say:

"The MPLS LSP Ping procedures MAY be initiated by the headend..."

I doubt that it can be started from the headend, but is the requirement
language really needed in this draft??

1.5 Section 4
-------------

In section, discussing  "validity checks", you have three statements of the
format:

       Length = 12 or 36

I think it would be clearer if you said

       Length = 12 or 36 octets

1.6 Security Considerations

The section says:

   This document defines additional MPLS LSP Ping sub-TLVs and follows the
mechanisms defined in [RFC8029].  All the security considerations
defined in [RFC8029] will be applicable for this document and, in
addition, they do not impose any additional security challenges to be
considered.

I assume that "they" in "they do not" says "the MPLS Ping sub-TLVs defined
in this document do not".



2. To be fixed before WGLC

2.1. Abstract
-------------

I find the Abstract a bit too thin, only 3 lines and does not give me
enough to understand what the draft is about.

As a rule of thumb, I have said that if you go back to your immediate
manager when you started to do IETF work, he should be able to understand
what the draft is about from the Abstract :).

2.2 Introduction
------------
Do we have a good reference to Target Forwarding Equivalence Class?

2.x IANA Considerations

The registry that you are requesting  your three new Sub-TLVs should be
assigned from has 8 different ranges, you need to pick one per sub-TLV
and indicate that as the range you want it to be assigned from.


3. Questions and suggestions

3.1 Section 2.2
-----------

You mention that the reader should be  familiar with the terminology form
RFC 8029 and RFC 8402, which is fine. Shouldn't RFC 3031 be mentioned
also?

3.2 Section 3
-------------

In section 3 you say:

  "As specified in Section 2 of [I-D.ietf-spring-mpls-path-segment], a
   Path Segment can be used to identify a Segment List, some or all
Segment lists in a Candidate path or an SR policy, so three different
Target FEC sub-TLVs need to be defined for Path Segment ID."

I understand that as you create a common name for your new Path Segment ID
Sub-TLVs, i.e. Target FEC sub-TLVs. Should that name be "Target FEC Stack
Sub-TLV"?

3.3 Grammar
-------

This is grammar and it is not my cup tea (especially not English grammar :).

In section 3 you say (and there are variants on it):

   When a
   Path Segment is used to identify an SR Policy, the Target FEC sub-TLV
of SR Policy's Path SID would be used to validate the control plane to
forwarding plane synchronization for this Path-SID

Should tat be?

   When a
   Path Segment is used to identify an SR Policy, the Target FEC sub-TLV
of the sub-type "SR Policy's Path SID" would be used to validate the
control plane to forwarding plane synchronization for this Path-SID"

Or maybe sub-type is overkill and we can do with:

   When a
   Path Segment is used to identify an SR Policy, the Target FEC sub-TLV
of the type "SR Policy's Path SID" would be used to validate the
control plane to forwarding plane synchronization for this Path-SID

/Loa