Refactor BGP community sets #883
Conversation
|
No major YANG version changes in commit 5f6999b |
|
Compatibility Report for commit 1e6c495: |
|
Darren,
Link BW functionality as described in draft-ietf-idr-link-bandwidth is
rather limited, please take a look at draft-ietf-bess-ebgp-dmz which
addresses most of the limitations (transitivity is still upto
implementation though), there are implementation in EOS, IOS-XR and FRR.
Thanks,
Jeff
…On Mon, Jun 5, 2023 at 20:14 Darren Loher ***@***.***> wrote:
Change Scope
- Add BGP Link bandwidth extended community type
- This change is backwards compatible
This change follows the OpenConfig model precedent for configuring
communities by adding a link bandwidth community type in the format
bandwidth:[0-4294967295]:[1-65535].
Platform Implementations
-
Cisco enables link bandwidth using a dmzlink-bw
<https://www.cisco.com/c/en/us/td/docs/ios-xml/ios/iproute_bgp/configuration/xe-16/irg-xe-16-book/bgp-link-bandwidth.pdf>
command at the address family and neighbor level and auto-calculates the
unequal load balancing based on interface bandwidth
-
JunOS implements link bandwidth via configuration of the bgp community
with a user specified bandwidth
<https://www.juniper.net/documentation/us/en/software/junos/bgp/topics/topic-map/load-balancing-bgp-session.html#d29e113__d29e120>
------------------------------
You can view, comment on, or merge this pull request online at:
#883
Commit Summary
- d595ed0
<d595ed0>
add bgp linkbw ext community
File Changes
(1 file <https://github.com/openconfig/public/pull/883/files>)
- *M* release/models/bgp/openconfig-bgp-types.yang
<https://github.com/openconfig/public/pull/883/files#diff-32e3ff8896cce4fbd6a6920426d642a6dfbac07a89adddeaf3990847534e50ae>
(32)
Patch Links:
- https://github.com/openconfig/public/pull/883.patch
- https://github.com/openconfig/public/pull/883.diff
—
Reply to this email directly, view it on GitHub
<#883>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABSIDQ3NG3DAAHYEI5OT4VLXJ2N7ZANCNFSM6AAAAAAY3YXDGQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Thanks for the updated reference. I see the new draft adds another configuration option for 'cumulative' link bandwidth. At the moment we don't have a use case for this, but it could be added in a subsequent pull request. If you are motivated to add it in a way that is similar to how FRR, EOS and IOS-XR expose it, probably it could be added as another community type here: |
|
I am working on a major rewrite of this PR to add strongly typed community types. Please ignore this PR for now. |
dplore
changed the title
|
A few high level comments on link bandwidth. As Jeff Tantsura notes above, there is work in BESS in IETF that discusses how to change some of the rules about how transitivity is managed. That work will eventually run up against discussion in IDR about how transitivity for extended communities needs to be dealt with; i.e. are we even breaking rules for transitivity if an implementation simply originates a non-transitive over an eBGP session and how is that distinct from a bug in the transitivity scoping being enforced. There's also the headache that Juniper's implementation of link bandwidth is not correct vs. the draft. Juniper created the original draft, but at the same time has been shipping code that uses TRANSITIVE link-bandwidth. This creates interop issues and work is ongoing to figure out how to change Juniper's implementation to both provide continuing support to our customers and cover existing transitive use cases while at the same time adding support for the non-transitive one in the existing draft. This work is being discussed with the current IETF draft authors. |
|
High level comment on this proposal: I feel your pain. The fundamental issue is that extended communities are an on-the-wire easy thing but in config and display are a unstructured mess. From a display standpoint, there's a desire to provide a simple string-based canonical form. For the configuration standpoint, trying to get out of regex hell is desirable. As an example, the ambiguity for a 2-byte AS route target from RFC 4360 and RFC 5668 4-byte ASes I highlighted in the diff provide an example where display ambiguity for on-the-wire encoding can be troublesome to distinguish without better keywords (I think IETF has a bug still in this respect), but can be made fully clear with structured containers to remove such ambiguity. The challenge is when configuration moves to structured elements that there should be clear coupling between the configuration style and the resultant string display style. See my comments covering the on-the-wire for link bandwidth being float vs. configuration as integer. |
|
Regarding bandwidth encoding in OpenConfig. In contrast to CLI, the user would be OpenConfig client/software. So why not keep baldwidth in on-the-wire format to avoid uncertanity? |
Address comments.
|
With
traditionally this is achived by use of reg-exp. If we want to keep spitit of strongly typed, structured data, perhaps we shall provide ability to express ASN range and/or value range, instead of just singelton values. |
Thanks for catching this. The regex I'm attempting to remove is the typedefs which use a string and regex validator in the yang model. Regex as a user supplied match criteria I think is a feature we need to keep. I added a container and leaf for a user supplied regex. Please let me know what you think. There are two concepts to evaluate for this I think.
Item 2 has two parts which are very important I think. The first is we should use a standard, testable regex format. Today each vendor has their own regex format and special rules which are only testable if you have a running binary image from the vendor and send routes. By using a standard regex format, one can write code or use any standard regex evaluator tool to test how a match works. The second part is I proposed a standard way to format the community list to be matched. We need this also so the standard regex that will give us the same match behavior no matter which implementation it's pushed to. |
dplore
changed the title
| type uint16; | ||
| description | ||
| "Represents an index of a member entry."; | ||
| } |
There was a problem hiding this comment.
This needs to have a community value.
There should also be a community regex option (list of regexes)
Change Scope
Platform Implementations
Cisco IOS XR implements a command like set extcommunity bandwidth (2906:45750000) with a user specified bandwidth in a route policy similar to this PR.
JunOS implements link bandwidth via configuration of the bgp community with a user specified bandwidth in a route policy list similar to this PR
Arista EOS enables link bandwidth using a per neighbor command. Bandwidths can optionally be set using a route-map and a percent format. This is similar to this PR, but uses % bandwidth instead of a bandwidth in bits/second.
FRR implements link bandwidth via configuration applied to a policy. Bandwidth is automatically calculated and not configurable.
Tree view
module: openconfig-routing-policy +--rw routing-policy +--rw defined-sets | +--rw oc-bgp-pol:bgp-defined-sets | +--rw oc-bgp-pol:community-sets (changes for standard community sets) 9323c9323 < | | | +--rw oc-bgp-pol:community-member* union --- > | | | x--rw oc-bgp-pol:community-member* union (<-- now deprecated) 9326,9328c9326,9347 < | | +--ro oc-bgp-pol:community-set-name string < | | +--ro oc-bgp-pol:community-member* union < | | x--ro oc-bgp-pol:match-set-options? oc-pol-types:match-set-options-type --- > | | | +--ro oc-bgp-pol:community-set-name string > | | | x--ro oc-bgp-pol:community-member* union > | | | x--ro oc-bgp-pol:match-set-options? oc-pol-types:match-set-options-type > | | +--rw oc-bgp-pol:members > | | +--rw oc-bgp-pol:member* [index] > | | +--rw oc-bgp-pol:index -> ../config/index > | | +--rw oc-bgp-pol:config > | | | +--rw oc-bgp-pol:index? uint16 > | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:index? uint16 > | | +--rw oc-bgp-pol:asn-type > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:asn? oc-inet:as-number > | | | | +--rw oc-bgp-pol:value? uint32 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:asn? oc-inet:as-number > | | | +--ro oc-bgp-pol:value? uint32 > | | +--rw oc-bgp-pol:asn-regex-type > | | +--rw oc-bgp-pol:config > | | | +--rw oc-bgp-pol:regex? oc-types:posix-eregexp > | | +--ro oc-bgp-pol:state > | | +--ro oc-bgp-pol:regex? oc-types:posix-eregexp 9334c9353 < | | | +--rw oc-bgp-pol:ext-community-member* union --- > | | | x--rw oc-bgp-pol:ext-community-member* union 9337,9339c9356,9428 < | | +--ro oc-bgp-pol:ext-community-set-name? string < | | +--ro oc-bgp-pol:ext-community-member* union < | | x--ro oc-bgp-pol:match-set-options? oc-pol-types:match-set-options-type --- > | | | +--ro oc-bgp-pol:ext-community-set-name? string > | | | x--ro oc-bgp-pol:ext-community-member* union > | | | x--ro oc-bgp-pol:match-set-options? oc-pol-types:match-set-options-type > | | +--rw oc-bgp-pol:members > | | +--rw oc-bgp-pol:member* [index] > | | +--rw oc-bgp-pol:index -> ../config/index > | | +--rw oc-bgp-pol:config > | | | +--rw oc-bgp-pol:index? uint16 > | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:index? uint16 > | | +--rw oc-bgp-pol:asn4-type > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:asn? oc-inet:as-number > | | | | +--rw oc-bgp-pol:value? uint16 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:asn? oc-inet:as-number > | | | +--ro oc-bgp-pol:value? uint16 > | | +--rw oc-bgp-pol:asn4-regex-type > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:regex? oc-types:posix-eregexp > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:regex? oc-types:posix-eregexp > | | +--rw oc-bgp-pol:ip-type > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:ipv4-address? oc-inet:ipv4-address > | | | | +--rw oc-bgp-pol:value? uint16 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:ipv4-address? oc-inet:ipv4-address > | | | +--ro oc-bgp-pol:value? uint16 > | | +--rw oc-bgp-pol:link-bw > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:asn? oc-inet:as-number > | | | | +--rw oc-bgp-pol:bandwidth? oc-types:ieeefloat32 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:asn? oc-inet:as-number > | | | +--ro oc-bgp-pol:bandwidth? oc-types:ieeefloat32 > | | +--rw oc-bgp-pol:route-target > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:asn? oc-inet:as-number > | | | | +--rw oc-bgp-pol:value? uint16 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:asn? oc-inet:as-number > | | | +--ro oc-bgp-pol:value? uint16 > | | +--rw oc-bgp-pol:route-target-ip > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:ipv4-address? oc-inet:ipv4-address > | | | | +--rw oc-bgp-pol:value? uint16 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:ipv4-address? oc-inet:ipv4-address > | | | +--ro oc-bgp-pol:value? uint16 > | | +--rw oc-bgp-pol:route-origin > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:asn? oc-inet:as-number > | | | | +--rw oc-bgp-pol:value? uint16 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:asn? oc-inet:as-number > | | | +--ro oc-bgp-pol:value? uint16 > | | +--rw oc-bgp-pol:route-origin-ip > | | | +--rw oc-bgp-pol:config > | | | | +--rw oc-bgp-pol:ipv4-address? oc-inet:ipv4-address > | | | | +--rw oc-bgp-pol:value? uint16 > | | | +--ro oc-bgp-pol:state > | | | +--ro oc-bgp-pol:ipv4-address? oc-inet:ipv4-address > | | | +--ro oc-bgp-pol:value? uint16 > | | +--rw oc-bgp-pol:color > | | +--rw oc-bgp-pol:config > | | | +--rw oc-bgp-pol:match-type? oc-bgp-types:bgp-color-match-type > | | | +--rw oc-bgp-pol:value? uint32 > | | | +--rw oc-bgp-pol:flags? enumeration > | | +--ro oc-bgp-pol:state > | | +--ro oc-bgp-pol:match-type? oc-bgp-types:bgp-color-match-type > | | +--ro oc-bgp-pol:value? uint32 > | | +--ro oc-bgp-pol:flags? enumeration