Skip to content

Commit

Permalink
bgpd: fix AIGP calculation in route advertisement
Browse files Browse the repository at this point in the history 
Currently the AIGP is always incremented when a route with the
attribute is advertised. That is incorrect when the nexthop is
unchanged, as is commonly the case in route reflection.

Adjust the AIGP for propagation only when the nexthop is set
to ourselves.

Signed-off-by: Enke Chen <[email protected]>
  • Loading branch information
@enkechen-panw
enkechen-panw committed Oct 22, 2024
1 parent 7e7f65c commit fc82d77
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 35 deletions.
27 changes: 10 additions & 17 deletions bgpd/bgp_attr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,12 +480,11 @@ static bool bgp_attr_aigp_get_tlv_metric(uint8_t *pnt, int length,
return false;
}

static void stream_put_bgp_aigp_tlv_metric(struct stream *s,
struct bgp_path_info *bpi)
static void stream_put_bgp_aigp_tlv_metric(struct stream *s, uint64_t aigp)
{
stream_putc(s, BGP_AIGP_TLV_METRIC);
stream_putw(s, BGP_AIGP_TLV_METRIC_LEN);
stream_putq(s, bgp_aigp_metric_total(bpi));
stream_putq(s, aigp);
}

static bool bgp_attr_aigp_valid(uint8_t *pnt, int length)
Expand Down Expand Up @@ -4546,14 +4545,11 @@ static void bgp_packet_ecommunity_attribute(struct stream *s, struct peer *peer,
}

/* Make attribute packet. */
bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
struct stream *s, struct attr *attr,
struct bpacket_attr_vec_arr *vecarr,
struct prefix *p, afi_t afi, safi_t safi,
struct peer *from, struct prefix_rd *prd,
mpls_label_t *label, uint8_t num_labels,
bool addpath_capable, uint32_t addpath_tx_id,
struct bgp_path_info *bpi)
bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct stream *s,
struct attr *attr, struct bpacket_attr_vec_arr *vecarr,
struct prefix *p, afi_t afi, safi_t safi, struct peer *from,
struct prefix_rd *prd, mpls_label_t *label, uint8_t num_labels,
bool addpath_capable, uint32_t addpath_tx_id)
{
size_t cp;
size_t aspath_sizep;
Expand Down Expand Up @@ -5032,10 +5028,7 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
}

/* AIGP */
if (bpi && CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) &&
(CHECK_FLAG(peer->flags, PEER_FLAG_AIGP) ||
peer->sub_sort == BGP_PEER_EBGP_OAD ||
peer->sort != BGP_PEER_EBGP)) {
if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && AIGP_TRANSMIT_ALLOWED(peer)) {
/* At the moment only AIGP Metric TLV exists for AIGP
* attribute. If more comes in, do not forget to update
* attr_len variable to include new ones.
Expand All @@ -5045,7 +5038,7 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
stream_putc(s, BGP_ATTR_FLAG_OPTIONAL);
stream_putc(s, BGP_ATTR_AIGP);
stream_putc(s, attr_len);
stream_put_bgp_aigp_tlv_metric(s, bpi);
stream_put_bgp_aigp_tlv_metric(s, attr->aigp_metric);
}

/* Unknown transit attribute. */
Expand Down Expand Up @@ -5314,7 +5307,7 @@ void bgp_dump_routes_attr(struct stream *s, struct bgp_path_info *bpi,
stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS);
stream_putc(s, BGP_ATTR_AIGP);
stream_putc(s, attr_len);
stream_put_bgp_aigp_tlv_metric(s, bpi);
stream_put_bgp_aigp_tlv_metric(s, attr->aigp_metric);
}

/* Return total size of attribute. */
Expand Down
16 changes: 10 additions & 6 deletions bgpd/bgp_attr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,12 +387,12 @@ extern struct attr *bgp_attr_aggregate_intern(
struct community *community, struct ecommunity *ecommunity,
struct lcommunity *lcommunity, struct bgp_aggregate *aggregate,
uint8_t atomic_aggregate, const struct prefix *p);
extern bgp_size_t bgp_packet_attribute(
struct bgp *bgp, struct peer *peer, struct stream *s, struct attr *attr,
struct bpacket_attr_vec_arr *vecarr, struct prefix *p, afi_t afi,
safi_t safi, struct peer *from, struct prefix_rd *prd,
mpls_label_t *label, uint8_t num_labels, bool addpath_capable,
uint32_t addpath_tx_id, struct bgp_path_info *bpi);
extern bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct stream *s,
struct attr *attr, struct bpacket_attr_vec_arr *vecarr,
struct prefix *p, afi_t afi, safi_t safi, struct peer *from,
struct prefix_rd *prd, mpls_label_t *label,
uint8_t num_labels, bool addpath_capable,
uint32_t addpath_tx_id);
extern void bgp_dump_routes_attr(struct stream *s, struct bgp_path_info *bpi,
const struct prefix *p);
extern bool attrhash_cmp(const void *arg1, const void *arg2);
Expand Down Expand Up @@ -585,6 +585,10 @@ static inline void bgp_attr_set_transit(struct attr *attr,
attr->transit = transit;
}

#define AIGP_TRANSMIT_ALLOWED(peer) \
(CHECK_FLAG((peer)->flags, PEER_FLAG_AIGP) || ((peer)->sub_sort == BGP_PEER_EBGP_OAD) || \
((peer)->sort != BGP_PEER_EBGP))

static inline uint64_t bgp_attr_get_aigp_metric(const struct attr *attr)
{
return attr->aigp_metric;
Expand Down
5 changes: 2 additions & 3 deletions bgpd/bgp_bmp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,9 +1016,8 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
stream_putw(s, 0);

/* 5: Encode all the attributes, except MP_REACH_NLRI attr. */
total_attr_len =
bgp_packet_attribute(NULL, peer, s, attr, &vecarr, NULL, afi,
safi, peer, NULL, NULL, 0, 0, 0, NULL);
total_attr_len = bgp_packet_attribute(NULL, peer, s, attr, &vecarr, NULL, afi, safi, peer,
NULL, NULL, 0, 0, 0);

/* space check? */

Expand Down
15 changes: 15 additions & 0 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2821,6 +2821,21 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
false));
}

/*
* Adjust AIGP for propagation when the nexthop is set to ourselves,
* e.g., using "set ip nexthop peer-address" or when advertising to
* EBGP. Note in route reflection the nexthop is usually unmodified
* and the AIGP should not be adjusted in that case.
*/
if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && AIGP_TRANSMIT_ALLOWED(peer)) {
if (nh_reset ||
CHECK_FLAG(attr->rmap_change_flags, BATTR_RMAP_NEXTHOP_PEER_ADDRESS)) {
uint64_t aigp = bgp_aigp_metric_total(pi);

bgp_attr_set_aigp_metric(attr, aigp);
}
}

return true;
}

Expand Down
15 changes: 6 additions & 9 deletions bgpd/bgp_updgrp_packet.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,9 +738,9 @@ struct bpacket *subgroup_update_packet(struct update_subgroup *subgrp)

/* 5: Encode all the attributes, except MP_REACH_NLRI
* attr. */
total_attr_len = bgp_packet_attribute(
NULL, peer, s, adv->baa->attr, &vecarr, NULL,
afi, safi, from, NULL, NULL, 0, 0, 0, path);
total_attr_len = bgp_packet_attribute(NULL, peer, s, adv->baa->attr,
&vecarr, NULL, afi, safi, from, NULL,
NULL, 0, 0, 0);

space_remaining =
STREAM_CONCAT_REMAIN(s, snlri, STREAM_SIZE(s))
Expand Down Expand Up @@ -1149,12 +1149,9 @@ void subgroup_default_update_packet(struct update_subgroup *subgrp,
/* Make place for total attribute length. */
pos = stream_get_endp(s);
stream_putw(s, 0);
total_attr_len =
bgp_packet_attribute(NULL, peer, s, attr, &vecarr, &p, afi,
safi, from, NULL, &label, num_labels,
addpath_capable,
BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE,
NULL);
total_attr_len = bgp_packet_attribute(NULL, peer, s, attr, &vecarr, &p, afi, safi, from,
NULL, &label, num_labels, addpath_capable,
BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE);

/* Set Total Path Attribute Length. */
stream_putw_at(s, pos, total_attr_len);
Expand Down

0 comments on commit fc82d77

Please sign in to comment.