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>

4 files changed, 227 insertions, 12 deletions

diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c
index 1aacc341a..34e59c545 100644
--- a/ospfd/ospf_apiserver.c
+++ b/ospfd/ospf_apiserver.c
@@ -1764,6 +1764,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__);
@@ -1772,6 +1778,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) {
@@ -1846,6 +1857,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__,
@@ -1855,14 +1871,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 c2b40af1c..6894c6a00 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -2120,14 +2120,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;
@@ -2139,7 +2146,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 552acfd6d..fe94c34e4 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2031,7 +2031,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 b01c96226..2e8bea265 100644
--- a/tests/topotests/ospfapi/test_ospf_clientapi.py
+++ b/tests/topotests/ospfapi/test_ospf_clientapi.py
@@ -20,7 +20,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
@@ -936,6 +944,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))