Reviewer: Brian Carpenter Review result: Ready with Issues Gen-ART Last Call review of draft-ietf-bess-nsh-bgp-control-plane-12 I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. Document: draft-ietf-bess-nsh-bgp-control-plane-12.txt Reviewer: Brian Carpenter Review Date: 2019-12-10 IETF LC End Date: 2019-12-13 IESG Telechat date: Summary: Ready with questions -------- 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. Minor issues: ------------- Actually these are mainly questions: 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? 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?) 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? > 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...)". > 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? Nits: ----- "such errors should be logged" ... "should log the event" "should either withdraw the SFPR or re-advertise it" Intentional lower case "should"? 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? -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
