Merge pull request #14288 from opensourcerouting/fix/warn_the_user_if…

…_keepalive_was_changed bgpd: Add a warning for the operator that keepalive was changed
FRRouting · Aug 30, 2023 · 497584a · 497584a
2 parents f80262b + 7c4ed2a
commit 497584a
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 9 deletions.
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
@@ -590,7 +590,7 @@ int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name,
 	int ret = bgp_get(bgp, as, name, inst_type, as_pretty, asnotation);
 
 	if (ret == BGP_CREATED) {
-		bgp_timers_set(*bgp, DFLT_BGP_KEEPALIVE, DFLT_BGP_HOLDTIME,
+		bgp_timers_set(NULL, *bgp, DFLT_BGP_KEEPALIVE, DFLT_BGP_HOLDTIME,
 			       DFLT_BGP_CONNECT_RETRY, BGP_DEFAULT_DELAYOPEN);
 
 		if (DFLT_BGP_IMPORT_CHECK)
@@ -2661,7 +2661,7 @@ DEFUN (bgp_timers,
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	bgp_timers_set(bgp, keepalive, holdtime, DFLT_BGP_CONNECT_RETRY,
+	bgp_timers_set(vty, bgp, keepalive, holdtime, DFLT_BGP_CONNECT_RETRY,
 		       BGP_DEFAULT_DELAYOPEN);
 
 	return CMD_SUCCESS;
@@ -2677,7 +2677,7 @@ DEFUN (no_bgp_timers,
        "Holdtime\n")
 {
 	VTY_DECLVAR_CONTEXT(bgp, bgp);
-	bgp_timers_set(bgp, DFLT_BGP_KEEPALIVE, DFLT_BGP_HOLDTIME,
+	bgp_timers_set(vty, bgp, DFLT_BGP_KEEPALIVE, DFLT_BGP_HOLDTIME,
 		       DFLT_BGP_CONNECT_RETRY, BGP_DEFAULT_DELAYOPEN);
 
 	return CMD_SUCCESS;

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
@@ -519,11 +519,22 @@ void bgp_cluster_id_unset(struct bgp *bgp)
 }
 
 /* BGP timer configuration.  */
-void bgp_timers_set(struct bgp *bgp, uint32_t keepalive, uint32_t holdtime,
-		    uint32_t connect_retry, uint32_t delayopen)
+void bgp_timers_set(struct vty *vty, struct bgp *bgp, uint32_t keepalive,
+		    uint32_t holdtime, uint32_t connect_retry,
+		    uint32_t delayopen)
 {
-	bgp->default_keepalive =
-		(keepalive < holdtime / 3 ? keepalive : holdtime / 3);
+	uint32_t default_keepalive = holdtime / 3;
+
+	if (keepalive > default_keepalive) {
+		if (vty)
+			vty_out(vty,
+				"%% keepalive value %u is larger than 1/3 of the holdtime, setting to %u\n",
+				keepalive, default_keepalive);
+	} else {
+		default_keepalive = keepalive;
+	}
+
+	bgp->default_keepalive = default_keepalive;
 	bgp->default_holdtime = holdtime;
 	bgp->default_connect_retry = connect_retry;
 	bgp->default_delayopen = delayopen;

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
@@ -2254,8 +2254,9 @@ extern void bgp_confederation_peers_add(struct bgp *bgp, as_t as,
 					const char *as_str);
 extern void bgp_confederation_peers_remove(struct bgp *bgp, as_t as);
 
-extern void bgp_timers_set(struct bgp *, uint32_t keepalive, uint32_t holdtime,
-			   uint32_t connect_retry, uint32_t delayopen);
+extern void bgp_timers_set(struct vty *vty, struct bgp *, uint32_t keepalive,
+			   uint32_t holdtime, uint32_t connect_retry,
+			   uint32_t delayopen);
 extern void bgp_timers_unset(struct bgp *);
 
 extern void bgp_default_local_preference_set(struct bgp *bgp,