From: David Lamparter Date: Thu, 1 Aug 2019 16:50:56 +0000 (+0200) Subject: bgpd: use new defaults system (v2) X-Git-Tag: base_7.3~99^2~4 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=5d5393b943c500c63770a06df9036a1558d0f100;p=mirror%2Ffrr.git bgpd: use new defaults system (v2) This moves all the DFLT_BGP_* stuff over to the new defaults mechanism. bgp_timers_nondefault() added to get better file-scoping. v2: moved everything into bgp_vty.c so that the core BGP code is independent of the CLI-specific defaults. This should make the future northbound conversion easier. Signed-off-by: David Lamparter --- diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 48b714c7df..f6b90d3651 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -49,6 +49,7 @@ #include "bgpd/bgp_nexthop.h" #include "bgpd/bgp_addpath.h" #include "bgpd/bgp_mac.h" +#include "bgpd/bgp_vty.h" /* * Definitions and external declarations. @@ -5709,9 +5710,10 @@ int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id, int ret = 0; - ret = bgp_get(&bgp_vrf, &as, vrf_id_to_name(vrf_id), - vrf_id == VRF_DEFAULT ? BGP_INSTANCE_TYPE_DEFAULT - : BGP_INSTANCE_TYPE_VRF); + ret = bgp_get_vty(&bgp_vrf, &as, vrf_id_to_name(vrf_id), + vrf_id == VRF_DEFAULT + ? BGP_INSTANCE_TYPE_DEFAULT + : BGP_INSTANCE_TYPE_VRF); switch (ret) { case BGP_ERR_AS_MISMATCH: flog_err(EC_BGP_EVPN_AS_MISMATCH, diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index bb34354685..0ff64c527b 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -72,6 +72,35 @@ #include "bgpd/rfapi/bgp_rfapi_cfg.h" #endif +FRR_CFG_DEFAULT_BOOL(BGP_IMPORT_CHECK, + { .val_long = true, .match_profile = "datacenter", }, + { .val_long = false }, +) +FRR_CFG_DEFAULT_BOOL(BGP_SHOW_HOSTNAME, + { .val_long = true, .match_profile = "datacenter", }, + { .val_long = false }, +) +FRR_CFG_DEFAULT_BOOL(BGP_LOG_NEIGHBOR_CHANGES, + { .val_long = true, .match_profile = "datacenter", }, + { .val_long = false }, +) +FRR_CFG_DEFAULT_BOOL(BGP_DETERMINISTIC_MED, + { .val_long = true, .match_profile = "datacenter", }, + { .val_long = false }, +) +FRR_CFG_DEFAULT_ULONG(BGP_CONNECT_RETRY, + { .val_ulong = 10, .match_profile = "datacenter", }, + { .val_ulong = 120 }, +) +FRR_CFG_DEFAULT_ULONG(BGP_HOLDTIME, + { .val_ulong = 9, .match_profile = "datacenter", }, + { .val_ulong = 180 }, +) +FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE, + { .val_ulong = 3, .match_profile = "datacenter", }, + { .val_ulong = 60 }, +) + DEFINE_HOOK(bgp_inst_config_write, (struct bgp *bgp, struct vty *vty), (bgp, vty)) @@ -356,6 +385,29 @@ int argv_find_and_parse_safi(struct cmd_token **argv, int argc, int *index, return ret; } +int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name, + enum bgp_instance_type inst_type) +{ + int ret = bgp_get(bgp, as, name, inst_type); + + if (ret == BGP_CREATED) { + bgp_timers_set(*bgp, DFLT_BGP_KEEPALIVE, DFLT_BGP_HOLDTIME, + DFLT_BGP_CONNECT_RETRY); + + if (DFLT_BGP_IMPORT_CHECK) + bgp_flag_set(*bgp, BGP_FLAG_IMPORT_CHECK); + if (DFLT_BGP_SHOW_HOSTNAME) + bgp_flag_set(*bgp, BGP_FLAG_SHOW_HOSTNAME); + if (DFLT_BGP_LOG_NEIGHBOR_CHANGES) + bgp_flag_set(*bgp, BGP_FLAG_LOG_NEIGHBOR_CHANGES); + if (DFLT_BGP_DETERMINISTIC_MED) + bgp_flag_set(*bgp, BGP_FLAG_DETERMINISTIC_MED); + + ret = BGP_SUCCESS; + } + return ret; +} + /* * bgp_vty_find_and_parse_afi_safi_bgp * @@ -1077,7 +1129,7 @@ DEFUN_NOSH (router_bgp, if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) is_new_bgp = (bgp_lookup(as, name) == NULL); - ret = bgp_get(&bgp, &as, name, inst_type); + ret = bgp_get_vty(&bgp, &as, name, inst_type); switch (ret) { case BGP_ERR_AS_MISMATCH: vty_out(vty, "BGP is already running; AS is %u\n", as); @@ -1830,7 +1882,7 @@ DEFUN (bgp_timers, return CMD_WARNING_CONFIG_FAILED; } - bgp_timers_set(bgp, keepalive, holdtime); + bgp_timers_set(bgp, keepalive, holdtime, DFLT_BGP_CONNECT_RETRY); return CMD_SUCCESS; } @@ -1845,7 +1897,8 @@ DEFUN (no_bgp_timers, "Holdtime\n") { VTY_DECLVAR_CONTEXT(bgp, bgp); - bgp_timers_unset(bgp); + bgp_timers_set(bgp, DFLT_BGP_KEEPALIVE, DFLT_BGP_HOLDTIME, + DFLT_BGP_CONNECT_RETRY); return CMD_SUCCESS; } @@ -6965,8 +7018,8 @@ DEFPY(af_import_vrf_route_map, af_import_vrf_route_map_cmd, as_t as = bgp->as; /* Auto-create assuming the same AS */ - ret = bgp_get(&bgp_default, &as, NULL, - BGP_INSTANCE_TYPE_DEFAULT); + ret = bgp_get_vty(&bgp_default, &as, NULL, + BGP_INSTANCE_TYPE_DEFAULT); if (ret) { vty_out(vty, @@ -7051,8 +7104,8 @@ DEFPY(bgp_imexport_vrf, bgp_imexport_vrf_cmd, bgp_default = bgp_get_default(); if (!bgp_default) { /* Auto-create assuming the same AS */ - ret = bgp_get(&bgp_default, &as, NULL, - BGP_INSTANCE_TYPE_DEFAULT); + ret = bgp_get_vty(&bgp_default, &as, NULL, + BGP_INSTANCE_TYPE_DEFAULT); if (ret) { vty_out(vty, @@ -7067,7 +7120,7 @@ DEFPY(bgp_imexport_vrf, bgp_imexport_vrf_cmd, vrf_bgp = bgp_default; else /* Auto-create assuming the same AS */ - ret = bgp_get(&vrf_bgp, &as, import_name, bgp_type); + ret = bgp_get_vty(&vrf_bgp, &as, import_name, bgp_type); if (ret) { vty_out(vty, @@ -9734,9 +9787,8 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, json_neigh, "bgpTimerConfiguredKeepAliveIntervalMsecs", p->keepalive * 1000); - } else if ((bgp->default_holdtime != BGP_DEFAULT_HOLDTIME) - || (bgp->default_keepalive - != BGP_DEFAULT_KEEPALIVE)) { + } else if ((bgp->default_holdtime != SAVE_BGP_HOLDTIME) + || (bgp->default_keepalive != SAVE_BGP_KEEPALIVE)) { json_object_int_add(json_neigh, "bgpTimerConfiguredHoldTimeMsecs", bgp->default_holdtime); @@ -9798,9 +9850,8 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, p->holdtime); vty_out(vty, ", keepalive interval is %d seconds\n", p->keepalive); - } else if ((bgp->default_holdtime != BGP_DEFAULT_HOLDTIME) - || (bgp->default_keepalive - != BGP_DEFAULT_KEEPALIVE)) { + } else if ((bgp->default_holdtime != SAVE_BGP_HOLDTIME) + || (bgp->default_keepalive != SAVE_BGP_KEEPALIVE)) { vty_out(vty, " Configured hold time is %d", bgp->default_holdtime); vty_out(vty, ", keepalive interval is %d seconds\n", @@ -13297,6 +13348,14 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, if (peergroup_flag_check(peer, PEER_FLAG_TIMER_CONNECT)) vty_out(vty, " neighbor %s timers connect %u\n", addr, peer->connect); + /* need special-case handling for changed default values due to + * config profile / version (because there is no "timers bgp connect" + * command, we need to save this per-peer :/) + */ + else if (!peer_group_active(peer) && !peer->connect && + peer->bgp->default_connect_retry != SAVE_BGP_CONNECT_RETRY) + vty_out(vty, " neighbor %s timers connect %u\n", addr, + peer->bgp->default_connect_retry); /* capability dynamic */ if (peergroup_flag_check(peer, PEER_FLAG_DYNAMIC_CAPABILITY)) @@ -13739,7 +13798,7 @@ int bgp_config_write(struct vty *vty) /* BGP log-neighbor-changes. */ if (!!bgp_flag_check(bgp, BGP_FLAG_LOG_NEIGHBOR_CHANGES) - != DFLT_BGP_LOG_NEIGHBOR_CHANGES) + != SAVE_BGP_LOG_NEIGHBOR_CHANGES) vty_out(vty, " %sbgp log-neighbor-changes\n", bgp_flag_check(bgp, BGP_FLAG_LOG_NEIGHBOR_CHANGES) @@ -13770,7 +13829,7 @@ int bgp_config_write(struct vty *vty) /* BGP default show-hostname */ if (!!bgp_flag_check(bgp, BGP_FLAG_SHOW_HOSTNAME) - != DFLT_BGP_SHOW_HOSTNAME) + != SAVE_BGP_SHOW_HOSTNAME) vty_out(vty, " %sbgp default show-hostname\n", bgp_flag_check(bgp, BGP_FLAG_SHOW_HOSTNAME) ? "" @@ -13815,7 +13874,7 @@ int bgp_config_write(struct vty *vty) /* BGP deterministic-med. */ if (!!bgp_flag_check(bgp, BGP_FLAG_DETERMINISTIC_MED) - != DFLT_BGP_DETERMINISTIC_MED) + != SAVE_BGP_DETERMINISTIC_MED) vty_out(vty, " %sbgp deterministic-med\n", bgp_flag_check(bgp, BGP_FLAG_DETERMINISTIC_MED) ? "" @@ -13904,15 +13963,15 @@ int bgp_config_write(struct vty *vty) /* BGP network import check. */ if (!!bgp_flag_check(bgp, BGP_FLAG_IMPORT_CHECK) - != DFLT_BGP_IMPORT_CHECK) + != SAVE_BGP_IMPORT_CHECK) vty_out(vty, " %sbgp network import-check\n", bgp_flag_check(bgp, BGP_FLAG_IMPORT_CHECK) ? "" : "no "); /* BGP timers configuration. */ - if (bgp->default_keepalive != BGP_DEFAULT_KEEPALIVE - && bgp->default_holdtime != BGP_DEFAULT_HOLDTIME) + if (bgp->default_keepalive != SAVE_BGP_KEEPALIVE + && bgp->default_holdtime != SAVE_BGP_HOLDTIME) vty_out(vty, " timers bgp %u %u\n", bgp->default_keepalive, bgp->default_holdtime); diff --git a/bgpd/bgp_vty.h b/bgpd/bgp_vty.h index 2f7074d6da..5f3ce9cd8e 100644 --- a/bgpd/bgp_vty.h +++ b/bgpd/bgp_vty.h @@ -21,6 +21,8 @@ #ifndef _QUAGGA_BGP_VTY_H #define _QUAGGA_BGP_VTY_H +#include "bgpd/bgpd.h" + struct bgp; #define BGP_INSTANCE_HELP_STR "BGP view\nBGP VRF\nView/VRF name\n" @@ -46,6 +48,8 @@ struct bgp; extern void bgp_vty_init(void); extern const char *get_afi_safi_str(afi_t afi, safi_t safi, bool for_json); +extern int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name, + enum bgp_instance_type inst_type); extern void bgp_config_write_update_delay(struct vty *vty, struct bgp *bgp); extern void bgp_config_write_wpkt_quanta(struct vty *vty, struct bgp *bgp); extern void bgp_config_write_rpkt_quanta(struct vty *vty, struct bgp *bgp); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 896687c083..9b0e81491a 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -404,19 +404,23 @@ time_t bgp_clock(void) } /* BGP timer configuration. */ -int bgp_timers_set(struct bgp *bgp, uint32_t keepalive, uint32_t holdtime) +int bgp_timers_set(struct bgp *bgp, uint32_t keepalive, uint32_t holdtime, + uint32_t connect_retry) { bgp->default_keepalive = (keepalive < holdtime / 3 ? keepalive : holdtime / 3); bgp->default_holdtime = holdtime; + bgp->default_connect_retry = connect_retry; return 0; } +/* mostly for completeness - CLI uses its own defaults */ int bgp_timers_unset(struct bgp *bgp) { bgp->default_keepalive = BGP_DEFAULT_KEEPALIVE; bgp->default_holdtime = BGP_DEFAULT_HOLDTIME; + bgp->default_connect_retry = BGP_DEFAULT_CONNECT_RETRY; return 0; } @@ -1118,7 +1122,7 @@ struct peer *peer_new(struct bgp *bgp) /* Set default value. */ peer->fd = -1; peer->v_start = BGP_INIT_START_TIMER; - peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + peer->v_connect = bgp->default_connect_retry; peer->status = Idle; peer->ostatus = Idle; peer->cur_event = peer->last_event = peer->last_major_event = 0; @@ -2417,7 +2421,7 @@ static void peer_group2peer_config_copy(struct peer_group *group, if (CHECK_FLAG(conf->flags, PEER_FLAG_TIMER_CONNECT)) peer->v_connect = conf->connect; else - peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + peer->v_connect = peer->bgp->default_connect_retry; } /* advertisement-interval apply */ @@ -2904,26 +2908,13 @@ static struct bgp *bgp_create(as_t *as, const char *name, bgp->default_local_pref = BGP_DEFAULT_LOCAL_PREF; bgp->default_subgroup_pkt_queue_max = BGP_DEFAULT_SUBGROUP_PKT_QUEUE_MAX; - bgp->default_holdtime = BGP_DEFAULT_HOLDTIME; - bgp->default_keepalive = BGP_DEFAULT_KEEPALIVE; + bgp_timers_unset(bgp); bgp->restart_time = BGP_DEFAULT_RESTART_TIME; bgp->stalepath_time = BGP_DEFAULT_STALEPATH_TIME; bgp->dynamic_neighbors_limit = BGP_DYNAMIC_NEIGHBORS_LIMIT_DEFAULT; bgp->dynamic_neighbors_count = 0; bgp->ebgp_requires_policy = DEFAULT_EBGP_POLICY_DISABLED; bgp->reject_as_sets = BGP_REJECT_AS_SETS_DISABLED; -#if DFLT_BGP_IMPORT_CHECK - bgp_flag_set(bgp, BGP_FLAG_IMPORT_CHECK); -#endif -#if DFLT_BGP_SHOW_HOSTNAME - bgp_flag_set(bgp, BGP_FLAG_SHOW_HOSTNAME); -#endif -#if DFLT_BGP_LOG_NEIGHBOR_CHANGES - bgp_flag_set(bgp, BGP_FLAG_LOG_NEIGHBOR_CHANGES); -#endif -#if DFLT_BGP_DETERMINISTIC_MED - bgp_flag_set(bgp, BGP_FLAG_DETERMINISTIC_MED); -#endif bgp_addpath_init_bgp_data(&bgp->tx_addpath); bgp->as = *as; @@ -3176,7 +3167,7 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name, bgp_zebra_instance_register(bgp); } - return BGP_SUCCESS; + return BGP_CREATED; } /* @@ -4980,7 +4971,7 @@ int peer_timers_connect_unset(struct peer *peer) if (peer->connect) peer->v_connect = peer->connect; else - peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + peer->v_connect = peer->bgp->default_connect_retry; /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) @@ -4998,7 +4989,7 @@ int peer_timers_connect_unset(struct peer *peer) /* Remove flag and configuration on peer-group member. */ UNSET_FLAG(member->flags, PEER_FLAG_TIMER_CONNECT); member->connect = 0; - member->v_connect = BGP_DEFAULT_CONNECT_RETRY; + member->v_connect = peer->bgp->default_connect_retry; } return 0; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 568438b0ce..7d81579009 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -457,6 +457,7 @@ struct bgp { /* BGP default timer. */ uint32_t default_holdtime; uint32_t default_keepalive; + uint32_t default_connect_retry; /* BGP graceful restart */ uint32_t restart_time; @@ -1429,13 +1430,17 @@ struct bgp_nlri { #define BGP_EVENTS_MAX 15 /* BGP timers default value. */ -/* note: the DFLT_ ones depend on compile-time "defaults" selection */ #define BGP_INIT_START_TIMER 1 -#define BGP_DEFAULT_HOLDTIME DFLT_BGP_HOLDTIME -#define BGP_DEFAULT_KEEPALIVE DFLT_BGP_KEEPALIVE +/* The following 3 are RFC defaults that are overridden in bgp_vty.c with + * version-/profile-specific values. The values here do not matter, they only + * exist to provide a clear layering separation between core and CLI. + */ +#define BGP_DEFAULT_HOLDTIME 180 +#define BGP_DEFAULT_KEEPALIVE 60 +#define BGP_DEFAULT_CONNECT_RETRY 120 + #define BGP_DEFAULT_EBGP_ROUTEADV 0 #define BGP_DEFAULT_IBGP_ROUTEADV 0 -#define BGP_DEFAULT_CONNECT_RETRY DFLT_BGP_TIMERS_CONNECT /* BGP default local preference. */ #define BGP_DEFAULT_LOCAL_PREF 100 @@ -1480,6 +1485,7 @@ enum bgp_clear_type { /* BGP error codes. */ #define BGP_SUCCESS 0 +#define BGP_CREATED 1 #define BGP_ERR_INVALID_VALUE -1 #define BGP_ERR_INVALID_FLAG -2 #define BGP_ERR_INVALID_AS -3 @@ -1626,7 +1632,8 @@ extern int bgp_confederation_peers_check(struct bgp *, as_t); extern int bgp_confederation_peers_add(struct bgp *, as_t); extern int bgp_confederation_peers_remove(struct bgp *, as_t); -extern int bgp_timers_set(struct bgp *, uint32_t keepalive, uint32_t holdtime); +extern int bgp_timers_set(struct bgp *, uint32_t keepalive, uint32_t holdtime, + uint32_t connect_retry); extern int bgp_timers_unset(struct bgp *); extern int bgp_default_local_preference_set(struct bgp *, uint32_t); diff --git a/lib/defaults.h b/lib/defaults.h index b35bdfeaf3..4ccdfb5c1c 100644 --- a/lib/defaults.h +++ b/lib/defaults.h @@ -26,27 +26,11 @@ #ifdef HAVE_DATACENTER -#define DFLT_BGP_IMPORT_CHECK 1 -#define DFLT_BGP_TIMERS_CONNECT 10 -#define DFLT_BGP_HOLDTIME 9 -#define DFLT_BGP_KEEPALIVE 3 -#define DFLT_BGP_LOG_NEIGHBOR_CHANGES 1 -#define DFLT_BGP_SHOW_HOSTNAME 1 -#define DFLT_BGP_DETERMINISTIC_MED 1 - #define DFLT_OSPF_LOG_ADJACENCY_CHANGES 1 #define DFLT_OSPF6_LOG_ADJACENCY_CHANGES 1 #else /* !HAVE_DATACENTER */ -#define DFLT_BGP_IMPORT_CHECK 0 -#define DFLT_BGP_TIMERS_CONNECT 120 -#define DFLT_BGP_HOLDTIME 180 -#define DFLT_BGP_KEEPALIVE 60 -#define DFLT_BGP_LOG_NEIGHBOR_CHANGES 0 -#define DFLT_BGP_SHOW_HOSTNAME 0 -#define DFLT_BGP_DETERMINISTIC_MED 0 - #define DFLT_OSPF_LOG_ADJACENCY_CHANGES 0 #define DFLT_OSPF6_LOG_ADJACENCY_CHANGES 0 diff --git a/tests/bgpd/test_capability.c b/tests/bgpd/test_capability.c index 96e398512b..1b3f90434b 100644 --- a/tests/bgpd/test_capability.c +++ b/tests/bgpd/test_capability.c @@ -923,7 +923,7 @@ int main(void) if (fileno(stdout) >= 0) tty = isatty(fileno(stdout)); - if (bgp_get(&bgp, &asn, NULL, BGP_INSTANCE_TYPE_DEFAULT)) + if (bgp_get(&bgp, &asn, NULL, BGP_INSTANCE_TYPE_DEFAULT) < 0) return -1; peer = peer_create_accept(bgp); diff --git a/tests/bgpd/test_mp_attr.c b/tests/bgpd/test_mp_attr.c index fbf2a9fed2..af9e5791b7 100644 --- a/tests/bgpd/test_mp_attr.c +++ b/tests/bgpd/test_mp_attr.c @@ -1087,7 +1087,7 @@ int main(void) if (fileno(stdout) >= 0) tty = isatty(fileno(stdout)); - if (bgp_get(&bgp, &asn, NULL, BGP_INSTANCE_TYPE_DEFAULT)) + if (bgp_get(&bgp, &asn, NULL, BGP_INSTANCE_TYPE_DEFAULT) < 0) return -1; peer = peer_create_accept(bgp); diff --git a/tests/bgpd/test_packet.c b/tests/bgpd/test_packet.c index 7a038fb02e..d2c093fbea 100644 --- a/tests/bgpd/test_packet.c +++ b/tests/bgpd/test_packet.c @@ -63,7 +63,7 @@ int main(int argc, char *argv[]) vrf_init(NULL, NULL, NULL, NULL, NULL); bgp_option_set(BGP_OPT_NO_LISTEN); - if (bgp_get(&bgp, &asn, NULL, BGP_INSTANCE_TYPE_DEFAULT)) + if (bgp_get(&bgp, &asn, NULL, BGP_INSTANCE_TYPE_DEFAULT) < 0) return -1; peer = peer_create_accept(bgp);