Re: Last Call: <draft-ietf-sfc-nsh-18.txt> (Network Service Header (NSH)) to Proposed Standard

"Carlos Pignataro (cpignata)" <cpignata@cisco.com> Mon, 04 September 2017 22:45 UTC

Return-Path: <cpignata@cisco.com>
X-Original-To: ietf@ietfa.amsl.com
Delivered-To: ietf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id EF2F81321A0; Mon, 4 Sep 2017 15:45:35 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.521
X-Spam-Level:
X-Spam-Status: No, score=-14.521 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_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
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 iPPcEYlhsYLQ; Mon, 4 Sep 2017 15:45:31 -0700 (PDT)
Received: from rcdn-iport-6.cisco.com (rcdn-iport-6.cisco.com [173.37.86.77]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id F0C24132192; Mon, 4 Sep 2017 15:45:30 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=40418; q=dns/txt; s=iport; t=1504565130; x=1505774730; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=phDkuN7ScO5fRywypEwmmfH78ODCoXoOP4/DRjPJtWA=; b=SOAaNJkCBIhQpetREWvKHvrqq1piaWBZmZsKanGP2njB6h3r5p/eBjj8 Z82mOQ+W1SurwlsIEdafhLFBE+QbYaGxsEsTGf2NKhQ9AoLKB70YxgOY3 elulK8DaCooOB38u98xihAR0dNwEhvO3JR2aNrrHbtIMxDqQ216LR/zhm 8=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0BIAQBe1q1Z/4YNJK1TChkBAQEBAQEBAQEBAQcBAQEBAYNaZIEVB4NwiiCQIYFPIneVMQ6CBAojhRsCGoQDPxgBAgEBAQEBAQFrKIUYAQEBAQIBDgwJBA0xAgcLBQsCAQgSBgICJgICAjAVAg4CBA4FHooLCBCwdYFtOotTAQEBAQEBAQEBAQEBAQEBAQEBAQEBGAWBDYIdgWIggU6BYysLgWVYNYRKBQ4YgxMwgjEFiXiOPIhAAodZg1qJHIIThWeDfoZ5iiCKXgIDCwIYAYE4AR84gQ13FR88AYUCAxwZgU52iHaBMYEPAQEB
X-IronPort-AV: E=Sophos;i="5.41,476,1498521600"; d="scan'208";a="291279397"
Received: from alln-core-12.cisco.com ([173.36.13.134]) by rcdn-iport-6.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 04 Sep 2017 22:45:28 +0000
Received: from XCH-RTP-017.cisco.com (xch-rtp-017.cisco.com [64.101.220.157]) by alln-core-12.cisco.com (8.14.5/8.14.5) with ESMTP id v84MjSEB014577 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 4 Sep 2017 22:45:28 GMT
Received: from xch-rtp-020.cisco.com (64.101.220.160) by XCH-RTP-017.cisco.com (64.101.220.157) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Mon, 4 Sep 2017 18:45:27 -0400
Received: from xch-rtp-020.cisco.com ([64.101.220.160]) by XCH-RTP-020.cisco.com ([64.101.220.160]) with mapi id 15.00.1263.000; Mon, 4 Sep 2017 18:45:27 -0400
From: "Carlos Pignataro (cpignata)" <cpignata@cisco.com>
To: Warren Kumari <warren@kumari.net>
CC: IETF Discuss <ietf@ietf.org>, IETF-Announce <ietf-announce@ietf.org>, "draft-ietf-sfc-nsh@ietf.org" <draft-ietf-sfc-nsh@ietf.org>, "Joel M. Halpern" <jmh@joelhalpern.com>, "sfc-chairs@ietf.org" <sfc-chairs@ietf.org>, Service Function Chaining IETF list <sfc@ietf.org>, Alia Atlas <akatlas@gmail.com>, James N Guichard <james.n.guichard@huawei.com>
Subject: Re: Last Call: <draft-ietf-sfc-nsh-18.txt> (Network Service Header (NSH)) to Proposed Standard
Thread-Topic: Last Call: <draft-ietf-sfc-nsh-18.txt> (Network Service Header (NSH)) to Proposed Standard
Thread-Index: AQHTEvZcFCn6REBjS0aBK4QSCafTu6KLJ8CAgBSa4wCABfVJgIAABDUA
Date: Mon, 04 Sep 2017 22:45:27 +0000
Message-ID: <9338757B-7BB7-4CEB-8442-129743A94C7B@cisco.com>
References: <150249274009.24438.5552125434164888108.idtracker@ietfa.amsl.com> <CAHw9_iJWhJxJGngi6yv2Di3XsF-cMZv-mxKV=NNGruEDJcM+eg@mail.gmail.com> <2F5DB4AD-3672-43DE-B420-510B85440106@cisco.com> <CAHw9_i+A=TTj1D_FBQRtdeZ9siqidPCoXLt8xZ5e_OtaO1TwaQ@mail.gmail.com>
In-Reply-To: <CAHw9_i+A=TTj1D_FBQRtdeZ9siqidPCoXLt8xZ5e_OtaO1TwaQ@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.118.116.133]
Content-Type: text/plain; charset="utf-8"
Content-ID: <4E72CF69D9D5CB48BE43AC52CB700E99@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/ietf/g3KhTRmq2FVDvvgRy9U6i83MgjM>
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: IETF-Discussion <ietf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ietf>, <mailto:ietf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ietf/>
List-Post: <mailto:ietf@ietf.org>
List-Help: <mailto:ietf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ietf>, <mailto:ietf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 04 Sep 2017 22:45:36 -0000

Warren,

> On Sep 4, 2017, at 6:30 PM, Warren Kumari <warren@kumari.net> wrote:
> 
> "
> 
> On Thu, Aug 31, 2017 at 11:31 PM, Carlos Pignataro (cpignata)
> <cpignata@cisco.com> wrote:
>> Warren,
>> 
>> Many thanks for taking the time to review this document! And apologies for the delayed-ack.
> 
> No worries -- -20 looks good.

Good to hear that rev -20 addresses all your concerns.

That you for the comments that resulted in an improved, more clear, and less ambiguous document.

> 
>> 
>> If reading parts of this document, and “only skimming sections 6.1 - Section 8” left you so confused, bewildered and dizzy, there is nothing we can start with other than acknowledging that there is an issue that we will correct. It is an inadequate outcome, and we will rectify it. Much of it seems to either be tracked to localized sources, or has proposed resolutions in this response.
> 
> I think it is largely rectified.
> The additional text in S1, and Figure 1 goes a long way to fixing it.
> 
>> 
>> Before diving into inlined issue-by-issue comments, there’s a few high-level observations I’d like to make in response to your review:
>> 
>> 1. Several other reviewers, including Ops Dir, RTG Dir, and GenART, managed to read and understand the document, provide constructive comments, and survive the experience unharmed :-) Just to be clear, that does not mean that the document is as clear as it ought to be, or otherwise negates your confusion. We will improve it and fix it.
>> 
>> 2. Your suggestion of "a clear model at the beginning of the document” is spot on. We will add both a model and some prose early on. In fact, Jim Guichard suggested an outline and volunteered to craft a bit of that text.
> 
> 
> Win - as said above, this made a huge difference (at least, for me :-))

Win-win, the doc looks better now.

> 
>> 
>> Earlier version of this document included a number of examples that, in our view, would have clarified much confusion. See https://tools.ietf.org/html/draft-ietf-sfc-nsh-05#section-9. Several folks, including the responsible AD, objected to that section (on grounds of the inability of an example section to include everyone’s favorite encapsulation). There is some truth to that.
>> 
>> That said, do you believe that such example section might have helped you? (This is, in addition to some early-on clarifications).
> 
> Yes, I think that it would have made it much clearer, at least to me.
> It seems that there could be a bunch of "this is just one
> encapsulation, you can use whatever one suits you" or similar to help
> mitigate the "but I like $foo, why didn't you use that one?!”.

OK, we will think of a way to mitigate the “my favorite encapsulation is not there”, and weight pros/cons.

> 
>> 
>> 3. It is outside the scope of this document to partake into religious debates of the meaning of encapsulation versus tunneling versus shimming.
>> 
>> In fact, this document does not include the word “shim”, whatsoever. You seem to have taken intellectual offense to the combination “inserted onto”. And subsequently you say: '*inserted onto* sounds like a shim’, and then build a rebuttal of the use of the word ‘shim’ :-) I believe part of the problem is that you are reviewing “shim”...
>> 
> 
> Ok, fair. I still think that "inserted into" and the title "Network
> Service Header" leads the reader (or, perhaps just me!) to think shim
> -- but the "inserted into" is now gone, and the new text in section 1
> and figure 1 fixes all that. Thank you!
> 

You are welcome.

> 
> 
>> So, moving forward, we will do a couple of things:
>> 
>> First, add a model and some text, early on at the beginning of the document. This is needed for issue #1.
>> 
>> Second, wait for your thoughts and other folks’ guidance on whether to re-include the now removed “Examples” section. It seems to us that the looseness in some text is greatly aggravated by the lack of packet formats. This is also relevant to issue #1.
>> 
>> Would you agree?
> 
> Yup.
> 
>> 
>> Third, we will sharpen the text throughout in the use of encap.
>> 
>> Issue #4 remains unresolved, as I would like to check and not break the balance of text. I will punt this to the Shepherd.
>> 
>> In summary, we will clarify issues #1 and #3 for NSH, and would like to re-add pak formats. All other issues have proposed resolutions inline.
>> 
>> Thanks again for pointing all of these out.
> 
> Thank you, and thank you for taking them in a constructive manner.

Of course! 

As I said, the new revision addressing "the spirit of" your comments is improved.

> 
>> 
>> Please find responses and follow-ups inline.
>> 
>> We will come back with proposed diffs for the two pending issues labeled “major” (#1 and #4).
>> 
>> Additionally, all Nits are addressed in our working copy. Many thanks for taking the time to identify all those and propose suggestions. Some of them are fixed differently though (when you wrote “As SF or SFC Proxy -> A SF or SFC Proxy”, that is actually “An SF” (grammar).
> 
> Sure.
> 
>> 
>> As an aside, and as a review of your review, two quick suggestions:
>> 
>> 1. The stream-of-counciousness narrative very poorly suits a technical review of a spec. While I am sure a plurality of thoughts interconnect, somehow, it is very hard to follow.
>> 
> 
> Fair enough. Sorry.
> 
>> 2. While I am sure you know the legend for “[O] [P] and [R]”, the first thing that came to mind to me was “well, [Q] is missing!” :-) I can guess Original, Proposed, Reason/Rational after a while, but really, it does not universally translate...
> 
> Yup, was Original / Proposed / Reason - but, I'll try remember to
> include a lookup key in the future :-)

O: “but, I’ll try remember”
P: “consequently, I will try to remember”
R: Grammar! :-)

— Carlos.

> 
> Thank again,
> W
> 
>> 
>> 
>>> On Aug 18, 2017, at 8:51 PM, Warren Kumari <warren@kumari.net> wrote:
>>> 
>>> On Fri, Aug 11, 2017 at 7:05 PM, The IESG <iesg-secretary@ietf.org> wrote:
>>>> 
>>>> The IESG has received a request from the Service Function Chaining WG (sfc)
>>>> to consider the following document: - 'Network Service Header (NSH)'
>>>> <draft-ietf-sfc-nsh-18.txt> as Proposed Standard
>>>> 
>>>> The IESG plans to make a decision in the next few weeks, and solicits final
>>>> comments on this action. Please send substantive comments to the
>>>> ietf@ietf.org mailing lists by 2017-08-25. Exceptionally, comments may be
>>>> sent to iesg@ietf.org instead. In either case, please retain the beginning of
>>>> the Subject line to allow automated sorting.
>>>> 
>>>> Abstract
>>>> 
>>>> 
>>>>  This document describes a Network Service Header (NSH) inserted onto
>>>>  packets or frames to realize service function paths.  NSH also
>>>>  provides a mechanism for metadata exchange along the instantiated
>>>>  service paths.  NSH is the SFC encapsulation required to support the
>>>>  Service Function Chaining (SFC) architecture (defined in RFC7665).
>>>> 
>>> 
>>> <no hats>
>>> 
>>> In my opinion this document still needs significant work. Christian's
>>> great SecDir review lists many issues which need to be addressed;
>>> since he has already raised concerns, I am not focusing on the
>>> security and privacy stuff in my but review. Instead, I'll discuss
>>> some of the other issues.
>>> 
>>> I only skimmed sections 6.1 - Section 8.
>>> 
>>> As this is output of the SFC WG, which is chartered to, amongst other
>>> thing, create a "Generic SFC Encapsulation" (with some more details)
>>> I'll take it as given that this is desired / needed.
>>> 
>>> Issues:
>>> 
>>> Issue 1: I find the entire document to be confusing about what an NSH
>>> actually is, and after spending much time reading, am still fairly
>>> confused.
>>> A clear model at the beginning of the document which explains how
>>> these bits fit together would be very helpful. I am familiar with
>>> RFC7665 (the architecture document); my confusion stems from this
>>> draft.
>>> 
>>> For example the Abstract says:
>>> "This document describes a Network Service Header (NSH) inserted onto
>>> packets or frames to realize service function paths. NSH also provides
>>> a mechanism for metadata exchange along the instantiated service
>>> paths.  NSH is the SFC encapsulation required to support the Service
>>> Function Chaining (SFC) architecture ".
>>> 
>>> Oh, ok, so it is something inserted onto packets or frames...
>>> *inserted onto* sounds like a shim, however,  "NSH is the SFC
>>> encapsulation...".  indicates that I had the wrong mental model and
>>> this is actually an encapsulation, kinda like GRE. (This is reinforced
>>> with (end of Section 1) "NSH is the SFC encapsulation referenced in
>>> [RFC7665].") Ok, so this encapsulates the packet, got it...
>>> I read some more, and then find (in section 2) that "An outer
>>> transport header is imposed, on NSH and the original packet/frame, for
>>> network forwarding.". Oh, so this is *actually* a shim / header which
>>> goes before the packet, and then the whole thing gets encapsulated? It
>>> shouldn't have been this hard to figure out, but Ok, now I get it...Or
>>> do I? Oh, hang on, what transport header do I use?!
>>> It is only much much later, in Section 4 (page 15) that I find "Once
>>> NSH is added to a packet, an outer encapsulation is used to forward
>>> the original packet and the associated metadata to the start of a
>>> service chain. ... The service header is independent of the
>>> encapsulation used and is encapsulated in existing transports.  The
>>> presence of NSH is indicated via protocol type or other indicator in
>>> the outer encapsulation." - I'm now really confused - first I thought
>>> it was a shim / header, then I thought it was an encapsulation, now I
>>> (finally) see that it is a header which goes before the packet
>>> (encapsulating it) before it gets encapsulated again into something
>>> else. It would be much better if the document was clearer at the
>>> beginning about how all this fits together / what the NSH actually
>>> *is* -- an analogy to layering, or as a shim which goes between a
>>> tunneling header and the original packet would make this much clearer
>>> (and the reader much less annoyed!).
>> 
>> “inserted onto” substituted with “imposed on” :-)
>> 
>> As mentioned, we will also add clarifying text to the intro.
>> 
>>> 
>>> Issue 2: Section 1.2.  Definition of Terms
>>> "Metadata:  Defined in [RFC7665]." - The definition of metadata in
>>> RFC7665 says: "Metadata: Provides the ability to exchange context
>>> information between classifiers and SFs, and among SFs.". This is not
>>> an adequate definition for use in this document - the document defines
>>> a context header which carries metadata, and it need to be better
>>> defined than something which provides an ability.
>>> Apart from RFC7665 actually *defining* metadata, it doesn't really
>>> describe what you do with it. As this document discusses metadata much
>>> more (and how to carry it), I think that it needs to much better
>>> define what it is, what its purpose is, and how to use it.
>>> 
>> 
>> OK, new definition, comments welcome:
>> 
>>    <t hangText="Metadata:">
>>            Defined in <xref target="RFC7665"></xref>.
>> The metadata, or context information shared between classifiers and SFs, and among SFs,
>> is carried on the NSH's Context Headers. It allows summarizing a classification result
>> in the packet itself, avoiding subsequent re-classifications.
>> 
>>       Examples of metadata include classification information used for
>>       policy enforcement and network context for forwarding post
>>       service delivery.
>> 
>> 
>>            </t>
>> 
>> 
>>> Issue 3: Section 2.  Network Service Header
>>> "An outer transport header is imposed, on NSH and the original
>>> packet/frame, for network forwarding." - this is very poorly worded,
>>> and I don't really know what I'm supposed to do here. I'm assuming
>>> that I'm supposed to concatenate the NSH and the packet, and then
>>> encapsulate this in some transport of my choosing, but this needs more
>>> precision.
>>> 
>> 
>> Agreed. Reworded. Here’s a suggestion, explicit text welcome:
>> 
>>      <t>
>> An NSH is imposed on the original packet/frame.
>>   This NSH contains service path information and
>>   optionally metadata that are added to a packet or frame and used to
>>   create a service plane. Subsequently, an outer encapsulation is imposed on
>> the NSH, whic is used for network forwarding.
>>     </t>
>> 
>> 
>>> Issue 4: The document talks about using the TTL to avoid loops, and
>>> also the Service Index (SI). "The Service Index MUST be decremented by
>>> a value of 1 by Service Functions or by SFC Proxy nodes after
>>> performing required services  and the new decremented SI value MUST be
>>> used in the egress packet’s NSH. ... Additionally, while the TTL field
>>> is the main mechanism for service plane loop detection, the SI can
>>> also be used for detecting service plane loops.". Much better text is
>>> needed here to discuss how these interact. Lets say the TTL is 14, and
>>> the SI is 1. The packet is now handed to the next SF, which decrements
>>> it to 0. Now what? It this the same as the TTL hitting 0? If it an
>>> error? Do I wrap and go back to 255? What are there 2 mechanisms for
>>> this? Much further down, in Section 6.1, we have: "SI serves as a
>>> mechanism for detecting invalid service function paths.  In
>>> particular, an SI value of zero indicates that forwarding is incorrect
>>> and the packet must be discarded." This answers many of the above, but
>>> having the text better organized would simplify the document.
>>> 
>> 
>> I am hesitant to touch that text right now as it summarizes a delicate balance reached by the WG.
>> 
>> Let me go back to the WG and propose text for this.
>> 
>>> Issue 5: Section 6. Service Path Forwarding with NSH Subsection 6.1.
>>> SFFs and Overlay Selection
>>> I'm assuming that there is much more detail in other documents which
>>> cover how to use the SPI / SI to do the forwarding -- can you please
>>> insert a reference? I'm somewhat confused by the level of detail
>>> between this document and <whatever the other one is>. This doesn't
>>> contain enough to implement, but seem to include more than I'd expect
>>> to clarify how to interface with some other doc.
>> 
>> The detail here seems adequate.
>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> Questions / concerns:
>>> Section 2.2.  NSH Base Header
>>> "TTL: Indicates the maximum SFF hops for an SFP.  This field is used
>>> for service plane loop detection. ...  Each SFF involved in forwarding
>>> an NSH packet MUST decrement the TTL value by 1 prior to NSH
>>> forwarding lookup.  Decrementing by 1 from an incoming value of 0
>>> shall result in a TTL value of 63.  The packet MUST NOT be forwarded
>>> if TTL is, after decrement, 0."
>>> Why does decrementing by 1 from 0 result in 63? Under what (valid)
>>> conditions will I receive a packet with an incoming value of 0? (why
>>> is this not an error?)
>>> 
>> 
>> Wrap?
>> 
>>> Section 2.4.  NSH MD Type 1
>>> "When the Base Header specifies MD Type = 0x1, a Fixed Length Context
>>> Header (16-bytes) MUST be present immediately following the Service
>>> Path Header, as per Figure 4.  The value of a Fixed Length Context
>>> Header that carries no metadata MUST be set to zero.". "This
>>> specification does not make any assumptions about the content of the
>>> 16 byte Context Header that must be present when the MD Type field is
>>> set to 1, and does not describe the structure or meaning of the
>>> included metadata.". So, the specification doesn't make any
>>> assumptions about the Fixed Length Context Header (I guess it is an
>>> opaque blob), but if it has no metadata is must be 0? And if there is
>>> no metadata, why  would I need to waste 16 bytes? And this document
>>> defines 2 MD types -- why would I use this one instead of a Type 2?
>>> (which seems cleaner, and shorter).
>> 
>> There are two MD Types to use. Fixed is HW friendly, TLV is SW friendly.
>> 
>> As usual, there is a tradeoff, and you choose MD Type based on what you want to optimize for.
>> 
>> 
>>> Kind of related to Issue 2, I
>>> think that there needs to be more discussions (perhaps in some other
>>> document, which needs a reference) what the purpose of the metadata
>>> is, why it is sometimes metadata and sometimes Context Header, what a
>>> receiver is supposed to do with this, etc.
>>> 
>> 
>> Context header carries metadata.
>> 
>>> Section 2.4:
>>> "An SFC-aware SF MUST receive the data semantics first in order to
>>> process the data placed in the mandatory context field.  The data
>>> semantics include both the allocation schema and the meaning of the
>>> included data.  How an SFC-aware SF gets the data semantics is outside
>>> the scope of this specification."  -- related to the above, I still
>>> don't get this. Is "the data placed in the mandatory context field" ==
>>> "metadata"?  And there is no type information associated, so receiver
>>> cannot look at it and know what it is, other than "I expected a blob,
>>> this is a blob, blobs I receive should be of type X, let me try
>>> decoded it as one?" -- shouldn't there be some sort of hint in the NSH
>>> as to what the context field is carrying (like Type 2 has)? I've
>>> looked at  I-D.guichard-sfc-nsh-dc-allocation and
>>> I-D.napper-sfc-nsh-broadband-allocation which discuss how bits in the
>>> context header can be allocated, but would a receiver differentiate
>>> which type is in use (other than being configured to know that blobs
>>> are always of type X). Again, I'm hoping that there is an SFC document
>>> which explains the architecture in more detail then RFC7665.
>> 
>> MD Type 1 is not self-describing.
>> 
>>> 
>>> Section  2.5.1.  Optional Variable Length Metadata
>>> Optional variable length Context Headers have a Class (scope of the
>>> Type), a Type (explicit type of metadata, defined by the Class owner),
>>> and a Length.
>>> A length of 0 means that the context header has no data. It would be
>>> useful to explain what this actually means -- it sounds like a message
>>> with no content, but perhaps receivers are supposed to use the Type in
>>> this case to do something?
>> 
>> Yes :-) Precisely.
>> 
>>> 
>>> Section 3.  NSH Actions
>>> "NSH-aware nodes are the only nodes that may alter the content of NSH
>>> headers." -- this sentence is nonsensical. If a node doesn't know
>>> something is an NSH (is it not NSH-aware), telling it not to touch NSH
>>> headers is basically "Devices on the Internet shouldn't poke at things
>>> that they don't understand”.
>> 
>> It might be obvious or redundant, but it is not nonsensical.
>> 
>> We will reword it though. :-)
>> 
>>> 
>>> Section 3:
>>> "At the end of a service function path, an SFF, MUST be the last node
>>> operating on the service header and MUST remove NSH before forwarding
>>> or delivering the un-encapsulated packet." - I'm somewhat confused
>>> what this is trying to say, and assume that it is simply superfluous
>>> commas. But, with those removed the sentence is still confusing to me
>>> -- a SFF is the thing that does the forwarding, and so, by definition
>>> it has to be the last thing operating on the service header (unless
>>> the packet is dropped) -- so, what does the first MUST mean?
>> 
>> Yes, there is a superfluous comma. But the meaning of the sentence is “do not leak NSH outside the SFC domain”.
>> 
>> I’ll reword it.
>> 
>>> 
>>> Section 4.  NSH Transport Encapsulation
>>> "The presence of NSH is indicated via protocol type or other indicator
>>> in the outer encapsulation." -- just for my interest, what all
>>> encapsulations already have this defined / allocated?
>> 
>> Ethernet, GRE, VXLAN-GPE, others.
>> 
>>> 
>>> Section 5.  Fragmentation Considerations
>>> "As discussed in [I-D.ietf-rtgwg-dt-encap], within an administrative
>>> domain, an operator can ensure that the underlay MTU is sufficient to
>>> carry SFC traffic without requiring fragmentation." -- I think that
>>> that is stretching what I-D.ietf-rtgwg-dt-encap says a bit far. NSH
>>> Type 1 adds 24 octets (4 octet Base Header, 4 octet Service Path
>>> Header, 16 octet Context Header), plus the gets wrapped in a new
>>> transport, so let's say 24 octets for GRE. This means that, for a
>>> 1500byte packet I need an MTU of 1548 (I'm not sure all operators can
>>> support that everywhere). The Type 2 header is of variable length (and
>>> I don't think I saw a limit).
>> 
>> This text is reworded based on GenART.
>> 
>>> " For example, when NSH is encapsulated in IP, IP-level fragmentation
>>> coupled with Path MTU Discovery (PMTUD) is used. When, on the other
>>> hand, the underlay does not support fragmentation procedures, an error
>>> message SHOULD be logged when dropping a packet too big." -- I think
>>> that more guidance / text is needed here, e.g for v6. Just dropping
>>> the packet and logging an error doesn't solve the base issue causing
>>> the problem. If this protocol wasn't an "encapsulation" I wouldn't
>>> feel so strongly, but if the protocol says that it providing an
>>> encapsulation it needs to handle things that encapsulation protocols
>>> do.
>> 
>> This text came from OpsDir review.
>> 
>>> 
>>> 
>>> 
>>> 
>>> Nits:
>> 
>> 
>> All fixed.
>> 
>> Best,
>> 
>> —
>> Carlos Pignataro, carlos@cisco.com
>> 
>> “Sometimes I use big words that I do not fully understand, to make myself sound more photosynthesis."
>> 
>> 
>> 
>>> Section 1. Introduction
>>> ... current service function deployment models have been relatively
>>> static, and bound to topology
>>> [O] static, and bound
>>> [P] static and bound
>>> [R] grammar
>>> 
>>> ...
>>> Specifically, the following functions are necessary:
>>> 
>>>     The movement of service functions and application workloads in the
>>>     network.
>>> 
>>>     The ability to easily bind service policy to granular information,
>>>     such as per-subscriber state.
>>> 
>>> [R]: Numbers or bullets would make the much easier to read.
>>> 
>>> 
>>> Section 1.2 - Definition of Terms
>>> "Byte:  All references to "bytes" in this document refer to 8-bit
>>> bytes, or octets." -- this is a circular definition (the section is
>>> Definition of Terms, not Terminology) - suggest dropping the 8-bit
>>> bytes, octets covers it.
>>> 
>>> Throughout the document:
>>> The document says "A Service Classifier adds NSH.  NSH is removed by
>>> ...", and "NSH offers a common and standards-based header" -- is NSH a
>>> technology (as implied by the second phrase), or a thing added to a
>>> packet (like the first implies). Whatever the case, for the first, "A
>>> Service Classifier adds *the* NSH.  *The* NSH is removed by ..." (or,
>>> "an").
>>> 
>>> 
>>> Section 1.4. NSH-based Service Chaining
>>>  NSH creates a dedicated service plane, more specifically, NSH
>>> [O] plane, more
>>> [P] plane; more
>>> [R] grammar
>>> 
>>> 1.  Topological Independence: Service forwarding occurs within the
>>>      service plane, the underlying network topology does not require
>>> 
>>> [O] plane, the
>>> [P] plane; the    (or "plane, so the")
>>> [R] grammar
>>> 
>>> 
>>> Section  2.2.  NSH Base Header
>>> "MD Type: Indicates the format of NSH beyond the mandatory Base Header
>>> and the Service Path Header.  MD Type defines the format of the
>>> metadata being carried." -- please expand MD here. I wasted much time
>>> before guessing that this was MetaData Type.
>>> 
>>> 
>>> Given the
>>>  widespread implementation of existing hardware that uses the first
>>>  nibble after an MPLS label stack for ECMP decision processing, this
>>>  document reserves version 01b and this value MUST NOT be used in
>>> [O] 01b and this value
>>> [P] 01b. This value
>>> [R] grammar -- run on sentence, and it reads awkwardly with a
>>> semicolon before the "and" (but that would be grammatically correct,
>>> if preferred.)
>>> 
>>> 
>>> Forwarding OAM
>>>  packets unmodified by SFC elements that do not support SFC OAM
>>>  procedures may be acceptable for a subset of OAM functions, but can
>>>  result in unexpected outcomes for others, thus it is recommended to
>>> [
>>> O] others, thus
>>> [P] others; thus
>>> [R] grammar
>>> 
>>> This specification does not disallow the MD Type value from changing
>>>  along an SFP; however, the specification of the necessary mechanism
>>>  to allow the MD Type to change along an SFP are outside the scope of
>>>  this document, and would need to be defined for that functionality to
>>> [
>>> O] document, and
>>> [P] document and
>>> [R] grammar
>>> 
>>> 
>>> 
>>> Section 2.4:
>>> As SF or SFC Proxy -> A SF or SFC Proxy
>>> 
>>> Section 2.5.1:
>>> "the SFC-aware SF MUST NOT process the packet and MUST log at least
>>> once per the SPI for which the mandatory metadata is missing." --
>>> feels like you are missing some words after "log" -- presumably "must
>>> log an error", or "this event", or something.
>>> 
>>> 
>>> Section 6.1. SFFs and Overlay Selection
>>> This indirection -- SPI to overlay -- creates a true service plane.
>>>  That is, the SFF/SF topology is constructed without impacting the
>>>  network topology but more importantly, service plane only
>>> 
>>> [O] topology but more importantly,
>>> [P] topology; but more importantly,
>>> [R] grammar
>>> 
>>> This can be via the overlay or
>>>  underlay and in some case require additional configuration on the SF.
>>> 
>>> [O] some case
>>> [P] some cases
>>> [R] I think this is what was intended
>>> 
>>> Section 6.4. Service Graphs
>>> These classifiers may also of course
>>> 
>>> [O] may also of course
>>> [P] may, of course, also
>>> [R] readability
>>> 
>>>  modify the metadata associated with the packet.
>>> 
>>> Section 7.1. NSH Metadata and Policy Enforcement
>>> 
>>>  As described in Section 2, NSH provides the ability to carry metadata
>>>  along a service path.  This metadata may be derived from several
>>>  sources, common examples include:
>>> 
>>> [O] sources, common
>>> [P] sources. Common
>>> [R] grammar
>>> 
>>>     Network nodes/devices: Information provided by network nodes can
>>>     indicate network-centric information (such as VRF or tenant) that
>>>     may be used by service functions, or conveyed to another network
>>> 
>>> [O] functions, or conveyed
>>> [P] functions or conveyed
>>> [R] grammar
>>> 
>>> The granularity of classification may vary.  For
>>>  example, a network switch, acting as a classifier, might only be able
>>>  to classify based on a 5-tuple, whereas, a service function may be
>>> 
>>> [O] whereas, a service
>>> [P] while a service
>>> [R] readability
>>> 
>>> In both of the examples above, the service functions perform policy
>>>  decisions based on the result of the initial classification: the SFs
>>>  did not need to perform re-classification, rather they rely on a
>>> 
>>> [O] re-classification, rather
>>> [P] re-classification; instead,
>>> [R] grammar/readability
>>> 
>>>  antecedent classification for local policy enforcement.
>>> 
>>>  Depending on the information carried in the metadata, data privacy
>>>  considerations may need to be considered.  For example, if the
>>> 
>>> This feels like its punting on the problem more than it should, but
>>> I'm avoiding privacy and security in this particular review.
>>> 
>>>  metadata conveys tenant information, that information may need to be
>>>  authenticated and/or encrypted between the originator and the
>>>  intended recipients (which may include intended SFs only) .  NSH
>>> 
>>> [O] ) .
>>> [P] ).
>>> [R] punctuation
>>> 
>>>  itself does not provide privacy functions, rather it relies on the
>>>  transport/overlay layer.  An operator can select the appropriate
>>>  transport to ensure confidentially (and other security)
>>> 
>>> [O] confidentially
>>> [P] confidentiality
>>> [R] word choice
>>> 
>>> 
>>> Section 8. Security Considerations
>>> 
>>> NSH is always encapsulated in a transport protocol (as detailed in
>>>  Section 4 of this specification) and therefore, when required,
>>> 
>>> [O] ) and therefore,
>>> [P] ); and, therefore,
>>> [R] grammar
>>> 
>>> 
>>> 
>>> 
>>> W
>>> 
>>> 
>>> 
>>> 
>>> --
>>> I don't think the execution is relevant when it was obviously a bad
>>> idea in the first place.
>>> This is like putting rabid weasels in your pants, and later expressing
>>> regret at having chosen those particular rabid weasels and that pair
>>> of pants.
>>>  ---maf
>> 
> 
> 
> 
> -- 
> I don't think the execution is relevant when it was obviously a bad
> idea in the first place.
> This is like putting rabid weasels in your pants, and later expressing
> regret at having chosen those particular rabid weasels and that pair
> of pants.
>   ---maf