From 11ca35875c8ee7316b47f6eeb422358212834615 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Tue, 3 Aug 2021 22:22:09 +0000 Subject: [PATCH 1/4] pimd: change ranges to 1..max, type align with IETF YANG. While defaults are good picks for "reasonable" guesses, min and max range values really aren't. Operators and experimenters often like to configure "unreasonable" values to stress test, tests boundary conditions and explore innovations. With that in mind, change all ranges to 1..max (of type). While we're here add optional ignored values in the "no" CLI forms. Signed-off-by: Christian Hopps --- doc/user/pim.rst | 38 ++++---- lib/command.h | 1 + pimd/pim_cmd.c | 223 ++++++++++++++++++++----------------------- pimd/pim_nb_config.c | 59 ++---------- yang/frr-igmp.yang | 14 +-- yang/frr-pim.yang | 42 ++++---- 6 files changed, 158 insertions(+), 219 deletions(-) diff --git a/doc/user/pim.rst b/doc/user/pim.rst index 6f9aa289b4..899a6b0078 100644 --- a/doc/user/pim.rst +++ b/doc/user/pim.rst @@ -93,7 +93,7 @@ Certain signals have special meanings to *pimd*. down. This command is vrf aware, to configure for a vrf, enter the vrf submode. -.. clicmd:: ip pim join-prune-interval (5-600) +.. clicmd:: ip pim join-prune-interval (1-65535) Modify the join/prune interval that pim uses to the new value. Time is specified in seconds. This command is vrf aware, to configure for a vrf, @@ -101,14 +101,14 @@ Certain signals have special meanings to *pimd*. a value smaller than 60 seconds be aware that this can and will affect convergence at scale. -.. clicmd:: ip pim keep-alive-timer (31-60000) +.. clicmd:: ip pim keep-alive-timer (1-65535) - Modify the time out value for a S,G flow from 31-60000 seconds. 31 seconds - is chosen for a lower bound because some hardware platforms cannot see data + Modify the time out value for a S,G flow from 1-60000 seconds. If choosing + a value below 31 seconds be aware that some hardware platforms cannot see data flowing in better than 30 second chunks. This command is vrf aware, to configure for a vrf, enter the vrf submode. -.. clicmd:: ip pim packets (1-100) +.. clicmd:: ip pim packets (1-255) When processing packets from a neighbor process the number of packets incoming at one time before moving on to the next task. The default value is @@ -116,7 +116,7 @@ Certain signals have special meanings to *pimd*. a large number of pim control packets flowing. This command is vrf aware, to configure for a vrf, enter the vrf submode. -.. clicmd:: ip pim register-suppress-time (5-60000) +.. clicmd:: ip pim register-suppress-time (1-65535) Modify the time that pim will register suppress a FHR will send register notifications to the kernel. This command is vrf aware, to configure for a @@ -162,7 +162,7 @@ Certain signals have special meanings to *pimd*. the existing IGMP general query timer.If no version is provided in the cli, it will be considered as default v2 query.This is a hidden command. -.. clicmd:: ip igmp watermark-warn (10-60000) +.. clicmd:: ip igmp watermark-warn (1-65535) Configure watermark warning generation for an igmp group limit. Generates warning once the configured group limit is reached while adding new groups. @@ -201,7 +201,7 @@ is in a vrf, enter the interface command with the vrf keyword at the end. Set the DR Priority for the interface. This command is useful to allow the user to influence what node becomes the DR for a lan segment. -.. clicmd:: ip pim hello (1-180) (1-630) +.. clicmd:: ip pim hello (1-65535) (1-65535) Set the pim hello and hold interval for a interface. @@ -227,11 +227,11 @@ is in a vrf, enter the interface command with the vrf keyword at the end. Join multicast group or source-group on an interface. -.. clicmd:: ip igmp query-interval (1-1800) +.. clicmd:: ip igmp query-interval (1-65535) Set the IGMP query interval that PIM will use. -.. clicmd:: ip igmp query-max-response-time (10-250) +.. clicmd:: ip igmp query-max-response-time (1-65535) Set the IGMP query response timeout value. If an report is not returned in the specified time we will assume the S,G or \*,G has timed out. @@ -246,12 +246,12 @@ is in a vrf, enter the interface command with the vrf keyword at the end. or IGMP report is received on this interface and the Group is denied by the prefix-list, PIM will ignore the join or report. -.. clicmd:: ip igmp last-member-query-count (1-7) +.. clicmd:: ip igmp last-member-query-count (1-255) Set the IGMP last member query count. The default value is 2. 'no' form of this command is used to to configure back to the default value. -.. clicmd:: ip igmp last-member-query-interval (1-255) +.. clicmd:: ip igmp last-member-query-interval (1-65535) Set the IGMP last member query interval in deciseconds. The default value is 10 deciseconds. 'no' form of this command is used to to configure back to the @@ -319,17 +319,17 @@ MSDP can be setup in different ways: Commands available for MSDP: -.. clicmd:: ip msdp timers (2-600) (3-600) [(1-600)] +.. clicmd:: ip msdp timers (1-65535) (1-65535) [(1-65535)] Configure global MSDP timers. - First value is the keep-alive interval and it must be less than the - second value which is hold-time. This configures the interval in - seconds between keep-alive messages. The default value is 60 seconds. + First value is the keep-alive interval. This configures the interval in + seconds between keep-alive messages. The default value is 60 seconds. It + should be less than the remote hold time. - Second value is the hold-time and it must be greater than the keep-alive - interval. This configures the interval in seconds before closing a non - responding connection. The default value is 75. + Second value is the hold-time. This configures the interval in seconds before + closing a non responding connection. The default value is 75. This value + should be greater than the remote keep alive time. Third value is the connection retry interval and it is optional. This configures the interval between connection attempts. The default value diff --git a/lib/command.h b/lib/command.h index 2b50bc2374..c76fc1e8eb 100644 --- a/lib/command.h +++ b/lib/command.h @@ -389,6 +389,7 @@ struct cmd_node { #define SRTE_STR "SR-TE information\n" #define SRTE_COLOR_STR "SR-TE Color information\n" #define NO_STR "Negate a command or set its defaults\n" +#define IGNORED_IN_NO_STR "Ignored value in no form\n" #define REDIST_STR "Redistribute information from another routing protocol\n" #define CLEAR_STR "Reset functions\n" #define RIP_STR "RIP information\n" diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index 812c8c1449..1238e03a5b 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -7179,7 +7179,7 @@ DEFPY (pim_register_accept_list, DEFUN (ip_pim_joinprune_time, ip_pim_joinprune_time_cmd, - "ip pim join-prune-interval (5-600)", + "ip pim join-prune-interval (1-65535)", IP_STR "pim multicast routing\n" "Join Prune Send Interval\n" @@ -7193,27 +7193,22 @@ DEFUN (ip_pim_joinprune_time, DEFUN (no_ip_pim_joinprune_time, no_ip_pim_joinprune_time_cmd, - "no ip pim join-prune-interval (5-600)", + "no ip pim join-prune-interval [(1-65535)]", NO_STR IP_STR "pim multicast routing\n" "Join Prune Send Interval\n" - "Seconds\n") + IGNORED_IN_NO_STR) { - char jp_default_timer[5]; - - snprintf(jp_default_timer, sizeof(jp_default_timer), "%d", - PIM_DEFAULT_T_PERIODIC); - nb_cli_enqueue_change(vty, "/frr-pim:pim/join-prune-interval", - NB_OP_MODIFY, jp_default_timer); + NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); } DEFUN (ip_pim_register_suppress, ip_pim_register_suppress_cmd, - "ip pim register-suppress-time (5-60000)", + "ip pim register-suppress-time (1-65535)", IP_STR "pim multicast routing\n" "Register Suppress Timer\n" @@ -7227,27 +7222,22 @@ DEFUN (ip_pim_register_suppress, DEFUN (no_ip_pim_register_suppress, no_ip_pim_register_suppress_cmd, - "no ip pim register-suppress-time (5-60000)", + "no ip pim register-suppress-time [(1-65535)]", NO_STR IP_STR "pim multicast routing\n" "Register Suppress Timer\n" - "Seconds\n") + IGNORED_IN_NO_STR) { - char rs_default_timer[5]; - - snprintf(rs_default_timer, sizeof(rs_default_timer), "%d", - PIM_REGISTER_SUPPRESSION_TIME_DEFAULT); - nb_cli_enqueue_change(vty, "/frr-pim:pim/register-suppress-time", - NB_OP_MODIFY, rs_default_timer); + NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); } DEFUN (ip_pim_rp_keep_alive, ip_pim_rp_keep_alive_cmd, - "ip pim rp keep-alive-timer (31-60000)", + "ip pim rp keep-alive-timer (1-65535)", IP_STR "pim multicast routing\n" "Rendevous Point\n" @@ -7274,20 +7264,26 @@ DEFUN (ip_pim_rp_keep_alive, DEFUN (no_ip_pim_rp_keep_alive, no_ip_pim_rp_keep_alive_cmd, - "no ip pim rp keep-alive-timer (31-60000)", + "no ip pim rp keep-alive-timer [(1-65535)]", NO_STR IP_STR "pim multicast routing\n" "Rendevous Point\n" "Keep alive Timer\n" - "Seconds\n") + IGNORED_IN_NO_STR) { const char *vrfname; - char rp_ka_timer[5]; + char rp_ka_timer[6]; char rp_ka_timer_xpath[XPATH_MAXLEN]; + uint v; - snprintf(rp_ka_timer, sizeof(rp_ka_timer), "%d", - PIM_RP_KEEPALIVE_PERIOD); + /* RFC4601 */ + v = yang_dnode_get_uint16(vty->candidate_config->dnode, + "/frr-pim:pim/register-suppress-time"); + v = 3 * v + PIM_REGISTER_PROBE_TIME_DEFAULT; + if (v > UINT16_MAX) + v = UINT16_MAX; + snprintf(rp_ka_timer, sizeof(rp_ka_timer), "%u", v); vrfname = pim_cli_get_vrf_name(vty); if (vrfname == NULL) @@ -7306,7 +7302,7 @@ DEFUN (no_ip_pim_rp_keep_alive, DEFUN (ip_pim_keep_alive, ip_pim_keep_alive_cmd, - "ip pim keep-alive-timer (31-60000)", + "ip pim keep-alive-timer (1-65535)", IP_STR "pim multicast routing\n" "Keep alive Timer\n" @@ -7331,19 +7327,16 @@ DEFUN (ip_pim_keep_alive, DEFUN (no_ip_pim_keep_alive, no_ip_pim_keep_alive_cmd, - "no ip pim keep-alive-timer (31-60000)", + "no ip pim keep-alive-timer [(1-65535)]", NO_STR IP_STR "pim multicast routing\n" "Keep alive Timer\n" - "Seconds\n") + IGNORED_IN_NO_STR) { const char *vrfname; - char ka_timer[5]; char ka_timer_xpath[XPATH_MAXLEN]; - snprintf(ka_timer, sizeof(ka_timer), "%d", PIM_KEEPALIVE_PERIOD); - vrfname = pim_cli_get_vrf_name(vty); if (vrfname == NULL) return CMD_WARNING_CONFIG_FAILED; @@ -7352,15 +7345,14 @@ DEFUN (no_ip_pim_keep_alive, "frr-pim:pimd", "pim", vrfname); strlcat(ka_timer_xpath, "/keep-alive-timer", sizeof(ka_timer_xpath)); - nb_cli_enqueue_change(vty, ka_timer_xpath, NB_OP_MODIFY, - ka_timer); + nb_cli_enqueue_change(vty, ka_timer_xpath, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); } DEFUN (ip_pim_packets, ip_pim_packets_cmd, - "ip pim packets (1-100)", + "ip pim packets (1-255)", IP_STR "pim multicast routing\n" "packets to process at one time per fd\n" @@ -7374,27 +7366,21 @@ DEFUN (ip_pim_packets, DEFUN (no_ip_pim_packets, no_ip_pim_packets_cmd, - "no ip pim packets (1-100)", + "no ip pim packets [(1-255)]", NO_STR IP_STR "pim multicast routing\n" "packets to process at one time per fd\n" - "Number of packets\n") + IGNORED_IN_NO_STR) { - char default_packet[3]; - - snprintf(default_packet, sizeof(default_packet), "%d", - PIM_DEFAULT_PACKET_PROCESS); - - nb_cli_enqueue_change(vty, "/frr-pim:pim/packets", NB_OP_MODIFY, - default_packet); + nb_cli_enqueue_change(vty, "/frr-pim:pim/packets", NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); } DEFPY (igmp_group_watermark, igmp_group_watermark_cmd, - "ip igmp watermark-warn (10-60000)$limit", + "ip igmp watermark-warn (1-65535)$limit", IP_STR IGMP_STR "Configure group limit for watermark warning\n" @@ -7408,12 +7394,12 @@ DEFPY (igmp_group_watermark, DEFPY (no_igmp_group_watermark, no_igmp_group_watermark_cmd, - "no ip igmp watermark-warn [(10-60000)$limit]", + "no ip igmp watermark-warn [(1-65535)$limit]", NO_STR IP_STR IGMP_STR "Unconfigure group limit for watermark warning\n" - "Group count to generate watermark warning\n") + IGNORED_IN_NO_STR) { PIM_DECLVAR_CONTEXT(vrf, pim); pim->igmp_watermark_limit = 0; @@ -8146,7 +8132,7 @@ DEFUN (interface_no_ip_igmp_join, DEFUN (interface_ip_igmp_query_interval, interface_ip_igmp_query_interval_cmd, - "ip igmp query-interval (1-1800)", + "ip igmp query-interval (1-65535)", IP_STR IFACE_IGMP_STR IFACE_IGMP_QUERY_INTERVAL_STR @@ -8174,19 +8160,14 @@ DEFUN (interface_ip_igmp_query_interval, DEFUN (interface_no_ip_igmp_query_interval, interface_no_ip_igmp_query_interval_cmd, - "no ip igmp query-interval", + "no ip igmp query-interval [(1-65535)]", NO_STR IP_STR IFACE_IGMP_STR - IFACE_IGMP_QUERY_INTERVAL_STR) + IFACE_IGMP_QUERY_INTERVAL_STR + IGNORED_IN_NO_STR) { - char default_query_interval[5]; - - snprintf(default_query_interval, sizeof(default_query_interval), "%d", - IGMP_GENERAL_QUERY_INTERVAL); - - nb_cli_enqueue_change(vty, "./query-interval", NB_OP_MODIFY, - default_query_interval); + nb_cli_enqueue_change(vty, "./query-interval", NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, "./frr-igmp:igmp"); } @@ -8222,7 +8203,7 @@ DEFUN (interface_no_ip_igmp_version, DEFUN (interface_ip_igmp_query_max_response_time, interface_ip_igmp_query_max_response_time_cmd, - "ip igmp query-max-response-time (10-250)", + "ip igmp query-max-response-time (1-65535)", IP_STR IFACE_IGMP_STR IFACE_IGMP_QUERY_MAX_RESPONSE_TIME_STR @@ -8251,27 +8232,21 @@ DEFUN (interface_ip_igmp_query_max_response_time, DEFUN (interface_no_ip_igmp_query_max_response_time, interface_no_ip_igmp_query_max_response_time_cmd, - "no ip igmp query-max-response-time (10-250)", + "no ip igmp query-max-response-time [(1-65535)]", NO_STR IP_STR IFACE_IGMP_STR IFACE_IGMP_QUERY_MAX_RESPONSE_TIME_STR - "Time for response in deci-seconds\n") + IGNORED_IN_NO_STR) { - char default_query_max_response_time[4]; - - snprintf(default_query_max_response_time, - sizeof(default_query_max_response_time), - "%d", IGMP_QUERY_MAX_RESPONSE_TIME_DSEC); - - nb_cli_enqueue_change(vty, "./query-max-response-time", NB_OP_MODIFY, - default_query_max_response_time); + nb_cli_enqueue_change(vty, "./query-max-response-time", NB_OP_DESTROY, + NULL); return nb_cli_apply_changes(vty, "./frr-igmp:igmp"); } DEFUN_HIDDEN (interface_ip_igmp_query_max_response_time_dsec, interface_ip_igmp_query_max_response_time_dsec_cmd, - "ip igmp query-max-response-time-dsec (10-250)", + "ip igmp query-max-response-time-dsec (1-65535)", IP_STR IFACE_IGMP_STR IFACE_IGMP_QUERY_MAX_RESPONSE_TIME_DSEC_STR @@ -8299,27 +8274,22 @@ DEFUN_HIDDEN (interface_ip_igmp_query_max_response_time_dsec, DEFUN_HIDDEN (interface_no_ip_igmp_query_max_response_time_dsec, interface_no_ip_igmp_query_max_response_time_dsec_cmd, - "no ip igmp query-max-response-time-dsec", + "no ip igmp query-max-response-time-dsec [(1-65535)]", NO_STR IP_STR IFACE_IGMP_STR - IFACE_IGMP_QUERY_MAX_RESPONSE_TIME_DSEC_STR) + IFACE_IGMP_QUERY_MAX_RESPONSE_TIME_DSEC_STR + IGNORED_IN_NO_STR) { - char default_query_max_response_time[4]; - - snprintf(default_query_max_response_time, - sizeof(default_query_max_response_time), - "%d", IGMP_QUERY_MAX_RESPONSE_TIME_DSEC); - - nb_cli_enqueue_change(vty, "./query-max-response-time", NB_OP_MODIFY, - default_query_max_response_time); + nb_cli_enqueue_change(vty, "./query-max-response-time", NB_OP_DESTROY, + NULL); return nb_cli_apply_changes(vty, "./frr-igmp:igmp"); } DEFUN (interface_ip_igmp_last_member_query_count, interface_ip_igmp_last_member_query_count_cmd, - "ip igmp last-member-query-count (1-7)", + "ip igmp last-member-query-count (1-255)", IP_STR IFACE_IGMP_STR IFACE_IGMP_LAST_MEMBER_QUERY_COUNT_STR @@ -8347,27 +8317,22 @@ DEFUN (interface_ip_igmp_last_member_query_count, DEFUN (interface_no_ip_igmp_last_member_query_count, interface_no_ip_igmp_last_member_query_count_cmd, - "no ip igmp last-member-query-count [(1-7)]", + "no ip igmp last-member-query-count [(1-255)]", NO_STR IP_STR IFACE_IGMP_STR IFACE_IGMP_LAST_MEMBER_QUERY_COUNT_STR - "Last member query count\n") + IGNORED_IN_NO_STR) { - char default_robustness[2]; - - snprintf(default_robustness, sizeof(default_robustness), "%d", - IGMP_DEFAULT_ROBUSTNESS_VARIABLE); - - nb_cli_enqueue_change(vty, "./robustness-variable", NB_OP_MODIFY, - default_robustness); + nb_cli_enqueue_change(vty, "./robustness-variable", NB_OP_DESTROY, + NULL); return nb_cli_apply_changes(vty, "./frr-igmp:igmp"); } DEFUN (interface_ip_igmp_last_member_query_interval, interface_ip_igmp_last_member_query_interval_cmd, - "ip igmp last-member-query-interval (1-255)", + "ip igmp last-member-query-interval (1-65535)", IP_STR IFACE_IGMP_STR IFACE_IGMP_LAST_MEMBER_QUERY_INTERVAL_STR @@ -8395,21 +8360,15 @@ DEFUN (interface_ip_igmp_last_member_query_interval, DEFUN (interface_no_ip_igmp_last_member_query_interval, interface_no_ip_igmp_last_member_query_interval_cmd, - "no ip igmp last-member-query-interval [(1-255)]", + "no ip igmp last-member-query-interval [(1-65535)]", NO_STR IP_STR IFACE_IGMP_STR IFACE_IGMP_LAST_MEMBER_QUERY_INTERVAL_STR - "Last member query interval in deciseconds\n") + IGNORED_IN_NO_STR) { - char default_last_member_query_count[4]; - - snprintf(default_last_member_query_count, - sizeof(default_last_member_query_count), - "%d", IGMP_SPECIFIC_QUERY_MAX_RESPONSE_TIME_DSEC); - - nb_cli_enqueue_change(vty, "./last-member-query-interval", NB_OP_MODIFY, - default_last_member_query_count); + nb_cli_enqueue_change(vty, "./last-member-query-interval", + NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, "./frr-igmp:igmp"); } @@ -8439,13 +8398,7 @@ DEFUN (interface_no_ip_pim_drprio, "Revert the Designated Router Priority to default\n" "Old Value of the Priority\n") { - char default_priority[10]; - - snprintf(default_priority, sizeof(default_priority), "%d", - PIM_DEFAULT_DR_PRIORITY); - - nb_cli_enqueue_change(vty, "./dr-priority", NB_OP_MODIFY, - default_priority); + nb_cli_enqueue_change(vty, "./dr-priority", NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, "./frr-pim:pim"); } @@ -8780,7 +8733,7 @@ DEFUN (interface_no_ip_mroute, DEFUN (interface_ip_pim_hello, interface_ip_pim_hello_cmd, - "ip pim hello (1-180) [(1-630)]", + "ip pim hello (1-65535) [(1-65535)]", IP_STR PIM_STR IFACE_PIM_HELLO_STR @@ -8815,21 +8768,15 @@ DEFUN (interface_ip_pim_hello, DEFUN (interface_no_ip_pim_hello, interface_no_ip_pim_hello_cmd, - "no ip pim hello [(1-180) [(1-630)]]", + "no ip pim hello [(1-65535) [(1-65535)]]", NO_STR IP_STR PIM_STR IFACE_PIM_HELLO_STR - IFACE_PIM_HELLO_TIME_STR - IFACE_PIM_HELLO_HOLD_STR) + IGNORED_IN_NO_STR + IGNORED_IN_NO_STR) { - char hello_default_timer[3]; - - snprintf(hello_default_timer, sizeof(hello_default_timer), "%d", - PIM_DEFAULT_HELLO_PERIOD); - - nb_cli_enqueue_change(vty, "./hello-interval", NB_OP_MODIFY, - hello_default_timer); + nb_cli_enqueue_change(vty, "./hello-interval", NB_OP_DESTROY, NULL); nb_cli_enqueue_change(vty, "./hello-holdtime", NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, "./frr-pim:pim"); @@ -9636,10 +9583,10 @@ DEFUN (no_ip_pim_ucast_bsm, } #if HAVE_BFDD > 0 -DEFUN_HIDDEN( +DEFUN_HIDDEN ( ip_pim_bfd_param, ip_pim_bfd_param_cmd, - "ip pim bfd (2-255) (50-60000) (50-60000)", + "ip pim bfd (2-255) (1-65535) (1-65535)", IP_STR PIM_STR "Enables BFD support\n" @@ -9650,7 +9597,7 @@ DEFUN_HIDDEN( DEFUN( ip_pim_bfd_param, ip_pim_bfd_param_cmd, - "ip pim bfd (2-255) (50-60000) (50-60000)", + "ip pim bfd (2-255) (1-65535) (1-65535)", IP_STR PIM_STR "Enables BFD support\n" @@ -9689,7 +9636,10 @@ DEFUN_HIDDEN( #if HAVE_BFDD == 0 ALIAS(no_ip_pim_bfd, no_ip_pim_bfd_param_cmd, - "no ip pim bfd (2-255) (50-60000) (50-60000)", NO_STR IP_STR PIM_STR + "no ip pim bfd (2-255) (1-65535) (1-65535)", + NO_STR + IP_STR + PIM_STR "Enables BFD support\n" "Detect Multiplier\n" "Required min receive interval\n" @@ -9728,7 +9678,7 @@ DEFPY(ip_msdp_peer, ip_msdp_peer_cmd, } DEFPY(ip_msdp_timers, ip_msdp_timers_cmd, - "ip msdp timers (2-600)$keepalive (3-600)$holdtime [(1-600)$connretry]", + "ip msdp timers (1-65535)$keepalive (1-65535)$holdtime [(1-65535)$connretry]", IP_STR CFG_MSDP_STR "MSDP timers configuration\n" @@ -9759,6 +9709,35 @@ DEFPY(ip_msdp_timers, ip_msdp_timers_cmd, return CMD_SUCCESS; } +DEFPY(no_ip_msdp_timers, no_ip_msdp_timers_cmd, + "no ip msdp timers [(1-65535) (1-65535) [(1-65535)]]", + NO_STR + IP_STR + CFG_MSDP_STR + "MSDP timers configuration\n" + IGNORED_IN_NO_STR + IGNORED_IN_NO_STR + IGNORED_IN_NO_STR) +{ + const char *vrfname; + char xpath[XPATH_MAXLEN]; + + vrfname = pim_cli_get_vrf_name(vty); + if (vrfname == NULL) + return CMD_WARNING_CONFIG_FAILED; + + snprintf(xpath, sizeof(xpath), FRR_PIM_MSDP_XPATH, "frr-pim:pimd", + "pim", vrfname, "frr-routing:ipv4"); + + nb_cli_enqueue_change(vty, "./hold-time", NB_OP_DESTROY, NULL); + nb_cli_enqueue_change(vty, "./keep-alive", NB_OP_DESTROY, NULL); + nb_cli_enqueue_change(vty, "./connection-retry", NB_OP_DESTROY, NULL); + + nb_cli_apply_changes(vty, xpath); + + return CMD_SUCCESS; +} + DEFUN (no_ip_msdp_peer, no_ip_msdp_peer_cmd, "no ip msdp peer A.B.C.D", @@ -11383,6 +11362,8 @@ void pim_cmd_init(void) install_element(CONFIG_NODE, &ip_msdp_timers_cmd); install_element(VRF_NODE, &ip_msdp_timers_cmd); + install_element(CONFIG_NODE, &no_ip_msdp_timers_cmd); + install_element(VRF_NODE, &no_ip_msdp_timers_cmd); install_element(CONFIG_NODE, &ip_msdp_mesh_group_member_cmd); install_element(VRF_NODE, &ip_msdp_mesh_group_member_cmd); install_element(CONFIG_NODE, &no_ip_msdp_mesh_group_member_cmd); diff --git a/pimd/pim_nb_config.c b/pimd/pim_nb_config.c index bd5e215027..5940c94570 100644 --- a/pimd/pim_nb_config.c +++ b/pimd/pim_nb_config.c @@ -956,21 +956,13 @@ int pim_msdp_hold_time_modify(struct nb_cb_modify_args *args) switch (args->event) { case NB_EV_VALIDATE: - if (yang_dnode_get_uint32(args->dnode, NULL) - <= yang_dnode_get_uint32(args->dnode, "../keep-alive")) { - snprintf( - args->errmsg, args->errmsg_len, - "Hold time must be greater than keep alive interval"); - return NB_ERR_VALIDATION; - } - break; case NB_EV_PREPARE: case NB_EV_ABORT: break; case NB_EV_APPLY: vrf = nb_running_get_entry(args->dnode, NULL, true); pim = vrf->info; - pim->msdp.hold_time = yang_dnode_get_uint32(args->dnode, NULL); + pim->msdp.hold_time = yang_dnode_get_uint16(args->dnode, NULL); break; } @@ -988,21 +980,13 @@ int pim_msdp_keep_alive_modify(struct nb_cb_modify_args *args) switch (args->event) { case NB_EV_VALIDATE: - if (yang_dnode_get_uint32(args->dnode, NULL) - >= yang_dnode_get_uint32(args->dnode, "../hold-time")) { - snprintf( - args->errmsg, args->errmsg_len, - "Keep alive must be less than hold time interval"); - return NB_ERR_VALIDATION; - } - break; case NB_EV_PREPARE: case NB_EV_ABORT: break; case NB_EV_APPLY: vrf = nb_running_get_entry(args->dnode, NULL, true); pim = vrf->info; - pim->msdp.keep_alive = yang_dnode_get_uint32(args->dnode, NULL); + pim->msdp.keep_alive = yang_dnode_get_uint16(args->dnode, NULL); break; } @@ -1027,7 +1011,7 @@ int pim_msdp_connection_retry_modify(struct nb_cb_modify_args *args) vrf = nb_running_get_entry(args->dnode, NULL, true); pim = vrf->info; pim->msdp.connection_retry = - yang_dnode_get_uint32(args->dnode, NULL); + yang_dnode_get_uint16(args->dnode, NULL); break; } @@ -2636,9 +2620,7 @@ int lib_interface_igmp_version_destroy(struct nb_cb_destroy_args *args) int lib_interface_igmp_query_interval_modify(struct nb_cb_modify_args *args) { struct interface *ifp; - struct pim_interface *pim_ifp; int query_interval; - int query_interval_dsec; switch (args->event) { case NB_EV_VALIDATE: @@ -2647,18 +2629,8 @@ int lib_interface_igmp_query_interval_modify(struct nb_cb_modify_args *args) break; case NB_EV_APPLY: ifp = nb_running_get_entry(args->dnode, NULL, true); - pim_ifp = ifp->info; query_interval = yang_dnode_get_uint16(args->dnode, NULL); - query_interval_dsec = 10 * query_interval; - if (query_interval_dsec <= - pim_ifp->igmp_query_max_response_time_dsec) { - snprintf(args->errmsg, args->errmsg_len, - "Can't set general query interval %d dsec <= query max response time %d dsec.", - query_interval_dsec, - pim_ifp->igmp_query_max_response_time_dsec); - return NB_ERR_INCONSISTENCY; - } - change_query_interval(pim_ifp, query_interval); + change_query_interval(ifp->info, query_interval); } return NB_OK; @@ -2671,9 +2643,7 @@ int lib_interface_igmp_query_max_response_time_modify( struct nb_cb_modify_args *args) { struct interface *ifp; - struct pim_interface *pim_ifp; int query_max_response_time_dsec; - int default_query_interval_dsec; switch (args->event) { case NB_EV_VALIDATE: @@ -2682,22 +2652,9 @@ int lib_interface_igmp_query_max_response_time_modify( break; case NB_EV_APPLY: ifp = nb_running_get_entry(args->dnode, NULL, true); - pim_ifp = ifp->info; query_max_response_time_dsec = - yang_dnode_get_uint8(args->dnode, NULL); - default_query_interval_dsec = - 10 * pim_ifp->igmp_default_query_interval; - - if (query_max_response_time_dsec - >= default_query_interval_dsec) { - snprintf(args->errmsg, args->errmsg_len, - "Can't set query max response time %d sec >= general query interval %d sec", - query_max_response_time_dsec, - pim_ifp->igmp_default_query_interval); - return NB_ERR_INCONSISTENCY; - } - - change_query_max_response_time(pim_ifp, + yang_dnode_get_uint16(args->dnode, NULL); + change_query_max_response_time(ifp->info, query_max_response_time_dsec); } @@ -2722,8 +2679,8 @@ int lib_interface_igmp_last_member_query_interval_modify( case NB_EV_APPLY: ifp = nb_running_get_entry(args->dnode, NULL, true); pim_ifp = ifp->info; - last_member_query_interval = yang_dnode_get_uint8(args->dnode, - NULL); + last_member_query_interval = + yang_dnode_get_uint16(args->dnode, NULL); pim_ifp->igmp_specific_query_max_response_time_dsec = last_member_query_interval; diff --git a/yang/frr-igmp.yang b/yang/frr-igmp.yang index e2971dc5cf..8d151e430d 100644 --- a/yang/frr-igmp.yang +++ b/yang/frr-igmp.yang @@ -84,9 +84,10 @@ module frr-igmp { leaf query-interval { type uint16 { - range "1..1800"; + range "1..max"; } units seconds; + must ". * 10 >= ../query-max-response-time"; default "125"; description "The Query Interval is the interval between General Queries @@ -94,10 +95,11 @@ module frr-igmp { } leaf query-max-response-time { - type uint8 { - range "10..250"; + type uint16 { + range "1..max"; } units deciseconds; + must ". <= ../query-interval * 10"; default "100"; description "Query maximum response time specifies the maximum time @@ -105,8 +107,8 @@ module frr-igmp { } leaf last-member-query-interval { - type uint8 { - range "1..255"; + type uint16 { + range "1..max"; } units deciseconds; default "10"; @@ -117,7 +119,7 @@ module frr-igmp { leaf robustness-variable { type uint8 { - range "1..7"; + range "1..max"; } default "2"; description diff --git a/yang/frr-pim.yang b/yang/frr-pim.yang index e846ffa1f8..109ce22309 100644 --- a/yang/frr-pim.yang +++ b/yang/frr-pim.yang @@ -97,7 +97,7 @@ module frr-pim { leaf keep-alive-timer { type uint16 { - range "31..60000"; + range "1..max"; } default "210"; description @@ -106,7 +106,7 @@ module frr-pim { leaf rp-keep-alive-timer { type uint16 { - range "31..60000"; + range "1..max"; } default "210"; description @@ -116,36 +116,34 @@ module frr-pim { grouping msdp-timers { leaf hold-time { - type uint32 { - range 3..600; + type uint16 { + range "1..max"; } units seconds; default 75; description "Hold period is started at the MSDP peer connection establishment and is reset every new message. When the period expires the - connection is closed. - - This value needs to be greater than `keep-alive-period`."; + connection is closed. This value should be greater than the + remote keep-alive time."; } leaf keep-alive { - type uint32 { - range 2..600; + type uint16 { + range "1..max"; } units seconds; default 60; description "To maintain a connection established it is necessary to send keep alive messages in a certain frequency and this allows its - configuration. - - This value needs to be lesser than `hold-time-period`."; + configuration. This value should be less than the remote + hold time."; } leaf connection-retry { - type uint32 { - range 1..600; + type uint16 { + range "1..max"; } units seconds; default 30; @@ -343,7 +341,7 @@ module frr-pim { leaf hello-interval { type uint8 { - range "1..180"; + range "1..max"; } default "30"; description @@ -352,7 +350,7 @@ module frr-pim { leaf hello-holdtime { type uint16 { - range "1..630"; + range "1..max"; } must ". > ./../hello-interval" { error-message "HoldTime must be greater than Hello"; @@ -367,7 +365,7 @@ module frr-pim { leaf min-rx-interval { type uint16 { - range "50..60000"; + range "1..max"; } default "300"; description @@ -376,7 +374,7 @@ module frr-pim { leaf min-tx-interval { type uint16 { - range "50..60000"; + range "1..max"; } default "300"; description @@ -422,7 +420,7 @@ module frr-pim { leaf dr-priority { type uint32 { - range "1..4294967295"; + range "1..max"; } default 1; description @@ -521,7 +519,7 @@ module frr-pim { "PIM router parameters."; leaf packets { type uint8 { - range "1..100"; + range "1..max"; } default "3"; description @@ -529,7 +527,7 @@ module frr-pim { } leaf join-prune-interval { type uint16 { - range "5..600"; + range "1..max"; } default "60"; description @@ -537,7 +535,7 @@ module frr-pim { } leaf register-suppress-time { type uint16 { - range "5..60000"; + range "1..max"; } default "60"; description From e8b7548c0d420e8e1ada7ae7db6aea14146c7b2d Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Wed, 18 Aug 2021 21:48:28 -0400 Subject: [PATCH 2/4] pimd: fix register suppress timer code Signed-off-by: Christian Hopps --- pimd/pim_instance.h | 4 ++-- pimd/pim_nb_config.c | 19 +++++++++++++++++++ pimd/pim_upstream.c | 14 +++++++++----- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/pimd/pim_instance.h b/pimd/pim_instance.h index 72c726690c..52ded08ae3 100644 --- a/pimd/pim_instance.h +++ b/pimd/pim_instance.h @@ -96,9 +96,9 @@ struct pim_router { int t_periodic; struct pim_assert_metric infinite_assert_metric; long rpf_cache_refresh_delay_msec; - int32_t register_suppress_time; + uint32_t register_suppress_time; int packet_process; - int32_t register_probe_time; + uint32_t register_probe_time; /* * What is the default vrf that we work in diff --git a/pimd/pim_nb_config.c b/pimd/pim_nb_config.c index 5940c94570..22a98f94dd 100644 --- a/pimd/pim_nb_config.c +++ b/pimd/pim_nb_config.c @@ -556,8 +556,27 @@ int pim_join_prune_interval_modify(struct nb_cb_modify_args *args) */ int pim_register_suppress_time_modify(struct nb_cb_modify_args *args) { + uint16_t value; switch (args->event) { case NB_EV_VALIDATE: + value = yang_dnode_get_uint16(args->dnode, NULL); + /* + * As soon as this is non-constant it needs to be replaced with + * a yang_dnode_get to lookup the candidate value, *not* the + * operational value. Since the code has a field assigned and + * used for this value it should have YANG/CLI to set it too, + * otherwise just use the #define! + */ + /* RFC7761: 4.11. Timer Values */ + if (value <= router->register_probe_time * 2) { + snprintf( + args->errmsg, args->errmsg_len, + "Register suppress time (%u) must be more than " + "twice the register probe time (%u).", + value, router->register_probe_time); + return NB_ERR_VALIDATION; + } + break; case NB_EV_PREPARE: case NB_EV_ABORT: break; diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c index 2b674b4234..d21c7b4008 100644 --- a/pimd/pim_upstream.c +++ b/pimd/pim_upstream.c @@ -1800,12 +1800,16 @@ void pim_upstream_start_register_stop_timer(struct pim_upstream *up, THREAD_OFF(up->t_rs_timer); if (!null_register) { - uint32_t lower = (0.5 * PIM_REGISTER_SUPPRESSION_PERIOD); - uint32_t upper = (1.5 * PIM_REGISTER_SUPPRESSION_PERIOD); - time = lower + (frr_weak_random() % (upper - lower + 1)) - - PIM_REGISTER_PROBE_PERIOD; + uint32_t lower = (0.5 * router->register_suppress_time); + uint32_t upper = (1.5 * router->register_suppress_time); + time = lower + (frr_weak_random() % (upper - lower + 1)); + /* Make sure we don't wrap around */ + if (time >= router->register_probe_time) + time -= router->register_probe_time; + else + time = 0; } else - time = PIM_REGISTER_PROBE_PERIOD; + time = router->register_probe_time; if (PIM_DEBUG_PIM_TRACE) { zlog_debug( From 0a76e764c846460cf3c0ad0b24a9153b00c305fd Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Thu, 29 Jul 2021 03:32:33 +0000 Subject: [PATCH 3/4] tests: Add global pim join-prune-interval json config option - cleanup pim configuration code Signed-off-by: Christian Hopps --- tests/topotests/lib/pim.py | 193 +++++++++--------- .../test_multicast_pim_sm_topo1.py | 7 +- .../test_multicast_pim_sm_topo3.py | 6 +- 3 files changed, 104 insertions(+), 102 deletions(-) diff --git a/tests/topotests/lib/pim.py b/tests/topotests/lib/pim.py index 7de1c7a2f9..e093e6de53 100644 --- a/tests/topotests/lib/pim.py +++ b/tests/topotests/lib/pim.py @@ -55,7 +55,7 @@ def create_pim_config(tgen, topo, input_dict=None, build=False, load_config=True input_dict = { "r1": { "pim": { - "disable" : ["l1-i1-eth1"], + "join-prune-interval": "5", "rp": [{ "rp_addr" : "1.0.3.17". "keep-alive-timer": "100" @@ -90,7 +90,7 @@ def create_pim_config(tgen, topo, input_dict=None, build=False, load_config=True if "rp" not in input_dict[router]["pim"]: continue - result = _create_pim_config( + result = _create_pim_rp_config( tgen, topo, input_dict, router, build, load_config ) if result is not True: @@ -100,9 +100,9 @@ def create_pim_config(tgen, topo, input_dict=None, build=False, load_config=True return result -def _create_pim_config(tgen, topo, input_dict, router, build=False, load_config=False): +def _create_pim_rp_config(tgen, topo, input_dict, router, build=False, load_config=False): """ - Helper API to create pim configuration. + Helper API to create pim RP configurations. Parameters ---------- @@ -119,96 +119,93 @@ def _create_pim_config(tgen, topo, input_dict, router, build=False, load_config= result = False logger.debug("Entering lib API: {}".format(sys._getframe().f_code.co_name)) - try: - pim_data = input_dict[router]["pim"] + pim_data = input_dict[router]["pim"] + rp_data = pim_data["rp"] - for dut in tgen.routers(): - if "pim" not in input_dict[router]: - continue + # Configure this RP on every router. + for dut in tgen.routers(): - for destLink, data in topo[dut]["links"].items(): - if "pim" not in data: - continue + # At least one interface must be enabled for PIM on the router + pim_if_enabled = False + for destLink, data in topo[dut]["links"].items(): + if "pim" in data: + pim_if_enabled = True + if not pim_if_enabled: + continue - if "rp" in pim_data: - config_data = [] - rp_data = pim_data["rp"] + config_data = [] - for rp_dict in deepcopy(rp_data): - # ip address of RP - if "rp_addr" not in rp_dict and build: - logger.error( - "Router %s: 'ip address of RP' not " - "present in input_dict/JSON", - router, - ) + for rp_dict in deepcopy(rp_data): + # ip address of RP + if "rp_addr" not in rp_dict and build: + logger.error( + "Router %s: 'ip address of RP' not " + "present in input_dict/JSON", + router, + ) - return False - rp_addr = rp_dict.setdefault("rp_addr", None) + return False + rp_addr = rp_dict.setdefault("rp_addr", None) - # Keep alive Timer - keep_alive_timer = rp_dict.setdefault("keep_alive_timer", None) + # Keep alive Timer + keep_alive_timer = rp_dict.setdefault("keep_alive_timer", None) - # Group Address range to cover - if "group_addr_range" not in rp_dict and build: - logger.error( - "Router %s:'Group Address range to cover'" - " not present in input_dict/JSON", - router, - ) + # Group Address range to cover + if "group_addr_range" not in rp_dict and build: + logger.error( + "Router %s:'Group Address range to cover'" + " not present in input_dict/JSON", + router, + ) - return False - group_addr_range = rp_dict.setdefault("group_addr_range", None) + return False + group_addr_range = rp_dict.setdefault("group_addr_range", None) - # Group prefix-list filter - prefix_list = rp_dict.setdefault("prefix_list", None) + # Group prefix-list filter + prefix_list = rp_dict.setdefault("prefix_list", None) - # Delete rp config - del_action = rp_dict.setdefault("delete", False) + # Delete rp config + del_action = rp_dict.setdefault("delete", False) - if keep_alive_timer: - cmd = "ip pim rp keep-alive-timer {}".format(keep_alive_timer) - config_data.append(cmd) + if keep_alive_timer: + cmd = "ip pim rp keep-alive-timer {}".format(keep_alive_timer) + if del_action: + cmd = "no {}".format(cmd) + config_data.append(cmd) + if rp_addr: + if group_addr_range: + if type(group_addr_range) is not list: + group_addr_range = [group_addr_range] + + for grp_addr in group_addr_range: + cmd = "ip pim rp {} {}".format(rp_addr, grp_addr) if del_action: cmd = "no {}".format(cmd) - config_data.append(cmd) + config_data.append(cmd) - if rp_addr: - if group_addr_range: - if type(group_addr_range) is not list: - group_addr_range = [group_addr_range] - - for grp_addr in group_addr_range: - cmd = "ip pim rp {} {}".format(rp_addr, grp_addr) - config_data.append(cmd) - - if del_action: - cmd = "no {}".format(cmd) - config_data.append(cmd) - - if prefix_list: - cmd = "ip pim rp {} prefix-list {}".format( - rp_addr, prefix_list - ) - config_data.append(cmd) - - if del_action: - cmd = "no {}".format(cmd) - config_data.append(cmd) + if prefix_list: + cmd = "ip pim rp {} prefix-list {}".format( + rp_addr, prefix_list + ) + if del_action: + cmd = "no {}".format(cmd) + config_data.append(cmd) + try: result = create_common_configuration( tgen, dut, config_data, "pim", build, load_config ) if result is not True: + logger.error("Error applying PIM config", exc_info=True) + logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) return False - except InvalidCLIError: - # Traceback - errormsg = traceback.format_exc() - logger.error(errormsg) - return errormsg + except InvalidCLIError as error: + logger.error("Error applying PIM config: %s", error, exc_info=error) + logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) + return False logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) return result @@ -338,35 +335,19 @@ def _enable_disable_pim(tgen, topo, input_dict, router, build=False): try: config_data = [] - enable_flag = True - # Disable pim on interface - if "pim" in input_dict[router]: - if "disable" in input_dict[router]["pim"]: - enable_flag = False - interfaces = input_dict[router]["pim"]["disable"] + # Enable pim on interfaces + for destRouterLink, data in sorted(topo[router]["links"].items()): + if "pim" in data and data["pim"] == "enable": - if type(interfaces) is not list: - interfaces = [interfaces] + # Loopback interfaces + if "type" in data and data["type"] == "loopback": + interface_name = destRouterLink + else: + interface_name = data["interface"] - for interface in interfaces: - cmd = "interface {}".format(interface) - config_data.append(cmd) - config_data.append("no ip pim") - - # Enable pim on interface - if enable_flag: - for destRouterLink, data in sorted(topo[router]["links"].items()): - if "pim" in data and data["pim"] == "enable": - - # Loopback interfaces - if "type" in data and data["type"] == "loopback": - interface_name = destRouterLink - else: - interface_name = data["interface"] - - cmd = "interface {}".format(interface_name) - config_data.append(cmd) - config_data.append("ip pim") + cmd = "interface {}".format(interface_name) + config_data.append(cmd) + config_data.append("ip pim") result = create_common_configuration( tgen, router, config_data, "interface_config", build=build @@ -374,6 +355,22 @@ def _enable_disable_pim(tgen, topo, input_dict, router, build=False): if result is not True: return False + config_data = [] + if "pim" in input_dict[router]: + pim_data = input_dict[router]["pim"] + for t in [ + "join-prune-interval", + "keep-alive-timer", + "register-suppress-time", + ]: + if t in pim_data: + cmd = "ip pim {} {}".format(t, pim_data[t]) + config_data.append(cmd) + + if config_data: + result = create_common_configuration( + tgen, router, config_data, "pim", build=build + ) except InvalidCLIError: # Traceback errormsg = traceback.format_exc() diff --git a/tests/topotests/multicast_pim_sm_topo1/test_multicast_pim_sm_topo1.py b/tests/topotests/multicast_pim_sm_topo1/test_multicast_pim_sm_topo1.py index 99a6e5bacf..36a3103c9d 100755 --- a/tests/topotests/multicast_pim_sm_topo1/test_multicast_pim_sm_topo1.py +++ b/tests/topotests/multicast_pim_sm_topo1/test_multicast_pim_sm_topo1.py @@ -78,6 +78,7 @@ from lib.common_config import ( step, iperfSendIGMPJoin, addKernelRoute, + apply_raw_config, reset_config_on_routers, iperfSendTraffic, kill_iperf, @@ -1553,8 +1554,10 @@ def test_modify_igmp_max_query_response_timer_p0(request): assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result) step("Delete the PIM and IGMP on FRR1") - input_dict_1 = {"l1": {"pim": {"disable": ["l1-i1-eth1"]}}} - result = create_pim_config(tgen, topo, input_dict_1) + raw_config = { + "l1": {"raw_config": ["interface l1-i1-eth1", "no ip pim"]} + } + result = apply_raw_config(tgen, raw_config) assert result is True, "Testcase {}: Failed Error: {}".format(tc_name, result) input_dict_2 = { diff --git a/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py b/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py index 33f476de44..f0d6ac5ccc 100755 --- a/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py +++ b/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py @@ -2231,8 +2231,10 @@ def test_verify_remove_add_pim_commands_when_igmp_configured_p1(request): step("Remove 'no ip pim' on receiver interface on FRR1") intf_l1_i1 = topo["routers"]["l1"]["links"]["i1"]["interface"] - input_dict_1 = {"l1": {"pim": {"disable": intf_l1_i1}}} - result = create_pim_config(tgen, topo, input_dict_1) + raw_config = { + "l1": {"raw_config": ["interface {}".format(intf_l1_i1), "no ip pim"]} + } + result = apply_raw_config(tgen, raw_config) assert result is True, "Testcase {}: Failed Error: {}".format(tc_name, result) step("Verify that no core is observed") From e09755b8f8f6051c3ff2eb7af7ea5b6e3cc05545 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Wed, 28 Jul 2021 10:31:13 +0000 Subject: [PATCH 4/4] tests: reduce multicast timer values - completes change made in #9190 - also lowers other pim and igmp timer values to speed things up. Signed-off-by: Christian Hopps --- .../multicast_pim_sm_topo1.json | 6 +++ .../multicast_pim_sm_topo2.json | 6 +++ .../multicast_pim_sm_topo3.json | 6 +++ .../multicast_pim_sm_topo4.json | 6 +++ .../test_multicast_pim_sm_topo3.py | 2 +- .../multicast_pim_static_rp.json | 38 ++++++++++++++++--- 6 files changed, 57 insertions(+), 7 deletions(-) diff --git a/tests/topotests/multicast_pim_sm_topo1/multicast_pim_sm_topo1.json b/tests/topotests/multicast_pim_sm_topo1/multicast_pim_sm_topo1.json index 71454c2ab2..cc20abbe6a 100644 --- a/tests/topotests/multicast_pim_sm_topo1/multicast_pim_sm_topo1.json +++ b/tests/topotests/multicast_pim_sm_topo1/multicast_pim_sm_topo1.json @@ -13,10 +13,12 @@ "r2": {"ipv4": "auto", "pim": "enable"}, "c1": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "igmp": { "interfaces": { "l1-i1-eth1" :{ "igmp":{ + "query": {"query-max-response-time": 40, "query-interval": 5}, "version": "2" } } @@ -38,6 +40,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i3": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["10.0.5.0/24", "10.0.6.0/24", "1.0.2.2/32", "10.0.1.0/24"], "next_hop": "10.0.7.1" @@ -55,6 +58,7 @@ "i2": {"ipv4": "auto", "pim": "enable"}, "i8": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.8.0/24", "10.0.9.0/24", "10.0.10.0/24", "10.0.12.0/24", "10.0.11.0/24"], "next_hop": "10.0.7.2" @@ -71,6 +75,7 @@ "l1": {"ipv4": "auto", "pim": "enable"}, "i4": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.6.0/24", "10.0.3.0/24", "10.0.8.0/24", "10.0.9.0/24", "10.0.12.0/24", "10.0.10.0/24", "10.0.11.0/24"], "next_hop": "10.0.2.2" @@ -87,6 +92,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i5": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.5.0/24", "10.0.6.0/24", "10.0.7.0/24", "10.0.8.0/24", "10.0.9.0/24", "10.0.7.0/24", "10.0.10.0/24", "10.0.11.0/24"], "next_hop": "10.0.3.2" diff --git a/tests/topotests/multicast_pim_sm_topo2/multicast_pim_sm_topo2.json b/tests/topotests/multicast_pim_sm_topo2/multicast_pim_sm_topo2.json index 71454c2ab2..cc20abbe6a 100644 --- a/tests/topotests/multicast_pim_sm_topo2/multicast_pim_sm_topo2.json +++ b/tests/topotests/multicast_pim_sm_topo2/multicast_pim_sm_topo2.json @@ -13,10 +13,12 @@ "r2": {"ipv4": "auto", "pim": "enable"}, "c1": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "igmp": { "interfaces": { "l1-i1-eth1" :{ "igmp":{ + "query": {"query-max-response-time": 40, "query-interval": 5}, "version": "2" } } @@ -38,6 +40,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i3": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["10.0.5.0/24", "10.0.6.0/24", "1.0.2.2/32", "10.0.1.0/24"], "next_hop": "10.0.7.1" @@ -55,6 +58,7 @@ "i2": {"ipv4": "auto", "pim": "enable"}, "i8": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.8.0/24", "10.0.9.0/24", "10.0.10.0/24", "10.0.12.0/24", "10.0.11.0/24"], "next_hop": "10.0.7.2" @@ -71,6 +75,7 @@ "l1": {"ipv4": "auto", "pim": "enable"}, "i4": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.6.0/24", "10.0.3.0/24", "10.0.8.0/24", "10.0.9.0/24", "10.0.12.0/24", "10.0.10.0/24", "10.0.11.0/24"], "next_hop": "10.0.2.2" @@ -87,6 +92,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i5": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.5.0/24", "10.0.6.0/24", "10.0.7.0/24", "10.0.8.0/24", "10.0.9.0/24", "10.0.7.0/24", "10.0.10.0/24", "10.0.11.0/24"], "next_hop": "10.0.3.2" diff --git a/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo3.json b/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo3.json index f582f4929d..89c54a41d6 100644 --- a/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo3.json +++ b/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo3.json @@ -13,10 +13,12 @@ "r2": {"ipv4": "auto", "pim": "enable"}, "c1": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "igmp": { "interfaces": { "l1-i1-eth1" :{ "igmp":{ + "query": {"query-max-response-time": 40, "query-interval": 5}, "version": "2" } } @@ -38,6 +40,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i3": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["10.0.5.0/24", "10.0.6.0/24", "1.0.2.2/32", "10.0.1.0/24", "1.0.3.5/32"], "next_hop": "10.0.7.1" @@ -55,6 +58,7 @@ "i2": {"ipv4": "auto", "pim": "enable"}, "i8": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.8.0/24", "10.0.9.0/24", "10.0.10.0/24", "10.0.11.0/24", "10.0.12.0/24"], "next_hop": "10.0.7.2" @@ -71,6 +75,7 @@ "l1": {"ipv4": "auto", "pim": "enable"}, "i4": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.6.0/24", "10.0.8.0/24", "10.0.9.0/24", "10.0.10.0/24", "10.0.11.0/24"], "next_hop": "10.0.2.2" @@ -87,6 +92,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i5": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.5.17/32", "10.0.5.0/24", "10.0.6.0/24", "10.0.7.0/24", "10.0.8.0/24", "10.0.9.0/24", "10.0.10.0/24", "10.0.11.0/24"], "next_hop": "10.0.3.2" diff --git a/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo4.json b/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo4.json index 4635dac7d2..afb55994a7 100644 --- a/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo4.json +++ b/tests/topotests/multicast_pim_sm_topo3/multicast_pim_sm_topo4.json @@ -13,10 +13,12 @@ "r2": {"ipv4": "auto", "pim": "enable"}, "c1": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "igmp": { "interfaces": { "l1-i1-eth1" :{ "igmp":{ + "query": {"query-max-response-time": 40, "query-interval": 5}, "version": "2" } } @@ -40,6 +42,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i3": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["10.0.4.0/24","10.0.3.1/24"], "next_hop": "10.0.7.1" @@ -57,6 +60,7 @@ "i2": {"ipv4": "auto", "pim": "enable"}, "i8": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["10.0.4.0/24","10.0.3.1/24"], "next_hop": "10.0.3.1" @@ -73,6 +77,7 @@ "l1": {"ipv4": "auto", "pim": "enable"}, "i4": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [{ "network": ["1.0.4.11/32","10.0.4.2/24", "10.0.3.1/24"], "next_hop": "10.0.2.2" @@ -87,6 +92,7 @@ "f1": {"ipv4": "auto", "pim": "enable"}, "i5": {"ipv4": "auto", "pim": "enable"} }, + "pim": { "join-prune-interval": "5", "keep-alive-timer": 15, "register-suppress-time": 12 }, "static_routes": [ { "network": ["1.0.4.11/32", "10.0.2.1/24", "10.0.1.2/24"], diff --git a/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py b/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py index f0d6ac5ccc..033c76081a 100755 --- a/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py +++ b/tests/topotests/multicast_pim_sm_topo3/test_multicast_pim_sm_topo3.py @@ -2014,7 +2014,7 @@ def test_verify_remove_add_igmp_commands_when_pim_configured_p0(request): intf_l1_i1 = topo["routers"]["l1"]["links"]["i1"]["interface"] input_dict_1 = { - "l1": {"igmp": {"interfaces": {intf_l1_i1: {"igmp": {"version": "2"}}}}} + "l1": {"igmp": {"interfaces": {intf_l1_i1: {"igmp": {"version": "2", "query": {"query-max-response-time": 40, "query-interval": 5}}}}}} } result = verify_igmp_config(tgen, input_dict_1) diff --git a/tests/topotests/multicast_pim_static_rp_topo1/multicast_pim_static_rp.json b/tests/topotests/multicast_pim_static_rp_topo1/multicast_pim_static_rp.json index 6d6c047b00..39c68408b4 100644 --- a/tests/topotests/multicast_pim_static_rp_topo1/multicast_pim_static_rp.json +++ b/tests/topotests/multicast_pim_static_rp_topo1/multicast_pim_static_rp.json @@ -4,7 +4,11 @@ "link_ip_start": {"ipv4": "10.0.0.0", "v4mask": 24}, "lo_prefix": {"ipv4": "1.0.", "v4mask": 32}, "routers": { - "r0": {"links": {"r1": {"ipv4": "auto"}}}, + "r0": { + "links": { + "r1": {"ipv4": "auto"} + } + }, "r1": { "links": { "lo": {"ipv4": "auto", "type": "loopback", "pim": "enable"}, @@ -14,9 +18,20 @@ "r4": {"ipv4": "auto", "pim": "enable"} }, "pim": { - "rp": [{"rp_addr": "1.0.2.17", "group_addr_range": ["224.0.0.0/4"]}] + "join-prune-interval": "5", + "keep-alive-timer": 15, + "register-suppress-time": 12 + }, + "igmp": { + "interfaces": { + "r1-r0-eth0": { + "igmp": { + "query": {"query-max-response-time": 40, "query-interval": 5}, + "version": "2" + } + } + } }, - "igmp": {"interfaces": {"r1-r0-eth0": {"igmp": {"version": "2"}}}}, "static_routes": [ {"network": "10.0.4.0/24", "next_hop": "10.0.2.2"}, {"network": "10.0.5.0/24", "next_hop": "10.0.2.2"}, @@ -35,6 +50,9 @@ "r3": {"ipv4": "auto", "pim": "enable"} }, "pim": { + "join-prune-interval": "5", + "keep-alive-timer": 15, + "register-suppress-time": 12, "rp": [{"rp_addr": "1.0.2.17", "group_addr_range": ["224.0.0.0/4"]}] }, "static_routes": [ @@ -57,7 +75,9 @@ "r5": {"ipv4": "auto", "pim": "enable"} }, "pim": { - "rp": [{"rp_addr": "1.0.2.17", "group_addr_range": ["224.0.0.0/4"]}] + "join-prune-interval": "5", + "keep-alive-timer": 15, + "register-suppress-time": 12 }, "static_routes": [ {"network": "10.0.0.0/24", "next_hop": "10.0.2.1"}, @@ -75,7 +95,9 @@ "r3": {"ipv4": "auto", "pim": "enable"} }, "pim": { - "rp": [{"rp_addr": "1.0.2.17", "group_addr_range": ["224.0.0.0/4"]}] + "join-prune-interval": "5", + "keep-alive-timer": 15, + "register-suppress-time": 12 }, "static_routes": [ {"network": "10.0.0.0/24", "next_hop": "10.0.3.1"}, @@ -88,6 +110,10 @@ {"network": "1.0.3.17/32", "next_hop": "10.0.5.1"} ] }, - "r5": {"links": {"r3": {"ipv4": "auto"}}} + "r5": { + "links": { + "r3": {"ipv4": "auto"} + } + } } }