[bess] John Scudder's Discuss on draft-ietf-bess-mvpn-evpn-aggregation-label-12: (with DISCUSS and COMMENT)

[John Scudder via Datatracker <[e-mail]>]

John Scudder has entered the following ballot position for
draft-ietf-bess-mvpn-evpn-aggregation-label-12: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-bess-mvpn-evpn-aggregation-label/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

# John Scudder, RTG AD, comments for
draft-ietf-bess-mvpn-evpn-aggregation-label-12 CC @jgscudder

Thanks for this spec. I have one serious concern (but I think it will be easy
to take care of) and a few comments and nits.

## DISCUSS

### Section 3.2, ignoring routes considered harmful

There are two places toward the end of this subsection where you specify that a
route must be ignored. The first is:

"A PE MUST ignore a received route with both the DCB-flag and the Context Label
Space ID EC attached, treating as if it was not received."

The second is:

"If a PE originates two x-PMSI/IMET routes with the same tunnel, it MUST ensure
one of the following" ... "Otherwise, a receiving PE MUST ignore the routes."

Literally ignoring routes is one of the classic Bad Ideas in BGP. There can be
exceptions, if the conditions for ignoring the routes are carefully chosen so
that correctness (or something like it) is preserved, but as a general matter,
ignoring routes is a one-way ticket to persistent traffic loss or worse. It's
for this reason that RFC 7606 specifies treat-as-withdraw for many error
conditions. I'll illustrate the general problem with an example that uses
simple IPv4 unicast routes:

- Suppose we receive 10/8, with nexthop 1.1.1.1, choose it as best, and install
it in the FIB. - Now suppose the router that advertised it to us sends a
replacement, an advertisement for 10/8, nexthop 2.2.2.2, including path
attribute P that we decide is malformed. We ignore the route as our error
handling strategy. - We are left in a state where we still have 10/8 via
1.1.1.1 selected and installed, because we ignored the replacement, "treating
it as if was not received". This is an incorrect state. I can easily show you
scenarios where it leads to traffic loss, persistent loops, etc. - The correct
behavior in this scenario would be to remove the 10/8 route received in the
first step; RFC 7606 calls this "treat-as-withdraw".

It might be that something special about MVPN/EVPN routes means this isn't an
issue for the two cases I've quoted, but you haven't made this clear in the
document. I think at minimum, some analysis is needed to show that the strategy
is OK. On the other hand if what you meant by "ignore" is something closer to
the "treat-as-withdraw" strategy, I think the language has to be made more
specific and leave less to the creativity and imagination of the implementor.

Let's have a discussion about which it is, and see where to go from there.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

## COMMENTS

### Section 3, EC

Please expand "EC" on first use, or put it in your glossary, or my personal
favorite, just use the words "Extended Community" and remove "EC" altogether,
it's unnecessary and (in my opinion) unhelpful to abbreviate it.

### Section 3.1, need registry?

You have an ID-Type and define the semantics of type 0. You probably should
create a registry for the unallocated types.

### Section 3.1, AND or OR?

You have:

   In the remainder of the document, when we say a BGP-MVPN/EVPN A-D
   route "carries DCB-flag" or "has DCB-flag attached" we mean the
   following:

   *  The route carries a PMSI Tunnel Attribute (PTA) and its Flags
      field has the Extension bit set

   *  The route carries an "Additional PMSI Tunnel Attribute Flags" EC
      and its DCB flag is set

I think you need to indicate if the bullets are ANDed or ORed. I infer from
later context that they're ORed, in which case perhaps "we mean one or the
other of the following".

## NITS

### Section 2.2

- "number of total number of labels" --> too many "number of"s

### Section 2.2.2.3

- "w/o" --> "without"

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues.

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments