Preference-based EVPN DF Election
draft-ietf-bess-evpn-pref-df-13

# Éric Vyncke, INT AD, comments for draft-ietf-bess-evpn-pref-df-11

Thank you for the work put into this document. Please note that I was close to ballot a DISCUSS based on my non-blocking COMMENT points, but as I am not an expert in EPVN, I am trusting the responsible AD.

Please find below osome non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and several nits (please run your I-D through a spell-checker).

Special thanks to Stéphane Litkowski for the shepherd's detailed write-up including the WG consensus and the justification of the intended status even if using the old template.

I hope that this review helps to improve the document,

Regards,

-éric

# COMMENTS

## Abstract

Please fix the expansion of BUM as it is not `Broadcast, Unknown unicast and Broadcast traffic (BUM)` ;-)

## Section 1.1

Please fix the expansion of BUM as it is not `Broadcast, Multicast and Unknown unicast traffic (BUM)` ;-)
I was amused to read two different wrong expansions for BUM ;-)

I am sure that non SP will also use this technique, i.e., I wonder whether s/Service Providers/Network Operators/ would be beneficial.

## Section 2

Rather than using "DP" in `DP - refers to the "Don't Preempt me"` should "DNP" be more logical ?

## Section 3

Should RFC 8584 be formally updated as its reserved field is carved out ?

`The DP capability is supported by DF Algorithms Highest-Preference or Lowest-Preference, and MAY be used with the default DF Algorithm or HRW [RFC8584]` Is there any migration concern with the use of MAY ? I.e., part of the participating routers will use the DP and the other part not => leading to an inconsistent election result.

# NITS

s/tie-breakers/tiebreakers/
s/The existence of both provide/The existence of both provides/
s/achive/achieve/ ?
s/decribed/described/

'e.g.' is usually surrounded by ','

Thank you to Peter Yee for the SECDIR review

I’ll repeat his counsel that the Security Considerations should reference the applicability of RFC7432.

I support John Scudder’s DISCUSS position.

Thanks for your work to address my discuss and comments. I have just one new comment and a nit.

## COMMENTS

### Abstract needs to cover the update

Thanks for adding the "updates" header. You'll notice that idnits now complains:

  Checking nits according to https://www.ietf.org/id-info/checklist :
  ----------------------------------------------------------------------------
...
  -- The draft header indicates that this document updates RFC8584, but the
     abstract doesn't seem to mention this, which it should.

You should fix this by adding something to the abstract, along the lines of, "This document updates RFC 8584 by modifying the definition of the DF Election Extended Community."

## NITS

### Section 4.1

"The procedures for the used of the DP bit" -- s/used/use/

# GEN AD review of draft-ietf-bess-evpn-pref-df-11

CC @larseggert

Thanks to Vijay Gurbani for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/yyKue3u0e4F2LuAMSzjlxaEKHrQ).

## Comments

### Section 1.1, paragraph 2
```
     While the Default Designated Forwarder Algorithm [RFC7432] or the
     Highest Random Weight algorithm (HRW) [RFC8584] provide an efficient
     and automated way of selecting the Designated Forwarder across
     different Ethernet Tags in the Ethernet Segment, there are some use-
     cases where a more 'deterministic' and user-controlled method is
     required.  At the same time, Service Providers require an easy way to
```
Why is "deterministic" in quotes here? Is this algorithm not
(always) in fact deterministic? Could a more accurate term be
chosen?

### Section 4.1, paragraph 1
```
     Assuming the operator wants to control - in a flexible way - what PE
     becomes the Designated Forwarder for a given virtual Ethernet Segment
     and the order in which the PEs become Designated Forwarder in case of
     multiple failures, the following procedure may be used:
```
It's not clear what kind of procedure the list items a-f describe. The
individual list items read more like standalone considerations than
any kind of procedure.

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Typos

#### Section 4.1, paragraph 2
```
-        (100,0,Highest-Preferance), (200,0,Highest-Preference) and
-                             ^
+        (100,0,Highest-Preference), (200,0,Highest-Preference) and
+                             ^
```

#### Section 4.1, paragraph 11
```
-           if PE2's IP addres is lower than PE1's.  Same example applies
+           if PE2's IP address is lower than PE1's.  Same example applies
+                            +
```

#### Section 4.2, paragraph 1
```
-    Segment.  A potential way to achive a more granular load balancing is
-    decribed below.
+    Segment.  A potential way to achieve a more granular load balancing is
+                                     +
+    described below.
+      +
```

### Outdated references

Document references `draft-ietf-bess-evpn-virtual-eth-segment-11`, but `-12` is
the latest available revision.

### Grammar/style

#### Section 4.1, paragraph 3
```
nce is more preferred than PE2's). Hence PE1 becomes the Designated Forwarde
                                   ^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Hence".

#### Section 4.1, paragraph 4
```
dered list for vES1 is <PE2, PE1>. Hence PE2 becomes the Designated Forwarde
                                   ^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Hence".

#### Section 4.1, paragraph 6
```
s are used as tie-breakers. If more that one PE is advertising itself as the
                                    ^^^^
```
Did you mean "than"?

#### Section 4.3, paragraph 11
```
with DP=1, that is, PE2 (Pref=200). Hence PE3 will inherit PE2's preference a
                                    ^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Hence".

#### Section 4.3, paragraph 19
```
warder Election Algorithm different than the one configured in the rest of t
                                    ^^^^
```
Did you mean "different from"? "Different than" is often considered colloquial
style.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Section 2 defines "BD" but it's not used anywhere in this document.  Nor are "NDF", "BDF", "vES", "ISID", or "MAC-VRF".

The sole "SHOULD" in Section 4.1 leaves me with two questions: (1) What happens if I don't do this?  (2) The sentence itself appears to parse like "You SHOULD do this, but you don't actually have to" which seems like an abuse of BCP 14.  I think this is better if you just change "SHOULD" to "should", and drop the BCP 14 references altogether.

I support John and Warren's DISCUSS positions.

I support John Scudder's DISCUSS, as well as his comments -- the Introduction seems quite incomplete, and just sort of throws the reader into the deep end.


My original DISCUSS position is below; I still think that this is needlessly complex, but will bow to the WG consensus....
---
Why are there two algorithms (Highest-Preference and Lowest-Preference)?[0] This seems operationally dangerous and will lead to additional operational complexity, tricky to debug behaviors, additional implementation complexity, etc. 
Assuming that there *is* a good reason (and "Well, we couldn't decide,, so... ¯\_(ツ)_/¯" isn't one) these should be a section helping operators decide which algorithm they should deploy, and the pro's and con's of each. 


[0]: I did try and find this, but the closest I got was a note in the Shepherd Writeup saying: "There was a "last minute" agreement on managing the highest/lowest pref algorithm using different DF algs rather than a single one+local configs." -- this doesn't actually answer the question.

Thanks for working on this specification. 

I don't have any transport related points on this specification. However, as similar != same, it would be great if we can illustrate where the procedure in section 4.1 bullet D might be different or have different considerations between Highest-Preference and Lowest-Preference algorithms.