Skip to content

Commit

Permalink
Merge pull request #17410 from opensourcerouting/fix/bgpd_ebgp_multih…
Browse files Browse the repository at this point in the history
…op_set_unset

BGP BFD session things
  • Loading branch information
riw777 authored Nov 12, 2024
2 parents 9ce07a1 + 2c08d9f commit fe56a65
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 35 deletions.
26 changes: 22 additions & 4 deletions bgpd/bgp_bfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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);
Expand Down
9 changes: 4 additions & 5 deletions bgpd/bgp_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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;

Expand Down
48 changes: 22 additions & 26 deletions bgpd/bgpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -5442,30 +5441,29 @@ 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;
}

int peer_ebgp_multihop_unset(struct peer *peer)
{
struct peer *member;
struct peer_group *group;
struct listnode *node, *nnode;
int ttl;
Expand Down Expand Up @@ -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;
Expand Down
Empty file.
14 changes: 14 additions & 0 deletions tests/topotests/bgp_bfd_session/r1/frr.conf
Original file line number Diff line number Diff line change
@@ -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
108 changes: 108 additions & 0 deletions tests/topotests/bgp_bfd_session/test_bgp_bfd_session.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
#!/usr/bin/env python
# SPDX-License-Identifier: ISC

#
# Copyright (c) 2024 by
# Donatas Abraitis <[email protected]>
#

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))

0 comments on commit fe56a65

Please sign in to comment.