-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DAN-49 - Route map add match by local-preference, (DAN-25 DAN-48 DAN-52) - IPv4 BGP Prefix List Broken #2
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: bbs2web <[email protected]>
Signed-off-by: bbs2web <[email protected]>
Thank you for your contribution! We are still finalizing all the logistics around accepting contributions. The process will be documented here. |
Signed-off-by: bbs2web <[email protected]>
Signed-off-by: bbs2web <[email protected]>
Signed-off-by: bbs2web <[email protected]>
"/policy/route/route-map/@element/rule/@element/match/as-path":"match as-path {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/as-path":"match as-path {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/interface":"match interface {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/address/standard-acl":"match ip address {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/address/extended-acl":"match ip address {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/address/prefix-lists":"match ip address prefix-list {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/nexthop/standard-acl":"match ip next-hop {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/nexthop/extended-acl":"match ip next-hop {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/nexthop/prefix-lists":"match ip next-hop prefix-list {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/address/access-list":"match ip address {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/address/prefix-list":"match ip address prefix-list {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/nexthop/access-list":"match ip next-hop {@text}", | ||
"/policy/route/route-map/@element/rule/@element/match/ip/nexthop/prefix-list":"match ip next-hop prefix-list {@text}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are based on the work by 'gbe0' and 'dewi-morgan' in the following pull request which is missing the DCO signed-off-by statement.
Reference: DAN-25
Signed-off-by: bbs2web <[email protected]>
@@ -36,6 +36,7 @@ | |||
"/policy/route/route-map/@element/rule/@element/set/ip-next-hop":"set ip next-hop {/@text}", | |||
"/policy/route/route-map/@element/rule/@element/set/weight":"set weight {/@text}", | |||
"/policy/route/route-map/@element/rule/@element/set/metric":"set metric {/@text}", | |||
"/policy/route/route-map/@element/rule/@element/set/src":"set src {/@text}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please could you explain the reason for this change, I don't see a src
node in the YANG module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I see your danos/vyatta-protocols-common#1 PR now! :-)
container multiple-paths { | ||
description "Forward packets over multiple paths"; | ||
configd:help "Forward packets over multiple paths"; | ||
leaf ebgp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where possible we try to expose default values in the data model. In this case you could expand the range and add a default statement on both of these leaves, for example:
leaf ibgp {
type uint32 {
range 1..256;
}
default 1;
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You certainly can't have a default outside the range. So, even if you don't specify the default here, you need to expand the range to include the default.
Thank you for the contribution @bbs2web! |
} | ||
leaf ibgp { | ||
type uint32 { | ||
range 2..256; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above - model default and extend range to include it.
A further point on YANG module changes, for info. We require a new or updated revision statement for pretty much any change made to a module (including those made in this PR). At a minimum a new revision statement must be added to any module changed between DANOS releases as this ensures proper interaction with NMSs (Network Management System). In this case, as we have not yet reached a final process for accepting community contributions, we can take care of adding the necessary revision statements. These requirements will be documented in due course, as this process is defined. |
The first two commits of this PR have been merged to master and will be included in the upcoming DANOS release. |
Allows DANOS to write FRRouting configuration file to match BGP local preference