Brian, thanks for your reviews of this document. Adrian, thanks for addressing Brian’s comments. I entered a No Objection ballot.
Alissa > On Dec 13, 2019, at 6:05 PM, Brian E Carpenter <brian.e.carpen...@gmail.com> > wrote: > > Hi, > > For the record, the -13 version addresses all my comments. > > Thanks > Brian Carpenter > > On 11-Dec-19 11:49, Brian E Carpenter wrote: >> Thanks Adrian. All OK for me, with one inserted comment below. >> >> Regards >> Brian >> >> On 10-Dec-19 23:07, Adrian Farrel wrote: >>> Hi Brian, >>> >>> Thanks for your time with this. >>> >>> In line... >>> >>>> Comments: >>>> --------- >>>> >>>> I am not a BGP expert and did not check the BGP details. This >>>> is a pretty complex mechanism so I would have liked to hear of >>>> at least a lab-scale implementation. I wouldn't be shocked if >>>> this was diverted to Experimental. >>> >>> At the moment I don't have access to a lab, so I won't comment about that. >>> I will note four things: >>> 1. I don't consider the mechanism to be "pretty complex", but "rather >>> simple". It may be that the difference is whether you have to pick up all >>> of BGP to understand this draft or whether it comes as a small increment. >>> 2. Obviously (?) the document has had eyes from a number of BGP experts >>> especially a very careful review by the document shepherd. It was shared >>> with IDR and caught comments from one of the IDR chairs. >>> 3. It's an IBGP mechanism not an EBGP mechanism, so the exposure to the >>> stability of the Internet is reduced. >>> 4. The BESS chairs ran a poll on the list to determine whether to progress >>> as is in advance of implementations. >>> >>>> Minor issues: >>>> ------------- >>>> Actually these are mainly questions: >>> >>> Questions are good. >>> >>>> There are numerous references, starting in the Abstract, to the >>>> "Controller" but it isn't defined or described in any one place. >>>> I expected to find it in RFC8300, but no. So what is the Controller? >>> >>> Right. This is a good catch. A "controller" is a centralised component >>> responsible for determining SFPs and maybe more. It is akin to an SDN >>> controller. We definitely need to add text for this. >>> >>> It is not an 8300 concept. Indeed, 8300 is principally focused on the >>> forwarding plane. >>> Furthermore, the control plane and orchestration aspects of SFC are a bit >>> sketchy in 7665. >>> draft-ietf-sfc-control-plane might have been a good source of information, >>> but the SFC WG appears to have given up on it. >>> >>> So, yes, we need a short definition in 1.2, and a paragraph in 2.2. >>> >>>> RFC8300 requires NSH+original_packet to be encapsulated in a Transport >>>> Encapsulation. In section 2.1 we find: >>>> >>>>> Note that the presence of the NSH can make it difficult for nodes in >>>>> the underlay network to locate the fields in the original packet that >>>>> would normally be used to constrain equal cost multipath (ECMP) >>>>> forwarding. Therefore, it is recommended that the node prepending >>>>> the NSH also provide some form of entropy indicator that can be used >>>>> in the underlay network. How this indicator is generated and >>>>> supplied, and how an SFF generates a new entropy indicator when it >>>>> forwards a packet to the next SFF are out of scope of this document. >>>> >>>> I would have expected that text to state that the entropy indicator is >>>> a property of the Transport Encapsulation required by RFC8300. (Isn't >>>> the Service Function Overlay Network in fact the embodiment of the >>>> Transport Encapsulation?) >>> >>> Well, yes and no. >>> The entropy indicator is carried in the transport encapsulation, and is >>> used by the transport (underlay) network. >>> But it is a property of the payload. In particular, it is a property of >>> what is encapsulated by the NSH. >>> The mechanism that encapsulates for the transport would normally have >>> visibility into the payload to create the entropy indicator (hashing on >>> specific fields), but the inclusion of the NSH makes that harder. Hence the >>> recommendation that the entropy indicator is provided by the mechanism that >>> prepends the NSH. >> >> Yes, understood. Of course IPv6 has its own header field precisely for this >> purpose and both RFC6437 and RFC6438 are there to help you ;-). Shame about >> IPv4. >> >>> >>> I think the text says this and that those skilled in the art (you have to >>> understand the use of the entropy indicators and the inclusion of the NSH) >>> will get this. >>> >>>> In section 2.2 we find: >>>> >>>>> When choosing the next SFI in a path, the SFF uses the SPI and SI as >>>>> well as the SFT to choose among the SFIs, applying, for example, a >>>>> load balancing algorithm or direct knowledge of the underlay network >>>>> topology as described in Section 4. >>>> >>>> I'm probably missing something, but doesn't that risk a conflict with >>>> the statement above about the entropy indicator? How would this choice >>>> of path be guaranteed congruent with the choice of path by the underlay >>>> network? Or doesn't that matter? >>> >>> No, this is a choice of SFIs, not a choice of paths between SFFs. >>> The former is determining the path in the overlay, the latter (using the >>> entropy indicator) is selecting the path through the underlay. >>> >>>>> 4.4. Classifier Operation >>>>> >>>>> As shown in Figure 1, the Classifier is a component that is used to >>>>> assign packets to an SFP. >>>>> >>>>> The Classifier is responsible for determining to which packet flow a >>>>> packet belongs (usually by inspecting the packet header),... >>>> >>>> Would it be better to state explicitly that the method of classification >>>> is out of scope for this document? There is a whole world of complexity >>>> in that "(usually...)". >>> >>> Yes, happy to say it is out of scope. >>> >>>>> 4.5. Service Function Forwarder Operation >>>> >>>> This section left me a bit puzzled. We've got the original packet, >>>> the classifier puts an NSH in front, we've got forwarding state, >>>> but we don't seem to have an IP header in front of the NSH to hand to >>>> the fowarding engine. Where's the Transport Encapsulation? >>> >>> OK. We can tweak that. We are principally interested in the overlay >>> forwarding in this section, but we should note that transmission between >>> SFFs is across the underlay and so there is a "transport" encapsulation. >>> >>>> Nits: >>>> ----- >>>> "such errors should be logged" ... "should log the event" >>>> "should either withdraw the SFPR or re-advertise it" >>>> Intentional lower case "should"? >>> >>> We'll go through these. The first few I looked at are reciting behaviour >>> defined in 8300 and I don't think it is appropriate to use upper case for >>> that. It is "as defined in RFC 8300" not new normative text. >>> >>>> IDnits said: >>>> -- The document has examples using IPv4 documentation addresses according >>>> to RFC6890, but does not use any IPv6 documentation addresses. Maybe >>>> there should be IPv6 examples, too? >>> >>> Maybe. I think we would need to add some v6 examples rather than convert >>> some of the existing (because there is a flow between the current examples). >>> I'm not sure it is very important because there is no use of prefixes, but >>> I'd be happy to include some v6 examples if someone wants to draft a couple. >>> >>> Best, >>> Adrian >>> >>> > > _______________________________________________ > Gen-art mailing list > Gen-art@ietf.org > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
