Early Review of draft-ietf-sfc-control-plane-08
review-ietf-sfc-control-plane-08-rtgdir-early-hardwick-2017-03-01-00

Hello

I have been selected as the routing directorate reviewer for this draft.
https://datatracker.ietf.org/doc/draft-ietf-sfc-control-plane/

The Routing Directorate seeks to review all routing or routing-related drafts
as they pass through IETF last call and IESG review, and sometimes on special
request, as in this case. The purpose of the review is to provide assistance to
the Routing ADs. For more information about the Routing Directorate, please see
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-sfc-control-plane-08.txt
Reviewer: Jonathan Hardwick
Review Date: 1 March 2017
Intended Status: Informational

Summary
I have significant concerns about this document and recommend that the Routing
ADs discuss these issues further with the authors.

Comments

Document Scope
This document aims to describe the requirements that the SFC data plane has on
the control plane.

My major concern with this document is that its scope is too narrow.  Without
giving a control plane architecture, this document’s usefulness is quite
limited.

The document is presumably useful to people working within the SFC WG as a
checklist of their requirements.  I assume (and hope) that SFC will in future
publish a control plane architecture that will satisfy these requirements.

Speaking with my implementer hat on, this document is not useful to me as it
does not describe anything that I can implement.  It does not describe what the
control plane looks like, what building blocks to use, etc.

Speaking with my IETF WG chair hat on, this document is not useful to me as it
gives no steer as to what protocols will be employed, or how.  This is causing
difficulties for other working groups.  For example, someone recently came to
the PCE WG with a draft describing an SFC control plane implemented with PCEP. 
I have no idea if the SFC WG wants to use PCEP in its control plane, so I am
unable to adopt said draft.  Meanwhile, there is a draft in BESS that documents
a completely different SFC control plane, and there may be others.  We are in a
confusing situation and the proposals are proliferating.  The SFC working group
needs to steer the protocol development work, but this document is completely
silent on it.

As such, I am not sure that it is worth the IETF publishing this document as it
stands.  However, I do very much encourage the SFC working group to continue
their work on the control plane and to develop an architecture which describes
it.

If we do go ahead and publish this, then I believe that the document should be
restructured and made much briefer.

Document Content
The document strays into discussions of management plane operation and
requirements.  For example, all of section 4.3 talks about management plane
operation and does not make it clear what the implications for the control
plane are.  I recommend that the document is reviewed within SFC and anything
that does not talk about a clear control plane implication is taken out.

The document frequently digresses into giving lists of examples.  For example,
section 3.3.1, 3.3.3, 4.4, 4.7, 4.8.  This is not a helpful way to capture
requirements.  Instead of giving open ended lists, the document should distil a
set of requirements out of the known examples, and state the requirements
instead.

Some of the passages in the document felt superfluous.

A)     Section 3.2.  I think this section adds nothing.

B)     The list of “functional objectives” in 3.3.1.

C)     Section 4.3, where, as stated above, I don’t see the relevance.

D)     Section 4.5.  This just seems to be saying that the control plane needs
to be able to modify SFCs.  I don’t find that surprising.  I’d be happier if
this was rewritten to talk about what the control plane must do.

The document is largely written in the passive voice, which makes understanding
it quite hard.  To take an example, in section 4.7.

OLD (passive voice)
   Liveness status records for all SF instances, and service function
   chains (including the SFPs bound to a given chain) are maintained by
   the SFC Control.
NEW (active voice)

   The SFC control plane must monitor the liveness of all SF instances, and all
   service function

   chains (including the SFPs bound to a given chain).

I recommend that an English speaker overhauls the text and replaces the passive
voice with the active voice.

Document Structure
I don’t understand why the document separates chapter 2 “Generic
Considerations” and chapter 4 “Additional considerations”.  They all just look
like requirements to me.  I would put chapter 3 first, then merge chapters 2
and 4, and try to distil the resulting text as much as possible.

Detailed comments

Section 2.2.
The list of information provided prior to bootstrapping includes “SFs serviced
by each SFF” but the information collected during bootstrapping includes “The
list of SFFs and the SFs that are attached to”.  Aren’t they the same thing?

Section 3.1
s/ Section 3.3.2 specifies such interface./ Section 3.3.2 specifies this
interface./ The final paragraph feels out of place in this section.

Section 3.2
s/during network attachment phase/during the network attachment phase/
s/Both centralized and distributed mechanism/Both centralized and distributed
mechanisms/

Section 3.3.1
“The SFC control plane should be responsible”  What do you mean “should be”? 
Responsibility should be clearly assigned. “…be part of the service function
discovery procedure” What is that?  First mention of it. “means to reduce
classification lookup time … should be supported by the SFC control plane” –
Why?  Isn’t that a job for the classifier to locally optimize? s/thanks to the
invocation of/by invoking/ s/does not convey that semantics/ does not convey
those semantics/ s/trust an existing SFC information/ trust existing SFC
information / s/configurable parameter/configurable parameters/

Section 3.3.2
s/Such table/This table/
s/Such instruct typically/This typically/
“groups of functionally equivalent SFs … may be used for load-balancing
purposes”  What is the relevance to the control plane that they may be used for
load balancing?  When you say “may be used” do you mean in all circumstances,
or do you mean only in some circumstances? “By default, an SFF relies on legacy
processing for forwarding these packets.”  Does “legacy processing” mean
forwarding using existing network headers?  I would not call that “legacy”.

Section 3.3.3
“SFs may need to output some processing results of packets” is too woolly.
“The SFC control needs the above status information for various tasks” is also
too woolly. In this section where you say (multiple times) “a context
information” do you mean “the context headers in the NSH”?  The latter would be
clearer. “Note that a context may be mandatory for "chain 1", but optional for
"chain 2".”  Sure, but what are the implications of this for the control plane?
“Multiple SFs may be located within the same physical node, but no SFF is
enabled in that same node, means to unambiguously forward the traffic from the
SFF to the appropriate SF must be supported.” – I can’t parse this sentence.
“Concretely, each SF must have a unique locator for unambiguous forwarding. 
This locator may be configured using this interface.”  This sounds like a
description of management plane function, not control plane function.  What
implications does the locator have for the control plane? “Special care should
be considered to avoid that instructions provided to distinct SFs lead to
loops.”  Rewrite to say what the control plane must do to avoid or mitigate
loops.

Section 4.1
s/variety if mechanisms/variety of mechanisms/

Section 4.2
s/both direction of/both directions of/
What does “full chain symmetry” mean?

Section 4.4
s/ accommodate such context/ accommodate this/
This section gives a couple of examples.  Are you giving a subset of all
possible options?  Are you saying the control plane can do either, or both, or
sometimes one and sometimes the other?

Section 4.6
“Specific criteria to send unsolicited notifications to a Control Element
should be fine tuned by the control plane”  What do you mean by “fine tuned”? 
What does the control plane actually need to do?

Section 4.7
s/liveliness/liveness/
“The classifier may be notified by the control plane” – Notified of what?
“The ability of an SFC Control Element to check the liveness of each SF present
in service function chain has several advantages…” – Advantages over what?  Do
you just mean it has several “features”?  What is the purpose of listing those
features?  What are the implications for the control plane?  Are you giving a
list of things the control plane must do? “Control Elements may be fed directly
or indirectly with inputs from these mechanisms.” – What do you mean by
directly versus indirectly?  Is this part of one of the Cx interfaces?

Section 4.8
“Even if setting the data collection cycle is deployment-specific, it is
recommended to support dynamic means…”  What do you mean by “dynamic means”?

Section 4.9
“Both short and long lifetimes may be assigned.” – Is not really a useful
statement.  What are the implications for the control plane?  A useful
statement would include what units these lifetimes should be conveyed in, and
their valid range.

Section 4.10.1
s/changes should monitored/changes should be monitored/
s/overladed/overloaded/
In the procedures for SFP adjustment, please give more detail on the control
plane requirements for “Replace target SF instances (e.g., in a failure or
overladed) with newly selected ones.”  How is this to be done?  I assume make
before break is required.  Are there any other control plane requirements?

Section 4.10.3
Please explain what you mean by “regional restoration”.
It isn’t clear to me what the first 3 paragraphs in this section have to do
with the final two, but perhaps if I understood what “regional restoration”
meant it would be clearer.