Last Call Review of draft-ietf-bess-evpn-vpws-fxc-07
review-ietf-bess-evpn-vpws-fxc-07-rtgdir-lc-mishra-2022-08-22-00

This specification specifies a important optimization for operators using VPWS
EVPN for scalability of millions of P2P AC provisioning using a VPWS EVPN
Flexible Cross connect service which multiplexes multiple attachment circuits
across different Ethernet Segments and physical interfaces into a single EVPN
VPWS service tunnel and still providing Single-Active and All-Active
multi-homing.

The draft is very well written and is ready for publication.

Few nits below need to be addressed prior to publication.  I did not find any
technical issues related to the solution.

Nits:
s/withdraw/s//withdrawal
s/control- plane/s//control-plane

Add to terminology section and / or expand and and show abbreviation in ()
example Flexible Cross Connect (FXC)

FXC - Flexible Cross Connect
VPWS -Virtual Private Wire Service
EVPN - Ethernet Virtual Private Network
MTU - Maximum Transmit Unit
L2 - Layer 2
VID - Vlan ID
ESI - Ethernet Segment Identififer
EVI - EVPN Instance Identifier
AC - Attachment Circuit
A-D - Auto Discovery
ES - Ethernet Segment
VRF - Virtual Route Forwarding
PW - Pseudowire
VCCV - Virtual circuit connection verification

In section 2 requirements 5th paragraph

An endpoint can be an AC, MAC-VRF, IP-VRF, global table, or etc.

I can see endpoint being AC as endpoint but not the others in the context of
this specification which is about multiplexing the ACs into a single tunnel.  I
think that line can be omitted or maybe changed to say an endpoint is one end
of an AC.

Section 3 solution

   Since there is only a single EVPN VPWS service tunnel associated with
   many normalized VIDs (either single or double) across multiple
   physical interfaces, MPLS lookup at the disposition PE is no longer
   sufficient to forward the packet to the right egress
   endpoint/interface.  Therefore, in addition to an EVPN label lookup
   corresponding to the VPWS service tunnel, a VID lookup (either single
   or double) is also required.

Is there any performance impact with the additional lookups?

Trade off for the operators deploying this feature providing scalability but at
a performance price.  If so this should be noticed in a deployment
considerations section.

Is section 3.2 the default FXC mode.  That needs to be made clear.

Section 3.2 states below

This mode of operation is only suitable for single-homing because in
   multi-homing the association between EVPN VPWS service tunnel and
   remote AC changes during the failure and therefore the VLANs
   (normalized VIDs) need to be signaled.

However section 3.2.1 states that multihome can use the default FXC mode,
however since multihome needs to signal as stated in section 3.2 it does not
seem that it can use the default FXC mode.  Please clarify.

3.2.1.  Multi-homing

   The default FXC mode can also be used for multi-homing.

Why don’t the normalized VIDs need to be signaled in BGP here for multihome?

Section 3.3

   In this solution, on each PE, the multi-homing ACs represented by
   their normalized VIDs are configured with a single EVI.  There is no
   need to configure VPWS service instance ID in here as it is the same
   as the normalized VID

Even though the normalized VID is the same as the VPN Service Instance ID, I
would think you would still need the Service Instance ID for for VPWS tunnel
creation.

In section 3.3 when the consistency check runs why does it not bring down the
VPWS or is the inconsistency superficial that it does not adversely effect
service.  Maybe should explain why the error does not bring down the service. 
This is like a false negative alarm.

Section 3.3.1 is local switching similar to analogy to RFC 8365 split horizon
local bias.

You should mention that there are only two new modes which you don’t realize
until you get to section 4 M mode bit.

M        00 mode of operation as defined in [RFC8214]
                01 VLAN-Signaled FXC
                10 Default FXC

Section 3.2 you should call the default FXC mode 1 and section 3.3 mode 2

Section 3.4 which talks about V bit you should call it service normalization I
think makes more sense then service instantiation

Section 3.2.1

When the default FXC mode is used for multi-homing, instead of a
   single EVPN VPWS service tunnel, there can be many service tunnels
   per pair of PEs - i.e, there is one tunnel per group of VIDs per pair
   of PEs and there can be many groups between a pair of PEs, thus
   resulting in many EVPN service tunnels.

How are the VIDs grouped into the service tunnels.  If you have an interface
with many subintefaces are all the subinterfaces grouped together into the same
service tunnel.  Is the grouping  an implementation specific aspect?

s/normalisation//normalization

   If single VID normalisation is signaled in the Ethernet Tag ID field
   (12-bits) yet dataplane is operating based double tags, the VID
   normalisation applies only to outer tag.  If double VID normalisation
   is signaled in the Ethernet Tag ID field (24-bits), VID normalisation
   applies to both inner and outer tags.

My understanding of the VID normalization is if you have > 4096 vlans now you
need normalization and you need the double tag so if the BGP control plane
signals 12 bits but the data plane us 24 bits then the control plane has to
match adding outer tag 12 bits total 24 bits.  If you agree with what I said
above I don’t think VID normalization applies to both inner and outer as it
only applies to inner if there is a control and data plane mismatch and to
rectify the issue the normalization occurs.
 minor nits related to normative language that I recommend should be addressed
 before publication.

Section 4
Why would you not make both M and V fields mandatory or at least the M fields
mandatory as that’s a critical part of the specification?

The
   reserved bit for flow label is defined as per
   [I-D.ietf-bess-rfc7432bis]

The reserved field defined in the bis does not have any bearing on this
document so I would leave that out

3.2.1 and 3.3.1 seem out of place with their section headers relating back to
section 3.2 and 3.3 respectively.  I would somehow change the heading so
multihome relates to default FXC and local switching relates to signaled FXC
mode.

I would make 3.4 part of section 4 maybe as sub section or within the same
section since it’s talking about normalization bits

Section 5 I would switch diagram 1 and 2 around so it flows with rest of
document make Default FXC first and signaled FXC second.

5.1.  EVPN VPWS Service Failure

   The failure detection of an EVPN VPWS service can be performed via
   OAM mechanisms such as VCCV-BFD and upon such failure detection, the
   switch over procedure to the backup S-PE is the same as the one
   described above.

Is there a informational draft you can reference related to VCCV-BFD.

Section 5.2 I would also do default FXC and then the signaled FXC.

In the failure scenario descriptions of the failures and maybe in the diagram
as well say you have N P2P VPWS ACs and now they are all over a single VPWS
tunnel versus separate P2P VPWS tunnel per AC maybe mention that and now all
the ACs are multiplexed over the VPWS tunnel showing the scale advantages as
well as how failure scenario is different compare to RFC 8214 or not with the
bundling of all ACs in a tunnel.

With this new feature how does it impact entropy label RFC 6790 With the mux
and demux.

Also maybe an operational considerations section.  Any caveats related to
backwards compatibility etc.  As well as deployments scenario and upgrading
code to support the new feature all PEs requiring the feature would need to be
upgraded.

Few minor nits related to normative language below:

Section 3

OLD

   When single normalized VID is used, the lower 12-bit of Ethernet tag
   field in EVPN routes is set to that VID and when double normalized
   VID is used, the lower 12-bit of Ethernet tag field is set to inner
   VID and the higher 12-bit is set to the outer VID.  As in [RFC8214],
   12-bit and 24-bit VPWS service instance identifiers representing
   normalised VIDs MUST be right-aligned.

New

   When single normalized VID is used, the lower 12-bit of Ethernet tag
   field in EVPN routes MUST be set to that VID and when double normalized
   VID is used, the lower 12-bit of Ethernet tag field MUST be set to inner
   VID and the higher 12-bit is set to the outer VID.  As in [RFC8214],
   12-bit and 24-bit VPWS service instance identifiers representing
   normalised VIDs MUST be right-aligned.

Old

The tag manipulation
   (translation from normalized VID(s) to local VID) can be performed
   either as part of the VID table lookup or at the egress interface
   itself.

New

The tag manipulation
   (translation from normalized VID(s) to local VID) SHOULD be performed
   either as part of the VID table lookup or at the egress interface
   itself.

Section 3.2

Old

   With respect to the data-plane aspects of the solution, both
   imposition and disposition PEs are aware of the VLANs as the
   imposition PE performs VID normalization and the disposition PE does
   VID lookup and translation.  In this solution, there is only a single
   P2P EVPN VPWS service tunnel between a pair of PEs for a set of ACs.

New

   With respect to the data-plane aspects of the solution, both
   imposition and disposition PEs MUST be aware of the VLANs as the
   imposition PE performs VID normalization and the disposition PE does
   VID lookup and translation.  In this solution, there SHOULD only be a single
   P2P EVPN VPWS service tunnel between a pair of PEs for a set of ACs.

Old

   As discussed previously, since the EVPN VPWS service tunnel is used
   to multiplex ACs across different ES's (e.g., physical interfaces),
   the EVPN label alone is not sufficient for proper forwarding of the
   received packets (over MPLS/IP network) to egress interfaces.
   Therefore, normalized VID lookup is required in the disposition

New - REQUIRED

   As discussed previously, since the EVPN VPWS service tunnel is used
   to multiplex ACs across different ES's (e.g., physical interfaces),
   the EVPN label alone is not sufficient for proper forwarding of the
   received packets (over MPLS/IP network) to egress interfaces.
   Therefore, normalized VID lookup is REQUIRED in the disposition

Check all normative language should be capitals but I have seen a few instances
of lower case.