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

From: Datatracker on behalf of Benjamin Kaduk <[email-masked]>
Date: Wed, 06 Mar 2019 20:52:47 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-mpls-sfc-05: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mpls-sfc/

----------------------------------------------------------------------
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.

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.

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

----------------------------------------------------------------------
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?)

Abstract (and Introduction)

   Multiprotocol Label Switching (MPLS) is a widely deployed forwarding
   technology that uses labels placed in a packet in a label stack to
   identify the forwarding actions to be taken at each hop through a
   network.  Actions may include swapping or popping the labels as well,
   as using the labels to determine the next hop for forwarding the
   packet.  [...]

nit: no comma after "as well".

   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?

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.

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.

   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.

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.

   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"

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.

   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-speific 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.

   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.

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.

      *  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.

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.)

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.)

Section 15

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

                                                 Where an SF is not
   encapsulation aware the encapsulation may be stripped by an SFC proxy

nit: "encapsulation-aware"

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.

   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'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).

   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.)

Section 19.2

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

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 7 Mar 2019 15:01:34 -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?

> 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

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?

> (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.

> nit: no comma after "as well".

Ack

> 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.

> This does not give any motivation for why a classifier might wish to

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.

> 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.

> 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.

> The word "reclassification" does not appear in the indicated reference.

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

> 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 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".

>    context-specific semantics such as indicating how to interpret the

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

> anything or attempting to defer to Section 5, so this seems

See your Discuss for resolution of both of these.

> 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.

> nit: probably best to use "new top" for consistency with the preceding

Sure

> since all three cases are potentially relevant.)

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

> 12 reserved bits would do that, too.)

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

> 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.

> nit: "encapsulation-aware"

Ack.

> doesn't provide any cryptographic integrity protection on the headers.

OK

> 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.

> (since "are addressed (or should be addressed)" is rather wishy-washy

OK.

> is, just for clarity.  (I assume it's the RFC8300 NSH.)

OK
Added 7665 and 8300.

> RECOMMENDED to insert Entropy Labels.

OK. Can do

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 7 Mar 2019 10:34:10 -0600

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

I don't see a particular need; I was mostly just concerned that the
typo-laden text wasn't saying anythign about the SF Label TTL, but that
concern seems to be unfounded.

> 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.

Okay, so I guess this follows from being in a single MPLS domain that the
topmost label in the "basic unit" is going to be uniquely interpretable,
and thus will indicate whether it's an SFP label or SFC context label (and
thus what form of processing); that makes sense.

>    SFI, it has four cases to consider.

That would be wonderful, thank you.

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

That part looks good, yes.

> 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 documen…

Hmm.  I think that this comment is basically just asking whether the word
"interim" is better in or out.  But it's not my call to make, of course.
(And English has these silly rules about the form of the indefinite article
changing when preceding a word that does or does not start with a vowel, of
course.)

> 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.

That's fair (and this was intentionally in the non-blocking comment
section).  I'm just always amazed at how many choices we have to offer in
some of these routing protocols, that the deployment scenarios don't admit
more consolidation of technologies.

>       bottom of stack, when it MUST be set.

Thanks.

> Dang! It's hyphenated in the other draft.

Shoot, I should have checked for that.  (I was actually looking it up to
get more background.)

> The label stack entry is a construct of its own (and thus a compound noun).

Ok.

> I think the resultant text is clear enough, although as you say "unusual".

Sure.

> This way aligns with "normal" LTV processing.

Well, it's potentially an interop issue, now that I think about it more.
If I treat the values as integers, then I could in theory use some
little-endian encoding, or even something more strange.  So I think we
probably do need more text about resolving the mismatch (even if we don't
change the diagram).

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

Maybe:

It is fundamental to the SFC design that the classifier is a fully trusted
element; the classification decision process is not visible to the other
elements and its output is treated as accurate.  As such, the classifier
has (sole?) responsibility for determining the processing that the packet
will be subject to, including, for example, firewall functions.

> Of course, a host of mechanisms exist for that, but are out of scope.

"payload packets" or "packet payloads"?  But sure, I think we're in
agreement.

Thanks!

-Benjamin

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 7 Mar 2019 17:26:17 -0000

Thanks again, Ben,

[AF] OK. Notwithstanding that the way fields are encoded in MPLS are well
 known we will add encoding rules

Maybe:

[AF] Yes. Working with that.

[AF] "Payload packets". The essence of SFC being that you have a packet stream and you send it (via an encapsulation) to be processed by SFs.

Thanks again,
Adrian