From 7f8f9aaee67b33433a0e0be37c7b4a1fb93d56c4 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sat, 18 Feb 2023 12:16:18 -0500 Subject: [PATCH 1/6] ospfd: inc. opaque data in `show opaque info detail json` output Signed-off-by: Christian Hopps (cherry picked from commit 1794afe01030b3111e83e26986cd004dce290049) --- ospfd/ospf_opaque.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c index ab647625fe..e053178a38 100644 --- a/ospfd/ospf_opaque.c +++ b/ospfd/ospf_opaque.c @@ -34,6 +34,7 @@ #include "thread.h" #include "hash.h" #include "sockunion.h" /* for inet_aton() */ +#include "printfrr.h" #include "ospfd/ospfd.h" #include "ospfd/ospf_interface.h" @@ -1162,11 +1163,13 @@ void ospf_opaque_config_write_debug(struct vty *vty) void show_opaque_info_detail(struct vty *vty, struct ospf_lsa *lsa, json_object *json) { + char buf[128], *bp; struct lsa_header *lsah = lsa->data; uint32_t lsid = ntohl(lsah->id.s_addr); uint8_t opaque_type = GET_OPAQUE_TYPE(lsid); uint32_t opaque_id = GET_OPAQUE_ID(lsid); struct ospf_opaque_functab *functab; + int len, lenValid; /* Switch output functionality by vty address. */ if (vty != NULL) { @@ -1185,11 +1188,19 @@ void show_opaque_info_detail(struct vty *vty, struct ospf_lsa *lsa, json, "opaqueType", ospf_opaque_type_name(opaque_type)); json_object_int_add(json, "opaqueId", opaque_id); - json_object_int_add(json, "opaqueDataLength", - ntohs(lsah->length) - - OSPF_LSA_HEADER_SIZE); + len = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; + json_object_int_add(json, "opaqueDataLength", len); + lenValid = VALID_OPAQUE_INFO_LEN(lsah); json_object_boolean_add(json, "opaqueDataLengthValid", - VALID_OPAQUE_INFO_LEN(lsah)); + lenValid); + if (lenValid) { + bp = asnprintfrr(MTYPE_TMP, buf, sizeof(buf), + "%*pHXn", (int)len, + (lsah + 1)); + json_object_string_add(json, "opaqueData", buf); + if (bp != buf) + XFREE(MTYPE_TMP, bp); + } } } else { zlog_debug(" Opaque-Type %u (%s)", opaque_type, From 5a51609d84bf2b67b4ee2516138dcf9576b3c2dd Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 6 Feb 2022 09:40:24 -0500 Subject: [PATCH 2/6] ospfd: small bugfix for miscounting A[S]BRs - improve the debug for the second add router call. Signed-off-by: Christian Hopps (cherry picked from commit 1eea62bb1c439ee66d502ef808695554370032e7) --- ospfd/ospf_route.c | 61 ++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/ospfd/ospf_route.c b/ospfd/ospf_route.c index 6360d8ec60..fea9f79bf7 100644 --- a/ospfd/ospf_route.c +++ b/ospfd/ospf_route.c @@ -363,45 +363,50 @@ void ospf_route_install(struct ospf *ospf, struct route_table *rt) /* RFC2328 16.1. (4). For "router". */ void ospf_intra_add_router(struct route_table *rt, struct vertex *v, - struct ospf_area *area, bool add_all) + struct ospf_area *area, bool add_only) { struct route_node *rn; struct ospf_route * or ; struct prefix_ipv4 p; struct router_lsa *lsa; - if (IS_DEBUG_OSPF_EVENT) - zlog_debug("%s: Start", __func__); - + if (IS_DEBUG_OSPF_EVENT) { + if (!add_only) + zlog_debug("%s: Start", __func__); + else + zlog_debug("%s: REACHRUN: Start", __func__); + } lsa = (struct router_lsa *)v->lsa; if (IS_DEBUG_OSPF_EVENT) zlog_debug("%s: LS ID: %pI4", __func__, &lsa->header.id); - if (!OSPF_IS_AREA_BACKBONE(area)) - ospf_vl_up_check(area, lsa->header.id, v); + if (!add_only) { + if (!OSPF_IS_AREA_BACKBONE(area)) + ospf_vl_up_check(area, lsa->header.id, v); - if (!CHECK_FLAG(lsa->flags, ROUTER_LSA_SHORTCUT)) - area->shortcut_capability = 0; + if (!CHECK_FLAG(lsa->flags, ROUTER_LSA_SHORTCUT)) + area->shortcut_capability = 0; - /* If the newly added vertex is an area border router or AS boundary - router, a routing table entry is added whose destination type is - "router". */ - if (!add_all && !IS_ROUTER_LSA_BORDER(lsa) && - !IS_ROUTER_LSA_EXTERNAL(lsa)) { - if (IS_DEBUG_OSPF_EVENT) - zlog_debug( - "%s: this router is neither ASBR nor ABR, skipping it", - __func__); - return; + /* If the newly added vertex is an area border router or AS + boundary router, a routing table entry is added whose + destination type is "router". */ + if (!IS_ROUTER_LSA_BORDER(lsa) && + !IS_ROUTER_LSA_EXTERNAL(lsa)) { + if (IS_DEBUG_OSPF_EVENT) + zlog_debug( + "%s: this router is neither ASBR nor ABR, skipping it", + __func__); + return; + } + + /* Update ABR and ASBR count in this area. */ + if (IS_ROUTER_LSA_BORDER(lsa)) + area->abr_count++; + if (IS_ROUTER_LSA_EXTERNAL(lsa)) + area->asbr_count++; } - /* Update ABR and ASBR count in this area. */ - if (IS_ROUTER_LSA_BORDER(lsa)) - area->abr_count++; - if (IS_ROUTER_LSA_EXTERNAL(lsa)) - area->asbr_count++; - /* The Options field found in the associated router-LSA is copied into the routing table entry's Optional capabilities field. Call the newly added vertex Router X. */ @@ -448,8 +453,12 @@ void ospf_intra_add_router(struct route_table *rt, struct vertex *v, listnode_add(rn->info, or); - if (IS_DEBUG_OSPF_EVENT) - zlog_debug("%s: Stop", __func__); + if (IS_DEBUG_OSPF_EVENT) { + if (!add_only) + zlog_debug("%s: Stop", __func__); + else + zlog_debug("%s: REACHRUN: Stop", __func__); + } } /* RFC2328 16.1. (4). For transit network. */ From 2b2061e2741f55a2c918cdc3ecf385c62a97c1c0 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 6 Feb 2022 15:01:28 -0500 Subject: [PATCH 3/6] ospfd: compare prefix values in host order Signed-off-by: Christian Hopps (cherry picked from commit 156a904cae87a1ea1cd51de5012c9bddc0d63747) --- ospfd/ospf_apiserver.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c index 6fd1c82c24..ee29ed8aae 100644 --- a/ospfd/ospf_apiserver.c +++ b/ospfd/ospf_apiserver.c @@ -2593,9 +2593,12 @@ static inline int cmp_route_nodes(struct route_node *orn, return 1; else if (!nrn) return -1; - else if (orn->p.u.prefix4.s_addr < nrn->p.u.prefix4.s_addr) + + uint32_t opn = ntohl(orn->p.u.prefix4.s_addr); + uint32_t npn = ntohl(nrn->p.u.prefix4.s_addr); + if (opn < npn) return -1; - else if (orn->p.u.prefix4.s_addr > nrn->p.u.prefix4.s_addr) + else if (opn > npn) return 1; else return 0; From 8d81aeaf19a102957579ce4addd70ecd89a3d4b0 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 19 Feb 2023 18:55:58 -0500 Subject: [PATCH 4/6] ospfclient: bugfix and no implicit register - dont delete the callback on opaque data delete - require explicit registration Signed-off-by: Christian Hopps (cherry picked from commit 703d2c0a3e30c18acd2426b50f97ea8c91c479f9) --- ospfclient/ospfclient.py | 63 ++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/ospfclient/ospfclient.py b/ospfclient/ospfclient.py index 8e3c68445f..793ab222ed 100755 --- a/ospfclient/ospfclient.py +++ b/ospfclient/ospfclient.py @@ -617,14 +617,6 @@ class OspfOpaqueClient(OspfApiClient): mp = struct.pack(msg_fmt[mt], lsa_type, otype) await self.msg_send_raises(mt, mp) - async def _assure_opaque_ready(self, lsa_type, otype): - async with self.ready_lock: - if self.ready_cond[lsa_type].get(otype) is True: - return - - await self._register_opaque_data(lsa_type, otype) - await self.wait_opaque_ready(lsa_type, otype) - async def _handle_msg_loop(self): try: logging.debug("entering async msg handling loop") @@ -825,6 +817,7 @@ class OspfOpaqueClient(OspfApiClient): Raises: See `msg_send_raises` """ + assert self.ready_cond.get(lsa_type, {}).get(otype) is True, "Not Registered!" if lsa_type == LSA_TYPE_OPAQUE_LINK: ifaddr, aid = int(addr), 0 @@ -842,7 +835,6 @@ class OspfOpaqueClient(OspfApiClient): *OspfOpaqueClient._opaque_args(lsa_type, otype, oid, data), ) msg += data - await self._assure_opaque_ready(lsa_type, otype) await self.msg_send_raises(mt, msg) async def delete_opaque_data(self, addr, lsa_type, otype, oid, flags=0): @@ -854,21 +846,31 @@ class OspfOpaqueClient(OspfApiClient): Args: addr: depends on lsa_type, LINK => ifaddr, AREA => area ID, AS => ignored lsa_type: LSA_TYPE_OPAQUE_{LINK,AREA,AS} - otype: (octet) opaque type. Note: the type will be registered if the user - has not explicity done that yet with `register_opaque_data`. + otype: (octet) opaque type. oid: (3 octets) ID of this opaque data flags: (octet) optional flags (e.g., OSPF_API_DEL_ZERO_LEN_LSA, defaults to no flags) Raises: See `msg_send_raises` """ - if (lsa_type, otype) in self.opaque_change_cb: - del self.opaque_change_cb[(lsa_type, otype)] + assert self.ready_cond.get(lsa_type, {}).get(otype) is True, "Not Registered!" mt = MSG_DELETE_REQUEST - await self._assure_opaque_ready(lsa_type, otype) mp = struct.pack(msg_fmt[mt], int(addr), lsa_type, otype, flags, oid) await self.msg_send_raises(mt, mp) + async def is_registered(self, lsa_type, otype): + """Determine if an (lsa_type, otype) tuple has been registered with FRR + + This determines if the type has been registered, but not necessarily if it is + ready, if that is required use the `wait_opaque_ready` metheod. + + Args: + lsa_type: LSA_TYPE_OPAQUE_{LINK,AREA,AS} + otype: (octet) opaque type. + """ + async with self.ready_lock: + return self.ready_cond.get(lsa_type, {}).get(otype) is not None + async def register_opaque_data(self, lsa_type, otype, callback=None): """Register intent to advertise opaque data. @@ -878,8 +880,7 @@ class OspfOpaqueClient(OspfApiClient): Args: lsa_type: LSA_TYPE_OPAQUE_{LINK,AREA,AS} - otype: (octet) opaque type. Note: the type will be registered if the user - has not explicity done that yet with `register_opaque_data`. + otype: (octet) opaque type. callback: if given, callback will be called when changes are received for LSA of the given (lsa_type, otype). The callbacks signature is: @@ -895,6 +896,10 @@ class OspfOpaqueClient(OspfApiClient): Raises: See `msg_send_raises` """ + assert not await self.is_registered( + lsa_type, otype + ), "Registering registered type" + if callback: self.opaque_change_cb[(lsa_type, otype)] = callback elif (lsa_type, otype) in self.opaque_change_cb: @@ -933,8 +938,7 @@ class OspfOpaqueClient(OspfApiClient): Args: lsa_type: LSA_TYPE_OPAQUE_{LINK,AREA,AS} - otype: (octet) opaque type. Note: the type will be registered if the user - has not explicity done that yet with `register_opaque_data`. + otype: (octet) opaque type. callback: if given, callback will be called when changes are received for LSA of the given (lsa_type, otype). The callbacks signature is: @@ -951,17 +955,8 @@ class OspfOpaqueClient(OspfApiClient): See `msg_send_raises` """ - if callback: - self.opaque_change_cb[(lsa_type, otype)] = callback - elif (lsa_type, otype) in self.opaque_change_cb: - logging.warning( - "OSPFCLIENT: register: removing callback for %s opaque-type %s", - lsa_typename(lsa_type), - otype, - ) - del self.opaque_change_cb[(lsa_type, otype)] - - return await self._assure_opaque_ready(lsa_type, otype) + await self.register_opaque_data(lsa_type, otype, callback) + await self.wait_opaque_ready(lsa_type, otype) async def unregister_opaque_data(self, lsa_type, otype): """Unregister intent to advertise opaque data. @@ -971,11 +966,13 @@ class OspfOpaqueClient(OspfApiClient): Args: lsa_type: LSA_TYPE_OPAQUE_{LINK,AREA,AS} - otype: (octet) opaque type. Note: the type will be registered if the user - has not explicity done that yet with `register_opaque_data`. + otype: (octet) opaque type. Raises: See `msg_send_raises` """ + assert await self.is_registered( + lsa_type, otype + ), "Unregistering unregistered type" if (lsa_type, otype) in self.opaque_change_cb: del self.opaque_change_cb[(lsa_type, otype)] @@ -1119,6 +1116,10 @@ async def async_main(args): except ValueError: addr = ip(aval) oargs = [addr, ltype, int(_s.pop(False)), int(_s.pop(False))] + + if not await c.is_registered(oargs[1], oargs[2]): + await c.register_opaque_data_wait(oargs[1], oargs[2]) + if what.casefold() == "add": try: b = bytes.fromhex(_s.pop(False)) From 49d5d11c3e2ed91c7b4af3bff7fa505faceb001e Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 20 Feb 2022 03:59:41 -0500 Subject: [PATCH 5/6] ospfclient: remove register "READY" requirement - also add ability of the apibin to process commands on stdin Signed-off-by: Christian Hopps (cherry picked from commit 6efa8fd5c1653372cea1b25a9fa764269960bb91) --- ospfclient/ospfclient.py | 136 +++++++++++++++++++++++++-------------- 1 file changed, 87 insertions(+), 49 deletions(-) diff --git a/ospfclient/ospfclient.py b/ospfclient/ospfclient.py index 793ab222ed..f93d633e18 100755 --- a/ospfclient/ospfclient.py +++ b/ospfclient/ospfclient.py @@ -255,6 +255,16 @@ def nsm_name(state): return names.get(state, str(state)) +class WithNothing: + "An object that does nothing when used with `with` statement." + + async def __aenter__(self): + return + + async def __aexit__(self, *args, **kwargs): + return + + # -------------- # Client Classes # -------------- @@ -560,15 +570,17 @@ class OspfOpaqueClient(OspfApiClient): Args: server: hostname or IP address of server default is "localhost" + wait_ready: if True then wait for OSPF to signal ready, in newer versions + FRR ospfd is always ready so this overhead can be skipped. + default is False. Raises: Will raise exceptions for failures with various `socket` modules functions such as `socket.socket`, `socket.setsockopt`, `socket.bind`. """ - def __init__(self, server="localhost"): + def __init__(self, server="localhost", wait_ready=False): handlers = { - MSG_READY_NOTIFY: self._ready_msg, MSG_LSA_UPDATE_NOTIFY: self._lsa_change_msg, MSG_LSA_DELETE_NOTIFY: self._lsa_change_msg, MSG_NEW_IF: self._if_msg, @@ -578,9 +590,13 @@ class OspfOpaqueClient(OspfApiClient): MSG_REACHABLE_CHANGE: self._reachable_msg, MSG_ROUTER_ID_CHANGE: self._router_id_msg, } + if wait_ready: + handlers[MSG_READY_NOTIFY] = self._ready_msg + super().__init__(server, handlers) - self.ready_lock = Lock() + self.wait_ready = wait_ready + self.ready_lock = Lock() if wait_ready else WithNothing() self.ready_cond = { LSA_TYPE_OPAQUE_LINK: {}, LSA_TYPE_OPAQUE_AREA: {}, @@ -617,6 +633,10 @@ class OspfOpaqueClient(OspfApiClient): mp = struct.pack(msg_fmt[mt], lsa_type, otype) await self.msg_send_raises(mt, mp) + # If we are not waiting, mark ready for register check + if not self.wait_ready: + self.ready_cond[lsa_type][otype] = True + async def _handle_msg_loop(self): try: logging.debug("entering async msg handling loop") @@ -648,6 +668,8 @@ class OspfOpaqueClient(OspfApiClient): return lsa async def _ready_msg(self, mt, msg, extra, lsa_type, otype, addr): + assert self.wait_ready + if lsa_type == LSA_TYPE_OPAQUE_LINK: e = "ifaddr {}".format(ip(addr)) elif lsa_type == LSA_TYPE_OPAQUE_AREA: @@ -918,6 +940,8 @@ class OspfOpaqueClient(OspfApiClient): if cond is True: return + assert self.wait_ready + logging.debug( "waiting for ready %s opaque-type %s", lsa_typename(lsa_type), otype ) @@ -1078,6 +1102,17 @@ class OspfOpaqueClient(OspfApiClient): # ================ # CLI/Script Usage # ================ +def next_action(action_list=None): + "Get next action from list or STDIN" + if action_list: + for action in action_list: + yield action + else: + while True: + action = input("") + if not action: + break + yield action.strip() async def async_main(args): @@ -1096,54 +1131,53 @@ async def async_main(args): await c.req_ism_states() await c.req_nsm_states() - if args.actions: - for action in args.actions: - _s = action.split(",") - what = _s.pop(False) - if what.casefold() == "wait": - stime = int(_s.pop(False)) - logging.info("waiting %s seconds", stime) - await asyncio.sleep(stime) - logging.info("wait complete: %s seconds", stime) - continue - ltype = int(_s.pop(False)) - if ltype == 11: - addr = ip(0) - else: - aval = _s.pop(False) - try: - addr = ip(int(aval)) - except ValueError: - addr = ip(aval) - oargs = [addr, ltype, int(_s.pop(False)), int(_s.pop(False))] + for action in next_action(args.actions): + _s = action.split(",") + what = _s.pop(False) + if what.casefold() == "wait": + stime = int(_s.pop(False)) + logging.info("waiting %s seconds", stime) + await asyncio.sleep(stime) + logging.info("wait complete: %s seconds", stime) + continue + ltype = int(_s.pop(False)) + if ltype == 11: + addr = ip(0) + else: + aval = _s.pop(False) + try: + addr = ip(int(aval)) + except ValueError: + addr = ip(aval) + oargs = [addr, ltype, int(_s.pop(False)), int(_s.pop(False))] - if not await c.is_registered(oargs[1], oargs[2]): - await c.register_opaque_data_wait(oargs[1], oargs[2]) + if not await c.is_registered(oargs[1], oargs[2]): + await c.register_opaque_data_wait(oargs[1], oargs[2]) - if what.casefold() == "add": + if what.casefold() == "add": + try: + b = bytes.fromhex(_s.pop(False)) + except IndexError: + b = b"" + logging.info("opaque data is %s octets", len(b)) + # Needs to be multiple of 4 in length + mod = len(b) % 4 + if mod: + b += b"\x00" * (4 - mod) + logging.info("opaque padding to %s octets", len(b)) + + await c.add_opaque_data(*oargs, b) + else: + assert what.casefold().startswith("del") + f = 0 + if len(_s) >= 1: try: - b = bytes.fromhex(_s.pop(False)) + f = int(_s.pop(False)) except IndexError: - b = b"" - logging.info("opaque data is %s octets", len(b)) - # Needs to be multiple of 4 in length - mod = len(b) % 4 - if mod: - b += b"\x00" * (4 - mod) - logging.info("opaque padding to %s octets", len(b)) - - await c.add_opaque_data(*oargs, b) - else: - assert what.casefold().startswith("del") - f = 0 - if len(_s) >= 1: - try: - f = int(_s.pop(False)) - except IndexError: - f = 0 - await c.delete_opaque_data(*oargs, f) - if args.exit: - return 0 + f = 0 + await c.delete_opaque_data(*oargs, f) + if not args.actions or args.exit: + return 0 except Exception as error: logging.error("async_main: unexpected error: %s", error, exc_info=True) return 2 @@ -1159,19 +1193,23 @@ async def async_main(args): def main(*args): ap = argparse.ArgumentParser(args) + ap.add_argument("--logtag", default="CLIENT", help="tag to identify log messages") ap.add_argument("--exit", action="store_true", help="Exit after commands") ap.add_argument("--server", default="localhost", help="OSPF API server") ap.add_argument("-v", "--verbose", action="store_true", help="be verbose") ap.add_argument( "actions", nargs="*", - help="(ADD|DEL),LSATYPE,[ADDR,],OTYPE,OID,[HEXDATA|DEL_FLAG]", + help="WAIT,SEC|(ADD|DEL),LSATYPE,[ADDR,],OTYPE,OID,[HEXDATA|DEL_FLAG]", ) args = ap.parse_args() level = logging.DEBUG if args.verbose else logging.INFO logging.basicConfig( - level=level, format="%(asctime)s %(levelname)s: CLIENT: %(name)s %(message)s" + level=level, + format="%(asctime)s %(levelname)s: {}: %(name)s %(message)s".format( + args.logtag + ), ) logging.info("ospfclient: starting") From 1869d3ab6e9ac9933d8ef727ec4bd1481c01d34c Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 6 Feb 2022 09:41:12 -0500 Subject: [PATCH 6/6] tests: improve the ospfapi test (move to square topology) Signed-off-by: Christian Hopps (cherry picked from commit 663a0c96d9c52895189bef7579a308a1b14120fa) --- tests/topotests/ospfapi/r1/ospfd.conf | 7 +- tests/topotests/ospfapi/r1/zebra.conf | 2 + tests/topotests/ospfapi/r2/ospfd.conf | 2 +- tests/topotests/ospfapi/r3/ospfd.conf | 2 +- tests/topotests/ospfapi/r4/ospfd.conf | 7 +- tests/topotests/ospfapi/r4/zebra.conf | 2 + .../topotests/ospfapi/test_ospf_clientapi.py | 212 ++++++++++-------- 7 files changed, 131 insertions(+), 103 deletions(-) diff --git a/tests/topotests/ospfapi/r1/ospfd.conf b/tests/topotests/ospfapi/r1/ospfd.conf index 8d13556847..338691bbf3 100644 --- a/tests/topotests/ospfapi/r1/ospfd.conf +++ b/tests/topotests/ospfapi/r1/ospfd.conf @@ -4,7 +4,12 @@ interface r1-eth0 ip ospf dead-interval 10 ip ospf area 1.2.3.4 ! +interface r1-eth1 + ip ospf hello-interval 2 + ip ospf dead-interval 10 + ip ospf area 1.2.3.4 +! router ospf - ospf router-id 192.168.0.1 + ospf router-id 1.0.0.0 capability opaque ! diff --git a/tests/topotests/ospfapi/r1/zebra.conf b/tests/topotests/ospfapi/r1/zebra.conf index aae87408c3..745159141d 100644 --- a/tests/topotests/ospfapi/r1/zebra.conf +++ b/tests/topotests/ospfapi/r1/zebra.conf @@ -2,3 +2,5 @@ interface r1-eth0 ip address 10.0.1.1/24 ! +interface r1-eth1 + ip address 10.0.4.1/24 diff --git a/tests/topotests/ospfapi/r2/ospfd.conf b/tests/topotests/ospfapi/r2/ospfd.conf index be0712742b..1bf2a0c7f2 100644 --- a/tests/topotests/ospfapi/r2/ospfd.conf +++ b/tests/topotests/ospfapi/r2/ospfd.conf @@ -10,6 +10,6 @@ interface r2-eth1 ip ospf area 1.2.3.4 ! router ospf - ospf router-id 192.168.0.2 + ospf router-id 2.0.0.0 capability opaque ! diff --git a/tests/topotests/ospfapi/r3/ospfd.conf b/tests/topotests/ospfapi/r3/ospfd.conf index 77cd86a975..ecf2042b42 100644 --- a/tests/topotests/ospfapi/r3/ospfd.conf +++ b/tests/topotests/ospfapi/r3/ospfd.conf @@ -10,6 +10,6 @@ interface r3-eth1 ip ospf area 1.2.3.4 ! router ospf - ospf router-id 192.168.0.3 + ospf router-id 3.0.0.0 capability opaque ! diff --git a/tests/topotests/ospfapi/r4/ospfd.conf b/tests/topotests/ospfapi/r4/ospfd.conf index 32e55d546b..4de8caeb17 100644 --- a/tests/topotests/ospfapi/r4/ospfd.conf +++ b/tests/topotests/ospfapi/r4/ospfd.conf @@ -4,7 +4,12 @@ interface r4-eth0 ip ospf dead-interval 10 ip ospf area 1.2.3.4 ! +interface r4-eth1 + ip ospf hello-interval 2 + ip ospf dead-interval 10 + ip ospf area 1.2.3.4 +! router ospf - ospf router-id 192.168.0.4 + ospf router-id 4.0.0.0 capability opaque ! diff --git a/tests/topotests/ospfapi/r4/zebra.conf b/tests/topotests/ospfapi/r4/zebra.conf index 702219720d..233ffa5c7b 100644 --- a/tests/topotests/ospfapi/r4/zebra.conf +++ b/tests/topotests/ospfapi/r4/zebra.conf @@ -2,3 +2,5 @@ interface r4-eth0 ip address 10.0.3.4/24 ! +interface r4-eth1 + ip address 10.0.4.4/24 \ No newline at end of file diff --git a/tests/topotests/ospfapi/test_ospf_clientapi.py b/tests/topotests/ospfapi/test_ospf_clientapi.py index 94c8c5c732..85c30b473e 100644 --- a/tests/topotests/ospfapi/test_ospf_clientapi.py +++ b/tests/topotests/ospfapi/test_ospf_clientapi.py @@ -34,7 +34,7 @@ from datetime import datetime, timedelta import pytest from lib.common_config import retry, run_frr_cmd, step -from lib.micronet import comm_error +from lib.micronet import Timeout, comm_error from lib.topogen import Topogen, TopoRouter from lib.topotest import interface_set_status, json_cmp @@ -56,15 +56,20 @@ assert os.path.exists( # Test Setup # ---------- +# +# r1 - r2 +# | | +# r4 - r3 +# + @pytest.fixture(scope="function", name="tgen") def _tgen(request): "Setup/Teardown the environment and provide tgen argument to tests" nrouters = request.param - if nrouters == 1: - topodef = {"sw1:": ("r1",)} - else: - topodef = {f"sw{i}": (f"r{i}", f"r{i+1}") for i in range(1, nrouters)} + topodef = {f"sw{i}": (f"r{i}", f"r{i+1}") for i in range(1, nrouters)} + if nrouters == 4: + topodef["sw4"] = ("r4", "r1") tgen = Topogen(topodef, request.module.__name__) tgen.start_topology() @@ -107,23 +112,23 @@ def verify_ospf_database(tgen, dut, input_dict, cmd="show ip ospf database json" def myreadline(f): buf = b"" while True: - # logging.info("READING 1 CHAR") + # logging.debug("READING 1 CHAR") c = f.read(1) if not c: return buf if buf else None buf += c - # logging.info("READ CHAR: '%s'", c) + # logging.debug("READ CHAR: '%s'", c) if c == b"\n": return buf -def _wait_output(p, regex, timeout=120): - retry_until = datetime.now() + timedelta(seconds=timeout) - while datetime.now() < retry_until: +def _wait_output(p, regex, maxwait=120): + timeout = Timeout(maxwait) + while not timeout.is_expired(): # line = p.stdout.readline() line = myreadline(p.stdout) if not line: - assert None, "Timeout waiting for '{}'".format(regex) + assert None, "EOF waiting for '{}'".format(regex) line = line.decode("utf-8") line = line.rstrip() if line: @@ -131,7 +136,9 @@ def _wait_output(p, regex, timeout=120): m = re.search(regex, line) if m: return m - assert None, "Failed to get output withint {}s".format(timeout) + assert None, "Failed to get output matching '{}' withint {} actual {}s".format( + regex, maxwait, timeout.elapsed() + ) # ----- @@ -141,12 +148,13 @@ def _wait_output(p, regex, timeout=120): def _test_reachability(tgen, testbin): waitlist = [ - "192.168.0.1,192.168.0.2,192.168.0.4", - "192.168.0.2,192.168.0.4", - "192.168.0.1,192.168.0.2,192.168.0.4", + "1.0.0.0,2.0.0.0,4.0.0.0", + "2.0.0.0,4.0.0.0", + "1.0.0.0,2.0.0.0,4.0.0.0", ] r2 = tgen.gears["r2"] r3 = tgen.gears["r3"] + r4 = tgen.gears["r4"] wait_args = [f"--wait={x}" for x in waitlist] @@ -164,10 +172,12 @@ def _test_reachability(tgen, testbin): step("reachable: check for modified reachability") interface_set_status(r2, "r2-eth0", False) + interface_set_status(r4, "r4-eth1", False) _wait_output(p, "SUCCESS: {}".format(waitlist[1])) step("reachable: check for restored reachability") interface_set_status(r2, "r2-eth0", True) + interface_set_status(r4, "r4-eth1", True) _wait_output(p, "SUCCESS: {}".format(waitlist[2])) except Exception as error: logging.error("ERROR: %s", error) @@ -182,16 +192,16 @@ def _test_reachability(tgen, testbin): def test_ospf_reachability(tgen): testbin = os.path.join(TESTDIR, "ctester.py") rc, o, e = tgen.gears["r2"].net.cmd_status([testbin, "--help"]) - logging.info("%s --help: rc: %s stdout: '%s' stderr: '%s'", testbin, rc, o, e) + logging.debug("%s --help: rc: %s stdout: '%s' stderr: '%s'", testbin, rc, o, e) _test_reachability(tgen, testbin) def _test_router_id(tgen, testbin): r1 = tgen.gears["r1"] waitlist = [ - "192.168.0.1", + "1.0.0.0", "1.1.1.1", - "192.168.0.1", + "1.0.0.0", ] mon_args = [f"--monitor={x}" for x in waitlist] @@ -213,7 +223,7 @@ def _test_router_id(tgen, testbin): _wait_output(p, "SUCCESS: {}".format(waitlist[1])) step("router id: check for restored router id") - r1.vtysh_multicmd("conf t\nrouter ospf\nospf router-id 192.168.0.1") + r1.vtysh_multicmd("conf t\nrouter ospf\nospf router-id 1.0.0.0") _wait_output(p, "SUCCESS: {}".format(waitlist[2])) except Exception as error: logging.error("ERROR: %s", error) @@ -228,7 +238,7 @@ def _test_router_id(tgen, testbin): def test_ospf_router_id(tgen): testbin = os.path.join(TESTDIR, "ctester.py") rc, o, e = tgen.gears["r1"].net.cmd_status([testbin, "--help"]) - logging.info("%s --help: rc: %s stdout: '%s' stderr: '%s'", testbin, rc, o, e) + logging.debug("%s --help: rc: %s stdout: '%s' stderr: '%s'", testbin, rc, o, e) _test_router_id(tgen, testbin) @@ -243,13 +253,13 @@ def _test_add_data(tgen, apibin): try: p = r1.popen([apibin, "-v", "add,9,10.0.1.1,230,2,00000202"]) input_dict = { - "routerId": "192.168.0.1", + "routerId": "1.0.0.0", "areas": { "1.2.3.4": { "linkLocalOpaqueLsa": [ { "lsId": "230.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", } ], @@ -265,7 +275,7 @@ def _test_add_data(tgen, apibin): "1.2.3.4": [ { "linkStateId": "230.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", "opaqueData": "00000202", }, @@ -285,13 +295,13 @@ def _test_add_data(tgen, apibin): p = None p = r1.popen([apibin, "-v", "add,10,1.2.3.4,231,1,00010101"]) input_dict = { - "routerId": "192.168.0.1", + "routerId": "1.0.0.0", "areas": { "1.2.3.4": { "linkLocalOpaqueLsa": [ { "lsId": "230.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", "lsaAge": 3600, } @@ -299,7 +309,7 @@ def _test_add_data(tgen, apibin): "areaLocalOpaqueLsa": [ { "lsId": "231.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", }, ], @@ -315,7 +325,7 @@ def _test_add_data(tgen, apibin): "1.2.3.4": [ { "linkStateId": "231.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", "opaqueData": "00010101", }, @@ -336,13 +346,13 @@ def _test_add_data(tgen, apibin): p = r1.popen([apibin, "-v", "add,11,232,3,deadbeaf01234567"]) input_dict = { - "routerId": "192.168.0.1", + "routerId": "1.0.0.0", "areas": { "1.2.3.4": { "areaLocalOpaqueLsa": [ { "lsId": "231.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", "lsaAge": 3600, }, @@ -352,7 +362,7 @@ def _test_add_data(tgen, apibin): "asExternalOpaqueLsa": [ { "lsId": "232.0.0.3", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", }, ], @@ -364,7 +374,7 @@ def _test_add_data(tgen, apibin): "asExternalOpaqueLsa": [ { "linkStateId": "232.0.0.3", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", "opaqueData": "deadbeaf01234567", }, @@ -382,11 +392,11 @@ def _test_add_data(tgen, apibin): p = None input_dict = { - "routerId": "192.168.0.1", + "routerId": "1.0.0.0", "asExternalOpaqueLsa": [ { "lsId": "232.0.0.3", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", "lsaAge": 3600, }, @@ -400,11 +410,11 @@ def _test_add_data(tgen, apibin): # Originate it again p = r1.popen([apibin, "-v", "add,11,232,3,ebadf00d"]) input_dict = { - "routerId": "192.168.0.1", + "routerId": "1.0.0.0", "asExternalOpaqueLsa": [ { "lsId": "232.0.0.3", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000002", }, ], @@ -415,7 +425,7 @@ def _test_add_data(tgen, apibin): "asExternalOpaqueLsa": [ { "linkStateId": "232.0.0.3", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000002", "opaqueData": "ebadf00d", }, @@ -425,6 +435,7 @@ def _test_add_data(tgen, apibin): json_cmd = "show ip ospf da opaque-as json" assert verify_ospf_database(tgen, r1, input_dict, json_cmd) is None + logging.debug("sending interrupt to writer api client") p.send_signal(signal.SIGINT) time.sleep(2) p.wait() @@ -439,6 +450,7 @@ def _test_add_data(tgen, apibin): raise finally: if p: + logging.debug("cleanup: sending interrupt to writer api client") p.terminate() p.wait() @@ -447,7 +459,7 @@ def _test_add_data(tgen, apibin): def test_ospf_opaque_add_data3(tgen): apibin = os.path.join(CLIENTDIR, "ospfclient.py") rc, o, e = tgen.gears["r2"].net.cmd_status([apibin, "--help"]) - logging.info("%s --help: rc: %s stdout: '%s' stderr: '%s'", apibin, rc, o, e) + logging.debug("%s --help: rc: %s stdout: '%s' stderr: '%s'", apibin, rc, o, e) _test_add_data(tgen, apibin) @@ -459,10 +471,12 @@ def _test_opaque_add_del(tgen, apibin): 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"], + ["/usr/bin/timeout", "120", apibin, "-v", "--logtag=READER", "wait,120"], encoding=None, # don't buffer stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, @@ -492,30 +506,30 @@ def _test_opaque_add_del(tgen, apibin): "linkLocalOpaqueLsa": [ { "lsId": "230.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "6d5f", + "checksum": "76bf", }, { "lsId": "230.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "8142", + "checksum": "8aa2", }, ], "linkLocalOpaqueLsaCount": 2, "areaLocalOpaqueLsa": [ { "lsId": "231.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "5278", + "checksum": "5bd8", }, { "lsId": "231.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "6d30", + "checksum": "7690", }, ], "areaLocalOpaqueLsaCount": 2, @@ -524,15 +538,15 @@ def _test_opaque_add_del(tgen, apibin): "asExternalOpaqueLsa": [ { "lsId": "232.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "5575", + "checksum": "5ed5", }, { "lsId": "232.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "d05d", + "checksum": "d9bd", }, ], "asExternalOpaqueLsaCount": 2, @@ -556,17 +570,17 @@ def _test_opaque_add_del(tgen, apibin): "1.2.3.4": [ { "linkStateId": "230.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "6d5f", + "checksum": "76bf", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "230.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "8142", + "checksum": "8aa2", "length": 24, "opaqueId": 2, "opaqueDataLength": 4, @@ -581,17 +595,17 @@ def _test_opaque_add_del(tgen, apibin): "1.2.3.4": [ { "linkStateId": "231.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "5278", + "checksum": "5bd8", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "231.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "6d30", + "checksum": "7690", "length": 28, "opaqueDataLength": 8, }, @@ -603,17 +617,17 @@ def _test_opaque_add_del(tgen, apibin): "asExternalOpaqueLsa": [ { "linkStateId": "232.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "5575", + "checksum": "5ed5", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "232.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "d05d", + "checksum": "d9bd", "length": 24, "opaqueDataLength": 4, }, @@ -655,32 +669,32 @@ def _test_opaque_add_del(tgen, apibin): "linkLocalOpaqueLsa": [ { "lsId": "230.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "6d5f", + "checksum": "76bf", }, { "lsId": "230.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "lsaAge": 3600, "sequenceNumber": "80000001", - "checksum": "8142", + "checksum": "8aa2", }, ], "linkLocalOpaqueLsaCount": 2, "areaLocalOpaqueLsa": [ { "lsId": "231.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "5278", + "checksum": "5bd8", }, { "lsId": "231.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "lsaAge": 3600, "sequenceNumber": "80000002", - "checksum": "4682", + "checksum": "4fe2", }, ], "areaLocalOpaqueLsaCount": 2, @@ -689,16 +703,16 @@ def _test_opaque_add_del(tgen, apibin): "asExternalOpaqueLsa": [ { "lsId": "232.0.0.1", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "lsaAge": 3600, "sequenceNumber": "80000001", - "checksum": "5575", + "checksum": "5ed5", }, { "lsId": "232.0.0.2", - "advertisedRouter": "192.168.0.1", + "advertisedRouter": "1.0.0.0", "sequenceNumber": "80000001", - "checksum": "d05d", + "checksum": "d9bd", }, ], "asExternalOpaqueLsaCount": 2, @@ -716,18 +730,18 @@ def _test_opaque_add_del(tgen, apibin): "1.2.3.4": [ { "linkStateId": "230.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "6d5f", + "checksum": "76bf", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "230.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaAge": 3600, "lsaSeqNumber": "80000001", - "checksum": "8142", + "checksum": "8aa2", "length": 24, "opaqueId": 2, "opaqueDataLength": 4, @@ -742,18 +756,18 @@ def _test_opaque_add_del(tgen, apibin): "1.2.3.4": [ { "linkStateId": "231.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "5278", + "checksum": "5bd8", "length": 20, "opaqueDataLength": 0, }, { "lsaAge": 3600, "linkStateId": "231.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000002", - "checksum": "4682", + "checksum": "4fe2", # data removed "length": 20, "opaqueDataLength": 0, @@ -766,18 +780,18 @@ def _test_opaque_add_del(tgen, apibin): "asExternalOpaqueLsa": [ { "linkStateId": "232.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaAge": 3600, "lsaSeqNumber": "80000001", - "checksum": "5575", + "checksum": "5ed5", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "232.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "d05d", + "checksum": "d9bd", "length": 24, "opaqueDataLength": 4, }, @@ -808,19 +822,19 @@ def _test_opaque_add_del(tgen, apibin): "1.2.3.4": [ { "linkStateId": "230.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaAge": 3600, "lsaSeqNumber": "80000001", - "checksum": "6d5f", + "checksum": "76bf", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "230.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaAge": 3600, "lsaSeqNumber": "80000001", - "checksum": "8142", + "checksum": "8aa2", "length": 24, "opaqueId": 2, "opaqueDataLength": 4, @@ -836,18 +850,18 @@ def _test_opaque_add_del(tgen, apibin): { "lsaAge": 3600, "linkStateId": "231.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000001", - "checksum": "5278", + "checksum": "5bd8", "length": 20, "opaqueDataLength": 0, }, { "lsaAge": 3600, "linkStateId": "231.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaSeqNumber": "80000002", - "checksum": "4682", + "checksum": "4fe2", # data removed "length": 20, "opaqueDataLength": 0, @@ -860,19 +874,19 @@ def _test_opaque_add_del(tgen, apibin): "asExternalOpaqueLsa": [ { "linkStateId": "232.0.0.1", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaAge": 3600, "lsaSeqNumber": "80000001", - "checksum": "5575", + "checksum": "5ed5", "length": 20, "opaqueDataLength": 0, }, { "linkStateId": "232.0.0.2", - "advertisingRouter": "192.168.0.1", + "advertisingRouter": "1.0.0.0", "lsaAge": 3600, "lsaSeqNumber": "80000001", - "checksum": "d05d", + "checksum": "d9bd", "length": 24, "opaqueDataLength": 4, }, @@ -931,7 +945,7 @@ def _test_opaque_add_del(tgen, apibin): def test_ospf_opaque_delete_data3(tgen): apibin = os.path.join(CLIENTDIR, "ospfclient.py") rc, o, e = tgen.gears["r2"].net.cmd_status([apibin, "--help"]) - logging.info("%s --help: rc: %s stdout: '%s' stderr: '%s'", apibin, rc, o, e) + logging.debug("%s --help: rc: %s stdout: '%s' stderr: '%s'", apibin, rc, o, e) _test_opaque_add_del(tgen, apibin)