Subject: [forces] AD Review of draft-ietf-forces-ceha

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 24 Jul 2013 09:48:54 +0100

Hi,

I have done my AD review of your document and see nothing major to hold
it back, but there are a large number of nits and a few petty questions.
We have a responsibility to send the I-D to IETF last call in the best 
possible shape so that it gets a good review.

Therefore, I have put the I-D into "Revised I-D Needed" state and will 
issue IETF last call when I see a new revision.

Thanks for the work,
Adrian

===

Section 1

Need to fix the 2119 boilerplate to include a reference to [RFC2119]

---

Section 1

OLD
   The following definitions are taken from [RFC3654]and [RFC3746]:
NEW
   The following definitions are taken from [RFC3654] and [RFC3746].  
   They are repeated here for convenience, but the normative definitions
   are found in the referenced RFCs.
END

---

Section 1

   o  ForCES Protocol -- The protocol used at the Fp reference point in
      the ForCES Framework in [RFC3746].

You need to explain what "Fp" is.

---

Section 1

   o  ForCES Protocol Layer (ForCES PL) -- A layer in the ForCES
      architecture that embodies the ForCES protocol and the state
      transfer mechanisms as defined in [RFC5810].

How can this definition come from 3654 or 3746 when it has a reference to
5810?

---

Figure 1

Maybe more normal to use "CEn" rather than "CEN"

---

Section 2

   The master CE controls the FEs via
   the ForCES protocol operating in the Fp interface.

s/in/on/

---

Section 3

   To achieve CE High Availability (HA), FEs and CEs MUST inter-operate
   per [RFC5810] definition which is repeated for contextual reasons in
   Section 3.1.

While you are repeating some of the material from 5810, you are also
restating some of it in new words, and adding text.

This gives a real problem with determining where the normative 
definition sits. We have to fix this!

Can you determine which sections are informational for this document and
which contain new text? 

Possibly the whole of Section 3 is just informational. If this is the
case you can replace the text quoted above with the following:

   To achieve CE High Availability (HA), FEs and CEs MUST inter-operate
   per [RFC5810].  The normative description of cold standby for CE HA
   is provided in [RFC5810].  This section provides a more wordy
   description of the procedures and is purely informational.  In the 
   event of any discrepancies between this text and that in RFC 5810,
   the text in RFC 5810 takes precedence.

---

Figure 2

Will it be obvious to the reader of this document what is meant by
"Asso Estb,Caps exchg"

---

Section 3.1.1

"CEID" is used without expansion. Although sometimes I find "CE ID" for
example in 3.1.2.

---

Section 3.1.1 has

   The FE connects to the CE specified on FEPO CEID component.  If it
   fails to connect to the defined CE, it moves it to the bottom of
   table BackupCEs and sets its CEID component to be the first CE
   retrieved from table BackupCEs.

This is not a problem, but is unusual. In many redundancy cases, the
primary object remains the favorite even when it has failed so that
when there is a restoration opportunity (such as a failure of the new
primary) it will resume its position as primary.

The question here is perhaps whether there is any distinction between
CEs except their role as primary or backup.

---

Figure 3 has "CEFTI" without explanation.

---

Section 3.1.1 has

   If the FE's FEPO CE Failover Policy is configured to mode 1, it
   indicates that the FE will run in HA restart recovery.  In such a
   case, the FE transitions to the Not Associated state and the CEFTI
   timer [RFC5810] is started.  The FE MAY continue to forward packets
   during this state.

This use of "MAY" implies to me that it is at least as common that the
FE does not forward packets in this state. Is that the intention?

---

Section 3.1.1

"HB" is used without expansion.

---

Section 4.2

"CEHB" and "FEHB" are used without expansion

---

Need to fix the line-length problems in Figure 4

---

Figure 5 implies that the association with the primary CE is the first
association formed. Is this a requirement?

---

Section 5 needs to give clear and unambiguous instructions to the IANA.
It seems that the text in Section 5 is currently a placeholder for the
correct text.

The shepherd write-up notes the need for this section to be reviewed by 
"ForCES IANA experts". If these people are not to be found in the ForCES
working group, then they exist nowhere. So the WG needs to look at this
section and work out what it should really say.

---

Section 6 is too sparse.

The Security Section in RFC 5810 is sound, so that is not the issue.
However, in considering HA you are considering a more complex scenario
where each CE must have its communications secured with the FE, and each
CE must be authenticated to the FE. That needs discussion.

Additionally, can the system be disrupted by simulating CE failure or by
disrupting CE-FE communications?

---

Section 7.2

I think 5812 is used in a normative way.

---

From: Jamal Hadi Salim <[email-masked]>
Date: Wed, 24 Jul 2013 08:57:34 -0400

Hi Adrian,
Thanks for the review. Comments below.

> Need to fix the 2119 boilerplate to include a reference to [RFC2119]

ok.

>    are found in the referenced RFCs.
Ok.

> You need to explain what "Fp" is.
Figure 1 points to what Fp is, but if it didnt jump at you right away then
we should
explain it.

---
> How can this definition come from 3654 or 3746 when it has a reference to
3654 and 3746 talked about the protocol in generalities.
5810 talks about the protocol. It came later and is only added
there for sake of clarity of what we mean when we say PL.

> Maybe more normal to use "CEn" rather than "CEN"

Ok.

>    the ForCES protocol operating in the Fp interface.
Ok.

> Can you determine which sections are informational for this document and
Ok, we'll review, however, note that there is intent in this document
to provide clarity in what is prescribed in 5810.
This is stated in Section 2.1, to quote:

"
The problem scope addressed by this document falls into 2 areas:
   1.  To describe with more clarity (than [RFC5810]) how current cold-
       standby approach operates within the NE cluster.
   2.  To describe how to evolve the [RFC5810] cold-standby setup to a
       hot-standby redundancy setup to improve the failover time and NE
       availability.
"

>    the text in RFC 5810 takes precedence.

The goal is for someone reading 5810 and not getting clarity on the
cold-standby HA\
to read this document instead. I think the part where you say "purely
informational"
may be overriding that intent.

> Will it be obvious to the reader of this document what is meant by
Will fix.

> "CEID" is used without expansion. Although sometimes I find "CE ID" for

Will fix for consistency.

> primary) it will resume its position as primary.
Depends. I actually have seen the sticky prioritization scheme you have
described being
requested for, but that desire subsidises after we describe that we
 provide for the master
CE to change the mastership if the older CE shows  up again i.e whatever
the FE does
it could be overriden by the CE.

> CEs except their role as primary or backup.

There is that implicit prioritization which says the ordering of the CEs in
the table
implies their priorities.

> Figure 3 has "CEFTI" without explanation.

It comes from 5810 - will fix.

> FE does not forward packets in this state. Is that the intention?

Abuse of MAY on our part. We'll fix.
[There is another knob which says whether to forward packets or not.
If that knob is on we continue to forward packets otherwise we dont]

> "HB" is used without expansion.

Will fix.

> "CEHB" and "FEHB" are used without expansion
Will fix.

> Need to fix the line-length problems in Figure 4
We'll fix

> association formed. Is this a requirement?

It is suggested in 5810 i.e that is part of the implicit prioritization
from above.
The administrator's listing of the controllers in a specific order
essentially
states which one needs to be connected-to first.
But if it is unavailable, then the next one in the list is connected to etc.

> It seems that the text in Section 5 is currently a placeholder for the

Will fix.

> section and work out what it should really say.
There are allocated ForCES IANA experts ;-> I will let the shepherd speak
for
himself - my gut-feel is he meant those people (some speaking in the third
person)
review those changes requested.

> disrupting CE-FE communications?
I cant think off the top of my head what new security issue
could be introduced merely because we are running HA (but i have not been
sleeping
well either, so likely missing something). We'll get back to you.

> I think 5812 is used in a normative way.
Will fix.

Thanks for the review Adrian.

cheers,
jamal

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 25 Jul 2013 20:37:46 +0100

Hi Jamal,

Cut down to issues for debate...

>> How can this definition come from 3654 or 3746 when it has a reference
>> to 5810?
>

I have no issue with this text, per se. However, the top of the section is
pretty clear that it is supposed to be a restatement of 3654 and 3746. You can't
have this cake and eat it.

>> Can you determine which sections are informational for this document
>> and which contain new text?
>
[snip]
cold-standby HA\
informational"
> may be overriding that intent.

OK. So it sounds (by point 1) like you *really* intend to update 5810 by
providing new normative text for cold-standby. If that is the case, let's go for
it full-steam ahead!
- metadata for updates 5810
- note in Abstract "This document updates RFC 5810 by doing foo"
- paragraph in Introduction to explain the update
- note in this section to say that this is a new normative description that
updates 5810

described being
> requested for, but that desire subsidises after we describe that we  provide
for the master
> CE to change the mastership if the older CE shows  up again i.e whatever the
FE does
> it could be overriden by the CE. 

OK, if it is a deliberate decision.

Thanks for the work,
Adrian

---

From: Jamal Hadi Salim <[email-masked]>
Date: Sat, 27 Jul 2013 03:10:29 -0400

> have this cake and eat it.

We'll fix the text to clarify.

ok.

> - note in this section to say that this is a new normative description that

Thanks for  catching this! Pretty sloppy of us.

> OK, if it is a deliberate decision.

It is -  when a CE dissappears in my experience at least it aint coming back
soon (either taken down for repair, or hardware failure).
So optimizing for that use case seemed reasonable (while providing a way for the
CE to override the policy).

cheers,
jamal