Handle BIRD receive limit issue gracefully

.travis.yml

@@ -35,6 +35,10 @@ jobs:
         - pip install coveralls
       after_success:
         - coveralls
+    - env: TOXENV=py36-cli
+      python: "3.6"
+      install:
+        - pip install tox
 
     # External resources tests, run on push/PR and via cron job.
     - stage: test external resources

pierky/arouteserver/builder.py

@@ -730,11 +730,15 @@ class BIRDConfigBuilder(ConfigBuilder):
              "scrub_communities_in", "scrub_communities_out",
              "apply_blackhole_filtering_policy"]
 
+    IGNORABLE_ISSUES = ["max_prefix_count_rejected_routes"]
+
     AVAILABLE_VERSION = ["1.6.3", "1.6.4", "1.6.6", "1.6.7", "1.6.8",
                          "2.0.7"]
     DEFAULT_VERSION = "1.6.8"
 
     def validate_bgpspeaker_specific_configuration(self):
+        res = True
+
         if self.ip_ver is None and \
            version.parse(self.target_version) < version.parse("2.0"):
             raise BuilderError(
@@ -750,7 +754,66 @@ def validate_bgpspeaker_specific_configuration(self):
                     "hooks must be a list of hook names"
                 )
 
-        return True
+        if version.parse(self.target_version) >= version.parse("2.0.7"):
+            max_prefix_count_rejected_routes_clients = []
+
+            for client in self.cfg_clients.cfg["clients"]:
+                max_prefix_count_rejected_routes = client["cfg"]["filtering"]["max_prefix"]["count_rejected_routes"]
+                if max_prefix_count_rejected_routes:
+                    max_prefix_count_rejected_routes_clients.append(client["ip"])
+
+            if (
+                self.cfg_general["filtering"]["max_prefix"]["count_rejected_routes"] or \
+                max_prefix_count_rejected_routes_clients
+            ):
+                if not self.process_bgpspeaker_specific_compatibility_issue(
+                    "max_prefix_count_rejected_routes",
+                    "In BIRD, the functionality represented by the "
+                    "'count_rejected_routes: True' configuration knob is "
+                    "implemented using the 'receive limit' statement. "
+                    "According to some tests, BIRD 2.0.7 is affected by "
+                    "an issue that prevents that statement from working "
+                    "correctly (see {link_bird} for more details). "
+                    "Even though beginning with ARouteServer 1.0.1 the "
+                    "default value of 'count_rejected_routes' is True, "
+                    "it's not advisable to deploy any configuration for "
+                    "BIRD 2.0 that uses it at the moment. "
+                    "There are two ways to unblock the build of the "
+                    "configuration: this error can be ignored using the "
+                    "command line argument mentioned later on, or the "
+                    "value of 'count_rejected_routes' can be set to "
+                    "False. "
+                    "If this error will be ignored, the configuration "
+                    "will be generated as if that option was set to "
+                    "False; this may be a good option if the goal is "
+                    "to have the desired behaviour of counting rejected "
+                    "routes towards the max-prefix limit implemented in "
+                    "any future release of BIRD 2.0 for which the issue "
+                    "will be solved. In this case, future releases of "
+                    "ARouteServer will skip this check if the "
+                    "--target-version will be set to the release for "
+                    "which the issue is fixed. "
+                    "The other option, that is to specifically configure "
+                    "'count_rejected_routes: False', may be the best one "
+                    "if the goal is to keep the currently available "
+                    "behaviour permanent in the configurations generated "
+                    "by this tool. For more details on how to configure "
+                    "it, see the general.yml file distributed with the "
+                    "package or check this URL: {link_ars}."
+                    "".format(
+                        link_bird=("http://trubka.network.cz/pipermail/"
+                                   "bird-users/2020-October/014932.html"),
+                        link_ars=("https://arouteserver.readthedocs.io/"
+                                  "en/latest/GENERAL.html#max-prefix-max-prefix")
+                    )
+                ):
+                    res = False
+                else:
+                    self.cfg_general["filtering"]["max_prefix"]["count_rejected_routes"] = False
+                    for client in self.cfg_clients.cfg["clients"]:
+                        client["cfg"]["filtering"]["max_prefix"]["count_rejected_routes"] = False
+
+        return res
 
     def _include_local_file(self, local_file_id):
         return 'include "{}";\n\n'.format(

pierky/arouteserver/commands/configure.py

@@ -24,7 +24,7 @@
 from .base import ARouteServerCommand
 
 from ..ask import Ask
-from ..builder import OpenBGPDConfigBuilder
+from ..builder import OpenBGPDConfigBuilder, BIRDConfigBuilder
 from ..config.program import program_config
 from ..errors import ARouteServerError
 from ..ipaddresses import IPNetwork
@@ -147,6 +147,12 @@ def collect_answers(self):
                 options=OpenBGPDConfigBuilder.AVAILABLE_VERSION,
                 default=OpenBGPDConfigBuilder.AVAILABLE_VERSION[-1]
             )
+        else:
+            self.add_answer("version", self.ask.ask,
+                "Which version?",
+                options=BIRDConfigBuilder.AVAILABLE_VERSION,
+                default=BIRDConfigBuilder.DEFAULT_VERSION
+            )
 
         self.wr_text(
             None, title="Router server's ASN"
@@ -336,6 +342,15 @@ def add_comm(name, std=None, lrg=None):
             "PeeringDB is used to fetch networks prefix count."
         )
 
+        if (
+            self.answers["daemon"] == "bird" and \
+            version.parse(self.answers["version"]) >= version.parse("2.0")
+        ):
+            filtering["max_prefix"]["count_rejected_routes"] = False
+            self.notes.append(
+                "Rejected routes are not counted towards max-prefix limit."
+            )
+
         cfg["graceful_shutdown"] = {"enabled": False}
         if self.answers["daemon"] == "bird":
             cfg["graceful_shutdown"] = {"enabled": True}

tests/cli

@@ -74,6 +74,7 @@ EOF
 SUB_TEST="$LINENO"
 GENERAL="$GENERAL_RS1"
 build_cmd "bird" --ip-ver 4 | must_contain "router id 192.0.2.1"
+build_cmd "bird" --ip-ver 4
 SUB_TEST="$LINENO"
 GENERAL="$GENERAL_RS2"
 build_cmd "bird" --ip-ver 4 | must_contain "router id 192.0.2.2"
@@ -259,6 +260,22 @@ EXP_ERR="the only available behaviour is to have the rejected routes counted tow
 SUB_TEST="$LINENO"
 build_cmd "openbgpd" | must_contain "$EXP_ERR"
 
+# ---------------------------------------------
+reset
+TITLE="BIRD 2.0.7 max_prefix_count_rejected_routes"
+GENERAL="tests/var/general.yml"
+cat << EOF > $GENERAL
+cfg:
+  rs_as: 999
+  router_id: "192.0.2.2"
+EOF
+
+EXP_ERR="the only available behaviour is to have the rejected routes counted towards the limit"
+
+SUB_TEST="$LINENO"
+build_cmd "bird" --target-version 2.0.7 | must_contain "BIRD 2.0.7 is affected by an issue that prevents that statement from working"
+build_cmd "bird" --target-version 2.0.7
+
 # ---------------------------------------------
 # clients-from-euroix
 reset

tests/live_tests/scenarios/max_prefix/README.rst

@@ -16,7 +16,8 @@ AS1:
 - n. of announced routes: 5 (all valid)
 - expectations:
 
-  - BIRD: only 4 routes received
+  - BIRD 1.6.x: only 4 routes received
+  - BIRD 2.0.7: 4 valid imported, 5 received
   - OpenBGPD: session down
 
 AS2 (client with peering_db.increment set to 0/0):
@@ -28,7 +29,8 @@ AS2 (client with peering_db.increment set to 0/0):
 - n. of announced routes: 5 (all valid)
 - expectations:
 
-  - BIRD: only 3 routes received
+  - BIRD 1.6.x: only 3 routes received
+  - BIRD 2.0.7: 3 imported, 5 received
   - OpenBGPD: session down
 
 AS3:
@@ -39,7 +41,8 @@ AS3:
 - n. of announced routes: 5 (all valid)
 - expectations:
 
-  - BIRD: only 2 routes received
+  - BIRD 1.6.x: only 2 routes received
+  - BIRD 2.0.7: 2 imported, 5 received
   - OpenBGPD: session down
 
 AS4:
@@ -51,7 +54,8 @@ AS4:
 - n. of announced routes: 7 (all valid)
 - expectations:
 
-  - BIRD: only 6 routes received
+  - BIRD 1.6.x: only 6 routes received
+  - BIRD 2.0.7: 6 imported, 7 received
   - OpenBGPD: session down
 
 AS5 (only for BIRD):
@@ -62,7 +66,8 @@ AS5 (only for BIRD):
 - n. of announced routes: 4 (2 valid, 2 bogons)
 - expectations:
 
-  - BIRD: session down
+  - BIRD 1.6.x: session down
+  - BIRD 2.0.7: session up, all routes received
 
 AS6 (only for BIRD):
 - specific limit: 3
@@ -72,4 +77,5 @@ AS6 (only for BIRD):
 - n. of announced routes: 4 (2 valid, 2 bogons)
 - expectations:
 
-  - BIRD: all 4 routes received
+  - BIRD 1.6.x: all 4 routes received
+  - BIRD 2.0.7: all 4 routes received

tests/live_tests/scenarios/max_prefix/base.py

@@ -100,6 +100,8 @@ class MaxPrefixScenarioBIRD(MaxPrefixScenario):
     TARGET_VERSION = None
     IP_VER = None
 
+    EXPECTED_LOG_MSG = "receive"
+
     @classmethod
     def _setup_instances(cls):
         super(MaxPrefixScenarioBIRD, cls)._setup_instances()
@@ -158,37 +160,37 @@ def test_020_sessions_up(self):
 
         self.session_is_up(self.rs, self.AS6)
 
-    def test_020_sessions_up(self):
+    def test_020_sessions_up_AS5(self):
         """{}: AS5 session is down (max-prefix hit, action == shutdown)"""
         with six.assertRaisesRegex(
             self,
             AssertionError, "is not up"
         ):
             self.session_is_up(self.rs, self.AS5)
 
-    def test_030_blocked_sessions_1(self):
+    def test_030_blocked_sessions(self):
         """{}: log is populated: receive limit, routes blocked"""
-        log_tpl = "Protocol {{inst}} hits route receive limit ({limit}), action: block"
+        log_tpl = "Protocol {{inst}} hits route {expected_log_msg} limit ({limit}), action: block"
 
-        log = log_tpl.format(limit=4)
+        log = log_tpl.format(limit=4, expected_log_msg=self.EXPECTED_LOG_MSG)
         self.log_contains(self.rs, log, {"inst": self.AS1})
 
-        log = log_tpl.format(limit=3)
+        log = log_tpl.format(limit=3, expected_log_msg=self.EXPECTED_LOG_MSG)
         self.log_contains(self.rs, log, {"inst": self.AS2})
 
-        log = log_tpl.format(limit=2)
+        log = log_tpl.format(limit=2, expected_log_msg=self.EXPECTED_LOG_MSG)
         self.log_contains(self.rs, log, {"inst": self.AS3})
 
-        log = log_tpl.format(limit=6)
+        log = log_tpl.format(limit=6, expected_log_msg=self.EXPECTED_LOG_MSG)
         self.log_contains(self.rs, log, {"inst": self.AS4})
 
-    def test_030_blocked_sessions_2(self):
+    def test_030_blocked_sessions_AS5(self):
         """{}: log is populated: receive limit, session shutdown (AS5)"""
-        log_tpl = "Protocol {{inst}} hits route receive limit ({limit}), action: disable"
-        log = log_tpl.format(limit=3)
+        log_tpl = "Protocol {{inst}} hits route {expected_log_msg} limit ({limit}), action: disable"
+        log = log_tpl.format(limit=3, expected_log_msg=self.EXPECTED_LOG_MSG)
         self.log_contains(self.rs, log, {"inst": self.AS5})
 
-    def test_030_blocked_sessions_3(self):
+    def test_030_blocked_sessions_AS6(self):
         """{}: log is populated: import limit, no warning in the log file (AS6)"""
 
         log = "Protocol {{inst}} hits route import limit"
@@ -210,35 +212,35 @@ def _get_routes_from(self, asn, include_filtered=False):
             )
         return routes
 
-    def test_030_count_received_prefixes_AS1(self):
+    def test_040_count_received_prefixes_AS1(self):
         """{}: number of prefixes received by rs from AS1"""
         asn = 1
 
         self.assertEqual(len(self._get_routes_from(asn)), 4)
         self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 4)
 
-    def test_031_count_received_prefixes_AS2(self):
+    def test_040_count_received_prefixes_AS2(self):
         """{}: number of prefixes received by rs from AS2"""
         asn = 2
 
         self.assertEqual(len(self._get_routes_from(asn)), 3)
         self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 3)
 
-    def test_032_count_received_prefixes_AS3(self):
+    def test_040_count_received_prefixes_AS3(self):
         """{}: number of prefixes received by rs from AS3"""
         asn = 3
 
         self.assertEqual(len(self._get_routes_from(asn)), 2)
         self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 2)
 
-    def test_032_count_received_prefixes_AS4(self):
+    def test_040_count_received_prefixes_AS4(self):
         """{}: number of prefixes received by rs from AS4"""
         asn = 4
 
         self.assertEqual(len(self._get_routes_from(asn)), 6)
         self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 6)
 
-    def test_032_count_received_prefixes_AS6(self):
+    def test_040_count_received_prefixes_AS6(self):
         """{}: number of prefixes received by rs from AS6"""
         asn = 6
 
@@ -255,6 +257,64 @@ class MaxPrefixScenarioBIRD2(MaxPrefixScenarioBIRD):
 
     TARGET_VERSION = "2.0.7"
 
+    EXPECTED_LOG_MSG = "import"
+
+    def test_020_sessions_up_AS5(self):
+        """{}: AS5 session is up"""
+        self.session_is_up(self.rs, self.AS5)
+
+    def test_030_blocked_sessions_AS5(self):
+        """{}: log is populated: receive limit, session NOT shutdown (AS5)"""
+
+        log = "Protocol {{inst}} hits route import limit"
+        # Please note: opposite=True, it fails if the msg is found in the logs:
+        # AS6 announces 2 valid routes + 2 invalid routes, so it doesn't hit
+        # the limit because invalid routes are not taken into account.
+        self.log_contains(self.rs, log, {"inst": self.AS5}, opposite=True)
+
+    def test_040_count_received_prefixes_AS1(self):
+        """{}: number of prefixes received by rs from AS1"""
+        asn = 1
+
+        self.assertEqual(len(self._get_routes_from(asn)), 4)
+        self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 5)
+
+    def test_040_count_received_prefixes_AS2(self):
+        """{}: number of prefixes received by rs from AS2"""
+        asn = 2
+
+        self.assertEqual(len(self._get_routes_from(asn)), 3)
+        self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 5)
+
+    def test_040_count_received_prefixes_AS3(self):
+        """{}: number of prefixes received by rs from AS3"""
+        asn = 3
+
+        self.assertEqual(len(self._get_routes_from(asn)), 2)
+        self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 5)
+
+    def test_040_count_received_prefixes_AS4(self):
+        """{}: number of prefixes received by rs from AS4"""
+        asn = 4
+
+        self.assertEqual(len(self._get_routes_from(asn)), 6)
+        self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 7)
+
+    def test_040_count_received_prefixes_AS5(self):
+        """{}: number of prefixes received by rs from AS5"""
+        asn = 5
+
+        self.assertEqual(len(self._get_routes_from(asn)), 2)
+        self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 4)
+
+    def test_040_count_received_prefixes_AS6(self):
+        """{}: number of prefixes received by rs from AS6"""
+        asn = 6
+
+        self.assertEqual(len(self._get_routes_from(asn)), 2)
+        self.assertEqual(len(self._get_routes_from(asn, include_filtered=True)), 4)
+
+
 class MaxPrefixScenarioOpenBGPD(LiveScenario_TagRejectPolicy, MaxPrefixScenario):
     __test__ = False
 

tests/static/irrdb_data/rset_AS2_ipv6.json

@@ -0,0 +1,5 @@
+{
+  "prefix_list": [
+    {"prefix": "2001:db8::/32"}
+  ]
+}