ospfd: OSPF opaque LSA stale processing fix and topotests.

        1. Fix OSPF opaque LSA processing to preserve the stale opaque
            LSAs in the Link State Database for 60 seconds consistent with
            what is done for other LSA types.
         2. Add a topotest that tests for cases where ospfd is restarted
            and a stale OSPF opaque LSA exists in the OSPF routing domain
            both when the LSA is purged and when the LSA is reoriginagted
            with a more recent instance.

Signed-off-by: Acee <aceelindem@gmail.com>
(cherry picked from commit 4e7eb1e62ce54ebcf78622682de962fdeff20b80)

diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c
index 6415fda1fcc9f88838bb0bf9fbf4042d3a9f07d4..bcfdddd39fba460be00f7b4e7921a98611128ebd 100644 (file)
--- a/ospfd/ospf_apiserver.c
+++ b/ospfd/ospf_apiserver.c
@@ -1779,6 +1779,12 @@ int ospf_apiserver_originate1(struct ospf_lsa *lsa, struct ospf_lsa *old)
                 * session. Dump it, but increment past it's seqnum.
                 */
                assert(!ospf_opaque_is_owned(old));
+               if (IS_DEBUG_OSPF_CLIENT_API)
+                       zlog_debug(
+                               "LSA[Type%d:%pI4]: OSPF API Server Originate LSA Old Seq: 0x%x Age: %d",
+                               old->data->type, &old->data->id,
+                               ntohl(old->data->ls_seqnum),
+                               ntohl(old->data->ls_age));
                if (IS_LSA_MAX_SEQ(old)) {
                        flog_warn(EC_OSPF_LSA_INSTALL_FAILURE,
                                  "%s: old LSA at maxseq", __func__);
@@ -1787,6 +1793,11 @@ int ospf_apiserver_originate1(struct ospf_lsa *lsa, struct ospf_lsa *old)
                lsa->data->ls_seqnum = lsa_seqnum_increment(old);
                ospf_discard_from_db(ospf, old->lsdb, old);
        }
+       if (IS_DEBUG_OSPF_CLIENT_API)
+               zlog_debug(
+                       "LSA[Type%d:%pI4]: OSPF API Server Originate LSA New Seq: 0x%x Age: %d",
+                       lsa->data->type, &lsa->data->id,
+                       ntohl(lsa->data->ls_seqnum), ntohl(lsa->data->ls_age));
 
        /* Install this LSA into LSDB. */
        if (ospf_lsa_install(ospf, lsa->oi, lsa) == NULL) {
@@ -1861,6 +1872,11 @@ struct ospf_lsa *ospf_apiserver_lsa_refresher(struct ospf_lsa *lsa)
        ospf = ospf_lookup_by_vrf_id(VRF_DEFAULT);
        assert(ospf);
 
+       if (IS_DEBUG_OSPF(lsa, LSA_GENERATE)) {
+               zlog_debug("LSA[Type%d:%pI4]: OSPF API Server LSA Refresher",
+                          lsa->data->type, &lsa->data->id);
+       }
+
        apiserv = lookup_apiserver_by_lsa(lsa);
        if (!apiserv) {
                zlog_warn("%s: LSA[%s]: No apiserver?", __func__,
@@ -1870,14 +1886,14 @@ struct ospf_lsa *ospf_apiserver_lsa_refresher(struct ospf_lsa *lsa)
                goto out;
        }
 
-       if (IS_LSA_MAXAGE(lsa)) {
-               ospf_opaque_lsa_flush_schedule(lsa);
-               goto out;
-       }
-
        /* Check if updated version of LSA instance has already prepared. */
        new = ospf_lsdb_lookup(&apiserv->reserve, lsa);
        if (!new) {
+               if (IS_LSA_MAXAGE(lsa)) {
+                       ospf_opaque_lsa_flush_schedule(lsa);
+                       goto out;
+               }
+
                /* This is a periodic refresh, driven by core OSPF mechanism. */
                new = ospf_apiserver_opaque_lsa_new(lsa->area, lsa->oi,
                                                    lsa->data);

diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
index e085814c66902c4f09bba1b63b7db85ff0184635..f4cfeeeb318ee4bf795d77efd343faf409a36eb1 100644 (file)
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -2127,14 +2127,21 @@ void ospf_opaque_self_originated_lsa_received(struct ospf_neighbor *nbr,
                        lsa->data->type, &lsa->data->id);
 
        /*
-        * Since these LSA entries are not yet installed into corresponding
-        * LSDB, just flush them without calling ospf_ls_maxage() afterward.
+        * Install the stale LSA into the Link State Database, add it to the
+        * MaxAge list, and flush it from the OSPF routing domain. For other
+        * LSA types, the installation is done in the refresh function. It is
+        * done inline here since the opaque refresh function is dynamically
+        * registered when opaque LSAs are originated (which is not the case
+        * for stale LSAs).
         */
        lsa->data->ls_age = htons(OSPF_LSA_MAXAGE);
+       ospf_lsa_install(
+               top, (lsa->data->type == OSPF_OPAQUE_LINK_LSA) ? nbr->oi : NULL,
+               lsa);
+       ospf_lsa_maxage(top, lsa);
+
        switch (lsa->data->type) {
        case OSPF_OPAQUE_LINK_LSA:
-               ospf_flood_through_area(nbr->oi->area, NULL /*inbr*/, lsa);
-               break;
        case OSPF_OPAQUE_AREA_LSA:
                ospf_flood_through_area(nbr->oi->area, NULL /*inbr*/, lsa);
                break;
@@ -2146,7 +2153,6 @@ void ospf_opaque_self_originated_lsa_received(struct ospf_neighbor *nbr,
                          __func__, lsa->data->type);
                return;
        }
-       ospf_lsa_discard(lsa); /* List "lsas" will be deleted by caller. */
 }
 
 /*------------------------------------------------------------------------*

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 43a707f0722076a95886aa54203e1d94207c1875..e8442ee38016d81f117bf31f932965001383fb88 100644 (file)
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2036,7 +2036,7 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph,
                        if (current == NULL) {
                                if (IS_DEBUG_OSPF_EVENT)
                                        zlog_debug(
-                                               "LSA[%s]: Previously originated Opaque-LSA,not found in the LSDB.",
+                                               "LSA[%s]: Previously originated Opaque-LSA, not found in the LSDB.",
                                                dump_lsa_key(lsa));
 
                                SET_FLAG(lsa->flags, OSPF_LSA_SELF);

diff --git a/tests/topotests/ospfapi/test_ospf_clientapi.py b/tests/topotests/ospfapi/test_ospf_clientapi.py
index 85c30b473e1c80bb7a9ef00b249d73740182cd20..a7179c554ec982aa15aea51d425fac95af19af79 100644 (file)
--- a/tests/topotests/ospfapi/test_ospf_clientapi.py
+++ b/tests/topotests/ospfapi/test_ospf_clientapi.py
@@ -33,7 +33,15 @@ from datetime import datetime, timedelta
 
 import pytest
 
-from lib.common_config import retry, run_frr_cmd, step
+from lib.common_config import (
+    retry,
+    run_frr_cmd,
+    step,
+    kill_router_daemons,
+    start_router_daemons,
+    shutdown_bringup_interface,
+)
+
 from lib.micronet import Timeout, comm_error
 from lib.topogen import Topogen, TopoRouter
 from lib.topotest import interface_set_status, json_cmp
@@ -949,6 +957,191 @@ def test_ospf_opaque_delete_data3(tgen):
     _test_opaque_add_del(tgen, apibin)
 
 
+def _test_opaque_add_restart_add(tgen, apibin):
+    "Test adding an opaque LSA and then restarting ospfd"
+
+    r1 = tgen.gears["r1"]
+    r2 = tgen.gears["r2"]
+
+    p = None
+    pread = None
+    # Log to our stdin, stderr
+    pout = open(os.path.join(r1.net.logdir, "r1/add-del.log"), "a+")
+    try:
+        step("reachable: check for add notification")
+        pread = r2.popen(
+            ["/usr/bin/timeout", "120", apibin, "-v", "--logtag=READER", "wait,120"],
+            encoding=None,  # don't buffer
+            stdin=subprocess.DEVNULL,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+        )
+        p = r1.popen(
+            [
+                apibin,
+                "-v",
+                "add,10,1.2.3.4,231,1",
+                "add,10,1.2.3.4,231,1,feedaceebeef",
+                "wait, 5",
+                "add,10,1.2.3.4,231,1,feedaceedeadbeef",
+                "wait, 5",
+                "add,10,1.2.3.4,231,1,feedaceebaddbeef",
+                "wait, 5",
+            ]
+        )
+        add_input_dict = {
+            "areas": {
+                "1.2.3.4": {
+                    "areaLocalOpaqueLsa": [
+                        {
+                            "lsId": "231.0.0.1",
+                            "advertisedRouter": "1.0.0.0",
+                            "sequenceNumber": "80000004",
+                            "checksum": "3128",
+                        },
+                    ],
+                    "areaLocalOpaqueLsaCount": 1,
+                },
+            },
+        }
+        step("Check for add LSAs")
+        json_cmd = "show ip ospf da json"
+        assert verify_ospf_database(tgen, r1, add_input_dict, json_cmd) is None
+        assert verify_ospf_database(tgen, r2, add_input_dict, json_cmd) is None
+
+        step("Shutdown the interface on r1 to isolate it for r2")
+        shutdown_bringup_interface(tgen, "r1", "r1-eth0", False)
+
+        time.sleep(2)
+        step("Reset the client")
+        p.send_signal(signal.SIGINT)
+        time.sleep(2)
+        p.wait()
+        p = None
+
+        step("Kill ospfd on R1")
+        kill_router_daemons(tgen, "r1", ["ospfd"])
+        time.sleep(2)
+
+        step("Bring ospfd on R1 back up")
+        start_router_daemons(tgen, "r1", ["ospfd"])
+
+        p = r1.popen(
+            [
+                apibin,
+                "-v",
+                "add,10,1.2.3.4,231,1",
+                "add,10,1.2.3.4,231,1,feedaceecafebeef",
+                "wait, 5",
+            ]
+        )
+
+        step("Bring the interface on r1 back up for connection to r2")
+        shutdown_bringup_interface(tgen, "r1", "r1-eth0", True)
+
+        step("Verify area opaque LSA refresh")
+        json_cmd = "show ip ospf da opaque-area json"
+        add_detail_input_dict = {
+            "areaLocalOpaqueLsa": {
+                "areas": {
+                    "1.2.3.4": [
+                        {
+                            "linkStateId": "231.0.0.1",
+                            "advertisingRouter": "1.0.0.0",
+                            "lsaSeqNumber": "80000005",
+                            "checksum": "a87e",
+                            "length": 28,
+                            "opaqueDataLength": 8,
+                        },
+                    ],
+                },
+            },
+        }
+        assert verify_ospf_database(tgen, r1, add_detail_input_dict, json_cmd) is None
+        assert verify_ospf_database(tgen, r2, add_detail_input_dict, json_cmd) is None
+
+        step("Shutdown the interface on r1 to isolate it for r2")
+        shutdown_bringup_interface(tgen, "r1", "r1-eth0", False)
+
+        time.sleep(2)
+        step("Reset the client")
+        p.send_signal(signal.SIGINT)
+        time.sleep(2)
+        p.wait()
+        p = None
+
+        step("Kill ospfd on R1")
+        kill_router_daemons(tgen, "r1", ["ospfd"])
+        time.sleep(2)
+
+        step("Bring ospfd on R1 back up")
+        start_router_daemons(tgen, "r1", ["ospfd"])
+
+        step("Bring the interface on r1 back up for connection to r2")
+        shutdown_bringup_interface(tgen, "r1", "r1-eth0", True)
+
+        step("Verify area opaque LSA Purging")
+        json_cmd = "show ip ospf da opaque-area json"
+        add_detail_input_dict = {
+            "areaLocalOpaqueLsa": {
+                "areas": {
+                    "1.2.3.4": [
+                        {
+                            "lsaAge": 3600,
+                            "linkStateId": "231.0.0.1",
+                            "advertisingRouter": "1.0.0.0",
+                            "lsaSeqNumber": "80000005",
+                            "checksum": "a87e",
+                            "length": 28,
+                            "opaqueDataLength": 8,
+                        },
+                    ],
+                },
+            },
+        }
+        assert verify_ospf_database(tgen, r1, add_detail_input_dict, json_cmd) is None
+        assert verify_ospf_database(tgen, r2, add_detail_input_dict, json_cmd) is None
+        step("Verify Area Opaque LSA removal after timeout (60 seconds)")
+        time.sleep(60)
+        json_cmd = "show ip ospf da opaque-area json"
+        timeout_detail_input_dict = {
+            "areaLocalOpaqueLsa": {
+                "areas": {
+                    "1.2.3.4": [],
+                },
+            },
+        }
+        assert (
+            verify_ospf_database(tgen, r1, timeout_detail_input_dict, json_cmd) is None
+        )
+        assert (
+            verify_ospf_database(tgen, r2, timeout_detail_input_dict, json_cmd) is None
+        )
+
+    except Exception:
+        if p:
+            p.terminate()
+            if p.wait():
+                comm_error(p)
+            p = None
+        raise
+    finally:
+        if pread:
+            pread.terminate()
+            pread.wait()
+        if p:
+            p.terminate()
+            p.wait()
+
+
+@pytest.mark.parametrize("tgen", [2], indirect=True)
+def test_ospf_opaque_restart(tgen):
+    apibin = os.path.join(CLIENTDIR, "ospfclient.py")
+    rc, o, e = tgen.gears["r2"].net.cmd_status([apibin, "--help"])
+    logging.debug("%s --help: rc: %s stdout: '%s' stderr: '%s'", apibin, rc, o, e)
+    _test_opaque_add_restart_add(tgen, apibin)
+
+
 if __name__ == "__main__":
     args = ["-s"] + sys.argv[1:]
     sys.exit(pytest.main(args))