diff --git a/bgpd/bgp_bfd.c b/bgpd/bgp_bfd.c index af6068cb1f..a331585d32 100644 --- a/bgpd/bgp_bfd.c +++ b/bgpd/bgp_bfd.c @@ -26,6 +26,7 @@ #include "bgpd/bgp_debug.h" #include "bgpd/bgp_vty.h" #include "bgpd/bgp_packet.h" +#include "bgpd/bgp_network.h" DEFINE_MTYPE_STATIC(BGPD, BFD_CONFIG, "BFD configuration data"); @@ -151,23 +152,40 @@ void bgp_peer_config_apply(struct peer *p, struct peer_group *pg) void bgp_peer_bfd_update_source(struct peer *p) { struct bfd_session_params *session = p->bfd_config->session; - const union sockunion *source; + const union sockunion *source = NULL; bool changed = false; int family; union { struct in_addr v4; struct in6_addr v6; } src, dst; + struct interface *ifp; + union sockunion addr; /* Nothing to do for groups. */ if (CHECK_FLAG(p->sflags, PEER_STATUS_GROUP)) return; /* Figure out the correct source to use. */ - if (CHECK_FLAG(p->flags, PEER_FLAG_UPDATE_SOURCE) && p->update_source) - source = p->update_source; - else + if (CHECK_FLAG(p->flags, PEER_FLAG_UPDATE_SOURCE)) { + if (p->update_source) { + source = p->update_source; + } else if (p->update_if) { + ifp = if_lookup_by_name(p->update_if, p->bgp->vrf_id); + if (ifp) { + sockunion_init(&addr); + if (bgp_update_address(ifp, &p->connection->su, &addr)) { + if (BGP_DEBUG(bfd, BFD_LIB)) + zlog_debug("%s: can't find the source address for interface %s", + __func__, p->update_if); + } + + source = &addr; + } + } + } else { source = p->su_local; + } /* Update peer's source/destination addresses. */ bfd_sess_addresses(session, &family, &src.v6, &dst.v6); diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 58e1ffa500..8c9050185b 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -1344,11 +1344,6 @@ enum bgp_fsm_state_progress bgp_stop(struct peer_connection *connection) peer->nsf_af_count = 0; - /* deregister peer */ - if (peer->bfd_config - && peer->last_reset == PEER_DOWN_UPDATE_SOURCE_CHANGE) - bfd_sess_uninstall(peer->bfd_config->session); - if (peer_dynamic_neighbor_no_nsf(peer) && !(CHECK_FLAG(peer->flags, PEER_FLAG_DELETE))) { if (bgp_debug_neighbor_events(peer)) @@ -1368,6 +1363,10 @@ enum bgp_fsm_state_progress bgp_stop(struct peer_connection *connection) if (peer_established(connection)) { peer->dropped++; + if (peer->bfd_config && (peer->last_reset == PEER_DOWN_UPDATE_SOURCE_CHANGE || + peer->last_reset == PEER_DOWN_MULTIHOP_CHANGE)) + bfd_sess_uninstall(peer->bfd_config->session); + /* Notify BGP conditional advertisement process */ peer->advmap_table_change = true; diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7e98735c14..aa2bd5c371 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -5394,7 +5394,7 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) { struct peer_group *group; struct listnode *node, *nnode; - struct peer *peer1; + struct peer *member; if (peer->sort == BGP_PEER_IBGP || peer->conf_if) return 0; @@ -5410,12 +5410,11 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) if (group->conf->gtsm_hops != BGP_GTSM_HOPS_DISABLED) return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, - peer1)) { - if (peer1->sort == BGP_PEER_IBGP) + for (ALL_LIST_ELEMENTS(group->peer, node, nnode, member)) { + if (member->sort == BGP_PEER_IBGP) continue; - if (peer1->gtsm_hops != BGP_GTSM_HOPS_DISABLED) + if (member->gtsm_hops != BGP_GTSM_HOPS_DISABLED) return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK; } } else { @@ -5442,23 +5441,21 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) } } else { group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - if (peer->sort == BGP_PEER_IBGP) + for (ALL_LIST_ELEMENTS(group->peer, node, nnode, member)) { + if (member->sort == BGP_PEER_IBGP) continue; - peer->ttl = group->conf->ttl; + member->ttl = group->conf->ttl; - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) + bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - bgp_session_reset(peer); + bgp_session_reset(member); /* Reconfigure BFD peer with new TTL. */ - if (peer->bfd_config) - bgp_peer_bfd_update_source(peer); + if (member->bfd_config) + bgp_peer_bfd_update_source(member); } } return 0; @@ -5466,6 +5463,7 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) int peer_ebgp_multihop_unset(struct peer *peer) { + struct peer *member; struct peer_group *group; struct listnode *node, *nnode; int ttl; @@ -5498,25 +5496,23 @@ int peer_ebgp_multihop_unset(struct peer *peer) bgp_peer_bfd_update_source(peer); } else { group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - if (peer->sort == BGP_PEER_IBGP) + for (ALL_LIST_ELEMENTS(group->peer, node, nnode, member)) { + if (member->sort == BGP_PEER_IBGP) continue; - peer->ttl = BGP_DEFAULT_TTL; + member->ttl = BGP_DEFAULT_TTL; - if (peer->connection->fd >= 0) { - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, + if (member->connection->fd >= 0) { + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) + bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - bgp_session_reset(peer); + bgp_session_reset(member); } /* Reconfigure BFD peer with new TTL. */ - if (peer->bfd_config) - bgp_peer_bfd_update_source(peer); + if (member->bfd_config) + bgp_peer_bfd_update_source(member); } } return 0; diff --git a/tests/topotests/bgp_bfd_session/__init__.py b/tests/topotests/bgp_bfd_session/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_bfd_session/r1/frr.conf b/tests/topotests/bgp_bfd_session/r1/frr.conf new file mode 100644 index 0000000000..a1560b09fa --- /dev/null +++ b/tests/topotests/bgp_bfd_session/r1/frr.conf @@ -0,0 +1,14 @@ +! +interface r1-eth0 + ip address 10.0.0.1/24 +! +router bgp 65000 + neighbor 192.168.1.2 remote-as auto + neighbor 192.168.1.2 bfd + neighbor 192.168.1.2 ebgp-multihop 10 + neighbor 192.168.1.2 update-source 10.0.0.1 + neighbor 192.168.1.3 remote-as auto + neighbor 192.168.1.3 bfd + neighbor 192.168.1.3 ebgp-multihop 20 + neighbor 192.168.1.3 update-source r1-eth0 +exit diff --git a/tests/topotests/bgp_bfd_session/test_bgp_bfd_session.py b/tests/topotests/bgp_bfd_session/test_bgp_bfd_session.py new file mode 100644 index 0000000000..adf557af7b --- /dev/null +++ b/tests/topotests/bgp_bfd_session/test_bgp_bfd_session.py @@ -0,0 +1,108 @@ +#!/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, TopoRouter +from lib.common_config import step + +pytestmark = [pytest.mark.bgpd] + + +def build_topo(tgen): + r1 = tgen.add_router("r1") + + switch = tgen.add_switch("s1") + switch.add_link(r1) + + +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)), + [ + (TopoRouter.RD_ZEBRA, None), + (TopoRouter.RD_MGMTD, None), + (TopoRouter.RD_BFD, None), + (TopoRouter.RD_BGP, None), + ], + ) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_bfd_session(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + def _bfd_session(): + output = json.loads(r1.vtysh_cmd("show bfd peers json")) + expected = [ + { + "multihop": True, + "peer": "192.168.1.2", + "local": "10.0.0.1", + "vrf": "default", + "minimum-ttl": 246, + "status": "down", + "diagnostic": "ok", + "remote-diagnostic": "ok", + "type": "dynamic", + }, + { + "multihop": True, + "peer": "192.168.1.3", + "local": "10.0.0.1", + "vrf": "default", + "minimum-ttl": 236, + "status": "down", + "diagnostic": "ok", + "remote-diagnostic": "ok", + "type": "dynamic", + } + ] + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bfd_session) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't see BFD session created" + + +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))