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