Re: [Gen-art] Genart last call review of draft-ietf-bess-nsh-bgp-control-plane-12

[Brian E Carpenter <[e-mail]>]

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
> 
>