diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 044e72cc1e..b034437a18 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -243,7 +243,7 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) peer->v_delayopen = from_peer->v_delayopen; peer->v_gr_restart = from_peer->v_gr_restart; peer->cap = from_peer->cap; - peer->neighbor_role = from_peer->neighbor_role; + peer->remote_role = from_peer->remote_role; status = peer->status; pstatus = peer->ostatus; last_evt = peer->last_event; @@ -1528,7 +1528,7 @@ int bgp_stop(struct peer *peer) peer->cap = 0; /* Resetting neighbor role to the default value */ - peer->neighbor_role = ROLE_UNDEFINE; + peer->remote_role = ROLE_UNDEFINED; FOREACH_AFI_SAFI (afi, safi) { /* Reset all negotiated variables */ diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index d8dd71788b..28cacd6086 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -900,7 +900,7 @@ static int bgp_capability_role(struct peer *peer, struct capability_header *hdr) } uint8_t role = stream_getc(BGP_INPUT(peer)); - peer->neighbor_role = role; + peer->remote_role = role; return 0; } @@ -1138,19 +1138,20 @@ static bool strict_capability_same(struct peer *peer) static bool bgp_role_violation(struct peer *peer) { uint8_t local_role = peer->local_role; - uint8_t neigh_role = peer->neighbor_role; + uint8_t remote_role = peer->remote_role; - if (local_role != ROLE_UNDEFINE && neigh_role != ROLE_UNDEFINE && - !((local_role == ROLE_PEER && neigh_role == ROLE_PEER) || - (local_role == ROLE_PROVIDER && neigh_role == ROLE_CUSTOMER) || - (local_role == ROLE_CUSTOMER && neigh_role == ROLE_PROVIDER) || - (local_role == ROLE_RS_SERVER && neigh_role == ROLE_RS_CLIENT) || - (local_role == ROLE_RS_CLIENT && neigh_role == ROLE_RS_SERVER))) { + if (local_role != ROLE_UNDEFINED && remote_role != ROLE_UNDEFINED && + !((local_role == ROLE_PEER && remote_role == ROLE_PEER) || + (local_role == ROLE_PROVIDER && remote_role == ROLE_CUSTOMER) || + (local_role == ROLE_CUSTOMER && remote_role == ROLE_PROVIDER) || + (local_role == ROLE_RS_SERVER && remote_role == ROLE_RS_CLIENT) || + (local_role == ROLE_RS_CLIENT && + remote_role == ROLE_RS_SERVER))) { bgp_notify_send(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_ROLE_MISMATCH); return true; } - if (neigh_role == ROLE_UNDEFINE && + if (remote_role == ROLE_UNDEFINED && CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE)) { const char *err_msg = "Strict mode. Please set the role on your side."; @@ -1729,7 +1730,7 @@ uint16_t bgp_open_capability(struct stream *s, struct peer *peer, stream_putc(s, CAPABILITY_CODE_EXT_MESSAGE_LEN); /* Role*/ - if (peer->local_role != ROLE_UNDEFINE) { + if (peer->local_role != ROLE_UNDEFINED) { SET_FLAG(peer->cap, PEER_CAP_ROLE_ADV); stream_putc(s, BGP_OPEN_OPT_CAP); stream_putc(s, CAPABILITY_CODE_ROLE_LEN + 2); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 03b66c7d45..baa894d67d 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -927,7 +927,7 @@ int bgp_vty_return(struct vty *vty, enum bgp_create_error_code ret) str = "Invalid role name"; break; case BGP_ERR_INVALID_INTERNAL_ROLE: - str = "Extrenal roles can be set only on eBGP session"; + str = "External roles can be set only on eBGP session"; break; } if (str) { @@ -6423,11 +6423,11 @@ static uint8_t get_role_by_name(const char *role_str) return ROLE_RS_SERVER; if (strncmp(role_str, "rs-client", 4) == 0) return ROLE_RS_CLIENT; - return ROLE_UNDEFINE; + return ROLE_UNDEFINED; } static int peer_role_set_vty(struct vty *vty, const char *ip_str, - const char *role_str, int strict_mode) + const char *role_str, bool strict_mode) { struct peer *peer; @@ -6436,7 +6436,7 @@ static int peer_role_set_vty(struct vty *vty, const char *ip_str, return CMD_WARNING_CONFIG_FAILED; uint8_t role = get_role_by_name(role_str); - if (role == ROLE_UNDEFINE) + if (role == ROLE_UNDEFINED) return bgp_vty_return(vty, BGP_ERR_INVALID_ROLE_NAME); return bgp_vty_return(vty, peer_role_set(peer, role, strict_mode)); } @@ -6451,7 +6451,7 @@ static int peer_role_unset_vty(struct vty *vty, const char *ip_str) return bgp_vty_return(vty, peer_role_unset(peer)); } -DEFUN(neighbor_role, +DEFPY(neighbor_role, neighbor_role_cmd, "neighbor local-role ", NEIGHBOR_STR @@ -6463,10 +6463,10 @@ DEFUN(neighbor_role, int idx_role = 3; return peer_role_set_vty(vty, argv[idx_peer]->arg, argv[idx_role]->arg, - 0); + false); } -DEFUN(neighbor_role_strict, +DEFPY(neighbor_role_strict, neighbor_role_strict_cmd, "neighbor local-role strict-mode", NEIGHBOR_STR @@ -6479,18 +6479,18 @@ DEFUN(neighbor_role_strict, int idx_role = 3; return peer_role_set_vty(vty, argv[idx_peer]->arg, argv[idx_role]->arg, - 1); + true); } -DEFUN(no_neighbor_role, +DEFPY(no_neighbor_role, no_neighbor_role_cmd, "no neighbor local-role [strict-mode]", NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2 - "Unset session role\n" + "Set session role\n" ROLE_STR - "Was used additional restriction on peer\n") + "Use additional restriction on peer\n") { int idx_peer = 2; @@ -12571,14 +12571,14 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, /* Roles */ if (use_json) { json_object_string_add(json_neigh, "localRole", - get_name_by_role(p->local_role)); - json_object_string_add(json_neigh, "neighRole", - get_name_by_role(p->neighbor_role)); + bgp_get_name_by_role(p->local_role)); + json_object_string_add(json_neigh, "remoteRole", + bgp_get_name_by_role(p->remote_role)); } else { vty_out(vty, " Local Role: %s\n", - get_name_by_role(p->local_role)); - vty_out(vty, " Neighbor Role: %s\n", - get_name_by_role(p->neighbor_role)); + bgp_get_name_by_role(p->local_role)); + vty_out(vty, " Remote Role: %s\n", + bgp_get_name_by_role(p->remote_role)); } @@ -16794,9 +16794,9 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, } /* role */ - if (peer->local_role != ROLE_UNDEFINE) { + if (peer->local_role != ROLE_UNDEFINED) { vty_out(vty, " neighbor %s local-role %s%s\n", addr, - get_name_by_role(peer->local_role), + bgp_get_name_by_role(peer->local_role), CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE) ? " strict-mode" : ""); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 4bf89a0375..7cfe5654ac 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1374,8 +1374,8 @@ struct peer *peer_new(struct bgp *bgp) peer->cur_event = peer->last_event = peer->last_major_event = 0; peer->bgp = bgp_lock(bgp); peer = peer_lock(peer); /* initial reference */ - peer->local_role = ROLE_UNDEFINE; - peer->neighbor_role = ROLE_UNDEFINE; + peer->local_role = ROLE_UNDEFINED; + peer->remote_role = ROLE_UNDEFINED; peer->password = NULL; peer->max_packet_size = BGP_STANDARD_MESSAGE_MAX_PACKET_SIZE; @@ -1999,12 +1999,12 @@ int peer_remote_as(struct bgp *bgp, union sockunion *su, const char *conf_if, return 0; } -const char *get_name_by_role(uint8_t role) +const char *bgp_get_name_by_role(uint8_t role) { static const char *const bgp_role_names[] = { "provider", "rs-server", "rs-client", "customer", "peer"}; - if (role == ROLE_UNDEFINE) - return "undefine"; + if (role == ROLE_UNDEFINED) + return "undefined"; if (role <= 5) return bgp_role_names[role]; return "unknown"; @@ -4247,6 +4247,7 @@ static const struct peer_flag_action peer_flag_action_list[] = { {PEER_FLAG_UPDATE_SOURCE, 0, peer_change_none}, {PEER_FLAG_DISABLE_LINK_BW_ENCODING_IEEE, 0, peer_change_none}, {PEER_FLAG_EXTENDED_OPT_PARAMS, 0, peer_change_reset}, + {PEER_FLAG_STRICT_MODE, 0, peer_change_reset}, {0, 0, 0}}; static const struct peer_flag_action peer_af_flag_action_list[] = { @@ -4917,7 +4918,7 @@ int peer_ebgp_multihop_unset(struct peer *peer) } /* Set Open Policy Role and check its correctness */ -int peer_role_set(struct peer *peer, uint8_t role, int strict_mode) +int peer_role_set(struct peer *peer, uint8_t role, bool strict_mode) { if (peer->local_role == role) { if (CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE) && @@ -4929,7 +4930,7 @@ int peer_role_set(struct peer *peer, uint8_t role, int strict_mode) SET_FLAG(peer->flags, PEER_FLAG_STRICT_MODE); /* Restart session to throw Role Mismatch Notification */ - if (peer->neighbor_role == ROLE_UNDEFINE) + if (peer->remote_role == ROLE_UNDEFINED) bgp_session_reset(peer); } } else { @@ -4950,7 +4951,7 @@ int peer_role_set(struct peer *peer, uint8_t role, int strict_mode) int peer_role_unset(struct peer *peer) { - return peer_role_set(peer, ROLE_UNDEFINE, 0); + return peer_role_set(peer, ROLE_UNDEFINED, 0); } /* Neighbor description. */ diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index b30a3059b9..26ac7a4c41 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1169,13 +1169,13 @@ struct peer { /* Roles in bgp session */ uint8_t local_role; - uint8_t neighbor_role; + uint8_t remote_role; #define ROLE_PROVIDER 0 #define ROLE_RS_SERVER 1 #define ROLE_RS_CLIENT 2 #define ROLE_CUSTOMER 3 #define ROLE_PEER 4 -#define ROLE_UNDEFINE 255 +#define ROLE_UNDEFINED 255 #define ROLE_NAME_MAX_LEN 20 @@ -2179,7 +2179,7 @@ extern int peer_ebgp_multihop_set(struct peer *, int); extern int peer_ebgp_multihop_unset(struct peer *); extern int is_ebgp_multihop_configured(struct peer *peer); -extern int peer_role_set(struct peer *peer, uint8_t role, int strict_mode); +extern int peer_role_set(struct peer *peer, uint8_t role, bool strict_mode); extern int peer_role_unset(struct peer *peer); extern void peer_description_set(struct peer *, const char *); @@ -2281,7 +2281,7 @@ extern void peer_tx_shutdown_message_set(struct peer *, const char *msg); extern void peer_tx_shutdown_message_unset(struct peer *); extern void bgp_route_map_update_timer(struct thread *thread); -extern const char *get_name_by_role(uint8_t role); +extern const char *bgp_get_name_by_role(uint8_t role); extern void bgp_route_map_terminate(void); diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 76af844b37..ee5c389bca 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -2653,7 +2653,12 @@ prevention, detection and mitigation. To enable its mechanics, you must set your local role to reflect your type of peering relationship with your neighbor. Possible values of ``LOCAL-ROLE`` are: -. + +- provider +- rs-server +- rs-client +- customer +- peer The local Role value is negotiated with the new BGP Role capability with a built-in check of the corresponding value. In case of mismatch the new OPEN @@ -2673,9 +2678,9 @@ The correct Role pairs are: Role: advertised and received If strict-mode is set BGP session won't become established until BGP neighbor -set local Role on its side. This configuratoin parameter is defined in +set local Role on its side. This configuration parameter is defined in :rfc:`9234` and used to enforce corresponding configuration at your -conter-part side. Default value - disabled. +counter-part side. Default value - disabled. Routes that sent from provider, rs-server, or peer local-role (or if received by customer, rs-clinet, or peer local-role) will be marked with a new @@ -2685,7 +2690,7 @@ Routes with this attribute can only be sent to your neighbor if your local-role is provider or rs-server. Routes with this attribute can be received only if your local-role is customer or rs-client. -In case of peer-peer relaitonship routes can be received only if +In case of peer-peer relationship routes can be received only if OTC value is equal to your neighbor AS number. All these rules with OTC help to detect and mitigate route leaks and @@ -2696,7 +2701,7 @@ happened automatically if local-role is set. This command set your local-role to ``LOCAL-ROLE``: . - This role help to detect and prevent route leaks. + This role helps to detect and prevent route leaks. If ``strict-mode`` is set, your neighbor must send you Capability with the value of his role (by setting local-role on his side). Otherwise, a Role diff --git a/tests/topotests/bgp_roles_capability/r1/bgpd.conf b/tests/topotests/bgp_roles_capability/r1/bgpd.conf index 4f152de960..4ec83093dc 100644 --- a/tests/topotests/bgp_roles_capability/r1/bgpd.conf +++ b/tests/topotests/bgp_roles_capability/r1/bgpd.conf @@ -4,12 +4,16 @@ router bgp 64501 ! Correct role pair neighbor 192.168.2.2 remote-as 64502 neighbor 192.168.2.2 local-role provider + neighbor 192.168.2.2 timers 3 10 ! Incorrect role pair neighbor 192.168.3.2 remote-as 64503 neighbor 192.168.3.2 local-role provider + neighbor 192.168.3.2 timers 3 10 ! Missed neighbor role neighbor 192.168.4.2 remote-as 64504 neighbor 192.168.4.2 local-role provider + neighbor 192.168.4.2 timers 3 10 ! Missed neighbor role with strict-mode neighbor 192.168.5.2 remote-as 64505 neighbor 192.168.5.2 local-role provider strict-mode + neighbor 192.168.5.2 timers 3 10 diff --git a/tests/topotests/bgp_roles_capability/r2/bgpd.conf b/tests/topotests/bgp_roles_capability/r2/bgpd.conf index efca633daa..e741aa16ce 100644 --- a/tests/topotests/bgp_roles_capability/r2/bgpd.conf +++ b/tests/topotests/bgp_roles_capability/r2/bgpd.conf @@ -2,3 +2,4 @@ router bgp 64502 bgp router-id 192.168.2.2 neighbor 192.168.2.1 remote-as 64501 neighbor 192.168.2.1 local-role customer + neighbor 192.168.2.1 timers 3 10 diff --git a/tests/topotests/bgp_roles_capability/r3/bgpd.conf b/tests/topotests/bgp_roles_capability/r3/bgpd.conf index 201c0afb2b..d1404260e6 100644 --- a/tests/topotests/bgp_roles_capability/r3/bgpd.conf +++ b/tests/topotests/bgp_roles_capability/r3/bgpd.conf @@ -1,3 +1,4 @@ router bgp 64503 neighbor 192.168.3.1 remote-as 64501 neighbor 192.168.3.1 local-role peer + neighbor 192.168.3.1 timers 3 10 diff --git a/tests/topotests/bgp_roles_capability/r4/bgpd.conf b/tests/topotests/bgp_roles_capability/r4/bgpd.conf index 30b97bb3a4..e6a0a9222a 100644 --- a/tests/topotests/bgp_roles_capability/r4/bgpd.conf +++ b/tests/topotests/bgp_roles_capability/r4/bgpd.conf @@ -1,2 +1,3 @@ router bgp 64504 neighbor 192.168.4.1 remote-as 64501 + neighbor 192.168.4.1 timers 3 10 diff --git a/tests/topotests/bgp_roles_capability/r5/bgpd.conf b/tests/topotests/bgp_roles_capability/r5/bgpd.conf index b4bf73fa3d..eeeed7ed30 100644 --- a/tests/topotests/bgp_roles_capability/r5/bgpd.conf +++ b/tests/topotests/bgp_roles_capability/r5/bgpd.conf @@ -1,2 +1,3 @@ router bgp 64505 neighbor 192.168.5.1 remote-as 64501 + neighbor 192.168.5.1 timers 3 10 diff --git a/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py b/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py index 55fc0972c1..28219da07b 100644 --- a/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py +++ b/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py @@ -82,7 +82,7 @@ def test_correct_pair(tgen): tgen.gears["r1"].vtysh_cmd(f"show bgp neighbors {neighbor_ip} json") )[neighbor_ip] assert neighbor_status["localRole"] == "provider" - assert neighbor_status["neighRole"] == "customer" + assert neighbor_status["remoteRole"] == "customer" assert neighbor_status["bgpState"] == "Established" assert ( neighbor_status["neighborCapabilities"].get("role") == "advertisedAndReceived" @@ -99,31 +99,31 @@ def test_role_pair_mismatch(tgen): def test_single_role_advertising(tgen): - # provider-undefine pair; we set role + # provider-undefined pair; we set role neighbor_ip = "192.168.4.2" neighbor_status = json.loads( tgen.gears["r1"].vtysh_cmd(f"show bgp neighbors {neighbor_ip} json") )[neighbor_ip] assert neighbor_status["localRole"] == "provider" - assert neighbor_status["neighRole"] == "undefine" + assert neighbor_status["remoteRole"] == "undefined" assert neighbor_status["bgpState"] == "Established" assert neighbor_status["neighborCapabilities"].get("role") == "advertised" def test_single_role_receiving(tgen): - # provider-undefine pair; we receive role + # provider-undefined pair; we receive role neighbor_ip = "192.168.4.1" neighbor_status = json.loads( tgen.gears["r4"].vtysh_cmd(f"show bgp neighbors {neighbor_ip} json") )[neighbor_ip] - assert neighbor_status["localRole"] == "undefine" - assert neighbor_status["neighRole"] == "provider" + assert neighbor_status["localRole"] == "undefined" + assert neighbor_status["remoteRole"] == "provider" assert neighbor_status["bgpState"] == "Established" assert neighbor_status["neighborCapabilities"].get("role") == "received" def test_role_strict_mode(tgen): - # provider-undefine pair bur strict-mode was set + # provider-undefined pair with strict-mode neighbor_ip = "192.168.5.2" neighbor_status = json.loads( tgen.gears["r1"].vtysh_cmd(f"show bgp neighbors {neighbor_ip} json") diff --git a/tests/topotests/bgp_roles_filtering/test_bgp_roles_filtering.py b/tests/topotests/bgp_roles_filtering/test_bgp_roles_filtering.py index 77116f474b..c739509b50 100644 --- a/tests/topotests/bgp_roles_filtering/test_bgp_roles_filtering.py +++ b/tests/topotests/bgp_roles_filtering/test_bgp_roles_filtering.py @@ -60,7 +60,7 @@ def tgen(request): BGP_CONVERGENCE = verify_bgp_convergence_from_running_config(tgen) assert BGP_CONVERGENCE, f"setup_module :Failed \n Error: {BGP_CONVERGENCE}" # Todo: What is the indented way to wait for convergence without json?! - time.sleep(3) + time.sleep(5) yield tgen tgen.stop_topology()