proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-07-27 05:28:51 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #17110 from FRRouting/mergify/bp/stable/10.0/pr-17093

Browse Source 
bgpd: fix route selection with AIGP (backport #17093)
This commit is contained in:
Donald Sharp 2024-10-15 16:28:06 -04:00 committed by GitHub
commit c6149b55ed
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 123 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
bgpd/bgp_attr.c
View File

@ -479,16 +479,6 @@ static bool bgp_attr_aigp_get_tlv_metric(uint8_t *pnt, int length,
return false;
}
static uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi)
{
uint64_t aigp = bgp_attr_get_aigp_metric(bpi->attr);
if (bpi->nexthop)
return aigp + bpi->nexthop->metric;
else
return aigp;
}
static void stream_put_bgp_aigp_tlv_metric(struct stream *s,
struct bgp_path_info *bpi)
{

10
bgpd/bgp_attr.h
View File

@ -601,6 +601,16 @@ static inline void bgp_attr_set_aigp_metric(struct attr *attr, uint64_t aigp)
SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP));
}
static inline uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi)
{
uint64_t aigp = bgp_attr_get_aigp_metric(bpi->attr);
if (bpi->nexthop)
return aigp + bpi->nexthop->metric;
else
return aigp;
}
static inline struct cluster_list *bgp_attr_get_cluster(const struct attr *attr)
{
return attr->cluster1;

4
bgpd/bgp_route.c
View File

@ -970,8 +970,8 @@ int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new,
if (CHECK_FLAG(newattr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) &&
CHECK_FLAG(existattr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) &&
CHECK_FLAG(bgp->flags, BGP_FLAG_COMPARE_AIGP)) {
uint64_t new_aigp = bgp_attr_get_aigp_metric(newattr);
uint64_t exist_aigp = bgp_attr_get_aigp_metric(existattr);
uint64_t new_aigp = bgp_aigp_metric_total(new);
uint64_t exist_aigp = bgp_aigp_metric_total(exist);
if (new_aigp < exist_aigp) {
*reason = bgp_path_selection_aigp;

2
tests/topotests/bgp_aigp/r1/ospfd.conf
View File

@ -1,6 +1,6 @@
!
interface lo
ip ospf cost 10
ip ospf passive
!
interface r1-eth0
ip ospf dead-interval 4

2
tests/topotests/bgp_aigp/r2/bgpd.conf
View File

@ -1,6 +1,7 @@
router bgp 65001
no bgp ebgp-requires-policy
no bgp network import-check
bgp route-reflector allow-outbound-policy
neighbor 10.0.0.1 remote-as internal
neighbor 10.0.0.1 timers 1 3
neighbor 10.0.0.1 timers connect 1
@ -11,5 +12,6 @@ router bgp 65001
neighbor 192.168.24.4 aigp
address-family ipv4
redistribute connected
neighbor 10.0.0.1 next-hop-self force
exit-address-family
!

2
tests/topotests/bgp_aigp/r2/ospfd.conf
View File

@ -1,6 +1,6 @@
!
interface lo
ip ospf cost 10
ip ospf passive
!
interface r2-eth0
ip ospf dead-interval 4

2
tests/topotests/bgp_aigp/r3/bgpd.conf
View File

@ -1,6 +1,7 @@
router bgp 65001
no bgp ebgp-requires-policy
no bgp network import-check
bgp route-reflector allow-outbound-policy
neighbor 10.0.0.1 remote-as internal
neighbor 10.0.0.1 timers 1 3
neighbor 10.0.0.1 timers connect 1
@ -11,5 +12,6 @@ router bgp 65001
neighbor 192.168.35.5 aigp
address-family ipv4
redistribute connected
neighbor 10.0.0.1 next-hop-self force
exit-address-family
!

2
tests/topotests/bgp_aigp/r3/ospfd.conf
View File

@ -1,6 +1,6 @@
!
interface lo
ip ospf cost 10
ip ospf passive
!
interface r3-eth0
ip ospf dead-interval 4

16
tests/topotests/bgp_aigp/r4/bgpd.conf
View File

@ -1,6 +1,7 @@
router bgp 65001
no bgp ebgp-requires-policy
no bgp network import-check
bgp route-reflector allow-outbound-policy
neighbor 192.168.24.2 remote-as internal
neighbor 192.168.24.2 timers 1 3
neighbor 192.168.24.2 timers connect 1
@ -11,7 +12,18 @@ router bgp 65001
neighbor 10.0.0.6 timers connect 1
neighbor 10.0.0.6 update-source lo
address-family ipv4
redistribute connected
redistribute ospf
redistribute connected route-map connected-to-bgp
neighbor 192.168.24.2 route-map set-nexthop out
exit-address-family
!
! Two OSPF domains should be isolated - otherwise the connected routes
! on r4 would be advertised to r3 (via r4 -> r6 -> r5 -> r3), and can
! mess up bgp bestpath calculation (igp metrics for the BGP nexthops).
!
route-map connected-to-bgp permit 10
set community no-advertise
!
route-map set-nexthop permit 10
set ip next-hop peer-address
exit
!

2
tests/topotests/bgp_aigp/r4/ospfd.conf
View File

@ -1,6 +1,6 @@
!
interface lo
ip ospf cost 10
ip ospf passive
!
interface r4-eth1
ip ospf dead-interval 4

16
tests/topotests/bgp_aigp/r5/bgpd.conf
View File

@ -1,6 +1,7 @@
router bgp 65001
no bgp ebgp-requires-policy
no bgp network import-check
bgp route-reflector allow-outbound-policy
neighbor 192.168.35.3 remote-as internal
neighbor 192.168.35.3 timers 1 3
neighbor 192.168.35.3 timers connect 1
@ -11,7 +12,18 @@ router bgp 65001
neighbor 10.0.0.6 timers connect 1
neighbor 10.0.0.6 update-source lo
address-family ipv4
redistribute connected
redistribute ospf
redistribute connected route-map connected-to-bgp
neighbor 192.168.35.3 route-map set-nexthop out
exit-address-family
!
! Two OSPF domains should be isolated - otherwise the connected routes
! on r5 would be advertised to r2 (via r5 -> r6 -> r4 -> r2), and can
! mess up bgp bestpath calculation (igp metrics for the BGP nexthops).
!
route-map connected-to-bgp permit 10
set community no-advertise
!
route-map set-nexthop permit 10
set ip next-hop peer-address
exit
!

2
tests/topotests/bgp_aigp/r5/ospfd.conf
View File

@ -1,6 +1,6 @@
!
interface lo
ip ospf cost 10
ip ospf passive
!
interface r5-eth1
ip ospf dead-interval 4

8
tests/topotests/bgp_aigp/r6/bgpd.conf
View File

@ -1,6 +1,7 @@
router bgp 65001
no bgp ebgp-requires-policy
no bgp network import-check
bgp route-reflector allow-outbound-policy
neighbor 10.0.0.4 remote-as internal
neighbor 10.0.0.4 timers 1 3
neighbor 10.0.0.4 timers connect 1
@ -15,6 +16,11 @@ router bgp 65001
neighbor 192.168.67.7 timers 1 3
neighbor 192.168.67.7 timers connect 1
address-family ipv4
redistribute ospf
neighbor 10.0.0.4 route-map set-nexthop out
neighbor 10.0.0.5 route-map set-nexthop out
exit-address-family
!
route-map set-nexthop permit 10
set ip next-hop peer-address
exit
!

2
tests/topotests/bgp_aigp/r6/ospfd.conf
View File

@ -1,6 +1,6 @@
!
interface lo
ip ospf cost 10
ip ospf passive
!
interface r6-eth0
ip ospf dead-interval 4

90
tests/topotests/bgp_aigp/test_bgp_aigp.py
View File

@ -12,9 +12,9 @@ r7 sets aigp-metric for 10.0.0.71/32 to 71, and 72 for 10.0.0.72/32.
r6 receives those routes with aigp-metric TLV.
r2 and r3 receives those routes with aigp-metric TLV increased by 20,
and 30 appropriately.
and 10 appropriately.
r1 receives routes with aigp-metric TLV 111,131 and 112,132 appropriately.
r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively.
"""
import os
@ -109,15 +109,29 @@ def test_bgp_aigp():
expected = {
"paths": [
{
"aigpMetric": 111,
"aigpMetric": 81,
"valid": True,
"nexthops": [{"hostname": "r3", "accessible": True}],
"nexthops": [
{
"ip": "10.0.0.3",
"hostname": "r3",
"metric": 30,
"accessible": True,
}
],
},
{
"aigpMetric": 131,
"aigpMetric": 91,
"valid": True,
"bestpath": {"selectionReason": "Neighbor IP"},
"nexthops": [{"hostname": "r2", "accessible": True}],
"bestpath": {"selectionReason": "IGP Metric"},
"nexthops": [
{
"ip": "10.0.0.2",
"hostname": "r2",
"metric": 10,
"accessible": True,
}
],
},
]
}
@ -141,30 +155,58 @@ def test_bgp_aigp():
"10.0.0.71/32": {
"paths": [
{
"aigpMetric": 111,
"bestpath": {"selectionReason": "AIGP"},
"aigpMetric": 81,
"valid": True,
"nexthops": [{"hostname": "r3", "accessible": True}],
"nexthops": [
{
"ip": "10.0.0.3",
"hostname": "r3",
"metric": 30,
"accessible": True,
}
],
},
{
"aigpMetric": 131,
"aigpMetric": 91,
"valid": True,
"nexthops": [{"hostname": "r2", "accessible": True}],
"bestpath": {"selectionReason": "AIGP"},
"nexthops": [
{
"ip": "10.0.0.2",
"hostname": "r2",
"metric": 10,
"accessible": True,
}
],
},
],
},
"10.0.0.72/32": {
"paths": [
{
"aigpMetric": 112,
"bestpath": {"selectionReason": "AIGP"},
"aigpMetric": 82,
"valid": True,
"nexthops": [{"hostname": "r3", "accessible": True}],
"nexthops": [
{
"ip": "10.0.0.3",
"hostname": "r3",
"metric": 30,
"accessible": True,
}
],
},
{
"aigpMetric": 132,
"aigpMetric": 92,
"valid": True,
"nexthops": [{"hostname": "r2", "accessible": True}],
"bestpath": {"selectionReason": "AIGP"},
"nexthops": [
{
"ip": "10.0.0.2",
"hostname": "r2",
"metric": 10,
"accessible": True,
}
],
},
],
},
@ -196,17 +238,17 @@ def test_bgp_aigp():
_, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.72/32 is not 72"
# r2, 10.0.0.71/32 with aigp-metric 101 (71 + 30)
test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 101)
# r2, 10.0.0.71/32 with aigp-metric 101 (71 + 20)
test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 91)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.71/32 is not 101"
assert result is None, "aigp-metric for 10.0.0.71/32 is not 91"
# r3, 10.0.0.72/32 with aigp-metric 92 (72 + 20)
test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 92)
# r3, 10.0.0.72/32 with aigp-metric 92 (72 + 10)
test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 82)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.0.72/32 is not 92"
assert result is None, "aigp-metric for 10.0.0.72/32 is not 82"
# r1, check if AIGP is considered in best-path selection (lowest wins)
# r1, check if AIGP is considered in best-path selection (lowest wins: aigp + nexthop-metric)
test_func = functools.partial(_bgp_check_aigp_metric_bestpath)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "AIGP attribute is not considered in best-path selection"