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 BESS@ietf.org https://www.ietf.org/mailman/listinfo/bess