Skip to content

Commit

Permalink
BGP: bestpath needs to prefer confed-external over confed-internal
Browse files Browse the repository at this point in the history 
Topology:
                    +-----------------------------------------+
                    |                                         |
                    |                 AS 100                  |
                    |                                         |
                    |  +----------------+                     |
  +-----------+     |  |                |                     |
  |           |     |  |   SubAS 65001  |                     |
  |   AS 90   |     |  |                |    +-------------+  |
  |    r9----------------r1---------r2----\  |             |  |
  |     |     |     |  |  |         |   | |  | SubAS 65002 |  |
  +-----|-----+     |  |  \--- r3 --/   | \-------r4       |  |
        \---------------------/  \---------------/ |       |  |
                    |  |                |    |     |       |  |
                    |  +----------------+    |     |       |  |
                    |                        |     |       |  |
                    |  +----------------+    |    r5       |  |
  +-----------+     |  |                |    |     |       |  |
  |           |     |  |   SubAS 65003  |    +-----|-------+  |
  |   AS 80   |     |  |                |          |          |
  |    r8----------------r7--------r6--------------/          |
  |           |     |  |                |                     |
  +-----------+     |  +----------------+                     |
                    +-----------------------------------------+

Important info:
- r8 originates 8.8.8.8/32
- r1, r2, r3 -> r7 are 10.0.0.1, 10.0.0.2, etc
- 'bgp bestpath compare-routerid' is configured everywhere (we could still hit
  the problem without this though)

Bestpath selection for 8.8.8.8/32 on r2 and r3 is inconsistent. Here r4
advertised the 8.8.8.8/32 to r2 first, r2 then advertised it to r3, r3 selected
the path from r2 as the bestpath due to lowest router-id.

r2
BGP routing table entry for 8.8.8.8/32
Paths: (1 available, best #1, table Default-IP-Routing-Table)
  Advertised to non peer-group peers:
  10.0.0.1 10.0.0.3 10.0.0.4
  (65002 65003) 80
    10.0.0.7 (metric 50) from 10.0.0.4 (10.0.0.4)
      Origin IGP, metric 0, localpref 100, valid, confed-external, best
      Last update: Fri May  1 14:46:57 2015

r3
BGP routing table entry for 8.8.8.8/32
Paths: (2 available, best #1, table Default-IP-Routing-Table)
  Advertised to non peer-group peers:
  10.0.0.4 90.1.1.6
  (65002 65003) 80
    10.0.0.7 (metric 50) from 10.0.0.2 (10.0.0.2)
      Origin IGP, metric 0, localpref 100, valid, confed-internal, best
      Last update: Fri May  1 14:46:58 2015

  (65002 65003) 80
    10.0.0.7 (metric 50) from 10.0.0.4 (10.0.0.4)
      Origin IGP, metric 0, localpref 100, valid, confed-external
      Last update: Fri May  1 14:46:57 2015

Here r4 advertised the 8.8.8.8/32 to r3 first, r3 then advertised it to r2, r2
selected the path from r3 as the bestpath due to lowest router-id.

r2
BGP routing table entry for 8.8.8.8/32
Paths: (2 available, best #2, table Default-IP-Routing-Table)
  Advertised to non peer-group peers:
  10.0.0.4
  (65002 65003) 80
    10.0.0.7 (metric 50) from 10.0.0.4 (10.0.0.4)
      Origin IGP, metric 0, localpref 100, valid, confed-external
      Last update: Fri May  1 15:37:27 2015

  (65002 65003) 80
    10.0.0.7 (metric 50) from 10.0.0.3 (10.0.0.3)
      Origin IGP, metric 0, localpref 100, valid, confed-internal, best
      Last update: Fri May  1 15:37:27 2015

r3
BGP routing table entry for 8.8.8.8/32
Paths: (1 available, best #1, table Default-IP-Routing-Table)
  Advertised to non peer-group peers:
  10.0.0.1 10.0.0.2 10.0.0.4 90.1.1.6
  (65002 65003) 80
    10.0.0.7 (metric 50) from 10.0.0.4 (10.0.0.4)
      Origin IGP, metric 0, localpref 100, valid, confed-external, best
      Last update: Fri May  1 15:37:22 2015

The fix is to have bestpath prefer a confed-external path over a confed-internal
path.  I added this just after the "nexthop IGP cost" step because some confed
customers will have one IGP covering multiple sub-ASs, in that case you want to
compare nexthop IGP cost.
  • Loading branch information
@donaldsharp
donaldsharp committed Jun 12, 2015
1 parent 356b329 commit 31a4638
Showing 1 changed file with 28 additions and 8 deletions.
36 changes: 28 additions & 8 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
&& (exist_sort == BGP_PEER_IBGP || exist_sort == BGP_PEER_CONFED))
{
if (debug)
zlog_debug("%s: path %s wins over path %s due to eBGP peer > iBGP peeer",
zlog_debug("%s: path %s wins over path %s due to eBGP peer > iBGP peer",
pfx_buf, new->peer->host, exist->peer->host);
return 1;
}
Expand All @@ -570,7 +570,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
&& (new_sort == BGP_PEER_IBGP || new_sort == BGP_PEER_CONFED))
{
if (debug)
zlog_debug("%s: path %s loses to path %s due to iBGP peer < eBGP peeer",
zlog_debug("%s: path %s loses to path %s due to iBGP peer < eBGP peer",
pfx_buf, new->peer->host, exist->peer->host);
return 0;
}
Expand Down Expand Up @@ -599,7 +599,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
ret = 0;
}

/* 8.1. Same IGP metric. Compare the cluster list length as
/* 9. Same IGP metric. Compare the cluster list length as
representative of IGP hops metric. Rewrite the metric value
pair (newm, existm) with the cluster list length. Prefer the
path with smaller cluster list length. */
Expand Down Expand Up @@ -633,7 +633,27 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
}
}

/* 9. Maximum path check. */
/* 10. confed-external vs. confed-internal */
if (CHECK_FLAG(bgp->config, BGP_CONFIG_CONFEDERATION))
{
if (new_sort == BGP_PEER_CONFED && exist_sort == BGP_PEER_IBGP)
{
if (debug)
zlog_debug("%s: path %s wins over path %s due to confed-external peer > confed-internal peer",
pfx_buf, new->peer->host, exist->peer->host);
return 1;
}

if (exist_sort == BGP_PEER_CONFED && new_sort == BGP_PEER_IBGP)
{
if (debug)
zlog_debug("%s: path %s loses to path %s due to confed-internal peer < confed-external peer",
pfx_buf, new->peer->host, exist->peer->host);
return 0;
}
}

/* 11. Maximum path check. */
if (newm == existm)
{
if (bgp_flag_check(bgp, BGP_FLAG_ASPATH_MULTIPATH_RELAX))
Expand Down Expand Up @@ -682,7 +702,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
return ret;
}

/* 10. If both paths are external, prefer the path that was received
/* 12. If both paths are external, prefer the path that was received
first (the oldest one). This step minimizes route-flap, since a
newer path won't displace an older one, even if it was the
preferred route based on the additional decision criteria below. */
Expand All @@ -707,7 +727,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
}
}

/* 11. Router-ID comparision. */
/* 13. Router-ID comparision. */
/* If one of the paths is "stale", the corresponding peer router-id will
* be 0 and would always win over the other path. If originator id is
* used for the comparision, it will decide which path is better.
Expand Down Expand Up @@ -737,7 +757,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
return 0;
}

/* 12. Cluster length comparision. */
/* 14. Cluster length comparision. */
new_cluster = BGP_CLUSTER_LIST_LENGTH(new->attr);
exist_cluster = BGP_CLUSTER_LIST_LENGTH(exist->attr);

Expand All @@ -759,7 +779,7 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist,
return 0;
}

/* 13. Neighbor address comparision. */
/* 15. Neighbor address comparision. */
/* Do this only if neither path is "stale" as stale paths do not have
* valid peer information (as the connection may or may not be up).
*/
Expand Down

0 comments on commit 31a4638

Please sign in to comment.