Revert frr route check (#2761)

* Revert "[route_check] remove check-frr_patch mock (#2732)"

This reverts commit f27dea0.

* Revert "[route_check] fix IPv6 address handling (#2722)"

This reverts commit ff68832.

* Revert "[route_check] implement a check for FRR routes not marked offloaded (#2531)"

This reverts commit 90d7015.

scripts/route_check.py

@@ -11,11 +11,11 @@
 How:
     NOTE: The flow from APPL-DB to ASIC-DB takes non zero milliseconds.
     1) Initiate subscribe for ASIC-DB updates.
-    2) Read APPL-DB & ASIC-DB
+    2) Read APPL-DB & ASIC-DB 
     3) Get the diff.
-    4) If any diff,
+    4) If any diff, 
         4.1) Collect subscribe messages for a second
-        4.2) check diff against the subscribe messages
+        4.2) check diff against the subscribe messages 
     5) Rule out local interfaces & default routes
     6) If still outstanding diffs, report failure.
 
@@ -29,7 +29,7 @@
         down to ensure failure.
         Analyze the reported failures to match expected.
     You may use the exit code to verify the result as success or not.
-
+    
 
 
 """
@@ -45,9 +45,7 @@
 import time
 import signal
 import traceback
-import subprocess
 
-from ipaddress import ip_network
 from swsscommon import swsscommon
 from utilities_common import chassis
 
@@ -73,9 +71,6 @@
 
 PRINT_MSG_LEN_MAX = 1000
 
-FRR_CHECK_RETRIES = 3
-FRR_WAIT_TIME = 15
-
 class Level(Enum):
     ERR = 'ERR'
     INFO = 'INFO'
@@ -146,7 +141,7 @@ def add_prefix(ip):
         ip = ip + PREFIX_SEPARATOR + "32"
     else:
         ip = ip + PREFIX_SEPARATOR + "128"
-    return str(ip_network(ip))
+    return ip
 
 
 def add_prefix_ifnot(ip):
@@ -155,7 +150,7 @@ def add_prefix_ifnot(ip):
     :param ip: IP to add prefix as string.
     :return ip with prefix
     """
-    return str(ip_network(ip)) if ip.find(PREFIX_SEPARATOR) != -1 else add_prefix(ip)
+    return ip if ip.find(PREFIX_SEPARATOR) != -1 else add_prefix(ip)
 
 
 def is_local(ip):
@@ -298,7 +293,7 @@ def get_routes():
 
 def get_route_entries():
     """
-    helper to read present route entries from ASIC-DB and
+    helper to read present route entries from ASIC-DB and 
     as well initiate selector for ASIC-DB:ASIC-state updates.
     :return (selector,  subscriber, <list of sorted routes>)
     """
@@ -314,39 +309,14 @@ def get_route_entries():
         res, e = checkout_rt_entry(k)
         if res:
             rt.append(e)
-
+                                
     print_message(syslog.LOG_DEBUG, json.dumps({"ASIC_ROUTE_ENTRY": sorted(rt)}, indent=4))
 
     selector = swsscommon.Select()
     selector.addSelectable(subs)
     return (selector, subs, sorted(rt))
 
 
-def is_suppress_fib_pending_enabled():
-    """
-    Returns True if FIB suppression is enabled, False otherwise
-    """
-    cfg_db = swsscommon.ConfigDBConnector()
-    cfg_db.connect()
-
-    state = cfg_db.get_entry('DEVICE_METADATA', 'localhost').get('suppress-fib-pending')
-
-    return state == 'enabled'
-
-
-def get_frr_routes():
-    """
-    Read routes from zebra through CLI command
-    :return frr routes dictionary
-    """
-
-    output = subprocess.check_output('show ip route json', shell=True)
-    routes = json.loads(output)
-    output = subprocess.check_output('show ipv6 route json', shell=True)
-    routes.update(json.loads(output))
-    return routes
-
-
 def get_interfaces():
     """
     helper to read interface table from APPL-DB.
@@ -384,7 +354,7 @@ def filter_out_local_interfaces(keys):
 
     chassis_local_intfs = chassis.get_chassis_local_interfaces()
     local_if_lst.update(set(chassis_local_intfs))
-
+    
     db = swsscommon.DBConnector(APPL_DB_NAME, 0)
     tbl = swsscommon.Table(db, 'ROUTE_TABLE')
 
@@ -523,61 +493,6 @@ def filter_out_standalone_tunnel_routes(routes):
     return updated_routes
 
 
-def check_frr_pending_routes():
-    """
-    Check FRR routes for offload flag presence by executing "show ip route json"
-    Returns a list of routes that have no offload flag.
-    """
-
-    missed_rt = []
-
-    retries = FRR_CHECK_RETRIES
-    for i in range(retries):
-        missed_rt = []
-        frr_routes = get_frr_routes()
-
-        for _, entries in frr_routes.items():
-            for entry in entries:
-                if entry['protocol'] != 'bgp':
-                    continue
-
-                # TODO: Also handle VRF routes. Currently this script does not check for VRF routes so it would be incorrect for us
-                # to assume they are installed in ASIC_DB, so we don't handle them.
-                if entry['vrfName'] != 'default':
-                    continue
-
-                if not entry.get('offloaded', False):
-                    missed_rt.append(entry)
-
-        if not missed_rt:
-            break
-
-        time.sleep(FRR_WAIT_TIME)
-
-    return missed_rt
-
-
-def mitigate_installed_not_offloaded_frr_routes(missed_frr_rt, rt_appl):
-    """
-    Mitigate installed but not offloaded FRR routes.
-
-    In case route exists in APPL_DB, this function will manually send a notification to fpmsyncd
-    to trigger the flow that sends offload flag to zebra.
-
-    It is designed to mitigate a problem when orchagent fails to send notification about installed route to fpmsyncd
-    or fpmsyncd not being able to read the notification or in case zebra fails to receive offload update due to variety of reasons.
-    All of the above mentioned cases must be considered as a bug, but even in that case we will report an error in the log but
-    given that this script ensures the route is installed in the hardware it will automitigate such a bug.
-    """
-    db = swsscommon.DBConnector('APPL_STATE_DB', 0)
-    response_producer = swsscommon.NotificationProducer(db, f'{APPL_DB_NAME}_{swsscommon.APP_ROUTE_TABLE_NAME}_RESPONSE_CHANNEL')
-    for entry in [entry for entry in missed_frr_rt if entry['prefix'] in rt_appl]:
-        fvs = swsscommon.FieldValuePairs([('err_str', 'SWSS_RC_SUCCESS'), ('protocol', entry['protocol'])])
-        response_producer.send('SWSS_RC_SUCCESS', entry['prefix'], fvs)
-
-        print_message(syslog.LOG_ERR, f'Mitigated route {entry["prefix"]}')
-
-
 def get_soc_ips(config_db):
     mux_table = config_db.get_table('MUX_CABLE')
     soc_ips = []
@@ -621,7 +536,7 @@ def check_routes():
     """
     The heart of this script which runs the checks.
     Read APPL-DB & ASIC-DB, the relevant tables for route checking.
-    Checkout routes in ASIC-DB to match APPL-DB, discounting local &
+    Checkout routes in ASIC-DB to match APPL-DB, discounting local & 
     default routes. In case of missed / unexpected entries in ASIC,
     it might be due to update latency between APPL & ASIC DBs. So collect
     ASIC-DB subscribe updates for a second, and checkout if you see SET
@@ -630,16 +545,12 @@ def check_routes():
     If there are still some unjustifiable diffs, between APPL & ASIC DB,
     related to routes report failure, else all good.
 
-    If there are FRR routes that aren't marked offloaded but all APPL & ASIC DB
-    routes are in sync report failure and perform a mitigation action.
-
     :return (0, None) on sucess, else (-1, results) where results holds
     the unjustifiable entries.
     """
     intf_appl_miss = []
     rt_appl_miss = []
     rt_asic_miss = []
-    rt_frr_miss = []
 
     results = {}
     adds = []
@@ -688,22 +599,11 @@ def check_routes():
     if rt_asic_miss:
         results["Unaccounted_ROUTE_ENTRY_TABLE_entries"] = rt_asic_miss
 
-    rt_frr_miss = check_frr_pending_routes()
-
-    if rt_frr_miss:
-        results["missed_FRR_routes"] = rt_frr_miss
-
     if results:
         print_message(syslog.LOG_WARNING, "Failure results: {",  json.dumps(results, indent=4), "}")
         print_message(syslog.LOG_WARNING, "Failed. Look at reported mismatches above")
         print_message(syslog.LOG_WARNING, "add: ", json.dumps(adds, indent=4))
         print_message(syslog.LOG_WARNING, "del: ", json.dumps(deletes, indent=4))
-
-        if rt_frr_miss and not rt_appl_miss and not rt_asic_miss:
-            print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR but all routes in APPL_DB and ASIC_DB are in sync")
-            if is_suppress_fib_pending_enabled():
-                mitigate_installed_not_offloaded_frr_routes(rt_frr_miss, rt_appl)
-
         return -1, results
     else:
         print_message(syslog.LOG_INFO, "All good!")
@@ -749,7 +649,7 @@ def main():
                 return ret, res
         else:
             return ret, res
-
+            
 
 
 if __name__ == "__main__":

tests/mock_tables/config_db.json

@@ -842,8 +842,7 @@
         "mac": "1d:34:db:16:a6:00",
         "platform": "x86_64-mlnx_msn3800-r0",
         "peer_switch": "sonic-switch",
-        "type": "ToRRouter",
-        "suppress-fib-pending": "enabled"
+        "type": "ToRRouter"
     },
     "SNMP_COMMUNITY|msft": {
         "TYPE": "RO"

tests/route_check_test.py

@@ -7,7 +7,7 @@
 import time
 from sonic_py_common import device_info
 from unittest.mock import MagicMock, patch
-from tests.route_check_test_data import APPL_DB, ARGS, ASIC_DB, CONFIG_DB, DEFAULT_CONFIG_DB, DESCR, OP_DEL, OP_SET, PRE, RESULT, RET, TEST_DATA, UPD, FRR_ROUTES
+from tests.route_check_test_data import APPL_DB, ARGS, ASIC_DB, CONFIG_DB, DEFAULT_CONFIG_DB, DESCR, OP_DEL, OP_SET, PRE, RESULT, RET, TEST_DATA, UPD
 
 import pytest
 
@@ -239,7 +239,6 @@ def setup(self):
 
     def init(self):
         route_check.UNIT_TESTING = 1
-        route_check.FRR_WAIT_TIME = 0
 
     @pytest.fixture
     def force_hang(self):
@@ -259,8 +258,7 @@ def mock_dbs(self):
              patch("route_check.swsscommon.Table") as mock_table, \
              patch("route_check.swsscommon.Select") as mock_sel, \
              patch("route_check.swsscommon.SubscriberStateTable") as mock_subs, \
-             patch("route_check.swsscommon.ConfigDBConnector", return_value=mock_config_db), \
-             patch("route_check.swsscommon.NotificationProducer"):
+             patch("route_check.swsscommon.ConfigDBConnector", return_value=mock_config_db):
             device_info.get_platform = MagicMock(return_value='unittest')
             set_mock(mock_table, mock_conn, mock_sel, mock_subs, mock_config_db)
             yield
@@ -274,16 +272,7 @@ def test_route_check(self, mock_dbs, test_num):
         set_test_case_data(ct_data)
         logger.info("Running test case {}: {}".format(test_num, ct_data[DESCR]))
 
-        with patch('sys.argv', ct_data[ARGS].split()), \
-            patch('route_check.subprocess.check_output') as mock_check_output:
-
-            routes = ct_data.get(FRR_ROUTES, {})
-
-            def side_effect(*args, **kwargs):
-                return json.dumps(routes)
-
-            mock_check_output.side_effect = side_effect
-
+        with patch('sys.argv', ct_data[ARGS].split()):
             ret, res = route_check.main()
             expect_ret = ct_data[RET] if RET in ct_data else 0
             expect_res = ct_data[RESULT] if RESULT in ct_data else None