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

From: Brian Carpenter via Datatracker <[email-masked]>
Date: Mon, 09 Dec 2019 19:08:36 -0800

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?

---

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 10 Dec 2019 10:07:55 -0000

Hi Brian,

Thanks for your time with this.

In line...

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

Questions are good.

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

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

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.

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

> is out of scope for this document? There is a whole world of complexity

Yes, happy to say it is out of scope.

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

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

>     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

---

From: Brian E Carpenter <[email-masked]>
Date: Wed, 11 Dec 2019 11:49:45 +1300

Thanks Adrian. All OK for me, with one inserted comment below.

Regards
   Brian

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

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.

---

From: Brian E Carpenter <[email-masked]>
Date: Sat, 14 Dec 2019 12:05:17 +1300

Hi,

For the record, the -13 version addresses all my comments.

Thanks
   Brian Carpenter

---

From: Alissa Cooper <[email-masked]>
Date: Tue, 17 Dec 2019 21:08:57 -0500

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 <[email-masked]> wrote:
> 
> Hi,
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art