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

["Jorge Rabadan (Nokia)" <[e-mail]>]

Hi John, all,

We published version 08. This version removes the use of D-PATH from SAFI 1 routes completely (this is still an ongoing discussion with the IDR chairs) and addresses your comments below.

Please see in-line with [jorge].

Thanks for the email!
Jorge


From: John Scudder <[e-mail]>
Date: Wednesday, January 11, 2023 at 3:55 PM
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>
Subject: Comments on draft-ietf-bess-evpn-ipvpn-interworking-07
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.)

[jorge] excellent point as usual. Indeed, in the first versions of the draft the text followed the loop detection as in the as_path case, however in the latest versions the authors agreed to change this since there were use cases where installing the route provided faster convergence upon certain failures in the Gateways and, in order to avoid loops, we only needed to prevent re-exporting the ‘looped’ route. So we missed to clarify this aspect in section 4. The new text says:


  1.  An ISF IPVPN or EVPN 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 for the purpose of re-exporting the route to the adjacent domain in a Gateway PE. The ISF route in this case MUST be flagged as "looped", MUST NOT be exported, and MAY be installed in the IP-VRF only in case there is no better route after the best path selection (Section 6<https://author-tools.ietf.org/api/export/79502ba6-6c80-49cf-8021-519ba10c48b2/draft-ietf-bess-evpn-ipvpn-interworking-08.html#sect-6>). The ISF_SAFI_TYPE is irrelevant for the purpose of loop detection of an ISF route. In other words, an ISF route is considered as a looped route if it contains a D-PATH attribute with at least one DOMAIN-ID matching a local DOMAIN-ID, irrespective of the ISF_SAFI_TYPE of the DOMAIN-ID.

For instance, in the example of Figure 2<https://author-tools.ietf.org/api/export/79502ba6-6c80-49cf-8021-519ba10c48b2/draft-ietf-bess-evpn-ipvpn-interworking-08.html#ure-multiple-domain-dci-example>, gateway GW1 receives TS1 prefix in two different ISF routes:

o    In an EVPN IP Prefix route with next-hop PE1 and no D-PATH attribute.

o    In a SAFI 128 route with next-hop GW2 and D-PATH = {length=1, <6500:1:EVPN>}, assuming that DOMAIN-ID for domain 1 is 6500:1.

Gateway GW1 flags the SAFI 128 route as "looped" (since 6500:1 is a local DOMAIN-ID in GW1) and it will not install it in the tenant IP-VRF, since the route selection process selects the EVPN IP Prefix route due to a shorter D-PATH attribute (Section 6<https://author-tools.ietf.org/api/export/79502ba6-6c80-49cf-8021-519ba10c48b2/draft-ietf-bess-evpn-ipvpn-interworking-08.html#sect-6>). Gateway GW1 identifies the route as "looped" even if the ISF_SAFI_TYPE value is unknown to GW1, i.e., any value different from the ones specified in this document).



## 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.
[jorge] the authors agreed on giving it a structure that could be more user friendly for configuration or auto-derivation if needed. Added the following:
“The Local Administrator sub-field is any local 2-octet value, and its allocation or configuration is a local implementation matter.”

## 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.
[jorge] added, thanks.

## 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."
[jorge] changed as per your suggestion, thanks!

## 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.
[jorge] most of the normative language that uses SHOULD is related to the propagation of attributes. We listed what we agreed the gateway PE has to propagate or block, but there may be other attributes (an example is the wide communities that you pointed out) that may come up in the future that need propagation and are not included in the list. That’s why we did not want to be so strict. For the rest of the sections we tried to use “MUST” as much as possible.

## 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...".
[jorge] changed, thanks.

# NITS:

- "as though it would" --> "as it would" (drop 'though')
[jorge] changed, thanks.