From cadfa693d618f56c9fcffd2cd227917f15953711 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 13 Sep 2024 10:51:41 +0300 Subject: [PATCH 1/3] bgpd: Implement BGP dual-as feature This is helpful for migrations, etc. The neighbor is configured with: ``` router bgp 65000 neighbor X local-as 65001 no-prepend replace-as dual-as ``` Neighbor X can use either 65000, or 65001 to peer with. Closes: https://github.com/FRRouting/frr/issues/13928 Signed-off-by: Donatas Abraitis --- bgpd/bgp_packet.c | 13 +++++++++++++ bgpd/bgp_updgrp.c | 9 ++++++++- bgpd/bgp_vty.c | 30 ++++++++++++++++++++---------- bgpd/bgpd.c | 24 ++++++++++++++++-------- bgpd/bgpd.h | 3 ++- 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index fa03f1d21d..62be7ffbf7 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2702,6 +2702,19 @@ static int bgp_notify_receive(struct peer_connection *connection, inner.subcode == BGP_NOTIFY_OPEN_UNSUP_PARAM) UNSET_FLAG(peer->sflags, PEER_STATUS_CAPABILITY_OPEN); + /* Resend the next OPEN message with a global AS number if we received + * a `Bad Peer AS` notification. This is only valid if `dual-as` is + * configured. + */ + if (inner.code == BGP_NOTIFY_OPEN_ERR && + inner.subcode == BGP_NOTIFY_OPEN_BAD_PEER_AS && + CHECK_FLAG(peer->flags, PEER_FLAG_DUAL_AS)) { + if (peer->change_local_as != peer->bgp->as) + peer->change_local_as = peer->bgp->as; + else + peer->change_local_as = peer->local_as; + } + /* If Graceful-Restart N-bit (Notification) is exchanged, * and it's not a Hard Reset, let's retain the routes. */ diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index 115bc35cdc..90c43b938f 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -783,8 +783,11 @@ static int update_group_show_walkcb(struct update_group *updgrp, void *arg) json_updgrp, "replaceLocalAs", CHECK_FLAG(updgrp->conf->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS)); + json_object_boolean_add(json_updgrp, "dualAs", + CHECK_FLAG(updgrp->conf->flags, + PEER_FLAG_DUAL_AS)); } else { - vty_out(vty, " Local AS %u%s%s\n", + vty_out(vty, " Local AS %u%s%s%s\n", updgrp->conf->change_local_as, CHECK_FLAG(updgrp->conf->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND) @@ -793,6 +796,10 @@ static int update_group_show_walkcb(struct update_group *updgrp, void *arg) CHECK_FLAG(updgrp->conf->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) ? " replace-as" + : "", + CHECK_FLAG(updgrp->conf->flags, + PEER_FLAG_DUAL_AS) + ? " dual-as" : ""); } } diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index f669564bb8..cf4269eb66 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5451,7 +5451,7 @@ DEFUN (neighbor_local_as, return CMD_WARNING_CONFIG_FAILED; } - ret = peer_local_as_set(peer, as, 0, 0, argv[idx_number]->arg); + ret = peer_local_as_set(peer, as, 0, 0, 0, argv[idx_number]->arg); return bgp_vty_return(vty, ret); } @@ -5480,19 +5480,20 @@ DEFUN (neighbor_local_as_no_prepend, return CMD_WARNING_CONFIG_FAILED; } - ret = peer_local_as_set(peer, as, 1, 0, argv[idx_number]->arg); + ret = peer_local_as_set(peer, as, 1, 0, 0, argv[idx_number]->arg); return bgp_vty_return(vty, ret); } -DEFUN (neighbor_local_as_no_prepend_replace_as, +DEFPY (neighbor_local_as_no_prepend_replace_as, neighbor_local_as_no_prepend_replace_as_cmd, - "neighbor local-as ASNUM no-prepend replace-as", + "neighbor local-as ASNUM no-prepend replace-as [dual-as$dual_as]", NEIGHBOR_STR NEIGHBOR_ADDR_STR2 "Specify a local-as number\n" "AS number expressed in dotted or plain format used as local AS\n" "Do not prepend local-as to updates from ebgp peers\n" - "Do not prepend local-as to updates from ibgp peers\n") + "Do not prepend local-as to updates from ibgp peers\n" + "Allow peering with a global AS number or local-as number\n") { int idx_peer = 1; int idx_number = 3; @@ -5510,20 +5511,21 @@ DEFUN (neighbor_local_as_no_prepend_replace_as, return CMD_WARNING_CONFIG_FAILED; } - ret = peer_local_as_set(peer, as, 1, 1, argv[idx_number]->arg); + ret = peer_local_as_set(peer, as, 1, 1, dual_as, argv[idx_number]->arg); return bgp_vty_return(vty, ret); } DEFUN (no_neighbor_local_as, no_neighbor_local_as_cmd, - "no neighbor local-as [ASNUM [no-prepend [replace-as]]]", + "no neighbor local-as [ASNUM [no-prepend [replace-as] [dual-as]]]", NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2 "Specify a local-as number\n" "AS number expressed in dotted or plain format used as local AS\n" "Do not prepend local-as to updates from ebgp peers\n" - "Do not prepend local-as to updates from ibgp peers\n") + "Do not prepend local-as to updates from ibgp peers\n" + "Allow peering with a global AS number or local-as number\n") { int idx_peer = 2; struct peer *peer; @@ -14052,6 +14054,10 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, if (CHECK_FLAG(p->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS)) json_object_boolean_true_add(json_neigh, "localAsReplaceAs"); + + json_object_boolean_add(json_neigh, "localAsReplaceAsDualAs", + !!CHECK_FLAG(p->flags, + PEER_FLAG_DUAL_AS)); } else { if (p->as_type == AS_SPECIFIED || CHECK_FLAG(p->as_type, AS_AUTO) || @@ -14066,13 +14072,15 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, vty_out(vty, ASN_FORMAT(bgp->asnotation), p->change_local_as ? &p->change_local_as : &p->local_as); - vty_out(vty, "%s%s, ", + vty_out(vty, "%s%s%s, ", CHECK_FLAG(p->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND) ? " no-prepend" : "", CHECK_FLAG(p->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) ? " replace-as" - : ""); + : "", + CHECK_FLAG(p->flags, PEER_FLAG_DUAL_AS) ? " dual-as" + : ""); } /* peer type internal or confed-internal */ if ((p->as == p->local_as) || (CHECK_FLAG(p->as_type, AS_INTERNAL))) { @@ -18664,6 +18672,8 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, vty_out(vty, " no-prepend"); if (peergroup_flag_check(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS)) vty_out(vty, " replace-as"); + if (peergroup_flag_check(peer, PEER_FLAG_DUAL_AS)) + vty_out(vty, " dual-as"); vty_out(vty, "\n"); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 9d36ed9008..1108b82f4e 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4696,6 +4696,7 @@ static const struct peer_flag_action peer_flag_action_list[] = { {PEER_FLAG_LOCAL_AS, 0, peer_change_reset}, {PEER_FLAG_LOCAL_AS_NO_PREPEND, 0, peer_change_reset}, {PEER_FLAG_LOCAL_AS_REPLACE_AS, 0, peer_change_reset}, + {PEER_FLAG_DUAL_AS, 0, peer_change_reset}, {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}, @@ -6638,9 +6639,9 @@ int peer_allowas_in_unset(struct peer *peer, afi_t afi, safi_t safi) } int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, - bool replace_as, const char *as_str) + bool replace_as, bool dual_as, const char *as_str) { - bool old_no_prepend, old_replace_as; + bool old_no_prepend, old_replace_as, old_dual_as; struct bgp *bgp = peer->bgp; struct peer *member; struct listnode *node, *nnode; @@ -6653,14 +6654,16 @@ int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, !!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); old_replace_as = !!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + old_dual_as = !!CHECK_FLAG(peer->flags, PEER_FLAG_DUAL_AS); /* Set flag and configuration on peer. */ peer_flag_set(peer, PEER_FLAG_LOCAL_AS); peer_flag_modify(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND, no_prepend); peer_flag_modify(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS, replace_as); + peer_flag_modify(peer, PEER_FLAG_DUAL_AS, dual_as); - if (peer->change_local_as == as && old_no_prepend == no_prepend - && old_replace_as == replace_as) + if (peer->change_local_as == as && old_no_prepend == no_prepend && + old_replace_as == replace_as && old_dual_as == dual_as) return 0; peer->change_local_as = as; if (as_str) { @@ -6689,10 +6692,11 @@ int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, PEER_FLAG_LOCAL_AS_NO_PREPEND); old_replace_as = CHECK_FLAG(member->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); - if (member->change_local_as == as - && CHECK_FLAG(member->flags, PEER_FLAG_LOCAL_AS) - && old_no_prepend == no_prepend - && old_replace_as == replace_as) + old_dual_as = !!CHECK_FLAG(member->flags, PEER_FLAG_DUAL_AS); + if (member->change_local_as == as && + CHECK_FLAG(member->flags, PEER_FLAG_LOCAL_AS) && + old_no_prepend == no_prepend && + old_replace_as == replace_as && old_dual_as == dual_as) continue; /* Set flag and configuration on peer-group member. */ @@ -6701,6 +6705,7 @@ int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, no_prepend); COND_FLAG(member->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS, replace_as); + COND_FLAG(member->flags, PEER_FLAG_DUAL_AS, dual_as); member->change_local_as = as; if (as_str) member->change_local_as_pretty = XSTRDUP(MTYPE_BGP_NAME, @@ -6723,12 +6728,14 @@ int peer_local_as_unset(struct peer *peer) peer_flag_inherit(peer, PEER_FLAG_LOCAL_AS); peer_flag_inherit(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND); peer_flag_inherit(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS); + peer_flag_inherit(peer, PEER_FLAG_DUAL_AS); PEER_ATTR_INHERIT(peer, peer->group, change_local_as); } else { /* Otherwise remove flag and configuration from peer. */ peer_flag_unset(peer, PEER_FLAG_LOCAL_AS); peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND); peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS); + peer_flag_unset(peer, PEER_FLAG_DUAL_AS); peer->change_local_as = 0; XFREE(MTYPE_BGP_NAME, peer->change_local_as_pretty); } @@ -6760,6 +6767,7 @@ int peer_local_as_unset(struct peer *peer) UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS); UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + UNSET_FLAG(member->flags, PEER_FLAG_DUAL_AS); member->change_local_as = 0; XFREE(MTYPE_BGP_NAME, member->change_local_as_pretty); member->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index def12ee642..795e4fbc58 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1507,6 +1507,7 @@ struct peer { #define PEER_FLAG_CAPABILITY_FQDN (1ULL << 37) /* fqdn capability */ #define PEER_FLAG_AS_LOOP_DETECTION (1ULL << 38) /* as path loop detection */ #define PEER_FLAG_EXTENDED_LINK_BANDWIDTH (1ULL << 39) +#define PEER_FLAG_DUAL_AS (1ULL << 40) /* *GR-Disabled mode means unset PEER_FLAG_GRACEFUL_RESTART @@ -2443,7 +2444,7 @@ extern int peer_allowas_in_set(struct peer *, afi_t, safi_t, int, int); extern int peer_allowas_in_unset(struct peer *, afi_t, safi_t); extern int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, - bool replace_as, const char *as_str); + bool replace_as, bool dual_as, const char *as_str); extern int peer_local_as_unset(struct peer *); extern int peer_prefix_list_set(struct peer *, afi_t, safi_t, int, From 573fa26bc7e39785a092938c553952d829a56ef0 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 13 Sep 2024 10:59:30 +0300 Subject: [PATCH 2/3] doc: Document `neighbox X local-as Y replace-as no-prepend dual-as` Signed-off-by: Donatas Abraitis --- doc/user/bgp.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index b6d289fac9..69a77568c6 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -1818,7 +1818,7 @@ Configuring Peers Since sent prefix count is managed by update-groups, this option creates a separate update-group for outgoing updates. -.. clicmd:: neighbor PEER local-as AS-NUMBER [no-prepend] [replace-as] +.. clicmd:: neighbor PEER local-as AS-NUMBER [no-prepend [replace-as [dual-as]]] Specify an alternate AS for this BGP process when interacting with the specified peer. With no modifiers, the specified local-as is prepended to @@ -1834,6 +1834,10 @@ Configuring Peers Note that replace-as can only be specified if no-prepend is. + The ``dual-as`` keyword is used to configure the neighbor to establish a peering + session using the real autonomous-system number (``router bgp ASN``) or by using + the autonomous system number configured with the ``local-as``. + This command is only allowed for eBGP peers. .. clicmd:: neighbor as-override From e229b5bad3ffe402e85c8e5fe9590f2139bc020e Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 13 Sep 2024 11:28:09 +0300 Subject: [PATCH 3/3] tests: Test `neighbor X no-prepend replace-as dual-as` Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_dual_as/__init__.py | 0 tests/topotests/bgp_dual_as/r1/frr.conf | 11 ++ tests/topotests/bgp_dual_as/r2/frr.conf | 10 ++ .../topotests/bgp_dual_as/test_bgp_dual_as.py | 124 ++++++++++++++++++ 4 files changed, 145 insertions(+) create mode 100644 tests/topotests/bgp_dual_as/__init__.py create mode 100644 tests/topotests/bgp_dual_as/r1/frr.conf create mode 100644 tests/topotests/bgp_dual_as/r2/frr.conf create mode 100644 tests/topotests/bgp_dual_as/test_bgp_dual_as.py diff --git a/tests/topotests/bgp_dual_as/__init__.py b/tests/topotests/bgp_dual_as/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_dual_as/r1/frr.conf b/tests/topotests/bgp_dual_as/r1/frr.conf new file mode 100644 index 0000000000..9dcfe05d69 --- /dev/null +++ b/tests/topotests/bgp_dual_as/r1/frr.conf @@ -0,0 +1,11 @@ +! +interface r1-eth0 + ip address 10.0.0.1/24 +! +router bgp 65000 + no bgp ebgp-requires-policy + neighbor 10.0.0.2 remote-as 65002 + neighbor 10.0.0.2 local-as 65001 no-prepend replace-as dual-as + neighbor 10.0.0.2 timers 3 10 + neighbor 10.0.0.2 timers connect 1 +! diff --git a/tests/topotests/bgp_dual_as/r2/frr.conf b/tests/topotests/bgp_dual_as/r2/frr.conf new file mode 100644 index 0000000000..cf5731b601 --- /dev/null +++ b/tests/topotests/bgp_dual_as/r2/frr.conf @@ -0,0 +1,10 @@ +! +interface r2-eth0 + ip address 10.0.0.2/24 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.0.1 remote-as 65001 + neighbor 10.0.0.1 timers 3 10 + neighbor 10.0.0.1 timers connect 1 +! diff --git a/tests/topotests/bgp_dual_as/test_bgp_dual_as.py b/tests/topotests/bgp_dual_as/test_bgp_dual_as.py new file mode 100644 index 0000000000..fcac9c94ec --- /dev/null +++ b/tests/topotests/bgp_dual_as/test_bgp_dual_as.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# Copyright (c) 2024 by +# Donatas Abraitis +# + +import os +import sys +import json +import pytest +import functools + +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, get_topogen +from lib.common_config import step + +pytestmark = [pytest.mark.bgpd] + + +def build_topo(tgen): + r1 = tgen.add_router("r1") + r2 = tgen.add_router("r2") + + switch = tgen.add_switch("s1") + switch.add_link(r1) + switch.add_link(r2) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + for _, (rname, router) in enumerate(tgen.routers().items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_dual_as(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r2 = tgen.gears["r2"] + + def _bgp_converge_65001(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 summary json")) + expected = { + "ipv4Unicast": { + "as": 65000, + "peers": { + "10.0.0.2": { + "hostname": "r2", + "remoteAs": 65002, + "localAs": 65001, + "state": "Established", + "peerState": "OK", + } + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_converge_65001) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't establish BGP session using local-as AS 65001" + + step("Change remote-as from r2 to use global AS 65000") + r2.vtysh_cmd( + """ + configure terminal + router bgp + neighbor 10.0.0.1 remote-as 65000 + """ + ) + + def _bgp_converge_65000(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 summary json")) + expected = { + "ipv4Unicast": { + "as": 65000, + "peers": { + "10.0.0.2": { + "hostname": "r2", + "remoteAs": 65002, + "localAs": 65000, + "state": "Established", + "peerState": "OK", + } + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_converge_65000) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't establish BGP session using global AS 65000" + + +def test_memory_leak(): + "Run the memory leak test and report results." + tgen = get_topogen() + if not tgen.is_memleak_enabled(): + pytest.skip("Memory leak test/report is disabled") + + tgen.report_memory_leaks() + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))