bgpd: fix AIGP calculation in route advertisement

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 <enchen@paloaltonetworks.com>
(cherry picked from commit fc82d7750f3cc54855ca399f3060428b66e1fbec)

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 7aa9bbbb91b875058821a81917e26759ecae003a..1a2fa8318e3445a0b9f51029a6fbb54f857e01e5 100644 (file)
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -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)
@@ -4549,14 +4548,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;
@@ -5035,10 +5031,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.
@@ -5048,7 +5041,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. */
@@ -5317,7 +5310,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. */

diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index c3c81a07c4eabcc35e806b542124022c33d170ad..5633c2f89af8c4cb6438cf23b6dc9da8e8ab782a 100644 (file)
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -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);
@@ -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;

diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c
index 9d99c2c7fda80882b5b81df13fa6786a6801e236..096fb48eaeb6bbd7377363876b51a4ba823b27dc 100644 (file)
--- a/bgpd/bgp_bmp.c
+++ b/bgpd/bgp_bmp.c
@@ -934,9 +934,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? */
 

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index f89327e463658463454dbed137e635ef19425359..359b6acef98ba97922fa8346e45ef70f12cc9ae7 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2820,6 +2820,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;
 }
 

diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c
index bed00a66402f1b3310773d40fcf84b10596b2712..3ce136ef873ae8e7f8ac244d6d375065a8cee032 100644 (file)
--- a/bgpd/bgp_updgrp_packet.c
+++ b/bgpd/bgp_updgrp_packet.c
@@ -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))
@@ -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);