BGP "Allow list" feature implementation #4563

pavel-shirshov · 2020-05-08T22:30:37Z

This PR implements a new feature: "BGP Allow list."

This feature allows us to control which IP prefixes will be imported from the routes received from BGP neighbors.

The feature can be enabled or disabled using "allow_list_enabled" flag in the /etc/sonic/deployment_id_asn_map.yml`

There is another flag for the feature: "allow_list_default_action". When this flag is equal "permit", this feature will not drop any BGP prefixes. It is useful for debugging. But when the flag is equal to "deny", all prefixes, which are not explicitly listed as "Allowed" will be dropped.

We can control the feature by using the following schema:

{
    "BGP_ALLOWED_PREFIXES": {
        "10.0.0.1": {
            "prefixes_v4": [
                "172.16.0.0/24",
                "172.16.1.0/24"
            ]
        },
        "10.0.0.1|1020:123": {
            "prefixes_v4": [
                "172.16.2.0/24",
                "172.16.3.0/24"
            ]
        },
        "10.0.0.1|1020:555": {
            "prefixes_v4": [
                "172.16.4.0/24",
                "192.16.5.0/24"
            ]
        },
        "fc00::2": {
            "prefixes_v6": [
                "20c0:a810::/64",
                "20c0:a820::/64"
            ]
        },
        "fc00::2|1020:123": {
            "prefixes_v6": [
                "20c0:a830::/64",
                "20c0:a840::/64"
            ]
        },
        "fc00::2|1020:555": {
            "prefixes_v6": [
                "20c0:a850::/64",
                "20c0:a860::/64"
            ]
        }
    }
}

The schema above means the following:

For BGP neighbor with IP address "10.0.0.1" do the following:
a. Import IPv4 prefixes "172.16.0.0/24" and "172.16.1.0/24". The community is not required.
b. Import IPv4 prefixes "172.16.2.0/24" and "172.16.3.0/24" only if they have associated BGP community "1020:123".
c. Import IPv4 prefixes "172.16.4.0/24" and "172.16.5.0/24" only if they have associated BGP community "1020:555".
For BGP neighbor with IP address "fc00::2" do the following:
a. Import IPv6 prefixes "20c0:a810::/64" and "20c0:a820::/64". The community is not required.
b. Import IPv6 prefixes "20c0:a830::/64" and "20c0:a840::/64" only if they have associated BGP community "1020:123".
c. Import IPv6 prefixes "20c0:a850::/64" and "20c0:a860::/64" only if they have associated BGP community "1020:555".

All other prefixes will be dropped (see above for "allow_list_default_action").

When BGP starts and this feature is enabled, all prefixes are enabled to be imported by default. The following BGP configuration is generated when we have one IPv4 neighbor "10.0.0.1" and one IPv6 neighbor "fc00::2"

! The feature introduced the following configuration
ip prefix-list PEER_10.0.0.1 seq 5 permit 10.0.0.1/32
!
ip prefix-list ALLOW_ADDRESS_ALLOW_ALL_V4 seq 10 permit any
!
ipv6 prefix-list ALLOW_ADDRESS_ALLOW_ALL_V6 seq 10 permit any
!
route-map ALLOW_LIST_V6 permit 30000
 match ipv6 address prefix-list ALLOW_ADDRESS_ALLOW_ALL_V6
 match ipv6 next-hop FC00::2
!
route-map ALLOW_LIST_V4 permit 30000
 match ip address prefix-list ALLOW_ADDRESS_ALLOW_ALL_V4
 match ip next-hop prefix-list PEER_10.0.0.1
!
route-map FROM_PEER_V4 permit 1
 call ALLOW_LIST_V4
 on-match next
!
route-map FROM_PEER_V4 permit 10
!
route-map FROM_PEER_V6 permit 1
 call ALLOW_LIST_V6
 on-match next
!
route-map FROM_PEER_V6 permit 10
!
route-map ALLOW_LIST_V4 deny 65535
!
route-map ALLOW_LIST_V6 deny 65535
!

When we apply the configuration you can find on the top of this PR description, the following BGP configuration will be generated.

ip prefix-list ALLOW_ADDRESS_10.0.0.1_1020:123 seq 5 permit 172.16.2.0/24
ip prefix-list ALLOW_ADDRESS_10.0.0.1_1020:123 seq 10 permit 172.16.3.0/24
ip prefix-list ALLOW_ADDRESS_10.0.0.1_1020:555 seq 5 permit 172.16.4.0/24
ip prefix-list ALLOW_ADDRESS_10.0.0.1_1020:555 seq 10 permit 192.16.5.0/24
ip prefix-list ALLOW_ADDRESS_10.0.0.1_empty seq 5 permit 172.16.0.0/24
ip prefix-list ALLOW_ADDRESS_10.0.0.1_empty seq 10 permit 172.16.1.0/24
ip prefix-list PEER_10.0.0.1 seq 5 permit 10.0.0.1/32
ip prefix-list ALLOW_ADDRESS_ALLOW_ALL_V4 seq 10 permit any
!
ipv6 prefix-list ALLOW_ADDRESS_FC00::2_1020:123 seq 5 permit 20c0:a830::/64
ipv6 prefix-list ALLOW_ADDRESS_FC00::2_1020:123 seq 10 permit 20c0:a840::/64
ipv6 prefix-list ALLOW_ADDRESS_FC00::2_1020:555 seq 5 permit 20c0:a850::/64
ipv6 prefix-list ALLOW_ADDRESS_FC00::2_1020:555 seq 10 permit 20c0:a860::/64
ipv6 prefix-list ALLOW_ADDRESS_FC00::2_empty seq 5 permit 20c0:a810::/64
ipv6 prefix-list ALLOW_ADDRESS_FC00::2_empty seq 10 permit 20c0:a820::/64
ipv6 prefix-list ALLOW_ADDRESS_ALLOW_ALL_V6 seq 10 permit any
!
ip community-list standard ALLOW_ADDRESS_10.0.0.1_1020:123 permit 1020:123
ip community-list standard ALLOW_ADDRESS_10.0.0.1_1020:555 permit 1020:555
ip community-list standard ALLOW_ADDRESS_FC00::2_1020:123 permit 1020:123
ip community-list standard ALLOW_ADDRESS_FC00::2_1020:555 permit 1020:555
!
route-map ALLOW_LIST_V6 permit 10
 match community 1020:555
 match ipv6 address prefix-list ALLOW_ADDRESS_FC00::2_1020:555
 match ipv6 next-hop FC00::2
!
route-map ALLOW_LIST_V6 permit 20
 match community 1020:123
 match ipv6 address prefix-list ALLOW_ADDRESS_FC00::2_1020:123
 match ipv6 next-hop FC00::2
!
route-map ALLOW_LIST_V6 permit 30020
 match ipv6 address prefix-list ALLOW_ADDRESS_FC00::2_empty
 match ipv6 next-hop FC00::2
!
route-map ALLOW_LIST_V4 permit 10
 match community 1020:555
 match ip address prefix-list ALLOW_ADDRESS_10.0.0.1_1020:555
 match ip next-hop prefix-list PEER_10.0.0.1
!
route-map ALLOW_LIST_V4 permit 20
 match community 1020:123
 match ip address prefix-list ALLOW_ADDRESS_10.0.0.1_1020:123
 match ip next-hop prefix-list PEER_10.0.0.1
!
route-map ALLOW_LIST_V4 permit 30010
 match ip address prefix-list ALLOW_ADDRESS_10.0.0.1_empty
 match ip next-hop prefix-list PEER_10.0.0.1
!
route-map FROM_PEER_V4 permit 1
 call ALLOW_LIST_V4
 on-match next
!
route-map FROM_PEER_V4 permit 10
!
route-map FROM_PEER_V6 permit 1
 call ALLOW_LIST_V6
 on-match next
!
route-map FROM_PEER_V6 permit 10
!
route-map ALLOW_LIST_V4 deny 65535
!
route-map ALLOW_LIST_V6 deny 65535

As you can see from the output above the feature is isolated inside of "ALLOW_LIST_V4" and "ALLOW_LIST_V6" route-maps. We call the route-maps on top of each "FROM" route-map for each neighbor. When the "ALLOW_LIST_V*" route-map evaluation returns "deny", the "FROM" route-map will return deny. But when "ALLOW_LIST_V*" route-map evaluation returns "permit", the "FROM" route-map will be evaluated.

After each change of configuration the feature "restart" bgp neighbor. The IPv4 neighbor restarts in a soft mode, IPv6 neighbor restarts in a hard mode (quagga doesn't support IPv6 hard mode restart).

To change a list of prefixes inside of each record, you can apply the new list of prefixes with the same key. bgpcfgd will change corrosponding prefix-lists automatically.

When you need to remove the feature from a specific BGP neighbor, you need to remove all "BGP Allow lists" for the neighbor, and BGP will have configuration, which allows the import of all prefixes by default.

When a BGP neighbor is deleted, all BGP configuration related to the feature will be removed.

When a new BGP neighbor is added, the default BGP configuration will be used to allow the import of all prefixes by default.

- Why I did it
I implemented the new feature. See the description above.

- How I did it
I introduced a new functionality into bgpcfgd. All code mostly isolated inside of BGPAllowList class. Also I made some changes inside of bgpd.conf.j2 and bgpd.peer.conf.j2 templates.

- How to verify it

You can save the schema example in the file and then load the file into ConfigDB using the following command: sonic-cfggen -j test_schema.conf --write-to-db. After that you can use show runningconfiguration bgp to check the changed configuration. Also take a look into /var/log/syslog. There you can find the following output:

May  9 00:39:59.746709 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Updating peer 'Allow list' 'FC00::2'. Prefixes: '['20c0:a850::/64', '20c0:a860::/64']' Community: '1020:555'
May  9 00:40:00.058845 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Restarting peer. Peer='FC00::2'
May  9 00:40:00.086069 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Updating peer 'Allow list' '10.0.0.1'. Prefixes: '['172.16.4.0/24', '192.16.5.0/24']' Community: '1020:555'
May  9 00:40:00.570277 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Restarting peer. Peer='10.0.0.1'
May  9 00:40:00.623143 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Updating peer 'Allow list' 'FC00::2'. Prefixes: '['20c0:a810::/64', '20c0:a820::/64']' Community: 'empty'
May  9 00:40:01.075521 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Restarting peer. Peer='FC00::2'
May  9 00:40:01.107332 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Updating peer 'Allow list' '10.0.0.1'. Prefixes: '['172.16.2.0/24', '172.16.3.0/24']' Community: '1020:123'
May  9 00:40:01.559661 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Restarting peer. Peer='10.0.0.1'
May  9 00:40:01.641081 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Updating peer 'Allow list' 'FC00::2'. Prefixes: '['20c0:a830::/64', '20c0:a840::/64']' Community: '1020:123'
May  9 00:40:02.174766 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Restarting peer. Peer='FC00::2'
May  9 00:40:02.223824 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Updating peer 'Allow list' '10.0.0.1'. Prefixes: '['172.16.0.0/24', '172.16.1.0/24']' Community: 'empty'
May  9 00:40:02.594231 str-s6100-acs-1 INFO bgp#bgpcfgd: AllowList::Restarting peer. Peer='10.0.0.1'

You can remove the feature configuration by the following commands. The default configuration will be restored.

redis-cli -n 4 del 'BGP_ALLOWED_PREFIXES|10.0.0.1'
redis-cli -n 4 del 'BGP_ALLOWED_PREFIXES|10.0.0.1|1020:123'
redis-cli -n 4 del 'BGP_ALLOWED_PREFIXES|10.0.0.1|1020:555'

redis-cli -n 4 del 'BGP_ALLOWED_PREFIXES|fc00::2|1020:555'
redis-cli -n 4 del 'BGP_ALLOWED_PREFIXES|fc00::2'
redis-cli -n 4 del 'BGP_ALLOWED_PREFIXES|fc00::2|1020:123'

After that you can use show runningconfiguration bgp to check the changed configuration.

lgtm-com · 2020-05-09T05:32:22Z

This pull request introduces 2 alerts and fixes 2 when merging 57afc49 into ceb8784 - view on LGTM.com

new alerts:

2 for Unreachable code

fixed alerts:

2 for Unused import

lgtm-com · 2020-05-11T17:07:41Z

This pull request introduces 2 alerts and fixes 2 when merging 9f5209e into 8d20030 - view on LGTM.com

new alerts:

2 for Unreachable code

fixed alerts:

2 for Unused import

venkatmahalingam · 2020-05-11T19:49:16Z

dockers/docker-fpm-quagga/bgpcfgd

@@ -93,6 +125,8 @@ class BGPConfigManager(object):
        daemon.add_manager(swsscommon.CONFIG_DB, swsscommon.CFG_DEVICE_NEIGHBOR_METADATA_TABLE_NAME, self.__neighbor_metadata_handler)
        daemon.add_manager(swsscommon.CONFIG_DB, swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME, self.__bgp_handler)
        daemon.add_manager(swsscommon.STATE_DB, swsscommon.STATE_INTERFACE_TABLE_NAME, self.__if_handler)
+        if g_allow_list_enabled:
+            daemon.add_manager(swsscommon.CONFIG_DB, "BGP_ALLOWED_PREFIXES", self.__bgp_allowed_handler)


Pavel,

Introducing new schema for prefixes specific to BGP might confuse the users while the generic schema (IP_PREFIX) is already defined in the below pull request, any thoughts?

https://github.com/Azure/SONiC/blob/48e9012c548528b6528745bda9d75b4164e785eb/doc/mgmt/SONiC_Design_Doc_Unified_FRR_Mgmt_Interface.md

Per FRR and mgmt-framework, IP prefix is the generic resource that can be used in multiple protocols (e.g BGP/route-map..etc), defining the prefixes specific to BGP might make the mapping of mgmt-framework's north bound interfaces (openconfig based) difficult, also, allowing two sources (config-DB schema 1) IP_PREFIX 2) BGP_ALLOWED_PREFIXES) to update FRR prefix list will lead to confusions as to which one to use with route-maps.

Per discussion with Guohan, we would like to use new field "bgp_config_mode" in DEVICE_META_DATA to generically make the tables e.g ROUTE_MAP, IP_PREFIX..etc usable for various other protocols as well not just for BGP.

lgtm-com · 2020-05-11T23:32:52Z

This pull request introduces 2 alerts and fixes 2 when merging 3ad2cf7 into 8d20030 - view on LGTM.com

new alerts:

2 for Unreachable code

fixed alerts:

2 for Unused import

pavel-shirshov · 2020-09-10T00:21:05Z

closed. Check #5309 instead

BGP "Allow list" feature implementation

5607abc

pavel-shirshov added Enhancement ➕ Not Ready to Merge 🕑 Request for 201811 Branch labels May 8, 2020

pavel-shirshov marked this pull request as ready for review May 8, 2020 23:16

pavel-shirshov added 2 commits May 8, 2020 16:17

Merge branch '201811' into pavelsh/201811_allow

8378764

Small fix

16372b2

pavel-shirshov removed the Not Ready to Merge 🕑 label May 9, 2020

pavel-shirshov self-assigned this May 9, 2020

pavel-shirshov requested a review from lguohan May 9, 2020 00:57

Add deployment_id into the test

57afc49

Merge branch '201811' into pavelsh/201811_allow

9f5209e

venkatmahalingam reviewed May 11, 2020

View reviewed changes

Add community to prefixes, which must be dropped.

3ad2cf7

pavel-shirshov closed this Sep 10, 2020

yxieca removed the Request for 201811 Branch label Sep 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BGP "Allow list" feature implementation #4563

BGP "Allow list" feature implementation #4563

pavel-shirshov commented May 8, 2020 •

edited

Loading

lgtm-com bot commented May 9, 2020

lgtm-com bot commented May 11, 2020

venkatmahalingam May 11, 2020 •

edited

Loading

lgtm-com bot commented May 11, 2020

pavel-shirshov commented Sep 10, 2020

BGP "Allow list" feature implementation #4563

BGP "Allow list" feature implementation #4563

Conversation

pavel-shirshov commented May 8, 2020 • edited Loading

lgtm-com bot commented May 9, 2020

lgtm-com bot commented May 11, 2020

venkatmahalingam May 11, 2020 • edited Loading

Choose a reason for hiding this comment

lgtm-com bot commented May 11, 2020

pavel-shirshov commented Sep 10, 2020

pavel-shirshov commented May 8, 2020 •

edited

Loading

venkatmahalingam May 11, 2020 •

edited

Loading