Skip to content
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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions scripts/frr/configs/commands/bgp.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
"/protocols/bgp/@element/address-family/ipv4-unicast/redistribute/rip": "redistribute rip [route-map {/route-map/@text},]",
"/protocols/bgp/@element/address-family/ipv4-unicast/parameters/dampening": "bgp dampening $if|{/half-life/@text},|half-life in {/@dict}$",
"/protocols/bgp/@element/address-family/ipv4-unicast/parameters/distance/global": "distance bgp {/external/@text} {/internal/@text} {/local/@text}",
"/protocols/bgp/@element/address-family/ipv4-unicast/parameters/multiple-paths/ebgp": "maximum-paths {/@text}",
"/protocols/bgp/@element/address-family/ipv4-unicast/parameters/multiple-paths/ibgp": "maximum-paths ibgp {/@text}",
"/protocols/bgp/@element/address-family/ipv4-unicast/network/@element": "network {/tagnode/@text} [route-map {/route-map/@text},] $if|backdoor,|backdoor in {/@dict}$",

"/protocols/bgp/@element/address-family/ipv6-unicast/@enter": "address-family ipv6 unicast",
Expand All @@ -63,11 +65,15 @@
"/protocols/bgp/@element/address-family/ipv6-unicast/redistribute/ripng": "redistribute ripng [route-map {/route-map/@text},]",
"/protocols/bgp/@element/address-family/ipv6-unicast/parameters/dampening": "bgp dampening $if|{/half-life/@text},|half-life in {/@dict}$",
"/protocols/bgp/@element/address-family/ipv6-unicast/parameters/distance/global": "distance bgp {/external/@text} {/internal/@text} {/local/@text}",
"/protocols/bgp/@element/address-family/ipv6-unicast/parameters/multiple-paths/ebgp": "maximum-paths {/@text}",
"/protocols/bgp/@element/address-family/ipv6-unicast/parameters/multiple-paths/ibgp": "maximum-paths ibgp {/@text}",
"/protocols/bgp/@element/address-family/ipv6-unicast/network/@element": "network {/tagnode/@text} [route-map {/route-map/@text},]",

"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/@enter": "address-family ipv4 unicast",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/@exit": "exit-address-family",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast": ["[neighbor {/../../tagnode/@text} peer-group {/peer-group/@text},]", "neighbor {/../../tagnode/@text} activate"],
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/addpath-tx-all-paths": "neighbor {/../../../tagnode/@text} addpath-tx-all-paths",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/addpath-tx-bestpath-per-AS": "neighbor {/../../../tagnode/@text} addpath-tx-bestpath-per-AS",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/allowas-in": "neighbor {/../../../tagnode/@text} allowas-in",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/allowas-in/number": "neighbor {/../../../../tagnode/@text} allowas-in {/@text}",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv4-unicast/attribute-unchanged": "neighbor {/../../../tagnode/@text} attribute-unchanged $if|as-path,|as-path in {/@dict}$ $if|med,|med in {/@dict}$ $if|next-hop,|next-hop in {/@dict}$",
Expand Down Expand Up @@ -97,6 +103,8 @@
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/@enter": "address-family ipv6 unicast",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/@exit": "exit-address-family",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast": ["[neighbor {/../../tagnode/@text} peer-group {/peer-group/@text},]", "neighbor {/../../tagnode/@text} activate"],
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/addpath-tx-all-paths": "neighbor {/../../../tagnode/@text} addpath-tx-all-paths",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/addpath-tx-bestpath-per-AS": "neighbor {/../../../tagnode/@text} addpath-tx-bestpath-per-AS",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/allowas-in": "neighbor {/../../../tagnode/@text} allowas-in",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/allowas-in/number": "neighbor {/../../../../tagnode/@text} allowas-in {/@text}",
"/protocols/bgp/@element/neighbor/@element/address-family/ipv6-unicast/attribute-unchanged": "neighbor {/../../../tagnode/@text} attribute-unchanged $if|as-path,|as-path in {/@dict}$ $if|med,|med in {/@dict}$ $if|next-hop,|next-hop in {/@dict}$",
Expand Down Expand Up @@ -148,6 +156,8 @@
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/@enter": "address-family ipv4 unicast",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/@exit": "exit-address-family",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast": ["[neighbor {/../../tagnode/@text} neighbor {/neighbor/@text},]", "neighbor {/../../tagnode/@text} activate"],
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/addpath-tx-all-paths": "neighbor {/../../../tagnode/@text} addpath-tx-all-paths",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/addpath-tx-bestpath-per-AS": "neighbor {/../../../tagnode/@text} addpath-tx-bestpath-per-AS",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/allowas-in": "neighbor {/../../../tagnode/@text} allowas-in",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/allowas-in/number": "neighbor {/../../../../tagnode/@text} allowas-in {/@text}",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv4-unicast/attribute-unchanged": "neighbor {/../../../tagnode/@text} attribute-unchanged $if|as-path,|as-path in {/@dict}$ $if|med,|med in {/@dict}$ $if|next-hop,|next-hop in {/@dict}$",
Expand Down Expand Up @@ -177,6 +187,8 @@
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/@enter": "address-family ipv6 unicast",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/@exit": "exit-address-family",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast": ["[neighbor {/../../tagnode/@text} neighbor {/neighbor/@text},]", "neighbor {/../../tagnode/@text} activate"],
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/addpath-tx-all-paths": "neighbor {/../../../tagnode/@text} addpath-tx-all-paths",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/addpath-tx-bestpath-per-AS": "neighbor {/../../../tagnode/@text} addpath-tx-bestpath-per-AS",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/allowas-in": "neighbor {/../../../tagnode/@text} allowas-in",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/allowas-in/number": "neighbor {/../../../../tagnode/@text} allowas-in {/@text}",
"/protocols/bgp/@element/peer-group/@element/address-family/ipv6-unicast/attribute-unchanged": "neighbor {/../../../tagnode/@text} attribute-unchanged $if|as-path,|as-path in {/@dict}$ $if|med,|med in {/@dict}$ $if|next-hop,|next-hop in {/@dict}$",
Expand Down
14 changes: 7 additions & 7 deletions scripts/frr/configs/commands/policy.json
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
{
"/policy/route/route-map/@enter": "!",
"/policy/route/route-map/@element/rule/@element":"route-map {/../../tagnode/@text} {/action/@text} {/tagnode/@text}",
"/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}",
Comment on lines -4 to +9
Copy link
Contributor Author

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

"/policy/route/route-map/@element/rule/@element/match/ip/source-protocol":"match source-protocol {@text}",
"/policy/route/route-map/@element/rule/@element/match/ipv6/address/access-list":"match ipv6 address {@text}",
"/policy/route/route-map/@element/rule/@element/match/ipv6/address/prefix-list":"match ipv6 address prefix-list {@text}",
"/policy/route/route-map/@element/rule/@element/match/ipv6/source-protocol":"match source-protocol $if|ospf6,{@text}|{@text}==ospfv3$",
"/policy/route/route-map/@element/rule/@element/match/local-preference":"match local-preference {@text}",
"/policy/route/route-map/@element/rule/@element/match/metric":"match metric {@text}",
"/policy/route/route-map/@element/rule/@element/match/origin":"match origin {@text}",
"/policy/route/route-map/@element/rule/@element/match/tag":"match tag {@text}",
Expand All @@ -37,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}",
Copy link
Contributor

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?

Copy link
Contributor

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! :-)

"/policy/route/prefix-list/@element/rule/@element":"ip prefix-list {/../../tagnode/@text} seq {/tagnode/@text} {/action/@text} {/prefix/@text} [ge {/ge/@text},] [le {/le/@text},]",
"/policy/route/prefix-list/@element/description":"ip prefix-list {/../tagnode/@text} description {/@text}",
"/policy/route/prefix-list6/@element/rule/@element":"ipv6 prefix-list {/../../tagnode/@text} seq {/tagnode/@text} {/action/@text} {/prefix/@text} [ge {/ge/@text},] [le {/le/@text},]",
Expand Down
37 changes: 36 additions & 1 deletion yang/vyatta-protocols-frr-bgp-v1.yang
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,25 @@ module vyatta-protocols-frr-bgp-v1 {
}
}

grouping parameters-multiple-paths {
container multiple-paths {
description "Forward packets over multiple paths";
configd:help "Forward packets over multiple paths";
leaf ebgp {
Copy link
Contributor

@deastoe deastoe Mar 23, 2020

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;
    ...
}

Copy link

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.

type uint32 {
range 2..256;
}
configd:help "Maximum number of parallel eBGP paths to consider, default is 1.";
}
leaf ibgp {
type uint32 {
range 2..256;
Copy link

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.

}
configd:help "Maximum number of parallel iBGP paths to consider, default is 1.";
}
}
}

grouping network-ipv4 {
list network {
description "BGP network";
Expand Down Expand Up @@ -1045,7 +1064,9 @@ module vyatta-protocols-frr-bgp-v1 {
- unsuppress-map
- capability orf prefix-list (both|receive|send))
- route-server-client
- nexthop-self*/
- nexthop-self
- addpath-tx-all-paths
- addpath-tx-bestpath-per-AS*/
/*
This grouping contains relative path to nodes outside the group scope.
It can only be used at a specific config level matching the relative path.
Expand Down Expand Up @@ -1207,6 +1228,18 @@ module vyatta-protocols-frr-bgp-v1 {
type empty;
configd:help "Peer-group as route server client";
}

leaf addpath-tx-all-paths {
description "Advertise all paths to this peer-group";
type empty;
configd:help "Advertise all paths to this peer-group";
}

leaf addpath-tx-bestpath-per-AS {
description "Advertise the bestpath per each neighboring AS";
type empty;
configd:help "Advertise the bestpath per each neighboring AS";
}
}

/* BGP Global parameters supported under default VRF */
Expand Down Expand Up @@ -1625,6 +1658,7 @@ module vyatta-protocols-frr-bgp-v1 {
configd:help "BGP parameters for the ipv4-unicast address family";
uses parameters-distance;
uses parameters-dampening;
uses parameters-multiple-paths;
}
uses ipv4-aggregate-address;
uses network-ipv4;
Expand All @@ -1640,6 +1674,7 @@ module vyatta-protocols-frr-bgp-v1 {
configd:help "BGP parameters for the ipv6-unicast address family";
uses parameters-distance;
uses parameters-dampening;
uses parameters-multiple-paths;
}
uses ipv6-aggregate-address;
uses network-ipv6;
Expand Down
6 changes: 3 additions & 3 deletions yang/vyatta-protocols-frr-ldp-v1.yang
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ module vyatta-protocols-frr-ldp-v1 {
"Basic discovery timer attributes.";
leaf hello-holdtime {
type uint16 {
range 15..3600;
range 3..3600;
}
units seconds;
default 15;
Expand All @@ -115,7 +115,7 @@ module vyatta-protocols-frr-ldp-v1 {
}
leaf hello-interval {
type uint16 {
range 5..1200;
range 1..1200;
}
units seconds;
default 5;
Expand All @@ -130,7 +130,7 @@ module vyatta-protocols-frr-ldp-v1 {
description "Neighbor configuration attributes.";
leaf session-ka-holdtime {
type uint16 {
range 30..3600;
range 15..3600;
}
units seconds;
default 30;
Expand Down