[bgpcfgd]: Support default action for "Allow prefix" feature #6370

pavel-shirshov · 2021-01-06T20:53:57Z

Change TSA route-map entry sequence numbers to 20 and 30 instead of 2 and 3. With this change standard SONiC sequence number rules order should be following:
- 1 for ipv6 default nexthop rule. Applicable for ipv6 route-maps only
- 10-11 for "Allow prefix" route-map rules
- 20-30 for "TSA" rules
- 100 and larger for standard rules
Add support dynamic default action for "Allow prefix"

- Why I did it
To enable dynamic default action parameter for "Allow prefix" feature.

- How I did it
I introduce another field into the "Allow prefix" controlling object.
I added field with name "default_action", which must have either "permit" or "deny" value

admin@str-s6100-acs-1:~$ cat 5
{
    "BGP_ALLOWED_PREFIXES": {
        "DEPLOYMENT_ID|0|1010:1010": {
            "prefixes_v4": [
                "10.20.0.0/16",
                "10.50.1.0/29"
            ],
            "prefixes_v6": [
                "fc01:10::/64",
                "fc02:20::/64"
            ],
            "default_action": "deny"
        },
        "DEPLOYMENT_ID|0": {
            "prefixes_v4": [
                "10.20.0.0/16",
                "10.50.1.0/29"
            ],
            "prefixes_v6": [
                "fc01:10::/64",
                "fc02:20::/64"
            ],
            "default_action": "deny"
        }
    }
}

When you apply this configuration to the DB by command: sonic-cfggen -j apply.json -w this configuration would be rendered into the following change

--- out 2021-01-06 20:49:37.971745161 +0000
+++ z   2021-01-06 20:50:58.328321185 +0000
@@ -151,7 +151,7 @@
  set src 10.1.0.32
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
- set community 5060:12345 additive
+ set community no-export additive
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 10
  match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010
@@ -161,7 +161,7 @@
  match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
- set community 5060:12345 additive
+ set community no-export additive
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 10
  match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010

That will mark differently prefixes which weren't matched by the "Allow prefix" rules.

By default or when "default_action" is not presented, the values from the constants.yml is used.

- How to verify it

Save this into apply.json file

admin@str-s6100-acs-1:~$ cat 5
{
    "BGP_ALLOWED_PREFIXES": {
        "DEPLOYMENT_ID|0|1010:1010": {
            "prefixes_v4": [
                "10.20.0.0/16",
                "10.50.1.0/29"
            ],
            "prefixes_v6": [
                "fc01:10::/64",
                "fc02:20::/64"
            ],
            "default_action": "deny"
        },
        "DEPLOYMENT_ID|0": {
            "prefixes_v4": [
                "10.20.0.0/16",
                "10.50.1.0/29"
            ],
            "prefixes_v6": [
                "fc01:10::/64",
                "fc02:20::/64"
            ],
            "default_action": "deny"
        }
    }
}

Apply with sonic-cfggen -j apply.json -w
Check that the following configuration was added to the bgp config

--- z1  2021-01-06 21:35:56.589811834 +0000
+++ z2  2021-01-06 21:36:19.740828696 +0000
@@ -120,10 +120,27 @@
 !
 ip prefix-list PL_LoopbackV4 seq 5 permit 10.1.0.32/32
 ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 5 permit 192.168.0.0/21
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V4 seq 10 deny 0.0.0.0/0 le 17
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V4 seq 20 permit 127.0.0.1/32
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V4 seq 30 permit 10.20.0.0/16 le 32
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V4 seq 40 permit 10.50.1.0/29 le 32
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4 seq 20 permit 127.0.0.1/32
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4 seq 30 permit 10.20.0.0/16 le 32
+ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4 seq 40 permit 10.50.1.0/29 le 32
 !
 ipv6 prefix-list PL_LoopbackV6 seq 5 permit fc00:1::/64
 ipv6 prefix-list LOCAL_VLAN_IPV6_PREFIX seq 10 permit fc02:1000::/64
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V6 seq 10 deny ::/0 le 59
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V6 seq 20 deny ::/0 ge 65
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V6 seq 30 permit fc01:10::/64 le 128
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V6 seq 40 permit fc02:20::/64 le 128
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V6 seq 10 deny ::/0 le 59
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V6 seq 20 deny ::/0 ge 65
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V6 seq 30 permit fc01:10::/64 le 128
+ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V6 seq 40 permit fc02:20::/64 le 128
 !
+bgp community-list standard COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010 seq 5 permit 1010:1010
 bgp community-list standard allow_list_default_community seq 5 permit no-export
 bgp community-list standard allow_list_default_community seq 10 permit 5060:12345
 !
@@ -134,10 +151,24 @@
  set src 10.1.0.32
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
- set community 5060:12345 additive
+ set community no-export additive
+!
+route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 10
+ match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010
+ match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V4
+!
+route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 30000
+ match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
- set community 5060:12345 additive
+ set community no-export additive
+!
+route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 10
+ match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010
+ match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V6
+!
+route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 30000
+ match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V6
 !
 route-map FROM_BGP_PEER_V4 permit 10
  call ALLOW_LIST_DEPLOYMENT_ID_0_V4

- Which release branch to backport (provide reason below if selected)

201811
201911
202006
202012

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

abdosi · 2021-01-06T23:22:27Z

dockers/docker-fpm-frr/frr/bgpd/templates/general/policies.conf.j2

 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
  set community {{ constants.bgp.allow_list.drop_community }} additive
 !
 route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
  set community {{ constants.bgp.allow_list.drop_community }} additive
 {%   endif %}
 !
-route-map FROM_BGP_PEER_V4 permit 2
+bgp community-list standard allow_list_default_community permit no-export
+bgp community-list standard allow_list_default_community permit {{ constants.bgp.allow_list.drop_community }}


@pavel-shirshov in my understanding we should this only "no-export" case. This is because if there is no allow list prefix configured we should tag them with drop community and continue to process other route-map. Otherwise we can break existing behaviour where we don't have prefix list by default

If we remove the second line here, the later routing policy could remove the mark from the prefix.

@pavel-shirshov i think we have to keep processing other route-map and can't break here . Because otherwise how we will permit default route from T0 ?

T0 will not send any default route, otherwise how do we choose between mulityple T0 for default routes?
Only T2 layer will send us default routes

abdosi

LGTM. Will have another PR on top of this for remaining changes/fixes

* Use 20 and 30 route-map entries instead of 2 and 3 for TSA * Added support for dynamic "Allow list" default action. Co-authored-by: Pavel Shirshov <[email protected]>

Updated test case to align with sonic-net/sonic-buildimage#6370 which made allow list action dynamic Added test case to cover sonic-net/sonic-buildimage#6671 Remove fixed value of Drop Community and read from /etc/sonic/constants.yml file. Organized the case as three test case one to verify default pre/post-config behavior when no prefix list there on device and then test case to verify with prefix list programmed with action as permit and deny

Use 20 and 30 route-map entries instead of 2 and 3 for TSA

7daa1be

pavel-shirshov self-assigned this Jan 6, 2021

pavel-shirshov force-pushed the pavelsh/default_community branch 2 times, most recently from 67dabec to 1f0edc4 Compare January 6, 2021 21:15

Added support for dynamic "Allow list" default action.

acc3c66

pavel-shirshov force-pushed the pavelsh/default_community branch from 1f0edc4 to acc3c66 Compare January 6, 2021 21:16

pavel-shirshov marked this pull request as ready for review January 6, 2021 21:37

pavel-shirshov requested a review from shi-su January 6, 2021 21:37

pavel-shirshov added bgp Enhancement ➕ Request for 201911 Branch Request for 202006 Branch Request for 202012 Branch labels Jan 6, 2021

abdosi reviewed Jan 6, 2021

View reviewed changes

abdosi approved these changes Jan 8, 2021

View reviewed changes

shi-su approved these changes Jan 8, 2021

View reviewed changes

pavel-shirshov merged commit 83715cf into sonic-net:master Jan 8, 2021

abdosi added the Included in 201911 Branch label Jan 8, 2021

daall added the Included in 202012 Branch label Jan 11, 2021

abdosi mentioned this pull request Feb 3, 2021

Update BGP Allow List test case sonic-net/sonic-mgmt#2911

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bgpcfgd]: Support default action for "Allow prefix" feature #6370

[bgpcfgd]: Support default action for "Allow prefix" feature #6370

pavel-shirshov commented Jan 6, 2021 •

edited

Loading

abdosi Jan 6, 2021

pavel-shirshov Jan 7, 2021

abdosi Jan 7, 2021

pavel-shirshov Jan 7, 2021

abdosi left a comment

[bgpcfgd]: Support default action for "Allow prefix" feature #6370

[bgpcfgd]: Support default action for "Allow prefix" feature #6370

Conversation

pavel-shirshov commented Jan 6, 2021 • edited Loading

abdosi Jan 6, 2021

Choose a reason for hiding this comment

pavel-shirshov Jan 7, 2021

Choose a reason for hiding this comment

abdosi Jan 7, 2021

Choose a reason for hiding this comment

pavel-shirshov Jan 7, 2021

Choose a reason for hiding this comment

abdosi left a comment

Choose a reason for hiding this comment

pavel-shirshov commented Jan 6, 2021 •

edited

Loading