Subject: [Teas] Genart last call review of draft-ietf-teas-pce-central-control-03

From: Elwyn Davies <[email-masked]>
Date: Sun, 20 Aug 2017 13:54:30 -0700

Reviewer: Elwyn Davies
Review result: Almost Ready

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

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-teas-pce-central-control-03
Reviewer: Elwyn Davies
Review Date: 2017-08-20
IETF LC End Date: 2017-08-24
IESG Telechat date: 2017-08-31

Summary:
Amost ready.  I found this a well-written and mostly readily comprehensible
document although it contains a couple of instances of unexplained SDN/PCE
jargon (notably 'southbound').  My main concern is that the archtecture
suggests that extensions to PCEP will a.s. be needed and implies that
mechanisms will be needed to provide multiple extensions but neither specifies
detailed guidance as to how this might be done or points to work in progress
that would provide this guidance.  The implication of the statements in this
document are that an implementation or deployment might need to check if
particlar extensions are implemented in communication partners and also how to
behave if an unreconixed extension is received especially to avoid possible
DoS.   The other minor issues are only just abve the level of nits.

Major issues:
None

Minor issues:

Abstract:  The abstract is probably overlong.

s1:  RFC 4655 is itself an architecture that introduces the PCE.  It would be
helpful to explain that this document is an extension of the basic PCE
arcitecture except that it relies on the specific use f the PCEP for
intercommunication between architectural elements providing traffic control and
routing whereas RFC 4655 does not assume any particular protocol.

s3.2: It would be desirable to reiterate the problem of synchronization
mentioned in s2.1.2 which is relevant to all the high level functions.

s4: Since this document effectively gurantees that extensions to PCEP will be
created and since RFC 5440 contains no extensibility procedures/guidelines, it
seems to me that either this document should indicate how PCEP extensions and
profiles are added to the base protocol or should point  to a (normative and
currently non-existent) document that updates RFC 5440 and defines how
extensions shoould be structured and provides ways for implementations to
determine what is supported, if necessary.

s5, para 2:  The second part of this paragraph;
   However, debate will rage over overall system security and the opportunity
   for attacks in an architecture with a central controller since the network
   can be vulnerable to denial of service attacks on the controller, and the
   forwarding system may be harmed by attacks on the messages sent to
   individual NEs. In short, while the interactions with a PCE-based controller
   are not substantially different from those in any other SDN architecture,
   the security implications of SDN are still open for discussion. The IRTF's
   SDN Research Group (SDNRG) discussed this topic.
This text needs to be rewritten to be suitable for inclusion in an RFC that is
a document of record.

s6:  The PCE, depending on which of the aspects mentioned in s3 and the
technology being managed (LSPs, transport paths, etc.) are involved in a
particular imlemetation/deployment, will need to access the relevant state
information in NEs and possibly other PCEs using relevant managent protcol(s)
and MIBs or similar.  Integrating this state information with the PCEP
management information wil be key to effective operation of the centralized PCE
system.

s10:  It is arguable, IMO, that RFC 5440 and I-D.ietf-pce-pce-initiated-lsp are
normative references.  The architecture implies their usage and someone
implementing the architecture would need to be familiar with the details of the
extended PCEP, particularly in respect of the manageability and security
aspects of the protocol.

Nits/editorial comments:

Abstract, para 1: Statements of implied omnipotence by mortals a.s. lead to
hubristic consequences...  s/any definition of "optimal"/virtually any
definition of "optimal"/

Abstract (and subseqently):  The term 'southbound' is SDN specific jargon and
should be explained.  Given that the abstract is already (IMO) too long, it
would be wise to remove it from the abstract (I don't think it is essential)
and provide the explation in s1.

s1, para 4: See comment above... s/any form of routed or switched
network./virtually any form of routed or switched network./

s2, para 2 and Figures 1-7:  It would be useful to add a note in s2 to indicate
that the TED box in all the figures should imply that other databases may also
be accessed.

s2, last para (just before Fig.3): s/use case- specific/use case-specific [or
use case specific]/

s2.1, para 1: The text at the beginning of the para doesn't read very easily -
the 'or' disguises the fact that the two problems are on either sideof the
'or'.  Suggest someting like: OLD: Systems with central controllers are
vulnerable to two problems: failure or overload of the single controller. NEW:
Systems with a single central controllers are vulnerable to two problems:
  - controller failure, and
  - controller overload.
ENDS

s3.1.4, last para:
     new protocol extensions may be needed, and some are already being worked on
     in [I-D.ietf-pce-wson-rwa-ext].
 The second clause of this is not future proof:   Suggest
s/and some are already being worked on in [I-D.ietf-pce-wson-rwa-ext]./as, for
example, described  in [I-D.ietf-pce-wson-rwa-ext]./

s3.1.5, para 1: s/the header or a packet/the header of a packet/ (I think -
certainly for IPv6 use case)

s3.2.2, last para:  The final sentence is probably superfluous - and, if it
remains, probably needs a reference to explain what a FEC is.

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 24 Aug 2017 17:06:24 +0100

Elwyn,

Thanks for the review.

> Summary:
> Almost ready.  I found this a well-written and mostly readily comprehensible
> that would provide this guidance.  

I'm pretty sure that this architecture should not tell the protocol designers how to do their jobs. So "detailed guidance" may be too much, but Section 4 does give "Guidance for Solution Developers".

Perhaps the simplest thing is to point at draft-zhao-pce-pcep-extension-for-pce-controller which is where early work on protocol extensions lives. It's not a working group document (yet?) and it still has some issues, but I've changed
OLD
   It is anticipated that new documents will be produced for each use
   case dependent on support and demand.  Such documents will explain
   the use case and define the necessary protocol extensions.
NEW
   It is anticipated that new documents (such as 
   [I-D.zhao-pce-pcep-extension-for-pce-controller]) will be produced
   for each use case dependent on support and demand.  Such documents
   will explain the use case and define the necessary protocol extensions.
END

> The implication of the statements in this
> DoS.

RFC 5440 already describes how to handle unknown protocol elements. Additionally, the PCEP Open message allows for exchange of capabilities information. We can make these points as:

OLD
   The only work expected to be needed is extensions to
   existing PCEP messages to carry additional or specific information
   elements for the individual use cases, which maintain backward
   compatibility and do not impact existing PCEP deployments.  Where
   possible, consistent with the general principles of how protocols are
   extended, any additions to the protocol should be made in a generic
   way such that they are open to use in a range of applications.
NEW
   The only work expected to be needed is extensions to
   existing PCEP messages to carry additional or specific information
   elements for the individual use cases, which maintain backward
   compatibility and do not impact existing PCEP deployments.  [RFC5440]
   already describes how legacy implementations handle unknown 
   PCEP extensions and how to use the PCEP Open message to indicate
   support for PCEP features.  Where possible, consistent with the general
   principles of how protocols are extended, any additions to the
   protocol should be made in a generic way such that they are open
   to use in a range of applications.
END

>  The other minor issues are only just abve the level of nits.
> 
> Major issues:
> None
> 
> Minor issues:
> 

Ah, why use 3 words when 10 will do?

RFC Ed suggests that up to 20 lines is OK. We have 21. I couldn't immediately see anything gratuitously superfluous, so I'm leaving it as is.

> arcitecture except that it relies on the specific use of the PCEP for

Hmmm. You're right. 4655 doesn't make protocol assumptions. In fact it pre-dated the WG's choice of a protocol.
Of course, since then PCEP has been adopted and is the de facto protocol for PCE. 

How about...

OLD
   This document introduces the architecture for PCE as a central
   controller, examines the motivations and applicability for PCEP as a
   southbound interface, and introduces the implications for the
   protocol.  A PCE-based central controller can simplify the processing
   of distributed control plane by blending it with elements of SDN and
   without necessarily completely replacing it.
NEW
   This document introduces the architecture for PCE as a central
   controller as an extension of the architecture described in [RFC4655]
   and assumes the continued use of PCEP as the protocol used between
   PCE and PCC.  The document also examines the motivations and 
   applicability for PCEP as a southbound interface and introduces the 
   implications for the protocol used in this way.  A PCE-based central 
   controller can simplify the processing of distributed control plane by
   blending it with elements of SDN and without necessarily completely
   replacing it.
END

> mentioned in s2.1.2 which is relevant to all the high level functions.

I agree that synchronization of "parallel" stateful PCEs is a very real (and potentially hard) problem. I agree that this issue needs to be exposed wherever  the concept of parallel controllers is present.

But how is this relevant to Section 3.2?

I have added a paragraph to the bottom of Section 4 as...

   Note that protocol mechanisms to handle synchronization of state
   in parallel PCE-based controllers will also be required if parallel controllers
   are used as described in Section 2.1.2.  There is a discussion of mechanisms
   to achieve PCE state synchronization in [I-D.ietf-pce-stateful-pce].

> determine what is supported, if necessary.

Actually (as above) I think 5440 does contain adequate procedures.

Section 6.2 describes the exchange of capabilities.
Section 6.9 handles receipt of unknown messages.
Section 7.2 shows how objects are marked as mandatory to process of not and goes on to describe processing of unknown objects.
Section 7.1 describes how to handle unknown TLVs.

> This text needs to be rewritten to be suitable for inclusion in an RFC that is

Oh. :-)

We could (of course?) not hope to reach consensus one a number of issues related to SDN. So we tried some weasel words. Apparently your BS detector is functioning perfectly.

How about something a little more definitive...

However, an architecture with a central controller has a central point of failure and
this is also a security weakness since the network can be vulnerable to
denial of service attacks on the controller. Similarly, the central controller
provides a focus for interception and modification of messages sent to
individual NEs. In short, while the interactions with a PCE-based controller
are not substantially different to those in any other SDN architecture,
the security implications of SDN have not been fully discussed or described.
Therefore, protocol and applicability work around solutions for this
architecture must take proper account of these concerns.

> management information wil be key to effective operation of the centralized PCE

Is this any different from any other use of PCE, and especially stateful PCE?

I have added a note to highlight it.

   An implementation of a PCE-based controller will require access to information about the state of
   the network, its nodes, and its links.  Some of this will be the TED as is normal for a PCE and can be
   collected using the mechanisms already in place (such as listening to the IGPs, using BGP-LS
   [RFC7752], or northbound export of YANG-encoded data [I-D.ietf-teas-yang-te-topo].  More
   information may be collected in the LSP database as described for stateful PCEs as described 
   in [RFC7399] and [I-D.ietf-pce-stateful-pce].  Additional information may be needed for other
   specific use cases and will need to be collected and passed to the controller.

> extended PCEP, particularly in respect of the manageability and security

Sure.

> hubristic consequences...  s/any definition of "optimal"/virtually any

Hubris is in the eye of the whichever one of the gods is beholding us today.

s/any definition/most definitions/

> and provide the explation in s1.

Removed from Abstract.
Added "(i.e.,  a control protocol for communicating from the central controller to network elements)" to s1.

> network./virtually any form of routed or switched network./

Ditto above.

> that the TED box in all the figures should imply that other databases may also

OK. Added a note

> s2, last para (just before Fig.3): s/use case- specific/use case-specific [or

Hmmm. An xml2rfc bug. Bypassed.

> 'or'.  Suggest someting like: 
>OLD:
> Systems with central controllers are

OK

> s3.1.4, last para:
>      new protocol extensions may be needed, and some are already being worked
>      on in [I-D.ietf-pce-wson-rwa-ext].

OK

> certainly for IPv6 use case)

Ack

> remains, probably needs a reference to explain what a FEC is.

What the FEC?
Removed.

Thanks again,
Adrian