Skip to content

Commit

Permalink
Merge pull request FRRouting#14154 from opensourcerouting/feature/bgp…
Browse files Browse the repository at this point in the history
…d_handle_role_capability_using_dynamic_capability

bgpd: Handle role capability using dynamic capability
  • Loading branch information
riw777 authored Aug 8, 2023
2 parents 2352bbf + 50c5908 commit a84dee7
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 42 deletions.
30 changes: 30 additions & 0 deletions bgpd/bgp_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,13 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
unsigned long cap_len;
uint16_t len;

if (!peer_established(peer))
return;

if (!CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_RCV) &&
!CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_ADV))
return;

/* Convert AFI, SAFI to values for packet. */
bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi, &pkt_safi);

Expand Down Expand Up @@ -1272,7 +1279,15 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_EXT_MESSAGE:
break;
case CAPABILITY_CODE_ROLE:
if (peer->local_role != ROLE_UNDEFINED) {
SET_FLAG(peer->cap, PEER_CAP_ROLE_ADV);
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_ROLE);
stream_putc(s, CAPABILITY_CODE_ROLE_LEN);
stream_putc(s, peer->local_role);
}
break;
default:
break;
Expand Down Expand Up @@ -2879,7 +2894,22 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_EXT_MESSAGE:
break;
case CAPABILITY_CODE_ROLE:
SET_FLAG(peer->cap, PEER_CAP_ROLE_RCV);
if (hdr->length != CAPABILITY_CODE_ROLE_LEN) {
flog_warn(EC_BGP_CAPABILITY_INVALID_LENGTH,
"Role: Received invalid length %d",
hdr->length);
bgp_notify_send(peer, BGP_NOTIFY_CEASE,
BGP_NOTIFY_SUBCODE_UNSPECIFIC);
return BGP_Stop;
}
uint8_t role;

memcpy(&role, pnt + 3, sizeof(role));

peer->remote_role = role;
break;
default:
flog_warn(
Expand Down
81 changes: 44 additions & 37 deletions bgpd/bgp_vty.c
Original file line number Diff line number Diff line change
Expand Up @@ -5738,14 +5738,10 @@ DEFPY(neighbor_capability_software_version,
ret = peer_flag_set_vty(vty, neighbor,
PEER_FLAG_CAPABILITY_SOFT_VERSION);

if (peer_established(peer)) {
if (CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_RCV) &&
CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_ADV))
bgp_capability_send(peer, AFI_IP, SAFI_UNICAST,
CAPABILITY_CODE_SOFT_VERSION,
no ? CAPABILITY_ACTION_UNSET
: CAPABILITY_ACTION_SET);
}
bgp_capability_send(peer, AFI_IP, SAFI_UNICAST,
CAPABILITY_CODE_SOFT_VERSION,
no ? CAPABILITY_ACTION_UNSET
: CAPABILITY_ACTION_SET);

return ret;
}
Expand Down Expand Up @@ -6786,75 +6782,86 @@ static uint8_t get_role_by_name(const char *role_str)
return ROLE_UNDEFINED;
}

static int peer_role_set_vty(struct vty *vty, const char *ip_str,
static int peer_role_set_vty(struct vty *vty, struct peer *peer,
const char *role_str, bool strict_mode)
{
struct peer *peer;

peer = peer_and_group_lookup_vty(vty, ip_str);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;
uint8_t role = get_role_by_name(role_str);

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));
}

static int peer_role_unset_vty(struct vty *vty, const char *ip_str)
{
struct peer *peer;

peer = peer_and_group_lookup_vty(vty, ip_str);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;
return bgp_vty_return(vty, peer_role_unset(peer));
}

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 <A.B.C.D|X:X::X:X|WORD>$neighbor local-role <provider|rs-server|rs-client|customer|peer>$role",
NEIGHBOR_STR
NEIGHBOR_ADDR_STR2
"Set session role\n"
ROLE_STR)
{
int idx_peer = 1;
int idx_role = 3;
int ret;
struct peer *peer;

peer = peer_and_group_lookup_vty(vty, neighbor);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;

ret = peer_role_set_vty(vty, peer, role, false);

return peer_role_set_vty(vty, argv[idx_peer]->arg, argv[idx_role]->arg,
false);
bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, CAPABILITY_CODE_ROLE,
CAPABILITY_ACTION_SET);

return ret;
}

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 <A.B.C.D|X:X::X:X|WORD>$neighbor local-role <provider|rs-server|rs-client|customer|peer>$role strict-mode",
NEIGHBOR_STR
NEIGHBOR_ADDR_STR2
"Set session role\n"
ROLE_STR
"Use additional restriction on peer\n")
{
int idx_peer = 1;
int idx_role = 3;
int ret;
struct peer *peer;

peer = peer_and_group_lookup_vty(vty, neighbor);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;

ret = peer_role_set_vty(vty, peer, role, true);

bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, CAPABILITY_CODE_ROLE,
CAPABILITY_ACTION_SET);

return peer_role_set_vty(vty, argv[idx_peer]->arg, argv[idx_role]->arg,
true);
return ret;
}

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 neighbor <A.B.C.D|X:X::X:X|WORD>$neighbor local-role <provider|rs-server|rs-client|customer|peer> [strict-mode]",
NO_STR
NEIGHBOR_STR
NEIGHBOR_ADDR_STR2
"Set session role\n"
ROLE_STR
"Use additional restriction on peer\n")
{
int idx_peer = 2;
int ret;
struct peer *peer;

return peer_role_unset_vty(vty, argv[idx_peer]->arg);
peer = peer_and_group_lookup_vty(vty, neighbor);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;

ret = bgp_vty_return(vty, peer_role_unset(peer));

bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, CAPABILITY_CODE_ROLE,
CAPABILITY_ACTION_UNSET);

return ret;
}

/* disable-connected-check */
Expand Down
6 changes: 2 additions & 4 deletions bgpd/bgpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -4444,8 +4444,8 @@ 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_ROLE_STRICT_MODE, 0, peer_change_reset},
{PEER_FLAG_ROLE, 0, peer_change_reset},
{PEER_FLAG_ROLE_STRICT_MODE, 0, peer_change_none},
{PEER_FLAG_ROLE, 0, peer_change_none},
{PEER_FLAG_PORT, 0, peer_change_reset},
{PEER_FLAG_AIGP, 0, peer_change_none},
{PEER_FLAG_GRACEFUL_SHUTDOWN, 0, peer_change_none},
Expand Down Expand Up @@ -5167,7 +5167,6 @@ int peer_role_set(struct peer *peer, uint8_t role, bool strict_mode)
else
UNSET_FLAG(peer->flags,
PEER_FLAG_ROLE_STRICT_MODE);
bgp_session_reset(peer);
}

return CMD_SUCCESS;
Expand Down Expand Up @@ -5212,7 +5211,6 @@ int peer_role_set(struct peer *peer, uint8_t role, bool strict_mode)
UNSET_FLAG(member->flags,
PEER_FLAG_ROLE_STRICT_MODE);
}
bgp_session_reset(member);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
#!/usr/bin/env python
# SPDX-License-Identifier: ISC

# Copyright (c) 2023 by
# Donatas Abraitis <[email protected]>
#

"""
Test if role capability is exchanged dynamically.
"""

import os
import re
import sys
import json
import pytest
import functools

pytestmark = pytest.mark.bgpd

CWD = os.path.dirname(os.path.realpath(__file__))
sys.path.append(os.path.join(CWD, "../"))

# pylint: disable=C0413
from lib import topotest
from lib.topogen import Topogen, TopoRouter, get_topogen
from lib.common_config import step

pytestmark = [pytest.mark.bgpd]


def setup_module(mod):
topodef = {"s1": ("r1", "r2")}
tgen = Topogen(topodef, mod.__name__)
tgen.start_topology()

router_list = tgen.routers()

for i, (rname, router) in enumerate(router_list.items(), 1):
router.load_config(
TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))
)
router.load_config(
TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname))
)

tgen.start_router()


def teardown_module(mod):
tgen = get_topogen()
tgen.stop_topology()


def test_bgp_dynamic_capability_role():
tgen = get_topogen()

if tgen.routers_have_failure():
pytest.skip(tgen.errors)

r1 = tgen.gears["r1"]
r2 = tgen.gears["r2"]

def _bgp_converge():
output = json.loads(r1.vtysh_cmd("show bgp neighbor json"))
expected = {
"192.168.1.2": {
"bgpState": "Established",
"localRole": "undefined",
"remoteRole": "undefined",
"neighborCapabilities": {
"dynamic": "advertisedAndReceived",
},
"connectionsEstablished": 1,
"connectionsDropped": 0,
}
}
return topotest.json_cmp(output, expected)

test_func = functools.partial(
_bgp_converge,
)
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Can't converge"

step("Set local-role and check if it's exchanged dynamically")

r1.vtysh_cmd(
"""
configure terminal
router bgp
neighbor 192.168.1.2 local-role customer
"""
)

r2.vtysh_cmd(
"""
configure terminal
router bgp
neighbor 192.168.1.1 local-role provider
"""
)

def _bgp_check_if_session_not_reset():
output = json.loads(r1.vtysh_cmd("show bgp neighbor json"))
expected = {
"192.168.1.2": {
"bgpState": "Established",
"localRole": "customer",
"remoteRole": "provider",
"neighborCapabilities": {
"dynamic": "advertisedAndReceived",
"role": "advertisedAndReceived",
},
"connectionsEstablished": 1,
"connectionsDropped": 0,
}
}
return topotest.json_cmp(output, expected)

test_func = functools.partial(
_bgp_check_if_session_not_reset,
)
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Session was reset after setting role capability"


if __name__ == "__main__":
args = ["-s"] + sys.argv[1:]
sys.exit(pytest.main(args))
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def teardown_module(mod):
tgen.stop_topology()


def test_bgp_dynamic_capability():
def test_bgp_dynamic_capability_software_version():
tgen = get_topogen()

if tgen.routers_have_failure():
Expand Down

0 comments on commit a84dee7

Please sign in to comment.