diff options
author | Russ White <russ@riw.us> | 2023-09-19 16:12:35 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-19 16:12:35 +0200 |
commit | 1e007847319252cb069a4c841517b027073ba324 (patch) | |
tree | b4c1415b0323ba1f8b5b3c43cff5f4de4db151af | |
parent | Merge pull request #14350 from FRIDM636/pcep-no-commands (diff) | |
parent | tests: Check if LLGR settings can be changed via BGP dynamic capabilities (diff) | |
download | frr-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.c | 12 | ||||
-rw-r--r-- | bgpd/bgp_open.h | 17 | ||||
-rw-r--r-- | bgpd/bgp_packet.c | 144 | ||||
-rw-r--r-- | bgpd/bgp_vty.c | 22 | ||||
-rw-r--r-- | tests/topotests/bgp_dynamic_capability/r1/bgpd.conf | 1 | ||||
-rw-r--r-- | tests/topotests/bgp_dynamic_capability/r2/bgpd.conf | 1 | ||||
-rw-r--r-- | tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py | 35 |
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, |