[bess] Shepherd's review of draft-ietf-bess-df-election-framework

[<[e-mail]>]

Hi,

Here is my review of the document:


General comments:
-------------------------

-          The document requires sometimes to be more generic as it describes a flexible framework for DF election rather than focusing on default election or HRW. See detailed comments.

-          The new FSM description is not enough clear in the text and AC-DF description provides updates to this FSM which are not aligned with the initial FSM description (events, states...).

-          It is not really clear if the document updates RFC7432 in some aspects (see detailed comments).

ID-Nits:
------


Miscellaneous warnings:

  ----------------------------------------------------------------------------



  == The document seems to lack the recommended RFC 2119 boilerplate, even if

     it appears to use RFC 2119 keywords -- however, there's a paragraph with

     a matching beginning. Boilerplate error?



     (The document does seem to have the reference to RFC 2119 which the

     ID-Checklist requires).

  -- The document date (April 12, 2018) is 12 days in the past.  Is this

     intentional?





  Checking references for intended status: Proposed Standard

  ----------------------------------------------------------------------------



     (See RFCs 3967 and 4897 for information about using normative references

     to lower-maturity documents in RFCs)



  == Missing Reference: 'RFC 7432' is mentioned on line 248, but not defined



  == Unused Reference: 'RFC7153' is defined on line 1034, but no explicit

     reference was found in the text



  == Unused Reference: 'RFC4271' is defined on line 1046, but no explicit

     reference was found in the text



  -- Possible downref: Non-RFC (?) normative reference: ref. 'HRW1999'


Detailed comments:
--------------------------

Section 2.1.

Using a linking word like "However" before "The described default DF election algorithm has some undesirable properties" would be suitable IMO to emphasis the contrast with the initial expectations of this algorithm mentioned in the previous paragraph.

"This document describes those issues..."
Maybe using "some of those issues" or "some known issues" is more accurate, I do not think that the document points all the issues.


"These mechanisms do involve changes to the default
   DF Election algorithm, however they do not require any protocol
   changes to the EVPN Route exchange and have minimal changes to their
   content per se."
Don't you consider that adding a community is a protocol change ?


I think this introduction should emphasis that there is a need for a flexible DF election as a single algorithm may not fit everyone's requirement.


Section 2.2


"In this particular case, in fact one of the PEs does not

      get elected all as the DF, so it does..."

[SLI] I do not understand completely this sentence (maybe an English issue on my side). Do you mean "get elected at all" ?





"So PE1, PE2 and PE3 are also the DFs for

      v1, v2 and v3 respectively."

[SLI] Maybe the word "also" can be removed.



"This need not be

   the case as there can be a v6 peering and supporting the EVPN

   address-family."

[SLI] "This need not be the case as the EVPN address-family can be carried over a v6 peering"





What does the two last paragraph bring in term of information ? It sounds redundant with the third point.





Section 2.2.2

"there is no procedure defined in EVPN [RFC7432<https://tools.ietf.org/html/rfc7432>] to trigger

   the DF re- election based on the ACS change on the DF."

s/re- election/re-election/



"This document modifies the default DF Election procedure so that the

   ACS may be taken into account as a variable in the DF election, and

   therefore EVPN can provide protection against logical failures."



That's not completely true that the document modifies the default DF election.

I think it was true when the AC-DF was a single document but in the framework of this document the sense is a bit different as the AC-DF may influence any DF election type not only the default one.



I would propose to remove this paragraph as section 2.3 clarifies what the document is proposing.

Or modify for example as below:

"This document proposes an optional modification of the DF Election procedure so that the

   ACS may be taken into account as a variable in the DF election, and

   therefore EVPN can provide protection against logical failures."





Section 2.3:

I think section 2.3 requires some enhancements to better explain the DF election framework and not only focusing on the HRW and the AC-DF.





OLD

"In order to address those issues, this document

   describes a new DF Election algorithm and a new capability that can

   influence the DF Election result:"



NEW PROPOSAL:

"In order to address those issues, this document

   introduces a new DF Election framework. This DF election framework allows the PEs to agree on a DF election type to use as well as the capabilities to enable. In general, a DF Election Type refers to the type of DF election

     algorithm that takes a number of parameters as input and determines

     the DF PE. A DF Election capability refers to an additional feature

     that can be executed along with the DF election algorithm, such as

     modifying the inputs (or list of candidate PEs) before the DF

     Election algorithm chooses the DF.



Within this framework, this document defines a new DF Election algorithm and a new capability that can influence the DF Election result:

o The new DF Election algorithm is referred to as "Highest Random

     Weight" (HRW). The HRW procedures are described in section 4<https://tools.ietf.org/html/draft-ietf-bess-evpn-df-election-framework-01#section-4>.



   o The new DF Election capability is referred to as "AC-Influenced DF

     Election" (AC-DF). The AC-DF procedures are described in section 5<https://tools.ietf.org/html/draft-ietf-bess-evpn-df-election-framework-01#section-5>.



   o HRW and AC-DF mechanisms are independent of each other. Therefore,

     a PE MAY support either HRW or AC-DF independently or MAY support

     both of them together. A PE MAY also support AC-DF capability along

     with the default DF election algorithm per [RFC7432<https://tools.ietf.org/html/rfc7432>].



In addition, this document defines a way to indicate the support of

   HRW and/or AC-DF along with the EVPN ES routes advertised for a given

   ES. Refer to section 3.2<https://tools.ietf.org/html/draft-ietf-bess-evpn-df-election-framework-01#section-3.2> for more details.

"







Section 3.1

"The FSM is normative in the sense that any design or implementation

   MUST behave towards external peers and as observable external

   behavior (DF) in a manner equivalent to this FSM."



What do you mean by "external peers" ? it usually refers to ebgp but I do not think this is the sense here.

Couldn't you just say: "Any design or implementation MUST comply with this FSM."



Do you consider this as an update of RFC7432, so RFC7432 implementations should apply this FSM ? You must be clear on this.





I have some concern about some FSM transition description:



   "2.  INIT on ES_UP: (i)do nothing."

Based on the diagram, it seems that the ES_UP event from the INIT state triggers a transition to DF_WAIT state. So it is not really "do nothing".





   "6.  DF_WAIT on DF_TIMER: do nothing."

Again a transition to DF_CALC state is expected here



"   7.  DF_CALC on entering or re-entering the state: (i) rebuild

       according list and hashes and perform election (ii) FSM generates

       CALCULATED event against itself."

Does (ii) need to be performed after (i) or simultaneously ? Not clear.





"   8.  DF_CALC on LOST_ES or VLAN_CHANGE: do nothing."

Don't you need to recompute the DF election (so go back to DF_WAIT) if an ES is withdrawn ?



"  9.  DF_CALC on RCVD_ES: do nothing."

Transition to DF_WAIT is expected here.



"  10. DF_CALC on CALCULATED: (i) mark election result for VLAN or

       bundle."

Transition to DF_DONE is expected



"   12. DF_DONE on VLAN_CHANGE or LOST_ES: do nothing."

Need to transition to DF_CALC per your diagram, but I would have expected a transition to DF_WAIT.



There is a missing transient: DF_DONE on RCVD_ES that should trigger a transition to DF_WAIT.





Section 3.2:

"it is necessary that all the participating PEs agree on

   the DF Election algorithm to be used."

I would propose the following change:

"it is necessary that all the participating PEs agree on

   the DF Election type and capabilities to be used."





"A PE SHOULD attach the DF Election Extended Community to any

     advertised ES route and the Extended Community MUST be sent if the

     ES is locally configured for DF Type HRW and/or AC-DF."

I would propose the following change:

"A PE SHOULD attach the DF Election Extended Community to any

     advertised ES route and the Extended Community MUST be sent if the

     ES is locally configured with a DF election type different from the Default Election algorithm or if a capability is requested to be used."



"In the case that they do, this particular PE will follow the

       procedures for the advertised DF Type and capabilities. "

Please use normative language here.





"- Otherwise if even a single advertisement for the type-4 route is

       not received with the locally configured DF Type and capability,

       the default DF Election algorithm (modulus) algorithm MUST be

       used as in [RFC7432<https://tools.ietf.org/html/rfc7432>]."

I think there is a missing case here. What if the DF type is different but the capabilities are matching ? Don't you want to use the capabilities with the Default Election algorithm ?





I think it would be worth to mention that:

-    For any new capability defined in future, the applicability/compatibility of this new capability to the existing DF types must be assessed

-    For any new DF type defined in future, the applicability/compatibility of this new DF type to the existing capabilities must be assessed

I don't know if we can say today that all DF types will be compatible with all capabilities and vice versa.







Section 3.3

Do you see this as an update of RFC7432 ? Does all EVPN implementations must comply to that ?







Section 4

"In the default DF Election algorithm (Section 2.1<https://tools.ietf.org/html/draft-ietf-bess-evpn-df-election-framework-01#section-2.1>), whenever a new PE

   comes up or an existing PE goes down, there is a significant interval

  before the change is noticed by all peer PEs as it has to be conveyed

   by the BGP update message involving the type-4 route. There is a

   timer to batch all the messages before triggering the service carving

   procedures.



   When the timer expires, each PE will build the ordered list and

   follow the procedures for DF Election. In the proposed method which

   we will describe shortly this "jittered" behavior is retained."



Is this text necessary ? As the FSM has already been defined, the reader knows that when using HRW, the implementation should comply to the new FSM.





Section 4.1

Any clue on why HRW has been picked compared to CHASH algorithms ?





Section 4.2

Do you confirm that Si could be either IPv4 or IPv6 ?



Nits here:

"BDF(v) = Sk: Weight(v, Es, Si) >= Weight(V, Es, Sk) and Weight(v, Sk) >= Weight(v, Es, Sj)."

Should be:

"BDF(v) = Sk: Weight(v, Es, Si) >= Weight(V, Es, Sk) and Weight(v, Es, Sk) >= Weight(v, Es, Sj)."







"and the actual length of the server IP address (whether V4

   or V6) is not really relevant The existing algorithm in [RFC7432<https://tools.ietf.org/html/rfc7432>] as

   is cannot employ both V4 and V6 neighbor peering address."

Missing "." between "relevant" and "The existing".

Moreover I'm not sure to understand this consideration.





Section 5



"It modifies the default DF Election procedures in [RFC7432<https://tools.ietf.org/html/rfc7432>] by removing

   from consideration any candidate PE in the ES that cannot forward

   traffic on the AC that belongs to the BD. "

It does not modify only the default election, it modifies all the DF types election.





"In particular, the AC-DF capability modifies the Step 3 in the

   default DF Election procedure described in [RFC7432] Section 8.5<https://tools.ietf.org/html/rfc7432#section-8.5>, as

   follows:"

This must be seen as an example only, as the AC-DF is applicable to any DF type, and not only the Default.



In the previous paragraph, it would be good to use normative language to describe that the candidate PE list is modified.







"The following example illustrates the AC-DF behavior"

Proposed change:

"The following example illustrates the AC-DF behavior applied to the Default DF election algorithm"









"In addition to the procedure described above, the following events

   SHALL modify the candidate PE list and trigger the DF re-election in

   a PE for a given <ES,VLAN> or <ES,VLAN-Bundle>:"

Please express your proposed changes as changes in the FSM described previously, especially using the same wording, events, states.

If you need to define new events or states, please state it.



Brgds,


Stephane



_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.