summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRuss White <russ@riw.us>2023-09-19 16:12:35 +0200
committerGitHub <noreply@github.com>2023-09-19 16:12:35 +0200
commit1e007847319252cb069a4c841517b027073ba324 (patch)
treeb4c1415b0323ba1f8b5b3c43cff5f4de4db151af
parentMerge pull request #14350 from FRIDM636/pcep-no-commands (diff)
parenttests: Check if LLGR settings can be changed via BGP dynamic capabilities (diff)
downloadfrr-1e007847319252cb069a4c841517b027073ba324.tar.xz
frr-1e007847319252cb069a4c841517b027073ba324.zip
Merge pull request #14382 from opensourcerouting/feature/long_lived_graceful_restart_dynamic_capability_split
bgpd: Handle LLGR capability using dynamic capabilities
-rw-r--r--bgpd/bgp_open.c12
-rw-r--r--bgpd/bgp_open.h17
-rw-r--r--bgpd/bgp_packet.c144
-rw-r--r--bgpd/bgp_vty.c22
-rw-r--r--tests/topotests/bgp_dynamic_capability/r1/bgpd.conf1
-rw-r--r--tests/topotests/bgp_dynamic_capability/r2/bgpd.conf1
-rw-r--r--tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py35
7 files changed, 200 insertions, 32 deletions
diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index da70f67c1..6ee5b5dc5 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -576,18 +576,6 @@ static int bgp_capability_restart(struct peer *peer,
static int bgp_capability_llgr(struct peer *peer,
struct capability_header *caphdr)
{
-/*
- * +--------------------------------------------------+
- * | Address Family Identifier (16 bits) |
- * +--------------------------------------------------+
- * | Subsequent Address Family Identifier (8 bits) |
- * +--------------------------------------------------+
- * | Flags for Address Family (8 bits) |
- * +--------------------------------------------------+
- * | Long-lived Stale Time (24 bits) |
- * +--------------------------------------------------+
- */
-#define BGP_CAP_LLGR_MIN_PACKET_LEN 7
struct stream *s = BGP_INPUT(peer);
size_t end = stream_get_getp(s) + caphdr->length;
diff --git a/bgpd/bgp_open.h b/bgpd/bgp_open.h
index 20f5fdb22..a92c56d1b 100644
--- a/bgpd/bgp_open.h
+++ b/bgpd/bgp_open.h
@@ -20,11 +20,24 @@ struct capability_mp_data {
};
struct graceful_restart_af {
- afi_t afi;
- safi_t safi;
+ uint16_t afi;
+ uint8_t safi;
uint8_t flag;
};
+/*
+ * +--------------------------------------------------+
+ * | Address Family Identifier (16 bits) |
+ * +--------------------------------------------------+
+ * | Subsequent Address Family Identifier (8 bits) |
+ * +--------------------------------------------------+
+ * | Flags for Address Family (8 bits) |
+ * +--------------------------------------------------+
+ * | Long-lived Stale Time (24 bits) |
+ * +--------------------------------------------------+
+ */
+#define BGP_CAP_LLGR_MIN_PACKET_LEN 7
+
/* Capability Code */
#define CAPABILITY_CODE_MP 1 /* Multiprotocol Extensions */
#define CAPABILITY_CODE_REFRESH 2 /* Route Refresh Capability */
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 6ae418b98..851432d56 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1211,6 +1211,8 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
unsigned long cap_len;
uint16_t len;
uint32_t gr_restart_time;
+ const char *capability = lookup_msg(capcode_str, capability_code,
+ "Unknown");
if (!peer_established(peer->connection))
return;
@@ -1254,12 +1256,12 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
stream_putc_at(s, cap_len, len);
if (bgp_debug_neighbor_events(peer))
- zlog_debug("%pBP sending CAPABILITY has %s Software Version for afi/safi: %s/%s",
+ zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s",
peer,
action == CAPABILITY_ACTION_SET
? "Advertising"
: "Removing",
- iana_afi2str(pkt_afi),
+ capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi));
break;
case CAPABILITY_CODE_MP:
@@ -1271,12 +1273,13 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
stream_putc(s, pkt_safi);
if (bgp_debug_neighbor_events(peer))
- zlog_debug(
- "%pBP sending CAPABILITY has %s MP_EXT CAP for afi/safi: %s/%s",
- peer,
- action == CAPABILITY_ACTION_SET ? "Advertising"
- : "Removing",
- iana_afi2str(pkt_afi), iana_safi2str(pkt_safi));
+ zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s",
+ peer,
+ action == CAPABILITY_ACTION_SET
+ ? "Advertising"
+ : "Removing",
+ capability, iana_afi2str(pkt_afi),
+ iana_safi2str(pkt_safi));
break;
case CAPABILITY_CODE_RESTART:
if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART) &&
@@ -1320,27 +1323,63 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
}
}
- /* Software Version capability Len. */
len = stream_get_endp(s) - cap_len - 1;
stream_putc_at(s, cap_len, len);
if (bgp_debug_neighbor_events(peer))
- zlog_debug("%pBP sending CAPABILITY has %s Graceful-Restart CAP for afi/safi: %s/%s",
+ zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s",
peer,
action == CAPABILITY_ACTION_SET
? "Advertising"
: "Removing",
- iana_afi2str(pkt_afi),
+ capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi));
break;
+ case CAPABILITY_CODE_LLGR:
+ if (!CHECK_FLAG(peer->cap, PEER_CAP_RESTART_ADV))
+ return;
+
+ SET_FLAG(peer->cap, PEER_CAP_LLGR_ADV);
+
+ stream_putc(s, action);
+ stream_putc(s, CAPABILITY_CODE_LLGR);
+ cap_len = stream_get_endp(s);
+ stream_putc(s, 0);
+
+ FOREACH_AFI_SAFI (afi, safi) {
+ if (!peer->afc[afi][safi])
+ continue;
+
+ bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi,
+ &pkt_safi);
+
+ stream_putw(s, pkt_afi);
+ stream_putc(s, pkt_safi);
+ stream_putc(s, LLGR_F_BIT);
+ stream_put3(s, peer->bgp->llgr_stale_time);
+
+ SET_FLAG(peer->af_cap[afi][safi], PEER_CAP_LLGR_AF_ADV);
+ }
+
+ len = stream_get_endp(s) - cap_len - 1;
+ stream_putc_at(s, cap_len, len);
+
+ if (bgp_debug_neighbor_events(peer))
+ zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s",
+ peer,
+ action == CAPABILITY_ACTION_SET
+ ? "Advertising"
+ : "Removing",
+ capability, iana_afi2str(pkt_afi),
+ iana_safi2str(pkt_safi));
+ break;
case CAPABILITY_CODE_REFRESH:
case CAPABILITY_CODE_ORF:
case CAPABILITY_CODE_AS4:
case CAPABILITY_CODE_DYNAMIC:
case CAPABILITY_CODE_ADDPATH:
case CAPABILITY_CODE_ENHANCED_RR:
- case CAPABILITY_CODE_LLGR:
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_EXT_MESSAGE:
@@ -2832,6 +2871,83 @@ static int bgp_route_refresh_receive(struct peer_connection *connection,
return BGP_PACKET_NOOP;
}
+static void bgp_dynamic_capability_llgr(uint8_t *pnt, int action,
+ struct capability_header *hdr,
+ struct peer *peer)
+{
+ uint8_t *data = pnt + 3;
+ uint8_t *end = data + hdr->length;
+ size_t len = end - data;
+
+ if (action == CAPABILITY_ACTION_SET) {
+ if (len < BGP_CAP_LLGR_MIN_PACKET_LEN) {
+ zlog_err("%pBP: Received invalid Long-Lived Graceful-Restart capability length %zu",
+ peer, len);
+ return;
+ }
+
+ SET_FLAG(peer->cap, PEER_CAP_LLGR_RCV);
+
+ while (data + BGP_CAP_LLGR_MIN_PACKET_LEN <= end) {
+ afi_t afi;
+ safi_t safi;
+ iana_afi_t pkt_afi;
+ iana_safi_t pkt_safi;
+ struct graceful_restart_af graf;
+
+ memcpy(&graf, data, sizeof(graf));
+ pkt_afi = ntohs(graf.afi);
+ pkt_safi = safi_int2iana(graf.safi);
+
+ /* Stale time is after AFI/SAFI/flags.
+ * It's encoded as 24 bits (= 3 bytes), so we need to
+ * put it into 32 bits.
+ */
+ uint32_t stale_time;
+ uint8_t *stale_time_ptr = data + 4;
+
+ stale_time = stale_time_ptr[0] << 16;
+ stale_time |= stale_time_ptr[1] << 8;
+ stale_time |= stale_time_ptr[2];
+
+ if (bgp_map_afi_safi_iana2int(pkt_afi, pkt_safi, &afi,
+ &safi)) {
+ if (bgp_debug_neighbor_events(peer))
+ zlog_debug("%s Addr-family %s/%s(afi/safi) not supported. Ignore the Long-lived Graceful Restart capability for this AFI/SAFI",
+ peer->host,
+ iana_afi2str(pkt_afi),
+ iana_safi2str(pkt_safi));
+ } else if (!peer->afc[afi][safi] ||
+ !CHECK_FLAG(peer->af_cap[afi][safi],
+ PEER_CAP_RESTART_AF_RCV)) {
+ if (bgp_debug_neighbor_events(peer))
+ zlog_debug("%s Addr-family %s/%s(afi/safi) not enabled. Ignore the Long-lived Graceful Restart capability",
+ peer->host,
+ iana_afi2str(pkt_afi),
+ iana_safi2str(pkt_safi));
+ } else {
+ if (bgp_debug_neighbor_events(peer))
+ zlog_debug("%s Addr-family %s/%s(afi/safi) Long-lived Graceful Restart capability stale time %u sec",
+ peer->host,
+ iana_afi2str(pkt_afi),
+ iana_safi2str(pkt_safi),
+ stale_time);
+
+ peer->llgr[afi][safi].flags = graf.flag;
+ peer->llgr[afi][safi].stale_time =
+ MIN(stale_time,
+ peer->bgp->llgr_stale_time);
+ SET_FLAG(peer->af_cap[afi][safi],
+ PEER_CAP_LLGR_AF_RCV);
+ }
+
+ data += BGP_CAP_LLGR_MIN_PACKET_LEN;
+ }
+ } else {
+ UNSET_FLAG(peer->cap, PEER_CAP_LLGR_RCV);
+ }
+}
+
static void bgp_dynamic_capability_graceful_restart(uint8_t *pnt, int action,
struct capability_header *hdr,
struct peer *peer)
@@ -3065,13 +3181,15 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
bgp_dynamic_capability_graceful_restart(pnt, action,
hdr, peer);
break;
+ case CAPABILITY_CODE_LLGR:
+ bgp_dynamic_capability_llgr(pnt, action, hdr, peer);
+ break;
case CAPABILITY_CODE_REFRESH:
case CAPABILITY_CODE_ORF:
case CAPABILITY_CODE_AS4:
case CAPABILITY_CODE_DYNAMIC:
case CAPABILITY_CODE_ADDPATH:
case CAPABILITY_CODE_ENHANCED_RR:
- case CAPABILITY_CODE_LLGR:
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_EXT_MESSAGE:
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index e76968cdb..5574afb0a 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -3619,10 +3619,16 @@ DEFUN(bgp_llgr_stalepath_time, bgp_llgr_stalepath_time_cmd,
VTY_DECLVAR_CONTEXT(bgp, bgp);
uint32_t llgr_stale_time;
+ struct listnode *node, *nnode;
+ struct peer *peer;
llgr_stale_time = strtoul(argv[3]->arg, NULL, 10);
bgp->llgr_stale_time = llgr_stale_time;
+ for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer))
+ bgp_capability_send(peer, AFI_IP, SAFI_UNICAST,
+ CAPABILITY_CODE_LLGR, CAPABILITY_ACTION_SET);
+
return CMD_SUCCESS;
}
@@ -3634,9 +3640,16 @@ DEFUN(no_bgp_llgr_stalepath_time, no_bgp_llgr_stalepath_time_cmd,
"Stale time value (seconds)\n")
{
VTY_DECLVAR_CONTEXT(bgp, bgp);
+ struct listnode *node, *nnode;
+ struct peer *peer;
bgp->llgr_stale_time = BGP_DEFAULT_LLGR_STALE_TIME;
+ for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer))
+ bgp_capability_send(peer, AFI_IP, SAFI_UNICAST,
+ CAPABILITY_CODE_LLGR,
+ CAPABILITY_ACTION_UNSET);
+
return CMD_SUCCESS;
}
@@ -12622,6 +12635,8 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi(
if (json) {
json_object_int_add(json_timer, "stalePathTimer",
peer->bgp->stalepath_time);
+ json_object_int_add(json_timer, "llgrStaleTime",
+ peer->llgr[afi][safi].stale_time);
if (peer->connection->t_gr_stale != NULL) {
json_object_int_add(json_timer,
@@ -12672,6 +12687,9 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi(
" Configured Selection Deferral Time(sec): %u\n",
peer->bgp->select_defer_time);
+ vty_out(vty, " LLGR Stale Path Time(sec): %u\n",
+ peer->llgr[afi][safi].stale_time);
+
if (peer->bgp->gr_info[afi][safi].t_select_deferral !=
NULL)
vty_out(vty,
@@ -12703,6 +12721,8 @@ static void bgp_show_neighbor_graceful_restart_time(struct vty *vty,
json_object_int_add(json_timer, "configuredRestartTimer",
p->bgp->restart_time);
+ json_object_int_add(json_timer, "configuredLlgrStaleTime",
+ p->bgp->llgr_stale_time);
json_object_int_add(json_timer, "receivedRestartTimer",
p->v_gr_restart);
@@ -12721,6 +12741,8 @@ static void bgp_show_neighbor_graceful_restart_time(struct vty *vty,
vty_out(vty, " Received Restart Time(sec): %u\n",
p->v_gr_restart);
+ vty_out(vty, " Configured LLGR Stale Path Time(sec): %u\n",
+ p->bgp->llgr_stale_time);
if (p->connection->t_gr_restart != NULL)
vty_out(vty, " Restart Time Remaining(sec): %ld\n",
event_timer_remain_second(
diff --git a/tests/topotests/bgp_dynamic_capability/r1/bgpd.conf b/tests/topotests/bgp_dynamic_capability/r1/bgpd.conf
index ad2fe4cd0..3d2c61177 100644
--- a/tests/topotests/bgp_dynamic_capability/r1/bgpd.conf
+++ b/tests/topotests/bgp_dynamic_capability/r1/bgpd.conf
@@ -4,6 +4,7 @@
router bgp 65001
no bgp ebgp-requires-policy
bgp graceful-restart
+ bgp long-lived stale-time 10
neighbor 192.168.1.2 remote-as external
neighbor 192.168.1.2 timers 1 3
neighbor 192.168.1.2 timers connect 1
diff --git a/tests/topotests/bgp_dynamic_capability/r2/bgpd.conf b/tests/topotests/bgp_dynamic_capability/r2/bgpd.conf
index 9b7292163..46f0b21a1 100644
--- a/tests/topotests/bgp_dynamic_capability/r2/bgpd.conf
+++ b/tests/topotests/bgp_dynamic_capability/r2/bgpd.conf
@@ -3,6 +3,7 @@
!
router bgp 65002
bgp graceful-restart
+ bgp long-lived stale-time 20
no bgp ebgp-requires-policy
neighbor 192.168.1.1 remote-as external
neighbor 192.168.1.1 timers 1 3
diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py
index a7feb3c58..846a68ea2 100644
--- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py
+++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py
@@ -6,8 +6,9 @@
#
"""
-Test if BGP graceful restart capability's restart time and notification
-flag are exchanged dynamically.
+Test if BGP graceful restart / long-lived graceful restart capabilities
+(restart time, stale time and notification flag) are exchanged dynamically
+via BGP dynamic capability.
"""
import os
@@ -70,11 +71,18 @@ def test_bgp_dynamic_capability_graceful_restart():
"neighborCapabilities": {
"dynamic": "advertisedAndReceived",
"gracefulRestart": "advertisedAndReceived",
+ "longLivedGracefulRestart": "advertisedAndReceived",
},
"gracefulRestartInfo": {
"nBit": True,
"timers": {
"receivedRestartTimer": 120,
+ "configuredLlgrStaleTime": 10,
+ },
+ "ipv4Unicast": {
+ "timers": {
+ "llgrStaleTime": 10,
+ }
},
},
"connectionsEstablished": 1,
@@ -89,17 +97,20 @@ def test_bgp_dynamic_capability_graceful_restart():
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Can't converge"
- step("Change Graceful-Restart restart-time, and check if it's changed dynamically")
+ step(
+ "Change Graceful-Restart restart-time, LLGR stale-time and check if they changed dynamically"
+ )
r2.vtysh_cmd(
"""
configure terminal
router bgp
bgp graceful-restart restart-time 123
+ bgp long-lived-graceful-restart stale-time 5
"""
)
- def _bgp_check_if_session_not_reset_after_changing_restart_time():
+ def _bgp_check_if_session_not_reset_after_changing_gr_settings():
output = json.loads(r1.vtysh_cmd("show bgp neighbor json"))
expected = {
"192.168.1.2": {
@@ -107,11 +118,18 @@ def test_bgp_dynamic_capability_graceful_restart():
"neighborCapabilities": {
"dynamic": "advertisedAndReceived",
"gracefulRestart": "advertisedAndReceived",
+ "longLivedGracefulRestart": "advertisedAndReceived",
},
"gracefulRestartInfo": {
"nBit": True,
"timers": {
"receivedRestartTimer": 123,
+ "configuredLlgrStaleTime": 10,
+ },
+ "ipv4Unicast": {
+ "timers": {
+ "llgrStaleTime": 5,
+ }
},
},
"connectionsEstablished": 1,
@@ -121,7 +139,7 @@ def test_bgp_dynamic_capability_graceful_restart():
return topotest.json_cmp(output, expected)
test_func = functools.partial(
- _bgp_check_if_session_not_reset_after_changing_restart_time,
+ _bgp_check_if_session_not_reset_after_changing_gr_settings,
)
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert (
@@ -148,11 +166,18 @@ def test_bgp_dynamic_capability_graceful_restart():
"neighborCapabilities": {
"dynamic": "advertisedAndReceived",
"gracefulRestart": "advertisedAndReceived",
+ "longLivedGracefulRestart": "advertisedAndReceived",
},
"gracefulRestartInfo": {
"nBit": False,
"timers": {
"receivedRestartTimer": 123,
+ "configuredLlgrStaleTime": 10,
+ },
+ "ipv4Unicast": {
+ "timers": {
+ "llgrStaleTime": 5,
+ }
},
},
"connectionsEstablished": 1,