From 85e9cd9aae0f590cce91ffedf96ba311f02e5756 Mon Sep 17 00:00:00 2001 From: Adriano Marto Reis Date: Sun, 3 Jan 2021 14:56:48 +1000 Subject: [PATCH 1/3] bgpd: bgpd listening on multiple addresses Changed bgpd so multiple IP addresses can be specified via -l option. Signed-off-by: "Adriano Marto Reis" --- bgpd/bgp_main.c | 24 ++++++++++++++++++------ bgpd/bgpd.c | 30 +++++++++++++++++++++++------- bgpd/bgpd.h | 8 ++++---- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c index f961647778..287555b1fc 100644 --- a/bgpd/bgp_main.c +++ b/bgpd/bgp_main.c @@ -253,6 +253,7 @@ static __attribute__((__noreturn__)) void bgp_exit(int status) bf_free(bm->rd_idspace); list_delete(&bm->bgp); + list_delete(&bm->addresses); bgp_lp_finish(); @@ -404,12 +405,16 @@ int main(int argc, char **argv) int tmp_port; int bgp_port = BGP_PORT_DEFAULT; - char *bgp_address = NULL; + struct list *addresses = list_new(); int no_fib_flag = 0; int no_zebra_flag = 0; int skip_runas = 0; int instance = 0; int buffer_size = BGP_SOCKET_SNDBUF_SIZE; + char *address; + struct listnode *node; + + addresses->cmp = (int (*)(void *, void *))strcmp; frr_preinit(&bgpd_di, argc, argv); frr_opt_add( @@ -463,7 +468,7 @@ int main(int argc, char **argv) break; } case 'l': - bgp_address = optarg; + listnode_add_sort_nodup(addresses, optarg); /* listenon implies -n */ /* fallthru */ case 'n': @@ -493,11 +498,10 @@ int main(int argc, char **argv) memset(&bgpd_privs, 0, sizeof(bgpd_privs)); /* BGP master init. */ - bgp_master_init(frr_init(), buffer_size); + bgp_master_init(frr_init(), buffer_size, addresses); bm->port = bgp_port; if (bgp_port == 0) bgp_option_set(BGP_OPT_NO_LISTEN); - bm->address = bgp_address; if (no_fib_flag || no_zebra_flag) bgp_option_set(BGP_OPT_NO_FIB); if (no_zebra_flag) @@ -513,8 +517,16 @@ int main(int argc, char **argv) /* BGP related initialization. */ bgp_init((unsigned short)instance); - snprintf(bgpd_di.startinfo, sizeof(bgpd_di.startinfo), ", bgp@%s:%d", - (bm->address ? bm->address : ""), bm->port); + if (list_isempty(bm->addresses)) { + snprintf(bgpd_di.startinfo, sizeof(bgpd_di.startinfo), + ", bgp@:%d", bm->port); + } else { + for (ALL_LIST_ELEMENTS_RO(bm->addresses, node, address)) + snprintf(bgpd_di.startinfo + strlen(bgpd_di.startinfo), + sizeof(bgpd_di.startinfo) + - strlen(bgpd_di.startinfo), + ", bgp@%s:%d", address, bm->port); + } frr_config_fork(); /* must be called after fork() */ diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 026b57193e..2c9403b86d 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -121,12 +121,20 @@ extern struct zclient *zclient; static int bgp_check_main_socket(bool create, struct bgp *bgp) { static int bgp_server_main_created; + struct listnode *node; + char *address; if (create) { if (bgp_server_main_created) return 0; - if (bgp_socket(bgp, bm->port, bm->address) < 0) - return BGP_ERR_INVALID_VALUE; + if (list_isempty(bm->addresses)) { + if (bgp_socket(bgp, bm->port, NULL) < 0) + return BGP_ERR_INVALID_VALUE; + } else { + for (ALL_LIST_ELEMENTS_RO(bm->addresses, node, address)) + if (bgp_socket(bgp, bm->port, address) < 0) + return BGP_ERR_INVALID_VALUE; + } bgp_server_main_created = 1; return 0; } @@ -3280,7 +3288,8 @@ struct bgp *bgp_get_evpn(void) int bgp_handle_socket(struct bgp *bgp, struct vrf *vrf, vrf_id_t old_vrf_id, bool create) { - int ret = 0; + struct listnode *node; + char *address; /* Create BGP server socket, if listen mode not disabled */ if (!bgp || bgp_option_check(BGP_OPT_NO_LISTEN)) @@ -3309,9 +3318,14 @@ int bgp_handle_socket(struct bgp *bgp, struct vrf *vrf, vrf_id_t old_vrf_id, */ if (vrf->vrf_id == VRF_UNKNOWN) return 0; - ret = bgp_socket(bgp, bm->port, bm->address); - if (ret < 0) - return BGP_ERR_INVALID_VALUE; + if (list_isempty(bm->addresses)) { + if (bgp_socket(bgp, bm->port, NULL) < 0) + return BGP_ERR_INVALID_VALUE; + } else { + for (ALL_LIST_ELEMENTS_RO(bm->addresses, node, address)) + if (bgp_socket(bgp, bm->port, address) < 0) + return BGP_ERR_INVALID_VALUE; + } return 0; } else return bgp_check_main_socket(create, bgp); @@ -7425,7 +7439,8 @@ char *peer_uptime(time_t uptime2, char *buf, size_t len, bool use_json, return buf; } -void bgp_master_init(struct thread_master *master, const int buffer_size) +void bgp_master_init(struct thread_master *master, const int buffer_size, + struct list *addresses) { qobj_init(); @@ -7435,6 +7450,7 @@ void bgp_master_init(struct thread_master *master, const int buffer_size) bm->bgp = list_new(); bm->listen_sockets = list_new(); bm->port = BGP_PORT_DEFAULT; + bm->addresses = addresses; bm->master = master; bm->start_time = bgp_clock(); bm->t_rmap_update = NULL; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 16210bed15..cfe707ff9a 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -124,8 +124,8 @@ struct bgp_master { /* BGP port number. */ uint16_t port; - /* Listener address */ - char *address; + /* Listener addresses */ + struct list *addresses; /* The Mac table */ struct hash *self_mac_hash; @@ -1852,8 +1852,8 @@ extern char *peer_uptime(time_t uptime2, char *buf, size_t len, bool use_json, extern int bgp_config_write(struct vty *); -extern void bgp_master_init(struct thread_master *master, - const int buffer_size); +extern void bgp_master_init(struct thread_master *master, const int buffer_size, + struct list *addresses); extern void bgp_init(unsigned short instance); extern void bgp_pthreads_run(void); From d1aed873ca4a35e43f482c31667b733fbca04a67 Mon Sep 17 00:00:00 2001 From: Adriano Marto Reis Date: Sun, 3 Jan 2021 14:58:55 +1000 Subject: [PATCH 2/3] doc: Explaining that bgpd can listen on multiple addresses Updated the documentation clarifying that multiple addresses can be specifyed via -l option. Signed-off-by: "Adriano Marto Reis" --- doc/user/bgp.rst | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 7bef7d19c8..a49c260264 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -31,12 +31,23 @@ be specified (:ref:`common-invocation-options`). .. option:: -l, --listenon - Specify a specific IP address for bgpd to listen on, rather than its default + Specify specific IP addresses for bgpd to listen on, rather than its default of ``0.0.0.0`` / ``::``. This can be useful to constrain bgpd to an internal - address, or to run multiple bgpd processes on one host. + address, or to run multiple bgpd processes on one host. Multiple addresses + can be specified. + + In the following example, bgpd is started listening for connections on the + addresses 100.0.1.2 and fd00::2:2. The options -d (runs in daemon mode) and + -f (uses specific configuration file) are also used in this example as we + are likely to run multiple bgpd instances, each one with different + configurations, when using -l option. Note that this option implies the --no_kernel option, and no learned routes will be installed into the linux kernel. +.. code-block:: shell + + # /usr/lib/frr/bgpd -d -f /some-folder/bgpd.conf -l 100.0.1.2 -l fd00::2:2 + .. option:: -n, --no_kernel Do not install learned routes into the linux kernel. This option is useful From 733367c04e6040486077db2fdba19fcc0024fa8f Mon Sep 17 00:00:00 2001 From: Adriano Marto Reis Date: Sun, 3 Jan 2021 15:02:29 +1000 Subject: [PATCH 3/3] tests: tests for bgpd listing on multiple addresses * Added a new topotest to test bgpd listening on multiple addresses. * Updated the existing bgpd tests according to the parameter added to bgp_master_init. Signed-off-by: "Adriano Marto Reis" --- tests/bgpd/test_aspath.c | 3 +- tests/bgpd/test_capability.c | 2 +- tests/bgpd/test_mp_attr.c | 2 +- tests/bgpd/test_mpath.c | 2 +- tests/bgpd/test_packet.c | 2 +- tests/bgpd/test_peer_attr.c | 2 +- .../bgp_listen_on_multiple_addresses.json | 154 +++++++++++++++++ .../test_bgp_listen_on_multiple_addresses.py | 160 ++++++++++++++++++ 8 files changed, 321 insertions(+), 6 deletions(-) create mode 100644 tests/topotests/bgp_listen_on_multiple_addresses/bgp_listen_on_multiple_addresses.json create mode 100755 tests/topotests/bgp_listen_on_multiple_addresses/test_bgp_listen_on_multiple_addresses.py diff --git a/tests/bgpd/test_aspath.c b/tests/bgpd/test_aspath.c index 439891b559..936ffaaad5 100644 --- a/tests/bgpd/test_aspath.c +++ b/tests/bgpd/test_aspath.c @@ -1265,7 +1265,8 @@ int main(void) { int i = 0; qobj_init(); - bgp_master_init(thread_master_create(NULL), BGP_SOCKET_SNDBUF_SIZE); + bgp_master_init(thread_master_create(NULL), BGP_SOCKET_SNDBUF_SIZE, + list_new()); master = bm->master; bgp_option_set(BGP_OPT_NO_LISTEN); bgp_attr_init(); diff --git a/tests/bgpd/test_capability.c b/tests/bgpd/test_capability.c index 1b3f90434b..153b83897d 100644 --- a/tests/bgpd/test_capability.c +++ b/tests/bgpd/test_capability.c @@ -912,7 +912,7 @@ int main(void) qobj_init(); master = thread_master_create(NULL); - bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE); + bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE, list_new()); vrf_init(NULL, NULL, NULL, NULL, NULL); bgp_option_set(BGP_OPT_NO_LISTEN); diff --git a/tests/bgpd/test_mp_attr.c b/tests/bgpd/test_mp_attr.c index 7fabaad7fa..f510760913 100644 --- a/tests/bgpd/test_mp_attr.c +++ b/tests/bgpd/test_mp_attr.c @@ -1086,7 +1086,7 @@ int main(void) cmd_init(0); bgp_vty_init(); master = thread_master_create("test mp attr"); - bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE); + bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE, list_new()); vrf_init(NULL, NULL, NULL, NULL, NULL); bgp_option_set(BGP_OPT_NO_LISTEN); bgp_attr_init(); diff --git a/tests/bgpd/test_mpath.c b/tests/bgpd/test_mpath.c index 520c460f15..92efd4c3d6 100644 --- a/tests/bgpd/test_mpath.c +++ b/tests/bgpd/test_mpath.c @@ -393,7 +393,7 @@ static int global_test_init(void) qobj_init(); master = thread_master_create(NULL); zclient = zclient_new(master, &zclient_options_default); - bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE); + bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE, list_new()); vrf_init(NULL, NULL, NULL, NULL, NULL); bgp_option_set(BGP_OPT_NO_LISTEN); diff --git a/tests/bgpd/test_packet.c b/tests/bgpd/test_packet.c index d2c093fbea..db5918745d 100644 --- a/tests/bgpd/test_packet.c +++ b/tests/bgpd/test_packet.c @@ -59,7 +59,7 @@ int main(int argc, char *argv[]) qobj_init(); bgp_attr_init(); master = thread_master_create(NULL); - bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE); + bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE, list_new()); vrf_init(NULL, NULL, NULL, NULL, NULL); bgp_option_set(BGP_OPT_NO_LISTEN); diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index 0a15886c10..123d97bc97 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -1399,7 +1399,7 @@ static void bgp_startup(void) master = thread_master_create(NULL); yang_init(true); nb_init(master, bgpd_yang_modules, array_size(bgpd_yang_modules), false); - bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE); + bgp_master_init(master, BGP_SOCKET_SNDBUF_SIZE, list_new()); bgp_option_set(BGP_OPT_NO_LISTEN); vrf_init(NULL, NULL, NULL, NULL, NULL); frr_pthread_init(); diff --git a/tests/topotests/bgp_listen_on_multiple_addresses/bgp_listen_on_multiple_addresses.json b/tests/topotests/bgp_listen_on_multiple_addresses/bgp_listen_on_multiple_addresses.json new file mode 100644 index 0000000000..95de8cc10c --- /dev/null +++ b/tests/topotests/bgp_listen_on_multiple_addresses/bgp_listen_on_multiple_addresses.json @@ -0,0 +1,154 @@ +{ + "ipv4base": "10.0.0.0", + "ipv4mask": 24, + "ipv6base": "fd00::", + "ipv6mask": 64, + "link_ip_start": { + "ipv4": "10.0.0.0", + "v4mask": 24, + "ipv6": "fd00::", + "v6mask": 64 + }, + "lo_prefix": { + "ipv4": "1.0.", + "v4mask": 32, + "ipv6": "2001:DB8:F::", + "v6mask": 128 + }, + "routers": { + "r1": { + "links": { + "lo": { + "ipv4": "auto", + "ipv6": "auto", + "type": "loopback" + }, + "r2": { + "ipv4": "auto", + "ipv6": "auto" + } + }, + "bgp": { + "local_as": "1000", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r1": {} + } + } + } + } + } + } + } + }, + "r2": { + "links": { + "lo": { + "ipv4": "auto", + "ipv6": "auto", + "type": "loopback" + }, + "r1": { + "ipv4": "auto", + "ipv6": "auto" + }, + "r3": { + "ipv4": "auto", + "ipv6": "auto" + } + }, + "bgp": { + "local_as": "2000", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r1": { + "dest_link": { + "r2": {} + } + }, + "r3": { + "dest_link": { + "r2": {} + } + } + } + } + } + } + } + }, + "r3": { + "links": { + "lo": { + "ipv4": "auto", + "ipv6": "auto", + "type": "loopback" + }, + "r2": { + "ipv4": "auto", + "ipv6": "auto" + }, + "r4": { + "ipv4": "auto", + "ipv6": "auto" + } + }, + "bgp": { + "local_as": "2000", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r3": {} + } + }, + "r4": { + "dest_link": { + "r3": {} + } + } + } + } + } + } + } + }, + "r4": { + "links": { + "lo": { + "ipv4": "auto", + "ipv6": "auto", + "type": "loopback" + }, + "r3": { + "ipv4": "auto", + "ipv6": "auto" + } + }, + "bgp": { + "local_as": "3000", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r3": { + "dest_link": { + "r4": {} + } + } + } + } + } + } + } + } + } +} diff --git a/tests/topotests/bgp_listen_on_multiple_addresses/test_bgp_listen_on_multiple_addresses.py b/tests/topotests/bgp_listen_on_multiple_addresses/test_bgp_listen_on_multiple_addresses.py new file mode 100755 index 0000000000..d773e87ef6 --- /dev/null +++ b/tests/topotests/bgp_listen_on_multiple_addresses/test_bgp_listen_on_multiple_addresses.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python + +# +# test_bgp_listen_on_multiple_addresses.py +# Part of NetDEF Topology Tests +# +# Copyright (c) 2021 by Boeing Defence Australia +# Adriano Marto Reis +# +# Permission to use, copy, modify, and/or distribute this software +# for any purpose with or without fee is hereby granted, provided +# that the above copyright notice and this permission notice appear +# in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE +# OF THIS SOFTWARE. +# + +""" +test_bgp_listen_on_multiple_addresses.py: Test BGP daemon listening for +connections on multiple addresses. + + +------+ +------+ +------+ +------+ + | | | | | | | | + | r1 |--------| r2 |--------| r3 |--------| r4 | + | | | | | | | | + +------+ +------+ +------+ +------+ + + | | | | + | AS 1000 | AS 2000 | AS 3000 | + | | | | + +------------+--------------------------------+-------------+ +""" + +import os +import sys +import json +import pytest + + +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +from lib.topogen import Topogen, get_topogen +from lib.topojson import build_topo_from_json, build_config_from_json +from lib.common_config import start_topology +from lib.topotest import router_json_cmp, run_and_expect +from mininet.topo import Topo +from functools import partial + + +LISTEN_ADDRESSES = { + "r1": ["10.0.0.1"], + "r2": ["10.0.0.2", "10.0.1.1"], + "r3": ["10.0.1.2", "10.0.2.1"], + "r4": ["10.0.2.2"], +} + + +# Reads data from JSON File for topology and configuration creation. +jsonFile = "{}/bgp_listen_on_multiple_addresses.json".format(CWD) +try: + with open(jsonFile, "r") as topoJson: + topo = json.load(topoJson) +except IOError: + assert False, "Could not read file {}".format(jsonFile) + + +class TemplateTopo(Topo): + "Topology builder." + + def build(self, *_args, **_opts): + "Defines the allocation and relationship between routers and switches." + tgen = get_topogen(self) + build_topo_from_json(tgen, topo) + + +def setup_module(mod): + "Sets up the test environment." + tgen = Topogen(TemplateTopo, mod.__name__) + + # Adds extra parameters to bgpd so they listen for connections on specific + # multiple addresses. + for router_name in tgen.routers().keys(): + tgen.net[router_name].daemons_options["bgpd"] = "-l " + " -l ".join( + LISTEN_ADDRESSES[router_name] + ) + + start_topology(tgen) + build_config_from_json(tgen, topo) + + +def teardown_module(_mod): + "Tears-down the test environment." + tgen = get_topogen() + tgen.stop_topology() + + +def test_peering(): + "Checks if the routers peer-up." + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + _bgp_converge_initial("r1", "10.0.0.2") + _bgp_converge_initial("r2", "10.0.0.1") + _bgp_converge_initial("r2", "10.0.1.2") + _bgp_converge_initial("r3", "10.0.1.1") + _bgp_converge_initial("r3", "10.0.2.2") + _bgp_converge_initial("r4", "10.0.2.1") + + +def test_listening_address(): + """ + Checks if bgpd is only listening on the specified IP addresses. + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + for router in tgen.routers().values(): + # bgpd must not be listening on the default address. + output = router.run("netstat -nlt4 | grep 0.0.0.0:179") + assert output == "", "{}: bpgd is listening on 0.0.0.0:179".format(router.name) + + # bgpd must be listening on the specified addresses. + for address in LISTEN_ADDRESSES[router.name]: + output = router.run("netstat -nlt4 | grep {}:179".format(address)) + assert output != "", "{}: bpgd is not listening on {}:179".format( + router.name, address + ) + + +def _bgp_converge_initial(router_name, peer_address, timeout=180): + """ + Waits for the BGP connection between a given router and a given peer + (specified by its IP address) to be established. If the connection is + not established within a given timeout, then an exception is raised. + """ + tgen = get_topogen() + router = tgen.routers()[router_name] + expected = {"ipv4Unicast": {"peers": {peer_address: {"state": "Established"}}}} + + test_func = partial(router_json_cmp, router, "show ip bgp summary json", expected) + _, result = run_and_expect(test_func, None, count=timeout, wait=1) + assert result is None, "{}: Failed to establish connection with {}".format( + router_name, peer_address + ) + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))