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 >> >> _______________________________________________ BESS mailing list [email protected] https://www.ietf.org/mailman/listinfo/bess
