Re: [sfc] Detailed review of draft-ietf-sfc-nsh-12.txt
"Paul Quinn (paulq)" <paulq@cisco.com> Mon, 20 March 2017 17:53 UTC
Return-Path: <paulq@cisco.com>
X-Original-To: sfc@ietfa.amsl.com
Delivered-To: sfc@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D12B0131617 for <sfc@ietfa.amsl.com>; Mon, 20 Mar 2017 10:53:11 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.523
X-Spam-Level:
X-Spam-Status: No, score=-14.523 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, RP_MATCHES_RCVD=-0.001, SPF_HELO_PASS=-0.001, 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 x5-HaG3sgikl for <sfc@ietfa.amsl.com>; Mon, 20 Mar 2017 10:53:07 -0700 (PDT)
Received: from alln-iport-3.cisco.com (alln-iport-3.cisco.com [173.37.142.90]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 2D72D13157E for <sfc@ietf.org>; Mon, 20 Mar 2017 10:52:33 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=32412; q=dns/txt; s=iport; t=1490032353; x=1491241953; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=1OJto4uN0N8J7wNaYh1MA4OaQNwdZl4fvFXmg8wBYns=; b=SF4lqggeDB7GgmSfxM5JGQwQ614L/YCrI/V752V5f9XDtqNuJjrQQvfJ 5GIWLbBophkFl6TWGlmhJuNjC8vj5kGo7qiyxGsUq+ck5s3yv4S7e2j22 WDhV6MWF7DHGpBfYYVfvYkZUo5XQ9ydP1qfJDqRcyz0doI0uX5LpDa1D6 M=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0AUAQBhFtBY/4QNJK1dGQEBAQEBAQEBAQEBBwEBAQEBgycqgWsHjWuRXZVDgg6CRwGDWgKDDj8YAQIBAQEBAQEBayiFFQEBAQECARoNEy0HBAcFCwIBCBgVCRAyJQEBBA4FGYlfCKpaOopGAQEBAQEBAQEBAQEBAQEBAQEBAQEBHYZOggWBYYEJhCUBEQEMED+CdYIxBY9djHABkkKBe4Uog1aGMohQiwcBHzh8CFgVQREBhEIDHYFjdYcuBQIIF4EKgQ0BAQE
X-IronPort-AV: E=Sophos;i="5.36,195,1486425600"; d="scan'208";a="399909337"
Received: from alln-core-10.cisco.com ([173.36.13.132]) by alln-iport-3.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 20 Mar 2017 17:52:31 +0000
Received: from XCH-RCD-008.cisco.com (xch-rcd-008.cisco.com [173.37.102.18]) by alln-core-10.cisco.com (8.14.5/8.14.5) with ESMTP id v2KHqVVr014962 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 20 Mar 2017 17:52:31 GMT
Received: from xch-rcd-008.cisco.com (173.37.102.18) by XCH-RCD-008.cisco.com (173.37.102.18) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Mon, 20 Mar 2017 12:52:30 -0500
Received: from xch-rcd-008.cisco.com ([173.37.102.18]) by XCH-RCD-008.cisco.com ([173.37.102.18]) with mapi id 15.00.1210.000; Mon, 20 Mar 2017 12:52:30 -0500
From: "Paul Quinn (paulq)" <paulq@cisco.com>
To: Adrian Farrel <adrian@olddog.co.uk>
CC: "sfc@ietf.org" <sfc@ietf.org>
Thread-Topic: [sfc] Detailed review of draft-ietf-sfc-nsh-12.txt
Thread-Index: AdKRRHVLY7SkxborSRGHk+YoYh/yhAQiDDYA
Date: Mon, 20 Mar 2017 17:52:30 +0000
Message-ID: <F6E3FD84-6DCF-4D53-B194-E6FB852A13B8@cisco.com>
References: <027d01d29144$d743bcb0$85cb3610$@olddog.co.uk>
In-Reply-To: <027d01d29144$d743bcb0$85cb3610$@olddog.co.uk>
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.82.237.180]
Content-Type: text/plain; charset="us-ascii"
Content-ID: <E25A3AD07E159340954908AB73E202A9@emea.cisco.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/sfc/ByFQ2ICtAPtVr_HYDQ837xAD1wo>
Subject: Re: [sfc] Detailed review of draft-ietf-sfc-nsh-12.txt
X-BeenThere: sfc@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Network Service Chaining <sfc.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/sfc>, <mailto:sfc-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/sfc/>
List-Post: <mailto:sfc@ietf.org>
List-Help: <mailto:sfc-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/sfc>, <mailto:sfc-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 20 Mar 2017 17:53:12 -0000
Hi Adrian, > On Feb 27, 2017, at 4:59 PM, Adrian Farrel <adrian@olddog.co.uk> wrote: > > Hi all, > > Many thanks to Paul for the updated revision. This makes life a lot > easier. > > Herewith a review. I suspect some points I've raised are tracked in > email threads or tickets, or in the minutes from the interim. That's > OK: I just read and scribbled in a continuous stream of consciousness. > > A bit of a wood/trees scenario here and I can't promise I have caught > every concern on this pass. > > Thanks to all for the work that has gone before and which makes this > detailed review possible. > Again, thank you for the great review! > Cheers, > Adrian > > --- > > Requirements Language should not show as a numbered section. > If you're using XML use <note> within <front> rather than <section>. > > --- > > NSH needs to be expanded on first use in the body of the document (in > the Introduction). > > --- > > Throughout the document you need to look at the use of "NSH". It is > often missing an article (sometimes definite, sometimes indefinite). > > --- PQ> Will update accordingly. > > In section 2 > > NSH defines a new service plane protocol specifically for the > creation of dynamic service chains and is composed of the following > elements: > > 1. Service Function Path identification > > 2. Transport independent service function chain > > 3. Per-packet network and service metadata or optional variable > type-length-value (TLV) metadata. > > Not sure about the middle of these 3. Isn't that a feature (which is > fine to talk about somewhere) not one of the components? I think the > component missing from the list is the SI. So you might write: PQ> That's a fair point, so, for #2, perhaps: 2. Indication of location within a Service Function Path. I think the concept of location is more accurate than progress. > > 2. Indication of the progress along the Service Function Path. > > And I have some trouble parsing the third item. I think you mean > network and service metadata that is per-packet (although that is > perhaps a given as the NSH is also per packet). And I think you mean > that the metadata is optional, fixed length, or TLV-based. How about: > > 3. Network and service metadata that may be optionally included > per packet and that may be fixed length or constructed from > type-length-value objects (TLVs). > PQ> Let's simplify even more: 3. Optional, per packet metadata (fixed length or TLV). > --- > > Why do you point to 7665 for the definition of "Classifier" but then > define "Service Classifier" to mean (AFAICS) exactly the same thing? > > --- PQ> The definition in NSH provides important NSH-centric details that aren't in 7665. > > In 2.3 I think point 4 is a use case of metadata and so should be part > of point 3. > PQ> That works for me. > --- > > 3.1 > s/draft/document/ > > --- PQ> Thanks. > > Wondering whether "Context Headers" should be "Context Header" > throughout. > PQ> Given that we've merged the figure into one "blob" this probably makes sense. > --- > > 3.1 > > An NSH is composed of a 4-byte (all references to bytes in this draft > refer to 8-bit bytes, or octets) Base Header, a 4-byte Service Path > Header and Context Headers, as shown in Figure 1 below. > > s/and Context/and optional Context/ > > --- > > 3.1 > Service Path Header: provide path identification and location within > > s/provide/provides/ > > --- PQ> Done. > > 3.2 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > |Ver|O|C|R|R|R|R|R|R| Length | MD Type | Next Protocol | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > Obviously, we have discussed the reformatting of the Base Header and > ended up with > > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > |Ver|O| TTL | Length |R|R|R|R|MD Type| Next Protocol | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > > Need to: > - Add a description of the TTL field (lots of mailing list discussion) PQ> AFAIK, the chairs haven't called consensus here. Once they convey that, we'll update accordingly. > - Delete the C-bit PQ> I just saw consensus around that, so the next version will reflect the change. > - Update the descriptions of Reserved bits > - Update the description of MD Type PQ> I believe you mean the updates that reflect TTL if/when added? > > --- > > 3.2 > > Version: The version field is used to ensure backward compatibility > going forward with future NSH updates. It MUST be set to 0x0 by the > sender, in this first revision of NSH. 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 01 and this value MUST NOT be used in future > versions of the protocol. Please see [RFC7325] for further > discussion of MPLS-related forwarding requirements. > > I believe this text is presuming the NSH-in-MPLS encapsulation. I don't > think it should do that, but if it does we have to look at the first > nibble not just the first two bits. > > Now, as 4385 points out, if the first nibble is 4 or 6, IPv4 or IPv6 > will be assumed. So 0b0100 or 0b0110 accounts for you reserving version > 1 (just in case the C bit is 0 and for any value of the O bit). So far, > so good! > > But 4385 defines a first nibble of 0000 to mean "here is a control word" > and that would mean you needed to reserve version 0 as well :-( > > So my advice here would be that encapsulation in MPLS is not in scope > for this document, and you should not play with reserving version > numbers. PQ> That is where we started, but there was a lot of discussion on the list about the potential for conflict. I have to go dig up the threads but I'm hoping that some of the folks who expressed concern here will weigh in. > > --- > > 3.2 > > There's been a long thread on the text about the O-bit. > > O bit: Setting this bit indicates an Operations, Administration, and > Maintenance (OAM) packet. The actual packet format and processing of > SFC OAM messages is outside the scope of this specification (see [I- > D.ietf-sfc-oam-framework]). > > First para OK. > > SF/SFF/SFC Proxy/Classifer implementations, which do not support SFC > OAM procedures, SHALL discard packets with O-bit set. > > SF/SFF/SFC Proxy/Classifer implementations MAY support a configurable > parameter to enable forwarding received SFC OAM packets unmodified to > the next element in the chain. Such behavior may be acceptable for a > subset of OAM functions, but can result in unexpected outcomes for > others, thus it is recommended to analyze the impact of forwarding an > OAM packet for all OAM functions prior to enabling this behavior. > The configurable parameter MUST be disabled by default. > > These paras are controversial as discarding OAM packets (rather than > passing them through) can also result in unexpected outcomes. Consider, > if you will, simple end-to-end ping functions. > > (Also s/Classifer/Classifier/ twice) > > There is also a mismatch of "SHALL" and "MAY". > > I would like this text to read... > > === > SF/SFF/SFC Proxy/Classifer implementations that do not support SFC > OAM procedures, SHALL ignore the setting of the O-bit and SHALL > attempt to process the packet as normal. > === > > ...but I could live with... > > === > SF/SFF/SFC Proxy/Classifer implementations that do not support SFC > OAM procedures SHOULD discard packets with O-bit set, but MAY > support a configurable parameter to enable forwarding received SFC > OAM packets unmodified to the next element in the chain. > === > PQ> I can live with this as well, however, I think we need to chairs to make a final call about how the WG has opted to move forward wrt OAM. > For non OAM packets, the O-bit MUST be cleared and MUST NOT be > modified along the SFP. > > This paragraph needs simple clarification as follows > > === > The O-bit MUST be set for OAM packets and MUST NOT be set for non-OAM > packets. The O-bit MUST NOT be modified along the SFP. > === > > --- > > 3.2 > > C-bit is being retired as discussed. > > --- > PQ> Indeed. > 3.2 > > Length: total length, in 4-byte words, of NSH including the Base > Header, the Service Path Header and the context headers or optional > variable length metadata. The Length MUST be of value 0x6 for MD > Type equal to 0x1 and MUST be of value 0x2 or greater for MD Type > equal to 0x2. The NSH header length MUST be an integer number of 4 > bytes. The length field indicates the "end" of NSH and where the > original packet/frame begins. > > There is no "or optional variable length metadata." That is exactly what > the context headers are, and the context headers are optional. And the > last sentence is a little too much. So... PQ> We tried to draw a distinction between the mandatory fixed vs. tlv headers. I can see how it's not clear in the above text. > > Length: total length, in 4-byte words, of the NSH including the Base > Header, the Service Path Header, and any Context Headers. It the > MD Type is 0x1, the Length MUST be 0x6. If the MD Type is 0x2, the > Length MUST be 0x2 or greater. The actual NSH header length MUST be > an integer multiple of 4 bytes and is padded if necessary. > PQ> That works for me. > --- > > 3.2 > > 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 see IANA Considerations section > below. > > Maybe "...indicates the format of the metadata carried in the Context > Headers..." PQ> I think the current text is more explicit. Do you have a specific concern? > > NSH defines two MD types: > > s/NSH/This document/ > s/types/Type values/ PQ> Will update accordingly. > > 0x1 - which indicates that the format of the header includes fixed > length context headers (see Figure 4 below). > > 0x2 - which does not mandate any headers beyond the Base Header and > Service Path Header, but may contain optional variable length context > information. > > Add "...not defined in this specification." > PQ> Will update. > --- > > 3.2 > > Next Protocol: indicates the protocol type of the encapsulated data. > NSH does not alter the inner payload, and the semantics on the inner > protocol remain unchanged due to NSH service function chaining. > Please see IANA Considerations section below. > > s/due to/by/ > > This draft defines the following Next Protocol values: > > s/draft/document/ > > 0x1 : IPv4 > 0x2 : IPv6 > 0x3 : Ethernet > 0x4: NSH > 0x5: MPLS > > Fix formatting > > 0x6-0xFD: Unassigned > 0xFE-0xFF: Experimental > > Suggest to delete the last two lines as they are not protocols, but info > for the IANA Considerations section. PQ> Will update to reflect all of the above. > > --- > > 3.3 > > Service Path Identifier (SPI): identifies a service path. > Participating nodes MUST use this identifier for Service Function > Path selection. The initial classifier MUST set the appropriate SPI > for a given classification result. > > Both uses of "MUST" are unnecessary and the words could be deleted > without loss of meaning or tightness of specification. PQ> New text: Service Path Identifier (SPI): identifies a service path. Participating nodes use this identifier for Service Function Path selection. The initial classifier MUST set the appropriate SPI for a given classification result. > > --- > > 3.3 > > Service Index (SI): provides location within the SFP. The initial > classifier for a given SFP SHOULD set the SI to 255, however the > control plane MAY configure the initial value of SI as appropriate > (i.e. taking into account the length of the service function path). > Service Index MUST be decremented by Service Functions or by SFC > Proxy nodes after performing required services and the new > decremented SI value MUST be used in the egress NSH packet. The > initial Classifier MUST send the packet to the first SFF in the > identified SFP for forwarding along an SFP. If re-classification > occurs, and that re-classification results in a new SPI, the > (re)classifier is, in effect, the initial classifier for the > resultant SPI. > > What you are saying, I think is that the initial classifier MUST set > the SI equal to the first SI in the SFP. This SHOULD be 255, but an > SFP MAY be configured to start at a lower SI value. PQ> Correct. > > s/decremented/decremented by one/ (reference the endless email threads) > PQ> Will be updated. > --- > > 3.3 > > SI SHOULD be used in conjunction with Service Path Identifier for > Service Function Path Selection and for determining the next SFF/SF > in the path. > > What is the variation to this "SHOULD"? PQ> I suspect this is a vestige from an earlier option where the path ID was considered sufficient in some cases: a SFF could, in principle, use the path ID to select the next SFF if, for example, there were no SFs, or could opt to send packets to a re-classifier based on path alone. Having said that, I'm not sure there's much need to those cases. Do others? > > In addition to indicating > the location within a Service Function Path, SI can be used for > service plane loop detection. > > I think that, although this might still be true, the TTL will be more > helpful for loop detection and we should strike this text. > PQ> Once we finalize the TTL details, I agree this'll be cleaned up. > --- > > 3.4 > I suggest deleting Figure 4. It does not differ from the information in > Figures 1, 2, and 3, and risks not being kept up-to-date with those > figures. > > OTOH, you should say here that Length has a specific value. > PQ> I think figure 4 provides a very concrete representation of a type-1 header so it probably makes sense to keep it. I do agree: length should reflect the specific value. > --- > > 3.4 > > When the Base Header specifies MD Type = 0x1, four Context Headers, > 4-byte each, MUST be added immediately following the Service Path > Header, as per Figure 4. Context Headers that carry no metadata MUST > be set to zero. > > I think this is adding new and strange meaning to "Context Header". > See previous comments, but why is a Context Header suddenly a 4-byte > thing? Why is this not 3 3-byte Context Headers followed by 1 7-byte > Context Header? Or more realistically, why not just talk about "Up to > 16 bytes of metadata carried in the 16 byte Context Header that follows > the Base Header. Any context Header bytes not used to carry metadata > MUST be set to zero." PQ> This text should have been changed, it's out of date. We've moved to a single 16 byte "blob" -- as per figure 4 -- and this text should reflect that. > > --- > > 3.4 > > OLD > This specification does not make any assumption about the content > placed in the mandatory context field of the NSH header, and does not > describe the structure or meaning of the included metadata. > NEW > 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. > END > PQ> New text will be added. > --- > > 3.4 > > Upon receiving an NSH MD-type 1 packet, if the SFC-aware SF is > configured for mandatory use of metadata but does not yet receive the > data semantics for the mandatory context field, it MUST NOT process > the packet and MUST log at least once per the SPI for which a > mandatory metadata is missing. > > There seem to be some assumptions here: > - An SF having mandatory use of metadata is a config item and can't be > an implementation thing (i.e., you are not allowing an SF that must > always use metadata). PQ> The text covers the case where the SF needs metadata (let's say for policy enforcement) but does not (yet) know how to parse the metadata. I don't see any implication on config vs implementation. > - You are assuming that the semantics for the metadata are always > dynamically installed in the SF and not known a priori. > PQ> There was no intent to imply that, perhaps the easiest way to clarify: "...not yet receive..." --> "...not yet know..." (or something similar) > I don't think you should make those restrictions (although, obviously, > we should allow them as options). > > You also seem to be assuming that a non-SFC-aware SF can receive a > packet with an NSH and metadata and will somehow manage to parse the NSH > and find the metadata that it doesn't understand. Frankly, I think this > document talks only about SFC-aware SFs and SFC proxies. Other SFs are > by definition not in scope as they will never receive an NSH (except for > a bug that will reasonably cause the SF to drop the packet - or crash). > PQ> I don't see that conveyed. The text is just unambiguous about an SFC-SF. It makes not claims about others. > Finally "must not process" is ambiguous. I think you mean "must discard" > PQ> Fair point. > So, maybe... > > An SF or SFC Proxy that does not know the format or semantics of the > Context Header for an NSH with MD Type 1 MUST discard any packet with > such an NSH (i.e., MUST NOT ignore the metadata that it cannot > process), and MUST log the event at least once per the SPI for which > the event occurs (subject to thresholding). PQ> This works for me. > > --- > > 3.5 > > Again, Figure 5 is de trop. > You also need to discuss the value of the Length field. PQ> Again, I think being explicit with figures helps the reader. Updating length field makes sense. > > --- > > 3.5 > > This section has the usual confusion of whether there are multiple > context headers (each of variable length), multiple variable length > metadata, a single variable length context header (also called a > variable context header) containing metadata, or whatever. > PQ> I re-read it several times, and although it's a bit clumsy, I'm not sure it's confusing. > Figure 6, for example, shows one piece of variable length metadata > in an encoding element labelled "Variable Context Headers". PQ> I agree, I'll update the figure to show a better representation of the structure. That will probably help with the 3.5 text. > > This is made more confusing in the context of 3.4 where you have > the concept of 4 byte context headers. > > Later, in 3.5.1, you call it a TLV. > > I wish we could sort out this language. > > I think what you are trying to say is that the Context Header (singular) > can contain zero, one, or multiple metadata elements each encoded as a > TLV as shown in Figure 6. > > --- PQ> I'll take a pass at cleaning it up. > > 3.5.1 > > Discussions on the list about the sizing of the Metadata Class and > Type fields, the deprecation of the C-bit, the naming of the Type field > (which looks like Metadata Type which is easily confused with MD Type), > and the deprecation of the split ranges of the Type field. > > --- PQ> Yup, will update accordingly once we finish the discussions. > > 3.5.1 > > If multiple instances of the same TLV are included in an NSH packet, > but the definition of that TLV does not allow for it, the SFC-aware > SF MUST NOT process the packet and MUST log at least once per the SPI > for which multiple instances of that TLV is supplied. > > This is OK, but I prefer the more usual (and future-proof)... > > If multiple instances of the same TLV are included in an NSH packet, > but the definition of that TLV does not allow for it, the SFC-aware > SF MUST process first instance and ignore subsequent instances. > > --- PQ> Barring objection from the list, that's fine with me. > > 4. > > NSH-aware nodes are the only nodes that MAY alter the content of the > NSH headers. > > s/MAY/may/ PQ> Thanks. > > Is there a difference between an "SFC-aware node" and an "NSH-aware > node"? > > Isn't this sentence silly? "Only nodes that can understand the content > of the NSH can change the content of the NSH" > PQ> I agree. Barring objection, will remove. > --- > > 4. > Bullet 1 > > OLD > At the end of a service function path, a SFF, MUST be > the last node operating on the service header and MUST remove it. > NEW > At the end of a service function path, a SFF, MUST be > the last node operating on the service header and MUST remove it > before forwarding or delivering the encapsulated packet. > END > PQ> I think that's a good clarification but I fear the last part is confusing since I think of post-NSH the packet being un-encapsulated (unless I'm missing something you are trying to convey): "...and MUST remove it before forwarding or delivering the un-encapsulated packet." > --- > > 4. > Bullet 1 > > Multiple logical classifiers may exist within a given service > path. > > How so "logical"? Are they or are they not "classifiers"? > (Multiple occurrences in this bullet.) PQ> Logical in the sense that they need not be distinct classifiers. We've been consistent with the concept that classifiers, SFFs, etc. are logical: they can be co-mingled or distinct. > > --- > > 4. > Bullet 1 > > OLD > Non-initial classifiers may re-classify data and that re- > classification MAY result in a new Service Function Path. > NEW > Non-initial classifiers may re-classify data and that re- > classification MAY result in a the packet being assigned to a > different Service Function Path. > END PQ> Minor tweak: Non-initial classifiers may re-classify data and that re-classification MAY result in the selection a different Service Function Path. > > --- > > 4. > > 2. Select service path: The Service Path Header provides service > chain information and is used by SFFs to determine correct > service path selection. SFFs MUST use the Service Path Header > for selecting the next SF or SFF in the service path. > > It is probably a minor point, but the Service Path Header provides > service *path* information and not service chain information. > PQ> Worth fixing, thanks. > --- > > 4. > > 3. Update NSH: NSH-aware service functions (SF) MUST decrement the > service index. If an SFF receives a packet with an SPI and SI > that do not correspond to a valid next hop in a valid Service > Function Path, that packet MUST be dropped by the SFF. > > Non-NSH-aware SFs will, of course, either not see NSHs or barf. So it is > enough to say "SFs" (you have already defined the abbreviation). > > Say "decrement the service index by one". > PQ> Thanks. > --- > > 4. > Bullet 3 > > Classifier(s) MAY update Context Headers if new/updated context > is available. > > s/Classifier(s)/Classifiers/ > PQ> Will update. > --- > > 4. > Bullet 3 > > If an SFC proxy is in use (acting on behalf of a non-NSH-aware > service function for NSH actions), then the proxy MUST update > Service Index and MAY update contexts. When an SFC proxy > receives an NSH-encapsulated packet, it MUST remove the NSH > headers before forwarding it to an NSH unaware SF. When the SFC > Proxy receives a packet back from an NSH unaware SF, it MUST re- > encapsulates it with the correct NSH, and MUST decrement the > Service Index. > > I think "update contexts" means "update the metadata carried in the > Context Header. This does not say whether a proxy can add a Context > Header if one is not already present, or whether it can add a metadata > TLV, or remove one. > PQ> Correct, this means update the existing contexts. I believe we decided that only classifiers can add/remove (but as per a point above, those classifiers are logical and can be "part' of an SF). I need to go back and check the archives. Joel might remember since I think we talked about it. > Decide on "non-NSH-aware", "NSH unaware", "non-SFC-aware". > > Say "decrement the service index by one". PQ> Will update. > > --- > > 4. > > 4. Service policy selection: Service Function instances derive > policy (i.e. service actions such as permit or deny) selection > and enforcement from the service header. Metadata shared in the > service header can provide a range of service-relevant > information such as traffic classification. Service functions > SHOULD use NSH to select local service policy. > > "Service Function instance" is not properly defined in 7665 and not > previously mentioned in this document. OTOH, the term could usefully be > used in 7.1 para 2. PQ> I think we can simplify and change service function instance to service function. > > s/service header/NSH/ (twice) PQ> Thanks. > > Does the last sentence add anything? It appears to say what the previous > sentences say, but the "SHOULD" is ambiguous. Suggest deleting it. PQ> Perhaps remove it, and beef up the first a bit. Let me fiddle with it. > > --- > > In Figure 8 > > - "Select Service Function Path" is ambiguous. Isn't the SFF bound by > the SPI and so cannot select the path, only use the one it is > instructed to use? OTOH, the Classifier is responsible for determining > which path to use. PQ> That's a fair point. The intent was to convey the forwarding aspect of the SFF. Update: "Forward NSH Packets" > > - Surely the SFC proxy can update the Context Header as in bullet 3, > above. > PQ> Yes. > - Might an SFC proxy also select service policy for an NSH-unaware SF? PQ> I'm not sure what you mean. The proxy maps the packet to a local connection. The proxy doesn't have any service policy insight. > > --- > > 5. > > The presence of NSH is > indicated via protocol type or other indicator in the outer > encapsulation. > > You are prejudging the "encapsulation of NSH" work that is to be done. > You may be largely correct, but not always. We should omit this text. PQ> Must there not be some form of indicator -- explicit or otherwise? > > --- > > 7.1 > > This indirection -- path ID 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 > participants (i.e. most SFs) need not be part of the network overlay > topology and its associated infrastructure (e.g. control plane, > routing tables, etc.). As mentioned above, an existing overlay > topology may be used provided it offers the requisite connectivity. > > There is a lot hidden or implied in this paragraph. But consider an SF > that is somehow remote from the SFF. The SFF knows how to send to the > SF over the underlay network. But does the SF know how to send back to > the SFF? In some underlay technologies you can just reverse the > encapsulation addresses. In others, you can't. > > Perhaps the relationship between SF and SFF is configured (i.e., known > a priori)? > > Now consider an SF that is used on multiple SFPs. And consider further > that the SF might be reachable from different SFFs on the different > SFPs. Now selecting the return path becomes a challenge. > > Maybe "most SFs" is the clue. Maybe "some SFs" participate in the > control plane and routing tables. > > Perhaps this could be clearer. PQ> We can add something down the lines of: SF need to be able to return a packet to the appropriate SFF when service processing is complete. This can be via the over or under-lay and in some case require additional configuration on the SF. > > --- > > 8.1 > > NSH > itself does not provide privacy functions, rather it relies on the > transport/overlay layer. An operator can select the appropriate > transport to ensure the confidentially (and other security) > considerations are met. > > I think you are making a big leap to decide what the metadata documents > will define. > PQ> See below. > You are also failing to consider metadata that needs to be kept private > between the SF at SI 250 and the SF at SI 240 when there are other SFs > in between. PQ> I don't think that's prohibited in the text above: pick the right transport. > > I suggest replacing this paragraph with something like... > > This specification does not provide any privacy or security functions > for the NSH. Hop-by-hop security and privacy can be achieved by using > features of the underlay (transport) connections. Metadata privacy > and security considerations are a matter for the documents that > define metadata formats, encodings, and TLVs. PQ> I'm fine with the last sentence. > > --- > > 8.2 > > Why do you not consider imposing metadata at a subsequent point along > the path? This would happen, for example, if one SF needed to convey > information to another SF for a packet that did not already have > metadata. PQ> We do, it's a core use case and depicted in figure 15 and described in 8.2, bullet 1. > > --- > > 12 > > Obviously the IANA stuff needs updating to reflect changes elsewhere: > > 12.2.1 The C bit has gone and the reserved bits have moved. > > 12.2.2 See my comment on section 3.2 > > 12.2.3 The size of the MD Type registry is reduced. > Allowing one experimental code point looks like enough to me. > PQ> Will update all three. > 12.2.4 There is a thread about this. > The registry has to indicate that MD Class 0 has been assigned > by this document for "IETF use". > The advice to DEs needs to be supplied. > The size of the range of classes may be reduced to allow an > increase in the number of "types" > > Missing registry for "types" within the MD Class 0 > The same thread applies. > (I recall there was also action from the interim) > The registry will be empty but should be created in this doc > The name of this registry is confusable with "MD type" > The size of the range may be increases as the MD class range is > reduced. PQ> I'll go re-read the thread.
- [sfc] Detailed review of draft-ietf-sfc-nsh-12.txt Adrian Farrel
- Re: [sfc] Detailed review of draft-ietf-sfc-nsh-1… mohamed.boucadair
- Re: [sfc] Detailed review of draft-ietf-sfc-nsh-1… Paul Quinn (paulq)
- Re: [sfc] Detailed review of draft-ietf-sfc-nsh-1… Paul Quinn (paulq)
- Re: [sfc] Detailed review of draft-ietf-sfc-nsh-1… Duarte Cardoso, Igor