[Gen-art] Genart last call review of draft-ietf-spring-mpls-path-segment-08

Stewart Bryant via Datatracker <noreply@ietf.org> Mon, 21 November 2022 13:57 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: gen-art@ietf.org
Delivered-To: gen-art@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id A221EC15270D; Mon, 21 Nov 2022 05:57:53 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Stewart Bryant via Datatracker <noreply@ietf.org>
To: gen-art@ietf.org
Cc: draft-ietf-spring-mpls-path-segment.all@ietf.org, last-call@ietf.org, spring@ietf.org, spring-chairs@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 9.1.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <166903907365.63514.883033837926360253@ietfa.amsl.com>
Reply-To: Stewart Bryant <stewart.bryant@gmail.com>
Date: Mon, 21 Nov 2022 05:57:53 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/rd0KTJzzs_10IjKwQy_kbNPJcaw>
Subject: [Gen-art] Genart last call review of draft-ietf-spring-mpls-path-segment-08
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.39
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 21 Nov 2022 13:57:53 -0000

Reviewer: Stewart Bryant
Review result: Not Ready

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-spring-mpls-path-segment-??
Reviewer: Stewart Bryant
Review Date: 2022-11-21
IETF LC End Date: 2022-11-20
IESG Telechat date: Not scheduled for a telechat

Summary: This document is not ready to progress. If it has not yet had one it
could usefully be reviewed by the MPLS Review Teams as this is clearly MPLS
technology that needs to validated by MPLS specialists. In my opinion the
document really ought to go back to the Spring WG to have a number of issues
resolved before the other review teams as asked to spend time on it.

Major issues:

   A Path Segment is a single label that is assigned from the Segment
SB> I think that should be a Path Segment Identifier, or a Path Segment Label
or a P-SID since a path segment is a subset of the path so the terms path
segment without qualification is confusing and aliases with other use of the
term, particularly in the transport network space.

=========

   When a Path Segment is used, the Path Segment MUST be inserted at the
   ingress node and MUST immediately follow the last label of the SR
   path, in other words, inserted after the routing segment
   (adjacency/node/prefix segment) pointing to the egress node.
SB> What happens if the payload is not IP - is the PSL before the PW (or other
payload) label?

=======

When Path Segment is allocated
   from the SRGB pool, an intermediate node MAY see the Path Segment
   label at the top of the label stack.
SB> Just checking - is it carried as it native value?
SB> If an intermediate node is looking at the PSL how does it know the context
of the label to know that it is a PSL and not some other label? SB> If the
label is the base label how do you deal with the problem that some hardware is
unable to process parts of the 20bit label space? SB> If we are going to
provide this as an option the text needs to be specific on the elements of
procedure

======

   A Path Segment can be used in the case of Penultimate Hop Popping
   (PHP), where some labels are be popped off at the penultimate hop of
   an SR path, but the Path Segment MUST NOT be popped off until it
   reaches at the egress node.

SB> What happens at the penultimate node - is it exposed there as a result of
PHP, and is it passed on as its native value or is the base added to it before
it is passed across?
=========
   Generic Associated Label (GAL) is used for Operations, Administration
SB: I think that should be "(GAL) MAY be used". A PW running in an MPLS network
can use the the CW first nibble 1, ACH mechanism.

========

SB> Section (3) is out of place WRT the rest of the text. Everything else is
data plane specific and control plane agnostic. I would suggest that the
control plane is out of scope for the RFC and you should simply say that a
method needs to be provided for the PSL imposition node to know what label to
use and that this label MUST be unique on the node stripping and processing the
PSL. I think information that belongs later in the text.

=========

SB> The following text is very hard to read

  BSID and Path SID (PSID) can be combined to achieve both sub-path and
   end-to-end path monitoring.  A reference model for such a combination
   in (Figure 2) shows an end-to-end path (A->D) that spans three
   domains (Access, Aggregation and Core domain) and consists of three
   sub-paths, one in each sub-domain (sub-path (A->B), sub-path (B->C)
   and sub-path (C->D)).  Each sub-path is allocated a BSID.  For
   nesting the sub-paths, each sub-path is allocated a PSID.  Then, the
   SID list of the end-to-end path can be expressed as <BSID1, BSID2,
   ..., BSIDn, e-PSID>, where the e-PSID is the PSID of the end-to-end
   path.  The SID list of a sub-path can be expressed as <SID1, SID2,
   ...SIDn, s-PSID>, where the s-PSID is the PSID of the sub-path.

SB> Doesn't this require that the (single) PSL is unique at all BSID end
points. I think that needs some discussion, such as whether you swap the PSL
when you enter the new segment and whether the PSL is its native value of the
locally resolved value and if the target locally resolved value how you know
what to swap it to.

=======

   Figure 2 shows the details of the label stacks when PSID and BSID are
   used to support both sub-path and end-to-end path monitoring in a
   multi-domain scenario.

SB> I think that there needs to be some consolidation of the terminology
between path segment and PSID since I think you are the term interchangeably.
SB> I am also concerned about SR developing a parallel universe with SR
specific approaches that apply to the general MPLS case, but using terms that
are not easily back ported into MPLS.

===========

   As defined in [RFC7799], performance measurement can be classified
   into Passive, Active, and Hybrid measurement.  Since Path Segment is
   encoded in the SR-MPLS Label Stack as shown in Figure 1, existing
   implementation on the egress node can be leveraged for measuring
   packet counts using the incoming SID (the Path Segment).  A different
   scheme such as carrying such identifier after the bottom of the label
   stack may require new implementation.
SB> That statement lacks detail and is speculative.

============

   For Passive performance measurement, path identification at the
   measuring points is the pre-requisite.  Path Segment can be used by
   the measuring points (e.g., the ingress and egress nodes of the SR
   path or a centralized controller) to correlate the packet counts and
   timestamps from the ingress and egress nodes for a specific SR path,
   then packet loss and delay can be calculated for the end-to-end path,
   respectively.
SB> Did you discuss the effect of ECMP if not all packets contain a PSID or you
turn the instrumentation on and off?

==========

   Path Segment can also be used for Active performance measurement for
   an SR path in SR-MPLS networks for collecting packet counters and
   timestamps from the egress node using probe messages
   [I-D.ietf-mpls-rfc6374-sr] and [I-D.ietf-spring-stamp-srpm].

SB> Again out old friend ECMP needs a discussion

============

   Path Segment can also be used for In-band PM for SR-MPLS to identify
   the SR Path associated with the collected performance metrics
   [I-D.ietf-mpls-inband-pm-encapsulation].
SB> In a standards track document you cannot make a claim without providing
supporting text showing how this is done and what the issues might be.

==========

   In the current SR architecture, an SR path is a unidirectional path
   [RFC8402].  In order to support bidirectional SR paths, a
   straightforward way is to bind two unidirectional SR paths to a
   single bidirectional SR path.  Path Segments can then be used to
   identify and correlate the traffic for the two unidirectional SR
   paths at both ends of the bidirectional path.
SB> SR uses loose source routing and ECMP to minimise segments. Even between a
pair of adjacent nodes you cannot be sure of path reciprocity unless you you
AdjSIDS. Is that the proposal?
=========

   An attacker could exploit path segment to manipulate the accounting
   of SR traffic at the egress.  Path segment could also be used to
   monitor traffic patterns for the E2E paths.  The control protocols
   used to allocate path segments could also be exploited to disseminate
   incorrect path segment information.  Note that, the path segment is
   imposed at the ingress and removed at the egress boundary and is not
   leaked out of the administered domain.
SB> What are the mitigations for the increase in attack surface?

============

Minor issues:

 For SR-MPLS, this can be the Path
   Segment label allocated by the egress node.
SB> Can be is not strict enough for standards track
=========

 It
   is obvious that this equivalence group can be instantiated in the
   network by an SDN controller using the Path Segments of the SR paths.
SB> Nothing is obvious until it is proven.

========

   Path Segment in SR-MPLS does not introduce any new behavior or any
   change in the way the MPLS data plane works.
SB> If it does not change anything we do not need an ST RFC.

========
 Path
   segment is additional metadata that is added to the packet consisting
   of the SR path.
SB> I think you mean that the change is to add a new label.

========

Nits/editorial comments:

6.  Path Segment for Bidirectional SR Path

   In some scenarios, for example, mobile backhaul transport networks,
   there are requirements to support bidirectional paths, and the path
   is normally treated as a single entity.  The both directions of the
SB> "The both directions" is not good grammar