Thank you Stephane. We’ll fix it for the next version.

Jorge

From: "stephane.litkow...@orange.com" <stephane.litkow...@orange.com>
Date: Thursday, May 24, 2018 at 2:03 PM
To: "Rabadan, Jorge (Nokia - US/Mountain View)" <jorge.raba...@nokia.com>, 
"bess@ietf.org" <bess@ietf.org>, 
"draft-ietf-bess-evpn-df-election-framework.auth...@ietf.org" 
<draft-ietf-bess-evpn-df-election-framework.auth...@ietf.org>
Subject: RE: Shepherd's review of draft-ietf-bess-df-election-framework

One small comment:

“this document
   elects a DF per <ES, BD>. This means that unlike [RFC 
7432<https://tools.ietf.org/html/rfc7432>],”

s/RFC 7432/RFC7432/

This creates a Nits


From: Rabadan, Jorge (Nokia - US/Mountain View) [mailto:jorge.raba...@nokia.com]
Sent: Wednesday, May 23, 2018 21:36
To: LITKOWSKI Stephane OBS/OINIS; bess@ietf.org; 
draft-ietf-bess-evpn-df-election-framework.auth...@ietf.org
Subject: Re: Shepherd's review of draft-ietf-bess-df-election-framework

Stephane,

We have addressed all your comments in rev 02 (just posted). Please see below 
with [S&J](Satya & Jorge).

Thank you for your thorough review!
Satya & Jorge


From: "stephane.litkow...@orange.com" <stephane.litkow...@orange.com>
Date: Tuesday, April 24, 2018 at 11:04 AM
To: "bess@ietf.org" <bess@ietf.org>, 
"draft-ietf-bess-evpn-df-election-framework.auth...@ietf.org" 
<draft-ietf-bess-evpn-df-election-framework.auth...@ietf.org>
Subject: Shepherd's review of draft-ietf-bess-df-election-framework
Resent-From: <alias-boun...@ietf.org>
Resent-To: <jorge.raba...@nokia.com>, <satya...@cisco.com>, 
<saja...@cisco.com>, <jdr...@juniper.com>, <kiran.naga...@nokia.com>, 
<senthil.sathap...@nokia.com>
Resent-Date: Tuesday, April 24, 2018 at 11:04 AM

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?
[S&J]The document includes the new text in RFC8174.





     (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
[S&J]fixed





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

     reference was found in the text
[S&J]fixed





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

     reference was found in the text
[S&J]fixed





  -- Possible downref: Non-RFC (?) normative reference: ref. 'HRW1999'
[S&J]it's not an I.D




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.
[S&J]ok, done.


“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.
[S&J]ok, done.



“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 ?
[S&J]ok, changed to:
These mechanisms do involve
   changes to the default DF Election algorithm, but they do not require
   any changes to the EVPN Route exchange and have minimal changes to
   their content per se.


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.
[S&J]added:
   In addition, there is a need to extend the DF Election procedures so
   that new algorithms and capabilities are possible. A single algorithm
   (the default DF Election algorithm) may not meet the requirements in
   all the use-cases.


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” ?
[S&J]fixed now







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

      v1, v2 and v3 respectively.”

[SLI] Maybe the word “also” can be removed.
[S&J]fixed



“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”

[S&J] We decided to change to this:

“One point to note is that the default DF election algorithm assumes that all 
the PEs who are multi-homed to the same Ethernet Segment (and interested in the 
DF Election by exchanging EVPN routes) use an Originating Router's IP Address 
of the same family. This does not need to be the case as the EVPN 
address-family can be carried over a v4 or v6 peering, and the PEs attached to 
the same ES may use an address of either family.”





What does the two last paragraph bring in term of information ? It sounds 
redundant with the third point.
[S&J]ok, the text is simplified now.







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/
[S&J]done





“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.”
[S&J]changed the text as per the above suggestion.







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.

“
[S&J]sounds good… took your suggestion with some minor modifications.









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

[S&J]external peers refers to other PEs in the ES, taking the FSM process as 
reference. Changed it to:

   The FSM is conceptual and any design or implementation MUST comply

   with a behavior equivalent to the one outlined in this FSM.



Do you consider this as an update of RFC7432, so RFC7432 implementations should 
apply this FSM ? You must be clear on this.
[S&J]we believe this is not an update of RFC7432, since all RFC7432 follow this 
FSM already. It is simply better to put it here to clarify the text in RFC7432.







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”.
[S&J]ok, changed







   “6.  DF_WAIT on DF_TIMER: do nothing.”

Again a transition to DF_CALC state is expected here
[S&J]ok, changed





“   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.
[S&J]ok, clarified







“   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 ?
[S&J]well, not since you want failures to converge fast





“  9.  DF_CALC on RCVD_ES: do nothing.”

Transition to DF_WAIT is expected here.
[S&J]ok, changed





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

       bundle.”

Transition to DF_DONE is expected
[S&J]ok, changed





“   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.
[S&J]changed to transition to DF_CALC





There is a missing transient: DF_DONE on RCVD_ES that should trigger a 
transition to DF_WAIT.
[S&J]ok, added







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.”
 [S&J]ok, changed







“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.”
 [S&J]changed





“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.
 [S&J]ok, done







“- 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 ?
[S&J]we don't think so. It may make sense for the AC-DF capability, but maybe 
not for others. So it is better to have predictable implementations and fall 
back to default if there is any inconsistency.







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.

[S&J]good point. Took your suggestion, slightly modified:

   o For any new capability defined in the future, the

     applicability/compatibility of this new capability to the existing

     DF types must be assessed on a per case by case basis.



   o Likewise, for any new DF type defined in future, its

     applicability/compatibility to the existing capabilities must be

     assessed on a per case by case basis.









Section 3.3

Do you see this as an update of RFC7432 ? Does all EVPN implementations must 
comply to that ?
[S&J]Not really, existing EVPN implementations are ok, they don't need to 
comply to this.









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.
 [S&J]ok, removed.







Section 4.1

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

[S&J] HRW is simpler to visualize and implement. Also, for small N 
(clusters/number of servers or participating PEs in case of EVPN ) it gives a 
good distribution.

Besides it does not require any precomputation and is of lesser complexity than 
Consistent hashing when servers go up/down and objects need to be re-mapped.





Section 4.2

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

[S&J] Yes, it should not matter.



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).”
 [S&J]ok, fixed



 [S&J] Also changed this in 4.2 (after the Wrand functions).

Here D(v,Es) is the 31-bit digest (CRC-32 and discarding the MSB as in 
[HRW1999] ) of the 14-byte stream, the Ethernet Tag v (4 bytes) followed by the 
Ethernet Segment Identifier (10 bytes). It is mandated that the 14-byte stream 
is formed by concatenation of the Ethernet tag and the Ethernet Segment 
identifier in network byte order. The CRC should proceed as if the architecture 
is in network byte order (big-endian).







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

[S&J]changed to the following:

A point to note is that the Weight function takes into consideration the 
combination of the Ethernet Tag, Ethernet Segment and the PE IP-address, and 
the actual length of the server IP address (whether V4 or V6) is not really 
relevant. The default algorithm in [RFC7432] cannot employ both V4 and V6 
neighbor peering addresses since RFC 7432 does not specify how to decide on the 
ordering (the ordinal list) when both V4 and V6 PEs are present





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.
 [S&J]ok, fixed







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

[S&J]changed to:

In particular, when used with the default DF Type, the AC-DF capability 
modifies the Step 3 in the DF Election procedure described in [RFC7432] Section 
8.5, as follows:







In the previous paragraph, it would be good to use normative language to 
describe that the candidate PE list is modified.
 [S&J]ok, done









“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”
 [S&J]ok, done











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

[S&J]para re-written to:

   In addition to the events defined in the FSM in Section 3.1, 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>. In

   the FSM of Figure 3, the events below MUST trigger a transition from

   DF_DONE to DF_CALC:

   i.   Local AC going DOWN/UP.

   ii.  Reception of a new Ethernet A-D per EVI update/withdraw for the

        <ES,VLAN> or <ES,VLAN-Bundle>.

   iii. Reception of a new Ethernet A-D per ES update/withdraw for the

        ES.



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.

_________________________________________________________________________________________________________________________



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.
_______________________________________________
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess

Reply via email to