Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: fix several issues in sourcing AIGP attribute #17091

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions bgpd/bgp_attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,7 @@ static inline uint64_t bgp_attr_get_aigp_metric(const struct attr *attr)
static inline void bgp_attr_set_aigp_metric(struct attr *attr, uint64_t aigp)
{
attr->aigp_metric = aigp;

if (aigp)
SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP));
SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP));
}

static inline uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi)
Expand Down
10 changes: 1 addition & 9 deletions bgpd/bgp_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -6754,9 +6754,6 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p,
if (afi == AFI_IP)
attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4;

if (bgp_static->igpmetric)
bgp_attr_set_aigp_metric(&attr, bgp_static->igpmetric);

if (bgp_static->atomic)
attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_ATOMIC_AGGREGATE);

Expand Down Expand Up @@ -9020,9 +9017,6 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p,
attr.distance = distance;
attr.tag = tag;

if (metric)
bgp_attr_set_aigp_metric(&attr, metric);

afi = family2afi(p->family);

red = bgp_redist_lookup(bgp, afi, type, instance);
Expand All @@ -9032,10 +9026,8 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p,
/* Copy attribute for modification. */
attr_new = attr;

if (red->redist_metric_flag) {
if (red->redist_metric_flag)
attr_new.med = red->redist_metric;
bgp_attr_set_aigp_metric(&attr_new, red->redist_metric);
}

/* Apply route-map. */
if (red->rmap.name) {
Expand Down
16 changes: 6 additions & 10 deletions bgpd/bgp_routemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -3524,19 +3524,15 @@ route_set_aigp_metric(void *rule, const struct prefix *pfx, void *object)
{
const char *aigp_metric = rule;
struct bgp_path_info *path = object;
uint32_t aigp = 0;
uint32_t aigp;

if (strmatch(aigp_metric, "igp-metric")) {
if (!path->nexthop)
return RMAP_NOMATCH;

bgp_attr_set_aigp_metric(path->attr, path->nexthop->metric);
} else {
/* Note: the metric is stored as MED for a locally redistributed. */
if (strmatch(aigp_metric, "igp-metric"))
aigp = path->nexthop ? path->nexthop->metric : path->attr->med;
ton31337 marked this conversation as resolved.
Show resolved Hide resolved
else
aigp = atoi(aigp_metric);
bgp_attr_set_aigp_metric(path->attr, aigp);
}

path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_AIGP);
bgp_attr_set_aigp_metric(path->attr, aigp);

return RMAP_OKAY;
}
Expand Down
7 changes: 7 additions & 0 deletions tests/topotests/bgp_aigp/r4/bgpd.conf
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,16 @@ router bgp 65001
neighbor 10.0.0.6 update-source lo
address-family ipv4
redistribute connected route-map connected-to-bgp
redistribute ospf route-map ospf-to-bgp
neighbor 192.168.24.2 route-map set-nexthop out
exit-address-family
!
ip prefix-list p66 seq 5 permit 10.0.6.6/32
!
route-map ospf-to-bgp permit 10
match ip address prefix-list p66
set aigp igp-metric
!
! 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).
Expand Down
1 change: 1 addition & 0 deletions tests/topotests/bgp_aigp/r6/zebra.conf
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
!
interface lo
ip address 10.0.0.6/32
ip address 10.0.6.6/32
!
interface r6-eth0
ip address 192.168.46.6/24
Expand Down
5 changes: 5 additions & 0 deletions tests/topotests/bgp_aigp/test_bgp_aigp.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ def _bgp_check_aigp_metric_bestpath():
"""
)

# r4, 10.0.6.6/32 with aigp-metric 20
test_func = functools.partial(_bgp_check_aigp_metric, r4, "10.0.6.6/32", 20)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
assert result is None, "aigp-metric for 10.0.6.6/32 is not 20"

# r4, 10.0.0.71/32 with aigp-metric 71
test_func = functools.partial(_bgp_check_aigp_metric, r4, "10.0.0.71/32", 71)
_, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
Expand Down
Loading