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>

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 596d820f1b508806047df1ffd489034ecc22e049..34b77b5eeccf618edb0b1b34191ab7eab06af05d 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)
@@ -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;
@@ -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.
@@ -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. */
@@ -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. */

diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index 0a43399be6c52b6c86eb6aef5c453a6af570986b..9cdac10f926660c37e5ddf3872816d2638381424 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 59efb35235d77870a65f8115955a5096f318ae08..48c5ee7ede94c536db1555176565003a05b8c65d 100644 (file)
--- a/bgpd/bgp_bmp.c
+++ b/bgpd/bgp_bmp.c
@@ -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? */
 

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 551547b7c09835c7df74b0594b07d799db39e218..cdd5fc080508078b04e62c75819fcdeebff8451d 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -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;
 }
 

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