Re: [bess] Genart last call review of draft-ietf-bess-evpn-prefix-advertisement-10

[Rabadan, Jorge (Nokia - US/Mountain View)]

Elwyn,

Thank you very much for your thorough review.
We addressed all your comments.. see below for more details, with [JORGE].

Thx
Jorge


-Original Message-
From: Elwyn Davies <elw...@dial.pipex.com>
Date: Saturday, April 14, 2018 at 12:59 PM
To: "gen-...@ietf.org" <gen-...@ietf.org>
Cc: "draft-ietf-bess-evpn-prefix-advertisement@ietf.org" 
<draft-ietf-bess-evpn-prefix-advertisement@ietf.org>, "i...@ietf.org" 
<i...@ietf.org>, "bess@ietf.org" <bess@ietf.org>
Subject: Genart last call review of draft-ietf-bess-evpn-prefix-advertisement-10
Resent-From: <alias-boun...@ietf.org>
Resent-To: <jorge.raba...@nokia.com>, <wim.henderi...@nokia.com>, 
<jdr...@juniper.net>, <w...@juniper.net>, <saja...@cisco.com>, 
<matthew.bo...@nokia.com>, <stephane.litkow...@orange.com>, 
<martin.vigour...@nokia.com>, <db3...@att.com>, <aretana.i...@gmail.com>, 
Zhaohui Zhang <zzh...@juniper.net>, <zzh...@juniper.net>
Resent-Date: Saturday, April 14, 2018 at 12:59 PM

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-bess-evpn-prefix-advertisement-10
Reviewer: Elwyn Davies
Review Date: 2018-04-14
IETF LC End Date: 2018-04-10
IESG Telechat date: 2018-05-10

Summary:  Almost ready.  My main concerns are the lack of a good 
introduction
and a rather weak definition of the format of the new EVPN route option
(looking back on RFC 7342, I think this could be said about that document
also!).  This is very technical material and  a good introduction would help
readers who are not  already deeply into the Data Center area to understand
what is going on.  Also this document has some remaining vestiges of being
written like an academc paper (some were fixed in the previous revision),
particularly the spurious notion of having 'conclusions' (material actually
deserves to be in the Intro)

Major issues:

None

Minor issues:
Lack of a proper introduction: An introduction should precede the 
terminology
section and needs to be somewhat clearer about the context (presumably
multi-tenant data centers). A reference to RFC 7365 which describes the data
center model in which the EVPN mechanism is expected to be employed would be
very useful. A somewhat expanded version of the text in s2 would be a good
basis for the introduction. The ss2.1 and 2.2 with a short new header would
become a new section (3) which is the Problem Statement.
[JORGE] OK, I shifted the intro and terminology sections, and expanded the 
introduction as you suggested.

s5: Associated with my previous comment... An RFC is not a scientific paper 
and
does not have 'conclusions'.  On reading s5, it strikes me that this text 
would
actually make quite a nice part of the introduction, probably interpolated
after the first paragraph of the current s2.
[JORGE] ok, removed the conclusions section and took the content to the 
introduction.

Terminology import: The current s1 contains a number of terms that are 
actually
defined in RFC 7365 (Data Center, Tenant System, Network Virtualization 
Edge,
etc). Pointing to RFC 7365 s1.1 would be helpful.
[JORGE] Added this in the terminology section:
"This document also assumes familiarity with the terminology of
 [RFC7432], [RFC8365] and [RFC7365]."


s1, VNI: There is some difference between the usage of VNI in RFC 8365 
(Section
5.1), in RFC 7365 (s3.1.2) where it means Virtual Network Instance and in 
this
draft (... Identifier). This is potentially confusing to the naive reader 
and
definitely confusing with the usage of VNI in item (4) of s4.1 where the 
VNI is
the VXLAN Network Identifier. It would be better if VNI meant the same 
thing in
all this closely related work. Please review all instances of VNI in the 
draft
to make sure they are talking about the same thing.
[JORGE] Done. I also added:
   "VNI: Virtual Network Identifier. As in [RFC8365], the term is used as
  a representation of a 24-bit NVO instance identifier, with the
  understanding that VNI will refer to a VSID in NVGRE, or VNI in
  VXLAN, etc. unless it is stated otherwise."


s2.1:
>[RFC7432] is used as the control plane for a Network Virtualization
>Overlay (NVO3) solution in Data Centers (DC),
The acronym NVO3 is used here as opposed to NVO which is used elsew

[bess] Genart last call review of draft-ietf-bess-evpn-prefix-advertisement-10

[Elwyn Davies]

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

.

Document: draft-ietf-bess-evpn-prefix-advertisement-10
Reviewer: Elwyn Davies
Review Date: 2018-04-14
IETF LC End Date: 2018-04-10
IESG Telechat date: 2018-05-10

Summary:  Almost ready.  My main concerns are the lack of a good introduction
and a rather weak definition of the format of the new EVPN route option
(looking back on RFC 7342, I think this could be said about that document
also!).  This is very technical material and  a good introduction would help
readers who are not  already deeply into the Data Center area to understand
what is going on.  Also this document has some remaining vestiges of being
written like an academc paper (some were fixed in the previous revision),
particularly the spurious notion of having 'conclusions' (material actually
deserves to be in the Intro)

Major issues:

None

Minor issues:
Lack of a proper introduction: An introduction should precede the terminology
section and needs to be somewhat clearer about the context (presumably
multi-tenant data centers). A reference to RFC 7365 which describes the data
center model in which the EVPN mechanism is expected to be employed would be
very useful. A somewhat expanded version of the text in s2 would be a good
basis for the introduction. The ss2.1 and 2.2 with a short new header would
become a new section (3) which is the Problem Statement.

s5: Associated with my previous comment... An RFC is not a scientific paper and
does not have 'conclusions'.  On reading s5, it strikes me that this text would
actually make quite a nice part of the introduction, probably interpolated
after the first paragraph of the current s2.

Terminology import: The current s1 contains a number of terms that are actually
defined in RFC 7365 (Data Center, Tenant System, Network Virtualization Edge,
etc). Pointing to RFC 7365 s1.1 would be helpful.

s1, VNI: There is some difference between the usage of VNI in RFC 8365 (Section
5.1), in RFC 7365 (s3.1.2) where it means Virtual Network Instance and in this
draft (... Identifier). This is potentially confusing to the naive reader and
definitely confusing with the usage of VNI in item (4) of s4.1 where the VNI is
the VXLAN Network Identifier. It would be better if VNI meant the same thing in
all this closely related work. Please review all instances of VNI in the draft
to make sure they are talking about the same thing.

s2.1:
>[RFC7432] is used as the control plane for a Network Virtualization
>Overlay (NVO3) solution in Data Centers (DC),
The acronym NVO3 is used here as opposed to NVO which is used elsewhere in the
document. NVO3 is used in RFC 7365 to refer to an overlay with an L3 underlay
network. It is not quite clear to me whether this is a typo with EVPN NOT being
an NVO3 example or whether the sentence really ought to refer to RFC 7365 and
explicitly say "Network Virtualization Overlay over Layer 3 tunnels". In any
case you can't use an RFC as a control plane - they don't come with knobs on
;-) ! Suggest: NEW: [RFC7432] describes how a BGP MPLS-based Ethernet VPN
(EVPN) can provide the control plane for a Network Virtualization Overlay [over
Layer 3 Tunnels] (NVO[3]) solution in Data Centers (DC), ENDS If this is a real
NVO3 then probably the NVO3 acronym should be used in the rest of the draft in
place of NVO.

s3.1: The encoding of the IP Prefix Route encoding is both underspecified and
to some extent confusing. [I note that this is, in part, inherited from RFC
7342, where I consider Section 7 to be grossly underspecified.] Specifically: -
Figure 3: Expand RD on first use. - 1st bullet after Fig 3: The usage of RD
appears to be defined on a per route type basis in RFC 7342. Accordingly I
don't think it is sufficient to refer to how it is done in RFC 7342. - 2nd
bullet after Fig 3: s/byte/octet/ for consistency - 3rd bullet: Encoding not
specified (presumably unsigned integer) - 4th bullet: The alignment of the
prefix bits in the field is not specified (presumably left aligned). The second
sentence about the size of the field is unnecessary and confusing if the total
length specification is given clearly. - 6th bullet: The alighment/encoding of
the field is not specified (high order doesn't have any meaning without this).
- 7th bullet: It would be better to have the length specification as the first
bullet - this then clarifies the length possibilities of the IP Prefix and GW
IP address fields. Given that the field lengths are given in octets it would be
clearer to specify the total length of the BGP EVPN NLRI variable portion in
octets (and to repeat in s3 as in RFC 7342 that the length is the length of the