Re: [mpls] Benjamin Kaduk's Discuss on draft-ietf-mpls-sfc-05: (with DISCUSS and COMMENT)

"Adrian Farrel" <adrian@olddog.co.uk> Thu, 07 March 2019 15:01 UTC

Return-Path: <adrian@olddog.co.uk>
X-Original-To: mpls@ietfa.amsl.com
Delivered-To: mpls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id F252212788F; Thu, 7 Mar 2019 07:01:45 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.6
X-Spam-Level:
X-Spam-Status: No, score=-2.6 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eXbPXoH7LoYN; Thu, 7 Mar 2019 07:01:42 -0800 (PST)
Received: from mta6.iomartmail.com (mta6.iomartmail.com [62.128.193.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 194A2124B19; Thu, 7 Mar 2019 07:01:41 -0800 (PST)
Received: from vs2.iomartmail.com (vs2.iomartmail.com [10.12.10.123]) by mta6.iomartmail.com (8.14.4/8.14.4) with ESMTP id x27F1cIb015305; Thu, 7 Mar 2019 15:01:38 GMT
Received: from vs2.iomartmail.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A4D9E22048; Thu, 7 Mar 2019 15:01:38 +0000 (GMT)
Received: from asmtp3.iomartmail.com (unknown [10.12.10.224]) by vs2.iomartmail.com (Postfix) with ESMTPS id 97F8D22050; Thu, 7 Mar 2019 15:01:38 +0000 (GMT)
Received: from LAPTOPK7AS653V ([87.112.237.8]) (authenticated bits=0) by asmtp3.iomartmail.com (8.14.4/8.14.4) with ESMTP id x27F1awV030878 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 7 Mar 2019 15:01:37 GMT
Reply-To: adrian@olddog.co.uk
From: Adrian Farrel <adrian@olddog.co.uk>
To: 'Datatracker on behalf of Benjamin Kaduk' <noreply@ietf.org>, 'The IESG' <iesg@ietf.org>
Cc: draft-ietf-mpls-sfc@ietf.org, 'Loa Andersson' <loa@pi.nu>, mpls-chairs@ietf.org, loa@pi.nu, mpls@ietf.org
References: <155193436776.13798.13710472280487229144.idtracker@ietfa.amsl.com>
In-Reply-To: <155193436776.13798.13710472280487229144.idtracker@ietfa.amsl.com>
Date: Thu, 07 Mar 2019 15:01:34 -0000
Organization: Old Dog Consulting
Message-ID: <016201d4d4f6$a91e5e10$fb5b1a30$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Microsoft Outlook 16.0
Content-Language: en-gb
Thread-Index: AQJzZtNl8vYRQS0gmn+2QKKbTlRPSqTC6nkw
X-Originating-IP: 87.112.237.8
X-Thinkmail-Auth: adrian@olddog.co.uk
X-TM-AS-GCONF: 00
X-TM-AS-Product-Ver: IMSVA-9.0.0.1623-8.2.0.1013-24476.000
X-TM-AS-Result: No--14.862-10.0-31-10
X-imss-scan-details: No--14.862-10.0-31-10
X-TMASE-Version: IMSVA-9.0.0.1623-8.2.1013-24476.000
X-TMASE-Result: 10--14.861900-10.000000
X-TMASE-MatchedRID: IeZYkn8zfFrxIbpQ8BhdbOYAh37ZsBDC4PxBuU3oo1VnnK6mXN72m4Xa KD3s/mOLX381PW63B5R3Ujz9Y2wFbv/qdXNk/x2CSEQN/D/3cG5D3kXYiJVSRGtPyIJPorIP/WN Hyoh/70JATBuuLHMJM94iSPt7D56Cx6itxkruJHueEco05hssvpZ6zKu0q4rtCVuEXtlNqcsWDG fKwstuO7FURUnPT9poYklZyHW6Io9Lc0v2hlJrc0hwlOfYeSqxpPZLm8rbDqL/MiRbve4ADmpHK tkQBynK1gmgH2BfZB9SZ35rifQIkEcEDUwCfLjBdMz8d6rgprf0IUHuSVWDhyfO9H6Y/dy+gW0g uPJuEMgkWm+Lrc1V3D25TAv6dW8egZi/ORh+nqDIpMSwJEh3JfmDoOtMnt41TDoylMQmcK84OJj GEod9DcNiqkhxQHrcIThB18cWramst3r4DzbynW0PWqD0pliRu2rcU2ygxCDMcZL/toKwUjKC++ TafYORJ7wMgcRo6adGvh8ozKAwsvh6oIsjCAtnxqZyCOmsQ/MIgWIk/VdnUm2Bc3xI+NwYn8nvM xMvT29CUKtIVPUsaq6V8+F/tHZwmjmuEzwoMy27bScJeyAvlkrCzBY4ZVGj7ynGrUv2DpRaapAz RzPL/khBxDp1GjrTO/OBelOfHeMmMPeO88gK4HxTDivx6nwGq8zHTxACKxbFHcwKL6UE/yBXsL8 Q5BFBcDYnLMG33ck5FeaVnwctxcwrDggcQ6d1TVa+L3Zgqc4ZKp0SZ4P+dbKMPhGFsui/166Xb3 /Hw4PTRaQ8SsYqIGOeTis4ExHJcyF6DsnU3FWeAiCmPx4NwFkMvWAuahr8m5N2YHMD0b8MyrfP9 j+C1d934/rDAK3zhG2qikEpQGU7asfoVDS3WmqZRGot6djOX28JHK2uRZV3T9Mv2BpCilOInFnJ 4pFI
X-TMASE-SNAP-Result: 1.821001.0001-0-1-12:0,22:0,33:0,34:0-0
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/qyr4009UwMGPURmGxiz87d2XjIw>
Subject: Re: [mpls] Benjamin Kaduk's Discuss on draft-ietf-mpls-sfc-05: (with DISCUSS and COMMENT)
X-BeenThere: mpls@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mpls>, <mailto:mpls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls/>
List-Post: <mailto:mpls@ietf.org>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mpls>, <mailto:mpls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 07 Mar 2019 15:01:46 -0000

Hi Ben,

Thanks for the detailed and thoughtful review. It's helpful.

Answers below, and a new revision in my buffer.

Adrian

> DISCUSS:
> 
> Thank you for this easy-to-read document!  The design is pretty
> elegant, but I think there are a few places that remain insufficiently
> specified so as to admit interoperable implementation.
>
> In particular, I don't think the TTL-handling behavior for the SF
> Label in the "label stacking" case is completely specified, though
> an apparent typo in Section 7 (see COMMENT) makes it hard to
> tell what was intended.

Fixing the typo would give us
   TTL:  The TTL fields in the SFC Context label stack entry and in the 
      SF label stack entry SHOULD be set to 1 as stated in Section 5,
      but MAY be set to larger values if the label indicated a 
      forwarding operation towards the node that hosts the SF.

In the SR case, it might not be necessary to tunnel the packets. In that case the SFC Context label provides sufficient information to route the packet through the SR network to the next SFF, but the TTL needs to reflect the distance between SFFs.

Do you think we need to add text to explain that?

> In Section 8, we see that:
>   When an SFF receives a packet containing an MPLS label stack, it
>   checks whether it is processing an {SFP, SI} label pair for label
>   swapping or a {context label, SFI index} label pair for label
>   stacking. [...]
>
> What data source is consulted to make this check?  The control
> plane? Is this distinction made on a per-node basis or a per-packet
> basis or otherwise?  My first thought was that it's per-SFC-Context-
> Label, but we're not guaranteed that those values will be interpretable
> equivalently in the swapping or stacking usages, IIUC.

All SFFs are programmed with context for SFC labels. So they know, from the top label (the SFP label or SFC context label) what form of processing is involved.

But, additionally, this is not going to be a rabid free-for-all. As section 8 says a bit higher up...
   it is also
   possible that different parts of the network utilize swapping or
   popping such that an end-to-end service chain has to utilize a
   combination of both techniques.

This "different parts of the network" is going to manifest as different interfaces to the SFF.

Lets upgrade the text to...
   When an SFF receives a packet containing an MPLS label stack, it
   checks from the context of the incoming interface, and from the SFP
   indicated by the top label whether it is processing an {SFP, SI} 
   label pair for label swapping or a {context label, SFI index} label
   pair for label stacking.  It then selects the appropriate SFI to
   which to send the packet.  When it receives the packet back from the
   SFI, it has four cases to consider.

> I also support Alissa's DISCUSS (and the secdir reviewer makes
> similar comments).

OK. Did you see my response to Alissa? Does that solve it for you?

> COMMENT:
>
> Am I supposed to assume that the control plane sufficiently informs
> everyone whether they're an SFF, SFI, or neither?  (Or does any given
> (virtual) device just inherently know?)

They know. Like a router knows it is not a telephone box.
If (improbable) a device could be hawk or a handsaw depending on the direction of the wind, then it would need to be configured to know what it was doing.

> Abstract (and Introduction)
>
> nit: no comma after "as well".

Ack

>  This document describes how Service Function Chaining can be achieved
>  in an MPLS network by means of a logical representation of the NSH in
>  an MPLS label stack.  That is, the NSH is not used, but the fields of
>  the NSH are mapped to fields in the MPLS label stack.  It does not
>  deprecate or replace the NSH, but acknowledges that there may be a
>  need for an interim deployment of SFC functionality in brownfield
>  networks.
>
> Am I supposed to read this as that the NSH is the way of the 
> future, and this mechanism is just a temporary interim measure?

It is unclear. The full use of the NSH may require upgrades to some forwarding plane devices. The IETF, through consensus, has approved the NSH on the standards track via RFC 8300, and this document is not intended to displace that or argue against that consensus. On the other hand, there is a desire to achieve SFC function largely equivalent to that provided by the NSH for more immediate deployment using existing hardware.

> Section 4.3
>
>  It may be that a service function chain (as described in Section 4.1
>  allows some leeway in the choice of service function instances along
>  the chain.  However, it may be that a service classifier wishes to
>  constrain the choice and this can be achieved using chain
>  concatenation so that the first chain ends at the point of choice,
>
> This does not give any motivation for why a classifier might wish to
> constrain the choice.

Yes. And I'm not sure that we should repeat the SFC architecture here, nor develop some of the use cases.

I would offer three possible reasons off the top of my head:
- The tool constructing the SFP does not trust (or knows better than) the SFF what SFI load-balancing decisions to make. 
- The SFIs are stateful, but the SFF is not capable of maintaining state as to which SFI is used for each SFP.
- There is a bidirectional SFP and the same SFI must be used in each direction.

I'm not inclined to put into this document a description of reasons why an SFP might or might not specify precise choice of SFIs. We just describe how it can be done.

> Section 5
>
> The decision to use the control plane to indicate "label stacking" vs.
> "label swapping" semantics as opposed to an in-band signal seems to
> create a new opportunity for misconfiguration and consequent service
> misbehavior.  I suppose it's not appreciably worse than any other way to
> configure the interpretation of the label field, though.

Not sure the comment applies to Section 5. But you're right, the use of a control plane is subject to misconfiguration. However, I think we only have the MPLS shim header to work with and unless we partition the label space (which the MPLS architects would scream about) I think we are stuck with what we have.

> S: The bottom of stack bit has its usual meaning in MPLS.  It MUST be
>      clear in the SFC Context label stack entry and MAY be set in the
>      SF label stack entry depending on whether the label is the bottom
>      of stack.
>
> I don't understand why this is only a MAY.

This is probably why the use of requirements language sucks sometimes 😊
Perhaps we should expand this a bit and write...

   S: The bottom of stack bit has its usual meaning in MPLS.  It MUST be
      clear in the SFC Context label stack entry.  In the SF label stack
      entry it MUST be clear in all cases except when the label is the 
      bottom of stack, when it MUST be set.

> Section 6
>
>    Under normal circumstances (with the exception of branching and
>    reclassification - see [I-D.ietf-bess-nsh-bgp-control-plane]) the
>
> The word "reclassification" does not appear in the indicated reference.

Dang! It's hyphenated in the other draft.
Fixing it here.

>   The following processing rules apply to the TTL field of the SF label
>   stack entry, and are derived from section 2.2 of [RFC8300]:
>
> nit: we seem to capitalize "SF Label"

Yeah, hmmm/ Seem to have consistent use as:
"SF Label"
"SF label stack entry"
The label stack entry is a construct of its own (and thus a compound noun).

> Section 7
>
> It's a bit unusual, style-wise, to have Section 6 introduce new terms
> for the SPI Label and SI Label that are specific to the swapping usage,
> and then have Section 7 just reuse the nominally generic terms from
> Section 5 as its own for the stacking usage.

I think this the consequence of some politics ☹
Two concepts and drafts were merged, and no one wanted to give up their language.
We might sort that out, but I don't know anyone with the energy!
I think the resultant text is clear enough, although as you say "unusual".

> SFC Context Label:  The Label field of the SFC Context label stack
>    entry contains a label that delivers SFC context.  This label may
>    be used to indicate the SPI encoded as a 20 bit integer using the
>    semantics of the SPI is exactly as defined in [RFC8300] and noting
>    that in this case a system using MPLS representation of the
>    logical NSH MUST NOT assign SPI values greater than 2^20 - 1 or
>    less than 16.  This label may also be used to convey other SFC
>    context-specific semantics such as indicating how to interpret the
>    SF Label or how to forward the packet to the node that offers the
>    SF.
>
> This "may also be used" behavior seems rather under-specified.

Yeah. Adding "if so configured and coordinated with the controller that programs the labels for the SFP."

> TTL:  The TTL fields in the SFC Context label stack entry SF label
>    stack entry SHOULD be set to 1 as stated in Section 5, but MAY be
>    set to larger values if the label indicated a forwarding operation
>    towards the node that hosts the SF.
>
> What is "SFC Context label stack entry SF label stack entry"?  It seems
> like there's a missing word or something.  I note that section 5 defers
> to here for the TTL of the SF Label and we are either not saying
> anything or attempting to defer to Section 5, so this seems
> under-specified.

See your Discuss for resolution of both of these.

> Section 8
>
> o  If the current hop requires an {SFP, SI} and the next hop requires
>     an {SFP, SI}, it selects an instance of the SF to be executed at
>     the next hop, sets the SI label to the SI value of the next hop,
>     and tunnels the packet to the SFF for that SFI.
>
> nit: I know the default behavior is to use the same SFP value, but (1)
> this should probably be stated explicitly, and (2) we've already talked
> about branching/etc. that could cause it to change.

OK

   o  If the current hop requires an {SPI, SI} and the next hop requires
      an {SPI, SI}, it sets the SPI label according to the SFP to be
      traversed, selects an instance of the SF to be executed at the 
      next hop, sets the SI label to the SI value of the next hop, and
      tunnels the packet to the SFF for that SFI.

>   *  If the top of the MPLS label stack contains a {context label,
>       SFI label}, it tunnels the packet to the SFF indicated by the
>       context label.
>
> nit: probably best to use "new top" for consistency with the preceding
> sub-bullet.

Sure

> Section 12.1
>
> The SFC Metadata Label (as a set of three labels as indicated in
> Figure 4) may be present zero, one, or more times in an MPLS SFC
> packet.  For MPLS label swapping, the SFC Metadata Labels are placed
> immediately after the basic unit of MPLS label stack for SFC as shown
> in Figure 5.  For MPLS label stacking, the SFC Metadata Labels can be
> present zero, one, or more times and are placed at the bottom of the
> label stack as shown in Figure 6.
>
> The "may be present zero, one, or more times" appears twice.
> (I actually don't mind the internal redundancy of that phrase here, though,
> since all three cases are potentially relevant.)

Yeah, but it's tacky.  Tidied it up.

> Section 12.2
>
>   Metadata Type:  The type of the metadata present.  Values for this
>      field are taken from the "MD Types" registry maintained by IANA
>      and defined in [RFC8300].
>
> The "MD Types" registry I'm finding in RFC 8300 is defined to hold
> four-bit values; why do we need a 16-bit field to hold it here in the
> TLV?  (I mean, "to keep the metadata itself aligned", sure, but having
> 12 reserved bits would do that, too.)

It's true. 
We could have jumped either way.
This way aligns with "normal" LTV processing.

> Section 15
>
> I agree with the secdir reviewer that the "trusted" nature of the
> classifier as a "trusted resource" could be further clarified.

I am having some trouble with the subtleties here.
I suppose this means that a trusted resource is one that will not behave in a way intended to cause harm to the network or to the payload data. That means that when a classifier puts a packet onto an SFP (by applying labels) its actions are not designed to cause the packet to avoid particular SFPs or be blackholed or be diverted to other parts of the network or SFs.
The point here, I think, is that there is no way for the rest of the network to have any visibility into the behaviour of the classifier, but must accept packets as they are forwarded.
Thus, the classifier is seen as within the MPLS trusted zone [RFC5920].

If you can suggest ways we could explain that to everyone's satisfaction, then I'd be happy to add text.

>                                                 Where an SF is not
>   encapsulation aware the encapsulation may be stripped by an SFC proxy
>
> nit: "encapsulation-aware"

Ack.

> Thank you for the new text in the -05 prompted by the secdir review; it
> is a huge improvement!
> In addition to encryption, I'd probably also note that MPLS at present
> doesn't provide any cryptographic integrity protection on the headers.

OK

>  o  The SFC-capable devices participating in an SFC system are
>      responsible for verifying and protecting packets and their
>      contents as well as providing other security capabilities that
>      might be required in the particular system.
>
> Do I recall correctly that we currently don't specify any mechanisms for
> them to do so?  If so, we probably need to note that this is currently
> in implementation-specific territory.

I believe this quote should say "payload packets and their contents."
Of course, a host of mechanisms exist for that, but are out of scope.

> I'd also suggest
> OLD:
>   Thus, the security vulnerabilities are addressed (or should be
>   addressed) in all the underlying technologies used by this design.
>NEW:
>   Thus, this design relies on the component underlying technologies to
>   address the potential security vulnerabilities, and documents the
>   necessary protections (or risk of their absence) above.  It does not
>   include any native security mechanisms in-band with the MPLS
>   encoding of the NSH functionality.
> (since "are addressed (or should be addressed)" is rather wishy-washy
> language).

OK.

>    No known new security vulnerabilities are introduced by this design,
>
> It's probably worth stating what the reference point for the comparison
> is, just for clarity.  (I assume it's the RFC8300 NSH.)

OK
Added 7665 and 8300.

> Section 19.2
>
> It seems that RFC 6790 needs to be a normative reference, since we are
> RECOMMENDED to insert Entropy Labels.

OK. Can do