From accef597dfcdabd0de65062f4baf44c4b585793a Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Sat, 6 Nov 2021 17:00:46 +0300 Subject: [PATCH] ospfd: remove commands for broken GR helper mode Issue #9983 explains what is wrong with the GR helper mode. To unblock the CI that fails almost all the time on the ospf_gr_topo1 test, remove the commands and disable the test. Also add a reminder to completely remove the helper mode if no one fixes the code in a month. Signed-off-by: Igor Ryzhov --- ospfd/ospf_vty.c | 522 +----------------- .../ospf_gr_helper/test_ospf_gr_helper.py | 2 + .../ospf_gr_topo1/test_ospf_gr_topo1.py | 2 + 3 files changed, 6 insertions(+), 520 deletions(-) diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index bbb458d8e..fcfc645a2 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -9863,161 +9863,10 @@ DEFUN (no_ospf_proactive_arp, return CMD_SUCCESS; } -/* Graceful Restart HELPER Commands */ -DEFPY(ospf_gr_helper_enable, ospf_gr_helper_enable_cmd, - "graceful-restart helper enable [A.B.C.D$address]", - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Enable Helper support\n" - "Advertising Router-ID\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - if (address_str) { - ospf_gr_helper_support_set_per_routerid(ospf, &address, - OSPF_GR_TRUE); - return CMD_SUCCESS; - } - - ospf_gr_helper_support_set(ospf, OSPF_GR_TRUE); - - return CMD_SUCCESS; -} - -DEFPY(no_ospf_gr_helper_enable, - no_ospf_gr_helper_enable_cmd, - "no graceful-restart helper enable [A.B.C.D$address]", - NO_STR - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Enable Helper support\n" - "Advertising Router-ID\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - if (address_str) { - ospf_gr_helper_support_set_per_routerid(ospf, &address, - OSPF_GR_FALSE); - return CMD_SUCCESS; - } - - ospf_gr_helper_support_set(ospf, OSPF_GR_FALSE); - return CMD_SUCCESS; -} - -#if CONFDATE > 20220921 -CPP_NOTICE( - "Time to remove the deprecated \"[no] graceful-restart helper-only\" commands") +#if CONFDATE > 20211209 +CPP_NOTICE("Time to remove broken OSPF GR helper") #endif -DEFPY_HIDDEN(ospf_gr_helper_only, ospf_gr_helper_only_cmd, - "graceful-restart helper-only [A.B.C.D]", - "OSPF Graceful Restart\n" - "Enable Helper support\n" - "Advertising router id\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - struct in_addr addr; - int ret; - - vty_out(vty, - "%% This command is deprecated. Please, use `graceful-restart helper enable` instead.\n"); - - if (argc == 3) { - ret = inet_aton(argv[2]->arg, &addr); - if (!ret) { - vty_out(vty, - "Please specify the valid routerid address.\n"); - return CMD_WARNING_CONFIG_FAILED; - } - - ospf_gr_helper_support_set_per_routerid(ospf, &addr, OSPF_GR_TRUE); - return CMD_SUCCESS; - } - - ospf_gr_helper_support_set(ospf, OSPF_GR_TRUE); - - return CMD_SUCCESS; -} - -ALIAS_HIDDEN(no_ospf_gr_helper_enable, - no_ospf_gr_helper_only_cmd, - "no graceful-restart helper-only [A.B.C.D]", - NO_STR - "OSPF Graceful Restart\n" - "Disable Helper support\n" - "Advertising router id\n") - -DEFPY(ospf_gr_helper_enable_lsacheck, - ospf_gr_helper_enable_lsacheck_cmd, - "graceful-restart helper strict-lsa-checking", - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Enable strict LSA check\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - ospf_gr_helper_lsa_check_set(ospf, OSPF_GR_TRUE); - return CMD_SUCCESS; -} - -DEFPY(no_ospf_gr_helper_enable_lsacheck, - no_ospf_gr_helper_enable_lsacheck_cmd, - "no graceful-restart helper strict-lsa-checking", - NO_STR - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Disable strict LSA check\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - ospf_gr_helper_lsa_check_set(ospf, OSPF_GR_FALSE); - return CMD_SUCCESS; -} - -DEFPY(ospf_gr_helper_supported_grace_time, - ospf_gr_helper_supported_grace_time_cmd, - "graceful-restart helper supported-grace-time (10-1800)$interval", - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Supported grace timer\n" - "Grace interval(in seconds)\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - ospf_gr_helper_supported_gracetime_set(ospf, interval); - return CMD_SUCCESS; -} - -DEFPY(no_ospf_gr_helper_supported_grace_time, - no_ospf_gr_helper_supported_grace_time_cmd, - "no graceful-restart helper supported-grace-time (10-1800)$interval", - NO_STR - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Supported grace timer\n" - "Grace interval(in seconds)\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - ospf_gr_helper_supported_gracetime_set(ospf, OSPF_MAX_GRACE_INTERVAL); - return CMD_SUCCESS; -} - -DEFPY(ospf_gr_helper_planned_only, - ospf_gr_helper_planned_only_cmd, - "graceful-restart helper planned-only", - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Supported only planned restart\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - ospf_gr_helper_set_supported_planned_only_restart(ospf, OSPF_GR_TRUE); - - return CMD_SUCCESS; -} - /* External Route Aggregation */ DEFUN (ospf_external_route_aggregation, ospf_external_route_aggregation_cmd, @@ -10100,256 +9949,6 @@ DEFUN (no_ospf_external_route_aggregation, return CMD_SUCCESS; } -DEFPY(no_ospf_gr_helper_planned_only, - no_ospf_gr_helper_planned_only_cmd, - "no graceful-restart helper planned-only", - NO_STR - "OSPF Graceful Restart\n" - "OSPF GR Helper\n" - "Supported only for planned restart\n") -{ - VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); - - ospf_gr_helper_set_supported_planned_only_restart(ospf, OSPF_GR_FALSE); - - return CMD_SUCCESS; -} - -static int ospf_print_vty_helper_dis_rtr_walkcb(struct hash_bucket *bucket, - void *arg) -{ - struct advRtr *rtr = bucket->data; - struct vty *vty = (struct vty *)arg; - static unsigned int count; - - vty_out(vty, "%-6pI4,", &rtr->advRtrAddr); - count++; - - if (count % 5 == 0) - vty_out(vty, "\n"); - - return HASHWALK_CONTINUE; -} - -static int ospf_show_gr_helper_details(struct vty *vty, struct ospf *ospf, - uint8_t use_vrf, json_object *json, - bool uj, bool detail) -{ - struct listnode *node; - struct ospf_interface *oi; - char buf[PREFIX_STRLEN]; - json_object *json_vrf = NULL; - - if (uj) { - if (use_vrf) - json_vrf = json_object_new_object(); - else - json_vrf = json; - } - - if (ospf->instance) { - if (uj) - json_object_int_add(json, "ospfInstance", - ospf->instance); - else - vty_out(vty, "\nOSPF Instance: %d\n\n", ospf->instance); - } - - ospf_show_vrf_name(ospf, vty, json_vrf, use_vrf); - - if (uj) { - if (use_vrf) - json_object_object_add(json, ospf_get_name(ospf), - json_vrf); - } else - vty_out(vty, "\n"); - - /* Show Router ID. */ - if (uj) { - json_object_string_add(json_vrf, "routerId", - inet_ntop(AF_INET, &ospf->router_id, - buf, sizeof(buf))); - } else { - vty_out(vty, "\n OSPF Router with ID (%pI4)\n\n", - &ospf->router_id); - } - - if (!uj) { - - if (ospf->is_helper_supported) - vty_out(vty, - " Graceful restart helper support enabled.\n"); - else - vty_out(vty, - " Graceful restart helper support disabled.\n"); - - if (ospf->strict_lsa_check) - vty_out(vty, " Strict LSA check is enabled.\n"); - else - vty_out(vty, " Strict LSA check is disabled.\n"); - - if (ospf->only_planned_restart) - vty_out(vty, - " Helper supported for planned restarts only.\n"); - else - vty_out(vty, - " Helper supported for Planned and Unplanned Restarts.\n"); - - vty_out(vty, - " Supported Graceful restart interval: %d(in seconds).\n", - ospf->supported_grace_time); - - if (OSPF_HELPER_ENABLE_RTR_COUNT(ospf)) { - vty_out(vty, " Enable Router list:\n"); - vty_out(vty, " "); - hash_walk(ospf->enable_rtr_list, - ospf_print_vty_helper_dis_rtr_walkcb, vty); - vty_out(vty, "\n\n"); - } - - if (ospf->last_exit_reason != OSPF_GR_HELPER_EXIT_NONE) { - vty_out(vty, " Last Helper exit Reason :%s\n", - ospf_exit_reason2str(ospf->last_exit_reason)); - } - - if (ospf->active_restarter_cnt) - vty_out(vty, - " Number of Active neighbours in graceful restart: %d\n", - ospf->active_restarter_cnt); - else - vty_out(vty, "\n"); - - } else { - json_object_string_add( - json_vrf, "helperSupport", - (ospf->is_helper_supported) ? "Enabled" : "Disabled"); - json_object_string_add(json_vrf, "strictLsaCheck", - (ospf->strict_lsa_check) ? "Enabled" - : "Disabled"); - json_object_string_add( - json_vrf, "restartSupoort", - (ospf->only_planned_restart) - ? "Planned Restart only" - : "Planned and Unplanned Restarts"); - - json_object_int_add(json_vrf, "supportedGracePeriod", - ospf->supported_grace_time); - - if (ospf->last_exit_reason != OSPF_GR_HELPER_EXIT_NONE) - json_object_string_add( - json_vrf, "LastExitReason", - ospf_exit_reason2str(ospf->last_exit_reason)); - - if (ospf->active_restarter_cnt) - json_object_int_add(json_vrf, "activeRestarterCnt", - ospf->active_restarter_cnt); - } - - - if (detail) { - int cnt = 1; - json_object *json_neighbors = NULL; - - for (ALL_LIST_ELEMENTS_RO(ospf->oiflist, node, oi)) { - struct route_node *rn; - struct ospf_neighbor *nbr; - json_object *json_neigh; - - if (ospf_interface_neighbor_count(oi) == 0) - continue; - - if (uj) { - json_object_object_get_ex(json_vrf, "Neighbors", - &json_neighbors); - if (!json_neighbors) { - json_neighbors = - json_object_new_object(); - json_object_object_add(json_vrf, - "Neighbors", - json_neighbors); - } - } - - for (rn = route_top(oi->nbrs); rn; - rn = route_next(rn)) { - - if (!rn->info) - continue; - - nbr = rn->info; - - if (!OSPF_GR_IS_ACTIVE_HELPER(nbr)) - continue; - - if (!uj) { - vty_out(vty, " Neighbour %d :\n", cnt); - vty_out(vty, " Address : %pI4\n", - &nbr->address.u.prefix4); - vty_out(vty, " Routerid : %pI4\n", - &nbr->router_id); - vty_out(vty, - " Received Grace period : %d(in seconds).\n", - nbr->gr_helper_info - .recvd_grace_period); - vty_out(vty, - " Actual Grace period : %d(in seconds)\n", - nbr->gr_helper_info - .actual_grace_period); - vty_out(vty, - " Remaining GraceTime:%ld(in seconds).\n", - thread_timer_remain_second( - nbr->gr_helper_info - .t_grace_timer)); - vty_out(vty, - " Graceful Restart reason: %s.\n\n", - ospf_restart_reason2str( - nbr->gr_helper_info - .gr_restart_reason)); - cnt++; - } else { - json_neigh = json_object_new_object(); - json_object_string_add( - json_neigh, "srcAddr", - inet_ntop(AF_INET, &nbr->src, - buf, sizeof(buf))); - - json_object_string_add( - json_neigh, "routerid", - inet_ntop(AF_INET, - &nbr->router_id, - buf, sizeof(buf))); - json_object_int_add( - json_neigh, - "recvdGraceInterval", - nbr->gr_helper_info - .recvd_grace_period); - json_object_int_add( - json_neigh, - "actualGraceInterval", - nbr->gr_helper_info - .actual_grace_period); - json_object_int_add( - json_neigh, "remainGracetime", - thread_timer_remain_second( - nbr->gr_helper_info - .t_grace_timer)); - json_object_string_add( - json_neigh, "restartReason", - ospf_restart_reason2str( - nbr->gr_helper_info - .gr_restart_reason)); - json_object_object_add( - json_neighbors, - inet_ntop(AF_INET, &nbr->src, - buf, sizeof(buf)), - json_neigh); - } - } - } - } - return CMD_SUCCESS; -} - DEFUN (ospf_external_route_aggregation_no_adrvertise, ospf_external_route_aggregation_no_adrvertise_cmd, "summary-address A.B.C.D/M no-advertise", @@ -10438,108 +10037,6 @@ DEFUN (ospf_route_aggregation_timer, return CMD_SUCCESS; } -DEFPY (show_ip_ospf_gr_helper, - show_ip_ospf_gr_helper_cmd, - "show ip ospf [vrf ] graceful-restart helper [detail] [json]", - SHOW_STR - IP_STR - "OSPF information\n" - VRF_CMD_HELP_STR - "All VRFs\n" - "OSPF Graceful Restart\n" - "Helper details in the router\n" - "Detailed informtion\n" - JSON_STR) -{ - char *vrf_name = NULL; - bool all_vrf = false; - int ret = CMD_SUCCESS; - int idx_vrf = 0; - int idx = 0; - uint8_t use_vrf = 0; - bool uj = use_json(argc, argv); - struct ospf *ospf = NULL; - json_object *json = NULL; - struct listnode *node = NULL; - int inst = 0; - bool detail = false; - - OSPF_FIND_VRF_ARGS(argv, argc, idx_vrf, vrf_name, all_vrf); - - if (argv_find(argv, argc, "detail", &idx)) - detail = true; - - if (uj) - json = json_object_new_object(); - - /* vrf input is provided */ - if (vrf_name) { - use_vrf = 1; - - if (all_vrf) { - for (ALL_LIST_ELEMENTS_RO(om->ospf, node, ospf)) { - if (!ospf->oi_running) - continue; - - ret = ospf_show_gr_helper_details( - vty, ospf, use_vrf, json, uj, detail); - } - - if (uj) { - vty_out(vty, "%s\n", - json_object_to_json_string_ext( - json, JSON_C_TO_STRING_PRETTY)); - json_object_free(json); - } - - return ret; - } - - ospf = ospf_lookup_by_inst_name(inst, vrf_name); - - if (ospf == NULL || !ospf->oi_running) { - - if (uj) { - vty_out(vty, "%s\n", - json_object_to_json_string_ext( - json, JSON_C_TO_STRING_PRETTY)); - json_object_free(json); - } else - vty_out(vty, "%% OSPF instance not found\n"); - - return CMD_SUCCESS; - } - - } else { - /* Default Vrf */ - ospf = ospf_lookup_by_vrf_id(VRF_DEFAULT); - - if (ospf == NULL || !ospf->oi_running) { - - if (uj) { - vty_out(vty, "%s\n", - json_object_to_json_string_ext( - json, JSON_C_TO_STRING_PRETTY)); - json_object_free(json); - } else - vty_out(vty, "%% OSPF instance not found\n"); - - return CMD_SUCCESS; - } - - ospf_show_gr_helper_details(vty, ospf, use_vrf, json, uj, - detail); - } - - if (uj) { - vty_out(vty, "%s\n", json_object_to_json_string_ext( - json, JSON_C_TO_STRING_PRETTY)); - json_object_free(json); - } - - return CMD_SUCCESS; -} -/* Graceful Restart HELPER commands end */ DEFUN (no_ospf_route_aggregation_timer, no_ospf_route_aggregation_timer_cmd, "no aggregation timer", @@ -12608,9 +12105,6 @@ void ospf_vty_show_init(void) /* "show ip ospf vrfs" commands. */ install_element(VIEW_NODE, &show_ip_ospf_vrfs_cmd); - /* "show ip ospf gr-helper details" command */ - install_element(VIEW_NODE, &show_ip_ospf_gr_helper_cmd); - /* "show ip ospf summary-address" command */ install_element(VIEW_NODE, &show_ip_ospf_external_aggregator_cmd); } @@ -12721,18 +12215,6 @@ static void ospf_vty_zebra_init(void) install_element(OSPF_NODE, &no_ospf_distance_ospf_cmd); install_element(OSPF_NODE, &ospf_distance_ospf_cmd); - /*Ospf garcefull restart helper configurations */ - install_element(OSPF_NODE, &ospf_gr_helper_enable_cmd); - install_element(OSPF_NODE, &no_ospf_gr_helper_enable_cmd); - install_element(OSPF_NODE, &ospf_gr_helper_only_cmd); - install_element(OSPF_NODE, &no_ospf_gr_helper_only_cmd); - install_element(OSPF_NODE, &ospf_gr_helper_enable_lsacheck_cmd); - install_element(OSPF_NODE, &no_ospf_gr_helper_enable_lsacheck_cmd); - install_element(OSPF_NODE, &ospf_gr_helper_supported_grace_time_cmd); - install_element(OSPF_NODE, &no_ospf_gr_helper_supported_grace_time_cmd); - install_element(OSPF_NODE, &ospf_gr_helper_planned_only_cmd); - install_element(OSPF_NODE, &no_ospf_gr_helper_planned_only_cmd); - /* External LSA summarisation config commands.*/ install_element(OSPF_NODE, &ospf_external_route_aggregation_cmd); install_element(OSPF_NODE, &no_ospf_external_route_aggregation_cmd); diff --git a/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py b/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py index 2c7c6df37..b7a10d630 100644 --- a/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py +++ b/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py @@ -100,6 +100,8 @@ TC8. Verify helper functionality when dut is helping RR and new grace lsa def setup_module(mod): + return pytest.skip("OSPF GR helper mode is currently broken") + """ Sets up the pytest environment diff --git a/tests/topotests/ospf_gr_topo1/test_ospf_gr_topo1.py b/tests/topotests/ospf_gr_topo1/test_ospf_gr_topo1.py index debf7ad76..4b69d8288 100755 --- a/tests/topotests/ospf_gr_topo1/test_ospf_gr_topo1.py +++ b/tests/topotests/ospf_gr_topo1/test_ospf_gr_topo1.py @@ -139,6 +139,8 @@ def build_topo(tgen): def setup_module(mod): + return pytest.skip("OSPF GR helper mode is currently broken") + "Sets up the pytest environment" tgen = Topogen(build_topo, mod.__name__) tgen.start_topology() -- 2.39.5