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

Benjamin Kaduk <kaduk@mit.edu> Thu, 07 March 2019 16:34 UTC

Return-Path: <kaduk@mit.edu>
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 9D832127971; Thu, 7 Mar 2019 08:34:28 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.001
X-Spam-Level:
X-Spam-Status: No, score=-2.001 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=mit.edu
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 2cC8Zwbd0KGS; Thu, 7 Mar 2019 08:34:24 -0800 (PST)
Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-eopbgr810132.outbound.protection.outlook.com [40.107.81.132]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 5A6EF12788F; Thu, 7 Mar 2019 08:34:24 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mit.edu; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6v9i/3PvmiWrfWIRwD14q5xFLGvwxUDG81z2sFM7G6k=; b=jsRw3P+VWjzSeaUhay9omPSSJ+3SoGpOUEAxML25PGXHXEpeAKfJKXqqfr55eLdIcUFsAWqCH2p17lmA06EQ2JGVh8YYwjDYgQ2wZFh1ok5SEd3abNMfnHfW+7UKeEZlcd1Arc0QfT7zE1pgsboZZMUjSK6FOjOYPpodvx8FQgI=
Received: from MWHPR01CA0029.prod.exchangelabs.com (2603:10b6:300:101::15) by DM6PR01MB4857.prod.exchangelabs.com (2603:10b6:5:6d::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1686.16; Thu, 7 Mar 2019 16:34:19 +0000
Received: from BY2NAM03FT034.eop-NAM03.prod.protection.outlook.com (2a01:111:f400:7e4a::201) by MWHPR01CA0029.outlook.office365.com (2603:10b6:300:101::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.1686.18 via Frontend Transport; Thu, 7 Mar 2019 16:34:18 +0000
Authentication-Results: spf=pass (sender IP is 18.9.28.11) smtp.mailfrom=mit.edu; pi.nu; dkim=none (message not signed) header.d=none;pi.nu; dmarc=bestguesspass action=none header.from=mit.edu;
Received-SPF: Pass (protection.outlook.com: domain of mit.edu designates 18.9.28.11 as permitted sender) receiver=protection.outlook.com; client-ip=18.9.28.11; helo=outgoing.mit.edu;
Received: from outgoing.mit.edu (18.9.28.11) by BY2NAM03FT034.mail.protection.outlook.com (10.152.84.211) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.1643.13 via Frontend Transport; Thu, 7 Mar 2019 16:34:16 +0000
Received: from kduck.mit.edu (24-107-191-124.dhcp.stls.mo.charter.com [24.107.191.124]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id x27GYBvk015984 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 7 Mar 2019 11:34:13 -0500
Date: Thu, 07 Mar 2019 10:34:10 -0600
From: Benjamin Kaduk <kaduk@mit.edu>
To: Adrian Farrel <adrian@olddog.co.uk>
CC: 'Datatracker on behalf of Benjamin Kaduk' <noreply@ietf.org>, 'The IESG' <iesg@ietf.org>, mpls@ietf.org, mpls-chairs@ietf.org, draft-ietf-mpls-sfc@ietf.org, loa@pi.nu
Message-ID: <20190307163410.GA9824@kduck.mit.edu>
References: <155193436776.13798.13710472280487229144.idtracker@ietfa.amsl.com> <016201d4d4f6$a91e5e10$fb5b1a30$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <016201d4d4f6$a91e5e10$fb5b1a30$@olddog.co.uk>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-EOPAttributedMessage: 0
X-Forefront-Antispam-Report: CIP:18.9.28.11; IPV:CAL; SCL:-1; CTRY:US; EFV:NLI; SFV:NSPM; SFS:(10019020)(39860400002)(376002)(346002)(396003)(136003)(2980300002)(51444003)(199004)(189003)(51914003)(1076003)(53416004)(106466001)(55016002)(66574012)(356004)(54906003)(305945005)(5660300002)(246002)(229853002)(2486003)(6916009)(478600001)(26826003)(33656002)(8676002)(23676004)(7696005)(76176011)(106002)(104016004)(8936002)(6246003)(47776003)(26005)(36906005)(956004)(126002)(2870700001)(316002)(476003)(336012)(11346002)(786003)(75432002)(14444005)(426003)(186003)(446003)(58126008)(486006)(4326008)(86362001)(50466002)(30864003)(88552002)(2906002); DIR:OUT; SFP:1102; SCL:1; SRVR:DM6PR01MB4857; H:outgoing.mit.edu; FPR:; SPF:Pass; LANG:en; PTR:outgoing-auth-1.mit.edu; A:1; MX:1;
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 03c23924-cdf5-4765-2c7d-08d6a31abe36
X-Microsoft-Antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4608103)(4709054)(2017052603328)(7153060); SRVR:DM6PR01MB4857;
X-MS-TrafficTypeDiagnostic: DM6PR01MB4857:
X-Microsoft-Exchange-Diagnostics: 1; DM6PR01MB4857; 20:p4a/05suPCmcFCQciZz1QA7e3rVgzCQ17nyd8VXmD8JZRLBOlOOUzm9Y/HVf1IXk75l66yrpO6gdrLvRMlgq4VQheS2BQjJaitwi6ULcBAS4Rq8lzuWr03s3/kkXJruuqqTvzqZbXVj4BlI6RoYkHUhd8TT4mjWSRnZae7T8HOfQZc2REAuEHt1QR2LZLTUun7nbBgpDXg4XmbSHT3aIxJqcSEHyXbpRL/43W33YyRp59qbKA9liVaUqQVsqpiRXT93yVsHK1MEZeTltLGWJK6Pij7z6kLv4UY/SxuKrJUsq+91EnESvJrG0IXYR3oSqtUB1T701cQzJql7WWcRixWR5lQhvVNN9AabS2HwGwd26yziMzTdNxU5kEnqkL2Z5161XA7ZjPYaTvctBhS6+y3M9QP2E5niMUaD5XFONEPWxgNmx5aG7FHd8VMCQ6CCgRKFrNHfC76rXBe3gBFZSoKgbNM/gZINfxJI5vduYYhW29tZn4MUdgG0WxKrkxym17dcrX9KDfDIxvUa1qv5EQvP3h1BPwWbbNSoEmf8MAiyEQv9tfFzP5zFMZkqskfs9OED1lkKR1eHYfA0MzW2TyzG0xkUxWi4H6KWDJli+bq8=
X-Microsoft-Antispam-PRVS: <DM6PR01MB4857E85F683767C5E07D9DF1A04C0@DM6PR01MB4857.prod.exchangelabs.com>
X-Forefront-PRVS: 096943F07A
X-Microsoft-Exchange-Diagnostics: 1;DM6PR01MB4857;23:vzK9jda+ECM9x1hICF463kCrqfLi7fDiijqmUdfNU13/Lfka18gSotVZgHbmBoN17ndUMWAzYsJtwqe5g5tlEeN//UJrwvGWZ6pcnCKMWmI7OZqMtVHaKZy4y8PbsQJ2O/YTLvNYigQTXEjwDiahirxls/JFKQQEbkzvnuizUO9Lz2K9lvisBsj/B6eAZycAAmVwfPj21o0pciQEvcbI24W1G36qKYT2twqkC1CemoD7igKWp1SMpiKQvsOFv1JFlqr6//2d3pQnoKrij2nLhf0Ce9zg6FUwvQn+skButogzN/F3Ulvg5P2jvK8lG9s3rV1pwHZ+3uktFJuu5A/WBvo9fHLcG2wY3fjLwEdCl3ku524oqQw4VvtmWuL3Lmr4YLNhDnWYz5NeMeVbUiw26x+BQBtd6ScZ8dXdK4TiwsdhghPaIJ5F1MpULmtlJfEz7xUgPcZ/b5gnWs59plzH/lqUyo72Qp5d36huPbbTaHiiwXlHn53iz6hPDJNERoTBLOQuIqaR371FTWJR8rWLQSLWjpS98ZDGN2BdfteysElYmeDJE8GvpFnUf1OHjjXvi/kXmzCNkjGNcbcZMk/3Asv7PJRsLIg8pA1ra6tq8TGtyY6ZyDeC2SiGS6x8PsoC1m/pSkSqLZ9yJ3msRTOXnSc/WOGo8PooFPPDys68ftsDAMZAwgkwbKuzuFIFz6ta3yumdBYEDu4EMNfyFWiO9EUzJvRaSgPdNP8XeAMR8BApEpnXtqMUgH5WgsYK/yD3Ne35DNfhPOYiNGRHOkEqkyXrqDgYPfXw0mnW9xBuNYoK4WwD1x6qC5kGE3087nyqOCV7y4pDqIx8ISjMRLVU/lacuiUoaYq5uobr5CdqirQ4psEFkC5TGLg5TrHNaatBWQOer0MESv9FJ9jBQ0/BXwtZ8MfOZFvA+kV4agPVGYp7LDfOc8Fb4zvRI4DWdnp/S8TFsphJJRUNXFhhGbQ/PAJJiLoGWlzokhUHLbAew6DmDAFxpVLJNQ0eDdcX0IdWMGJJmQWPctykukCt/WzrLeqfwzgIa7w6Cdj5jPyw408Y6BTZyxNez/427ou8pZW4dx9HVgT7pJxj48KUMUzSWXr5HKakpqwzWnoTmlYLuphHiVSibfNhf6Ovv1YYZ4RBNU+fFp6xDJMxKcnt5fh4ddUSuVwNThBx3VRpqGn4NHrt3Nx2yVDEdpYJshC6uc+iuiKH9hIqQLu18G74soUHpYirDT7og0ZN9wu83xPKLaGZTtjw51rZhqunWg4WOGbE
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam-Message-Info: PUvsMFAJdktRkC/JfqLY+by1G7n6+J5BC1WzJ9j1JTGbZAnTKm6vob4aG2C3WSHj5kyJGFCb9huP0LXBYsX7Z+qFlGw7of0M3wGf2PEt5U/2VJVuIpRjDXXbF/gigI5190/FoY2tHfAM38y3Rda89jGVuZZQ5aSojGLxI+n8BdXz7Ip7Qr1zC75ZC6BGbwBAWRXFQEhD74y9JLTQLqrsCu/Khv1HmYjZXV1cLGSg+PJfUivSLPWLLOjHkDLCOhX7dZo75nDSDtnkiSKrbtuiU20kshvX3qPt+Bw3wMtp8qF4dD08sc+3cJDEU6ZYVyk/mt2lBtYgQ7Q2uEPc3X5zZu9m1ry2v/nV6EHdHFbvaJtj7dPSXyzIokalTNnEQ1F4wtZ/21D3xv5gYr2FyheraxlN8y7VEZCvmsNTFLp3w4A=
X-OriginatorOrg: mit.edu
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2019 16:34:16.9527 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 03c23924-cdf5-4765-2c7d-08d6a31abe36
X-MS-Exchange-CrossTenant-Id: 64afd9ba-0ecf-4acf-bc36-935f6235ba8b
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=64afd9ba-0ecf-4acf-bc36-935f6235ba8b; Ip=[18.9.28.11]; Helo=[outgoing.mit.edu]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR01MB4857
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/Wvg46lXWoOcnVxGGcTv7DL9Phe8>
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 16:34:29 -0000

On Thu, Mar 07, 2019 at 03:01:34PM -0000, Adrian Farrel wrote:
> 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?

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.

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

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.

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

That would be wonderful, thank you.

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

That part looks good, yes.

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

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

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

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.

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

Thanks.

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

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

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

Ok.

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

Sure.

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

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

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

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.

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

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

Thanks!

-Benjamin

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