[bess] Comments on draft-ietf-bess-evpn-ipvpn-interworking-07

[John Scudder <[e-mail]>]

Hi Authors and WG,

I recently looked at some parts of draft-ietf-bess-evpn-ipvpn-interworking-07. This isn't a full review but I noticed some things of concern that I thought I'd share.

Regards,

--John 

# COMMENTS

## Section 4

```
An ISF route received by a gateway PE with a D-PATH attribute that contains one or more of its locally associated DOMAIN-IDs for the IP-VRF is considered to be a looped ISF route. The ISF route in this case MUST be flagged as "looped" and be installed in the IP-VRF only in case there is no better route after the best path selection (Section 6).
```

But... this is just a restatement of the general condition for route installation in any case. A route is never installed unless there is no better route after best path selection -- if there is a better route, the better route is installed instead.

It seems to me that either the explanatory text here is wrong/missing something, or loop detection is genuinely not being used for anything.

I did notice that Section 8 (2(b)) contains a rule related to loops ("In this case the route is considered to be a looped ISF route, as described in Section 4 and hence MUST NOT be exported in ISF SAFI-y."), maybe that is where the entire meat of loop suppression resides? But this was not clear to me.

For comparison, here is how RFC 4271 says to handle AS_PATH loops (Section 9.1.2):

```
   If the AS_PATH attribute of a BGP route contains an AS loop, the BGP
   route should be excluded from the Phase 2 decision function.
```

(If this were an IESG ballot, this would be a DISCUSS question, since loop suppression is presented as a major feature of this spec.)

## Section 4

Also, I don't think the method of assignment of the Local Admin part of the Domain ID is specified. I mean, presumably it's... local... but it seems like some words should be said. Indeed, I found the entire definition of the domain-id pretty mystifying -- it uses six bytes for two fields that appear to be arbitrary (the global-admin part) and even more arbitrary (the local-admin part) without getting any evident benefit from creating that structure. 

## Section 5.2

5.2 (1) implies that Wide Communities (draft-ietf-idr-wide-bgp-communities-08) SHOULD NOT be propagated (since your rule is in effect, "anything not permitted is prohibited"). Is this exclusion deliberate? FWIW I see that Wide Communities has passed IDR WGLC. 

## Section 5.2

While I'm talking about it, the final bullet of (1) is a little ambiguous:

```
The following set of Path Attributes SHOULD be propagated by the gateway PE to other ISF SAFIs (other BGP Path Attributes SHOULD NOT be propagated):
...
- Communities, Extended Communities and Large Communities, except for the EVPN extended communities, Route Target extended communities and BGP Encapsulation extended communities.
```

I guess you mean that everything after the "except for" is excluded? Because of the shortcomings of English grammar, this isn't clear as written. Perhaps something like, "Communities, Extended Communities and Large Communities, except in the exception cases detailed in point 4."

## General

By the way, I think the usual concern about the use of SHOULD (NOT) instead of MUST (NOT) is going to apply, and the document would benefit from a review to convert these to MUST when possible or supply further explanation about what exception cases exist otherwise.

## Section 5.3

```
AS_PATH is aggregated based on the rules in [RFC4271]. The gateway PEs SHOULD NOT receive AS_PATH attributes with path segments of type AS_SET [RFC6472].
```

That's not a good use of the RFC 2119 keyword, I think you mean something like "gateway PEs are not expected to receive...". 

# NITS:

- "as though it would" --> "as it would" (drop 'though')