[bgp] Add 'allow list' manager feature (sonic-net#5309)

implements a new feature: "BGP Allow list."

This feature allows us to control which IP prefixes are going to be advertised via ebgp from the routes received from EBGP neighbors.

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

@@ -24,7 +24,7 @@
 {% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
     neighbor PEER_V6 allowas-in 1
     neighbor PEER_V6_INT allowas-in 1
-  {% endif %}
+{% endif %}
 {% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
     neighbor PEER_V6_INT route-reflector-client
 {% endif %}

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

@@ -3,6 +3,33 @@
 !
 !
 !
+{% if constants.bgp.allow_list is defined and constants.bgp.allow_list.enabled is defined and constants.bgp.allow_list.enabled %}
+{%   if constants.bgp.allow_list.default_action is defined and constants.bgp.allow_list.default_action.strip() == 'deny' %}
+route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
+  set community no-export additive
+!
+route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
+  set community no-export additive
+{%   else %}
+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
+  call ALLOW_LIST_DEPLOYMENT_ID_0_V4
+  on-match next
+!
+route-map FROM_BGP_PEER_V6 permit 2
+  call ALLOW_LIST_DEPLOYMENT_ID_0_V6
+  on-match next
+!
+{% endif %}
+!
+!
+!
 route-map FROM_BGP_PEER_V4 permit 100
 !
 route-map TO_BGP_PEER_V4 permit 100

files/image_config/constants/constants.yml

@@ -18,6 +18,18 @@ constants:
       enabled: true
       ipv4: 64
       ipv6: 64
+    allow_list:
+      enabled: true
+      default_action: "permit"   # or "deny"
+      drop_community: 5060:12345  # value of the community to identify a prefix to drop. Make sense only with allow_list_default_action equal to 'permit'
+      default_pl_rules:
+        v4:
+          - "deny 0.0.0.0/0 le 17"
+          - "permit 127.0.0.1/32"
+        v6:
+          - "deny 0::/0 le 59"
+          - "deny 0::/0 ge 65"
+          - "permit fe80::/64"
     peers:
       general: # peer_type
         db_table: "BGP_NEIGHBOR"

rules/sonic_bgpcfgd.mk

@@ -6,6 +6,7 @@ $(SONIC_BGPCFGD)_SRC_PATH = $(SRC_PATH)/sonic-bgpcfgd
 # of sonic-config-engine and bgpcfgd explicitly calls sonic-cfggen
 # as part of its unit tests.
 # TODO: Refactor unit tests so that these dependencies are not needed
-$(SONIC_BGPCFGD)_DEPENDS += $(SWSSSDK_PY2) $(SONIC_PY_COMMON_PY2)
+$(SONIC_BGPCFGD)_DEPENDS += $(SONIC_PY_COMMON_PY2)
+$(SONIC_BGPCFGD)_DEBS_DEPENDS += $(LIBSWSSCOMMON) $(PYTHON_SWSSCOMMON)
 $(SONIC_BGPCFGD)_PYTHON_VERSION = 2
 SONIC_PYTHON_WHEELS += $(SONIC_BGPCFGD)

src/sonic-bgpcfgd/.gitignore

@@ -6,3 +6,4 @@ app/*.pyc
 tests/*.pyc
 tests/__pycache__/
 .idea
+.coverage

src/sonic-bgpcfgd/app/config.py

@@ -10,19 +10,33 @@ class ConfigMgr(object):
     """ The class represents frr configuration """
     def __init__(self):
         self.current_config = None
+        self.current_config_raw = None
 
     def reset(self):
         """ Reset stored config """
         self.current_config = None
+        self.current_config_raw = None
 
     def update(self):
         """ Read current config from FRR """
         self.current_config = None
+        self.current_config_raw = None
         ret_code, out, err = run_command(["vtysh", "-c", "show running-config"])
         if ret_code != 0:
+            # FIXME: should we throw exception here?
             log_crit("can't update running config: rc=%d out='%s' err='%s'" % (ret_code, out, err))
             return
-        self.current_config = self.to_canonical(out)
+        text = []
+        for line in out.split('\n'):
+            if line.lstrip().startswith('!'):
+                continue
+            text.append(line)
+        text += ["     "]  # Add empty line to have something to work on, if there is no text
+        self.current_config_raw = text
+        self.current_config = self.to_canonical(out)  # FIXME: use test as an input
+
+    def push_list(self, cmdlist):
+        return self.push("\n".join(cmdlist))
 
     def push(self, cmd):
         """
@@ -51,8 +65,12 @@ def write(self, cmd):
             log_err("ConfigMgr::push(): can't push configuration '%s', rc='%d', stdout='%s', stderr='%s'" % err_tuple)
         if ret_code == 0:
             self.current_config = None  # invalidate config
+            self.current_config_raw = None
         return ret_code == 0
 
+    def get_text(self):
+        return self.current_config_raw
+
     @staticmethod
     def to_canonical(raw_config):
         """

src/sonic-bgpcfgd/app/directory.py

@@ -0,0 +1,159 @@
+from collections import defaultdict
+
+from app.log import log_err
+
+
+class Directory(object):
+    """ This class stores values and notifies callbacks which were registered to be executed as soon
+        as some value is changed. This class works as DB cache mostly """
+    def __init__(self):
+        self.data = defaultdict(dict)  # storage. A key is a slot name, a value is a dictionary with data
+        self.notify = defaultdict(lambda: defaultdict(list))  # registered callbacks: slot -> path -> handlers[]
+
+    @staticmethod
+    def get_slot_name(db, table):
+        """ Convert db, table pair into a slot name """
+        return db + "__" + table
+
+    def path_traverse(self, slot, path):
+        """
+        Traverse a path in the storage.
+        If the path is an empty string, it returns a value as it is.
+        If the path is not an empty string, the method will traverse through the dictionary value.
+        Example:
+            self.data["key_1"] = { "abc": { "cde": { "fgh": "val_1", "ijk": "val_2" } } }
+            self.path_traverse("key_1", "abc/cde") will return True, { "fgh": "val_1", "ijk": "val_2" }
+        :param slot: storage key
+        :param path: storage path as a string where each internal key is separated by '/'
+        :return: a pair: True if the path was found, object if it was found
+        """
+        if slot not in self.data:
+            return False, None
+        elif path == '':
+            return True, self.data[slot]
+        d = self.data[slot]
+        for p in path.split("/"):
+            if p not in d:
+                return False, None
+            d = d[p]
+        return True, d
+
+    def path_exist(self, db, table, path):
+        """
+        Check if the path exists in the storage
+        :param db: db name
+        :param table: table name
+        :param path: requested path
+        :return: True if the path is available, False otherwise
+        """
+        slot = self.get_slot_name(db, table)
+        return self.path_traverse(slot, path)[0]
+
+    def get_path(self, db, table, path):
+        """
+        Return the requested path from the storage
+        :param db: db name
+        :param table: table name
+        :param path: requested path
+        :return: object if the path was found, None otherwise
+        """
+        slot = self.get_slot_name(db, table)
+        return self.path_traverse(slot, path)[1]
+
+    def put(self, db, table, key, value):
+        """
+        Put information into the storage. Notify handlers which are dependant to the information
+        :param db: db name
+        :param table: table name
+        :param key: key to change
+        :param value: value to put
+        :return:
+        """
+        slot = self.get_slot_name(db, table)
+        self.data[slot][key] = value
+        if slot in self.notify:
+            for path in self.notify[slot].keys():
+                if self.path_exist(db, table, path):
+                    for handler in self.notify[slot][path]:
+                        handler()
+
+    def get(self, db, table, key):
+        """
+        Get a value from the storage
+        :param db: db name
+        :param table: table name
+        :param key: ket to get
+        :return: value for the key
+        """
+        slot = self.get_slot_name(db, table)
+        return self.data[slot][key]
+
+    def get_slot(self, db, table):
+        """
+        Get an object from the storage
+        :param db: db name
+        :param table: table name
+        :return: object for the slot
+        """
+        slot = self.get_slot_name(db, table)
+        return self.data[slot]
+
+    def remove(self, db, table, key):
+        """
+        Remove a value from the storage
+        :param db: db name
+        :param table: table name
+        :param key: key to remove
+        """
+        slot = self.get_slot_name(db, table)
+        if slot in self.data:
+            if key in self.data[slot]:
+                del self.data[slot][key]
+            else:
+                log_err("Directory: Can't remove key '%s' from slot '%s'. The key doesn't exist" % (key, slot))
+        else:
+            log_err("Directory: Can't remove key '%s' from slot '%s'. The slot doesn't exist" % (key, slot))
+
+    def remove_slot(self, db, table):
+        """
+        Remove an object from the storage
+        :param db: db name
+        :param table: table name
+        """
+        slot = self.get_slot_name(db, table)
+        if slot in self.data:
+            del self.data[slot]
+        else:
+            log_err("Directory: Can't remove slot '%s'. The slot doesn't exist" % slot)
+
+    def available(self, db, table):
+        """
+        Check if the table is available
+        :param db: db name
+        :param table: table name
+        :return: True if the slot is available, False if not
+        """
+        slot = self.get_slot_name(db, table)
+        return slot in self.data
+
+    def available_deps(self, deps):
+        """
+        Check if all items from the deps list is available in the storage
+        :param deps: list of dependencies
+        :return: True if all dependencies are presented, False otherwise
+        """
+        res = True
+        for db, table, path in deps:
+            res = res and self.path_exist(db, table, path)
+        return res
+
+    def subscribe(self, deps, handler):
+        """
+        Subscribe the handler to be run as soon as all dependencies are presented
+        :param deps:
+        :param handler:
+        :return:
+        """
+        for db, table, path in deps:
+            slot = self.get_slot_name(db, table)
+            self.notify[slot][path].append(handler)

src/sonic-bgpcfgd/app/manager.py

@@ -0,0 +1,71 @@
+from swsscommon import swsscommon
+
+from app.log import log_debug, log_err
+
+
+class Manager(object):
+    """ This class represents a SONiC DB table """
+    def __init__(self, common_objs, deps, database, table_name):
+        """
+        Initialize class
+        :param common_objs: common object dictionary
+        :param deps: dependencies list
+        :param database: database name
+        :param table_name: table name
+        """
+        self.directory = common_objs['directory']
+        self.cfg_mgr = common_objs['cfg_mgr']
+        self.constants = common_objs['constants']
+        self.deps = deps
+        self.db_name = database
+        self.table_name = table_name
+        self.set_queue = []
+        self.directory.subscribe(deps, self.on_deps_change)  # subscribe this class method on directory changes
+
+    def get_database(self):
+        """ Return associated database """
+        return self.db_name
+
+    def get_table_name(self):
+        """ Return associated table name"""
+        return self.table_name
+
+    def handler(self, key, op, data):
+        """
+        This method is executed on each add/remove event on the table.
+        :param key: key of the table entry
+        :param op: operation on the table entry. Could be either 'SET' or 'DEL'
+        :param data: associated data of the event. Empty for 'DEL' operation.
+        """
+        if op == swsscommon.SET_COMMAND:
+            if self.directory.available_deps(self.deps):  # all required dependencies are set in the Directory?
+                res = self.set_handler(key, data)
+                if not res:  # set handler returned False, which means it is not ready to process is. Save it for later.
+                    log_debug("'SET' handler returned NOT_READY for the Manager: %s" % self.__class__)
+                    self.set_queue.append((key, data))
+            else:
+                log_debug("Not all dependencies are met for the Manager: %s" % self.__class__)
+                self.set_queue.append((key, data))
+        elif op == swsscommon.DEL_COMMAND:
+            self.del_handler(key)
+        else:
+            log_err("Invalid operation '%s' for key '%s'" % (op, key))
+
+    def on_deps_change(self):
+        """ This method is being executed on every dependency change """
+        if not self.directory.available_deps(self.deps):
+            return
+        new_queue = []
+        for key, data in self.set_queue:
+            res = self.set_handler(key, data)
+            if not res:
+                new_queue.append((key, data))
+        self.set_queue = new_queue
+
+    def set_handler(self, key, data):
+        """ Placeholder for 'SET' command """
+        log_err("set_handler() wasn't implemented for %s" % self.__class__.__name__)
+
+    def del_handler(self, key):
+        """ Placeholder for 'DEL' command """
+        log_err("del_handler wasn't implemented for %s" % self.__class__.__name__)

src/sonic-bgpcfgd/app/vars.py

@@ -1 +1 @@
-g_debug = False
+g_debug = True  # FIXME: read from env variable, or from constants

src/sonic-bgpcfgd/bgpcfgd

@@ -15,10 +15,13 @@ import jinja2
 import netaddr
 from swsscommon import swsscommon
 
+from app.directory import Directory
+from app.manager import Manager
 from app.vars import g_debug
 from app.log import log_debug, log_notice, log_info, log_warn, log_err, log_crit
 from app.template import TemplateFabric
 from app.config import ConfigMgr
+from app.allow_list import BGPAllowListMgr
 from app.util import run_command
 
 g_run = True
@@ -846,7 +849,7 @@ def wait_for_daemons(daemons, seconds):
 def read_constants():
     """ Read file with constants values from /etc/sonic/constants.yml """
     with open('/etc/sonic/constants.yml') as fp:
-        content = yaml.load(fp)
+        content = yaml.load(fp) # FIXME: , Loader=yaml.FullLoader)
         if "constants" not in content:
             log_crit("/etc/sonic/constants.yml doesn't have 'constants' key")
             raise Exception("/etc/sonic/constants.yml doesn't have 'constants' key")
@@ -878,6 +881,8 @@ def main():
         BGPPeerMgrBase(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME, "general", True),
         BGPPeerMgrBase(common_objs, "CONFIG_DB", "BGP_MONITORS", "monitors", True),
         BGPPeerMgrBase(common_objs, "CONFIG_DB", "BGP_PEER_RANGE", "dynamic", False),
+        # AllowList Managers
+        BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES"),
     ]
     runner = Runner()
     for mgr in managers:

src/sonic-bgpcfgd/pytest.ini

@@ -0,0 +1,2 @@
+[pytest]
+addopts = --cov=app --cov-report term