From 74f3656d12b5a99160aeabea2784514bf22548fa Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 28 Nov 2022 08:52:48 -0500 Subject: [PATCH 1/5] bgpd: Null checking is not needed on failure Memory allocations that fail crash the program. Checking for NULL is not going to do anything. Signed-off-by: Donald Sharp --- bgpd/bgp_mpath.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c index 64af8a5411..52f6b64c2c 100644 --- a/bgpd/bgp_mpath.c +++ b/bgpd/bgp_mpath.c @@ -278,8 +278,10 @@ void bgp_mp_list_add(struct list *mp_list, struct bgp_path_info *mpinfo) static struct bgp_path_info_mpath *bgp_path_info_mpath_new(void) { struct bgp_path_info_mpath *new_mpath; + new_mpath = XCALLOC(MTYPE_BGP_MPATH_INFO, sizeof(struct bgp_path_info_mpath)); + return new_mpath; } @@ -313,8 +315,6 @@ bgp_path_info_mpath_get(struct bgp_path_info *path) if (!path->mpath) { mpath = bgp_path_info_mpath_new(); - if (!mpath) - return NULL; path->mpath = mpath; mpath->mp_info = path; } From 15e78e64b41f166ad2176977b6c6eb4d698344e0 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 28 Nov 2022 08:53:20 -0500 Subject: [PATCH 2/5] ospfd: Do not always debug joining AllDRouters Multicast group My log file is filling up with: 2022-11-26 13:24:47.532 [DEBG] ospfd: [RY794-DQ7AK] interface 192.168.119.229 [2] join AllDRouters Multicast group. Every 1/2 hour. There is nothing an operator needs to do here and nothing that they can change. Let's guard this output. Signed-off-by: Donald Sharp --- ospfd/ospf_network.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ospfd/ospf_network.c b/ospfd/ospf_network.c index be06afe532..a8c9493ec8 100644 --- a/ospfd/ospf_network.c +++ b/ospfd/ospf_network.c @@ -104,11 +104,12 @@ int ospf_if_add_alldrouters(struct ospf *top, struct prefix *p, "can't setsockopt IP_ADD_MEMBERSHIP (fd %d, addr %pI4, ifindex %u, AllDRouters): %s; perhaps a kernel limit on # of multicast group memberships has been exceeded?", top->fd, &p->u.prefix4, ifindex, safe_strerror(errno)); - else - zlog_debug( - "interface %pI4 [%u] join AllDRouters Multicast group.", - &p->u.prefix4, ifindex); - + else { + if (IS_DEBUG_OSPF_EVENT) + zlog_debug( + "interface %pI4 [%u] join AllDRouters Multicast group.", + &p->u.prefix4, ifindex); + } return ret; } From 7b7ca2d31938c6420c9fc795592fcef9811c6bbe Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 28 Nov 2022 08:57:38 -0500 Subject: [PATCH 3/5] ospfd: Do not auto-debug DR-Election notifications Every 1/2 hour my logs are filling up with this: 2022-11-26 13:54:47.531 [DEBG] ospfd: [P4PQ9-K4XFD] DR-Election[1st]: Backup 192.168.119.229 2022-11-26 13:54:47.531 [DEBG] ospfd: [HBZ7F-65Y86] DR-Election[1st]: DR 192.168.119.229 2022-11-26 13:54:47.531 [DEBG] ospfd: [H01MF-RN00N] DR-Election[2nd]: Backup 0.0.0.0 2022-11-26 13:54:47.531 [DEBG] ospfd: [R7BJ4-KP8JT] DR-Election[2nd]: DR 192.168.119.229 This should be guarded by an if check to ensure that the operator really wants to see this. Signed-off-by: Donald Sharp --- ospfd/ospf_ism.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ospfd/ospf_ism.c b/ospfd/ospf_ism.c index ab75ab9a1a..d4565b6651 100644 --- a/ospfd/ospf_ism.c +++ b/ospfd/ospf_ism.c @@ -223,8 +223,10 @@ int ospf_dr_election(struct ospf_interface *oi) new_state = ospf_ism_state(oi); - zlog_debug("DR-Election[1st]: Backup %pI4", &BDR(oi)); - zlog_debug("DR-Election[1st]: DR %pI4", &DR(oi)); + if (IS_DEBUG_OSPF(ism, ISM_STATUS)) { + zlog_debug("DR-Election[1st]: Backup %pI4", &BDR(oi)); + zlog_debug("DR-Election[1st]: DR %pI4", &DR(oi)); + } if (new_state != old_state && !(new_state == ISM_DROther && old_state < ISM_DROther)) { @@ -233,8 +235,10 @@ int ospf_dr_election(struct ospf_interface *oi) new_state = ospf_ism_state(oi); - zlog_debug("DR-Election[2nd]: Backup %pI4", &BDR(oi)); - zlog_debug("DR-Election[2nd]: DR %pI4", &DR(oi)); + if (IS_DEBUG_OSPF(ism, ISM_STATUS)) { + zlog_debug("DR-Election[2nd]: Backup %pI4", &BDR(oi)); + zlog_debug("DR-Election[2nd]: DR %pI4", &DR(oi)); + } } list_delete(&el_list); From 8f1bf68740ed1db4d60966be9bdf5e44640e1939 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 28 Nov 2022 09:41:03 -0500 Subject: [PATCH 4/5] ospf6d: ospf6_route_cmp_nexthops make return sane The ospf6_route_cmp_nexthops function was returning 0 for same and 1 for not same. Let's reverse the polarity and actually make the returns useful long term. Signed-off-by: Donald Sharp --- ospf6d/ospf6_abr.c | 2 +- ospf6d/ospf6_route.c | 25 ++++++++++++++----------- ospf6d/ospf6_route.h | 6 +++--- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/ospf6d/ospf6_abr.c b/ospf6d/ospf6_abr.c index e9c42bb80c..e007709f99 100644 --- a/ospf6d/ospf6_abr.c +++ b/ospf6d/ospf6_abr.c @@ -1322,7 +1322,7 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa) ospf6_copy_nexthops(tmp_route->nh_list, o_path->nh_list); - if (ospf6_route_cmp_nexthops(tmp_route, route) != 0) { + if (!ospf6_route_cmp_nexthops(tmp_route, route)) { /* adv. router exists in the list, update nhs */ list_delete_all_node(o_path->nh_list); ospf6_copy_nexthops(o_path->nh_list, diff --git a/ospf6d/ospf6_route.c b/ospf6d/ospf6_route.c index 1cc1fcb47b..ecfd0469b2 100644 --- a/ospf6d/ospf6_route.c +++ b/ospf6d/ospf6_route.c @@ -246,7 +246,10 @@ void ospf6_merge_nexthops(struct list *dst, struct list *src) } } -int ospf6_route_cmp_nexthops(struct ospf6_route *a, struct ospf6_route *b) +/* + * If the nexthops are the same return true + */ +bool ospf6_route_cmp_nexthops(struct ospf6_route *a, struct ospf6_route *b) { struct listnode *anode, *bnode; struct ospf6_nexthop *anh, *bnh; @@ -264,14 +267,14 @@ int ospf6_route_cmp_nexthops(struct ospf6_route *a, struct ospf6_route *b) /* Currnet List A element not found List B * Non-Identical lists return */ if (identical == false) - return 1; + return false; } - return 0; + return true; } else - return 1; + return false; } /* One of the routes doesn't exist ? */ - return (1); + return false; } int ospf6_num_nexthops(struct list *nh_list) @@ -577,12 +580,12 @@ ospf6_route_lookup_identical(struct ospf6_route *route, for (target = ospf6_route_lookup(&route->prefix, table); target; target = target->next) { - if (target->type == route->type - && prefix_same(&target->prefix, &route->prefix) - && target->path.type == route->path.type - && target->path.cost == route->path.cost - && target->path.u.cost_e2 == route->path.u.cost_e2 - && ospf6_route_cmp_nexthops(target, route) == 0) + if (target->type == route->type && + prefix_same(&target->prefix, &route->prefix) && + target->path.type == route->path.type && + target->path.cost == route->path.cost && + target->path.u.cost_e2 == route->path.u.cost_e2 && + ospf6_route_cmp_nexthops(target, route)) return target; } return NULL; diff --git a/ospf6d/ospf6_route.h b/ospf6d/ospf6_route.h index c8411c015f..71a84a5c5d 100644 --- a/ospf6d/ospf6_route.h +++ b/ospf6d/ospf6_route.h @@ -304,7 +304,7 @@ extern const char *const ospf6_path_type_substr[OSPF6_PATH_TYPE_MAX]; (ra)->path.cost == (rb)->path.cost && \ (ra)->path.u.cost_e2 == (rb)->path.u.cost_e2 && \ listcount(ra->paths) == listcount(rb->paths) && \ - ospf6_route_cmp_nexthops(ra, rb) == 0) + ospf6_route_cmp_nexthops(ra, rb)) #define ospf6_route_is_best(r) (CHECK_FLAG ((r)->flag, OSPF6_ROUTE_BEST)) @@ -330,8 +330,8 @@ extern void ospf6_add_nexthop(struct list *nh_list, int ifindex, struct in6_addr *addr); extern void ospf6_add_route_nexthop_blackhole(struct ospf6_route *route); extern int ospf6_num_nexthops(struct list *nh_list); -extern int ospf6_route_cmp_nexthops(struct ospf6_route *a, - struct ospf6_route *b); +extern bool ospf6_route_cmp_nexthops(struct ospf6_route *a, + struct ospf6_route *b); extern void ospf6_route_zebra_copy_nexthops(struct ospf6_route *route, struct zapi_nexthop nexthops[], int entries, vrf_id_t vrf_id); From 0ec939675aeba665d025640266d061e50951d238 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 28 Nov 2022 09:46:39 -0500 Subject: [PATCH 5/5] ospf6d: Consolidate to ospf6_route_is_identical Signed-off-by: Donald Sharp --- ospf6d/ospf6_route.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ospf6d/ospf6_route.c b/ospf6d/ospf6_route.c index ecfd0469b2..9603c91a9a 100644 --- a/ospf6d/ospf6_route.c +++ b/ospf6d/ospf6_route.c @@ -580,12 +580,7 @@ ospf6_route_lookup_identical(struct ospf6_route *route, for (target = ospf6_route_lookup(&route->prefix, table); target; target = target->next) { - if (target->type == route->type && - prefix_same(&target->prefix, &route->prefix) && - target->path.type == route->path.type && - target->path.cost == route->path.cost && - target->path.u.cost_e2 == route->path.u.cost_e2 && - ospf6_route_cmp_nexthops(target, route)) + if (ospf6_route_is_identical(target, route)) return target; } return NULL;