summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--bgpd/bgp_fsm.c4
-rw-r--r--bgpd/bgp_open.c23
-rw-r--r--bgpd/bgp_vty.c38
-rw-r--r--bgpd/bgpd.c17
-rw-r--r--bgpd/bgpd.h8
-rw-r--r--doc/user/bgp.rst15
-rw-r--r--tests/topotests/bgp_roles_capability/r1/bgpd.conf4
-rw-r--r--tests/topotests/bgp_roles_capability/r2/bgpd.conf1
-rw-r--r--tests/topotests/bgp_roles_capability/r3/bgpd.conf1
-rw-r--r--tests/topotests/bgp_roles_capability/r4/bgpd.conf1
-rw-r--r--tests/topotests/bgp_roles_capability/r5/bgpd.conf1
-rw-r--r--tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py14
-rw-r--r--tests/topotests/bgp_roles_filtering/test_bgp_roles_filtering.py2
13 files changed, 72 insertions, 57 deletions
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;
-
- 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))) {
+ uint8_t remote_role = peer->remote_role;
+
+ 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 <A.B.C.D|X:X::X:X|WORD> local-role <provider|rs-server|rs-client|customer|peer>",
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 <A.B.C.D|X:X::X:X|WORD> local-role <provider|rs-server|rs-client|customer|peer> 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 <A.B.C.D|X:X::X:X|WORD> local-role <provider|rs-server|rs-client|customer|peer> [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>.
+
+- 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``:
<provider|rs-server|rs-client|customer|peer>.
- 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()
generated by cgit v1.2.3 (git 2.39.1) at 2025-12-10 17:13:44 +0000