bgp may add multiple path entries with the same nexthop

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-8129

- We have 14 paths for each prefix, 7 are from ipv4 peers and 7 are from ipv6 peers

- There are 7 unique nexthops

- When comparing the exact same path from an v4 peer vs. a v6 peer the path from
  the v4 peer wins.  This is due to the "lowest neighbor IP" check in the decision
  algorithm.  For example below we learn NEXTHOP 210.2.4.2 from 210.2.4.2 and
  2001:20:4::2 but only the one from the v4 peer is flagged as multipath.

- The problem is when our bestpath is from a v6 peer, 2001:20:2::2 in this case
  (see line 85). 2001:20:2::2 sent us 210.2.2.2 so we will install that nexthop
  because it is from our bestpath, the problem is we flag the path from 210.2.2.2
  (line 37) as multipath which causes us to install two paths with nexthop 210.2.2.2

  1 superm-redxp-05# show ip bgp 2.23.24.192/28
  2 BGP routing table entry for 2.23.24.192/28
  3 Paths: (14 available, best #14, table Default-IP-Routing-Table)
  4   Advertised to non peer-group peers:
  5   210.2.0.2 210.2.1.2 210.2.2.2 210.2.3.2 210.2.4.2 210.2.5.2 210.2.6.2 210.4.1.4 2001:20::2 2001:20:1::2 2001:20:2::2 2001:20:3::2 2001:20:4::2 2001:2
  6   205 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
  7     210.2.4.2 from 210.2.4.2 (10.0.0.2)
  8       Origin IGP, localpref 100, valid, external, multipath
  9       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 10       Last update: Wed Nov 11 20:54:57 2015
 11
 12   204 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 13     210.2.3.2 from 210.2.3.2 (10.0.0.2)
 14       Origin IGP, localpref 100, valid, external, multipath
 15       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 16       Last update: Wed Nov 11 20:54:57 2015
 17
 18   202 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 19     210.2.1.2 from 210.2.1.2 (10.0.0.2)
 20       Origin IGP, localpref 100, valid, external, multipath
 21       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 22       Last update: Wed Nov 11 20:54:57 2015
 23
 24   206 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 25     210.2.5.2 from 2001:20:5::2 (10.0.0.2)
 26       Origin IGP, localpref 100, valid, external
 27       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 28       Last update: Wed Nov 11 20:54:57 2015
 29
 30   205 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 31     210.2.4.2 from 2001:20:4::2 (10.0.0.2)
 32       Origin IGP, localpref 100, valid, external
 33       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 34       Last update: Wed Nov 11 20:54:57 2015
 35
 36   203 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 37     210.2.2.2 from 210.2.2.2 (10.0.0.2)
 38       Origin IGP, localpref 100, valid, external, multipath
 39       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 40       Last update: Wed Nov 11 20:54:57 2015
 41
 42   202 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 43     210.2.1.2 from 2001:20:1::2 (10.0.0.2)
 44       Origin IGP, localpref 100, valid, external
 45       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 46       Last update: Wed Nov 11 20:54:57 2015
 47
 48   201 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 49     210.2.0.2 from 210.2.0.2 (10.0.0.2)
 50       Origin IGP, localpref 100, valid, external, multipath
 51       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 52       Last update: Wed Nov 11 20:54:57 2015
 53
 54   206 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 55     210.2.5.2 from 210.2.5.2 (10.0.0.2)
 56       Origin IGP, localpref 100, valid, external, multipath
 57       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 58       Last update: Wed Nov 11 20:54:57 2015
 59
 60   207 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 61     210.2.6.2 from 2001:20:6::2 (10.0.0.2)
 62       Origin IGP, localpref 100, valid, external
 63       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 64       Last update: Wed Nov 11 20:54:57 2015
 65
 66   207 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 67     210.2.6.2 from 210.2.6.2 (10.0.0.2)
 68       Origin IGP, localpref 100, valid, external, multipath
 69       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 70       Last update: Wed Nov 11 20:54:57 2015
 71
 72   201 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 73     210.2.0.2 from 2001:20::2 (10.0.0.2)
 74       Origin IGP, localpref 100, valid, external
 75       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 76       Last update: Wed Nov 11 20:54:57 2015
 77
 78   204 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 79     210.2.3.2 from 2001:20:3::2 (10.0.0.2)
 80       Origin IGP, localpref 100, valid, external
 81       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 82       Last update: Wed Nov 11 20:54:57 2015
 83
 84   203 200 300 790 90 80 2334 544 56 67 889 3111 777 8 879 900 88 7654 3211 113 43434 666 343 4534 667 7688
 85     210.2.2.2 from 2001:20:2::2 (10.0.0.2)
 86       Origin IGP, localpref 100, valid, external, multipath, best
 87       Community: 0:100 0:200 0:300 0:324 0:2938 0:3344 0:3545 0:4466 0:5445 0:5754
 88       Last update: Wed Nov 11 20:54:57 2015
 89
 90 superm-redxp-05#

Here you can see the two paths with nexthop 210.2.2.2

superm-redxp-05# show ip route 2.23.24.192/28
Routing entry for 2.23.24.192/28
  Known via "bgp", distance 20, metric 0, best
  Last update 00:32:12 ago
  * 210.2.2.2, via swp3
  * 210.2.0.2, via swp1
  * 210.2.1.2, via swp2
  * 210.2.2.2, via swp3
  * 210.2.3.2, via swp4
  * 210.2.4.2, via swp5
  * 210.2.5.2, via swp6
  * 210.2.6.2, via swp7

superm-redxp-05#
superm-redxp-05#

The fix is to not flag a path as multipath if it has the same nexthop as the bestpath

diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c
index 30eefdb5fed18ba51dfab3b6e2900c5f7ae3f2d6..c0e1a61636a760fc84848a7f6041ca46477198c4 100644 (file)
--- a/bgpd/bgp_mpath.c
+++ b/bgpd/bgp_mpath.c
@@ -103,7 +103,7 @@ bgp_maximum_paths_unset (struct bgp *bgp, afi_t afi, safi_t safi,
  * or greater than zero if bi1 is respectively less than, equal to,
  * or greater than bi2.
  */
-static int
+int
 bgp_info_nexthop_cmp (struct bgp_info *bi1, struct bgp_info *bi2)
 {
   struct attr_extra *ae1, *ae2;

diff --git a/bgpd/bgp_mpath.h b/bgpd/bgp_mpath.h
index 0c8137041af6d381d01c1e2f8abf08720f4a06ab..729b2c8cabe0b4a18bed149cd60e45c870b248af 100644 (file)
--- a/bgpd/bgp_mpath.h
+++ b/bgpd/bgp_mpath.h
@@ -59,6 +59,7 @@ extern int bgp_maximum_paths_unset (struct bgp *, afi_t, safi_t, int);
 /* Functions used by bgp_best_selection to record current
  * multipath selections
  */
+extern int bgp_info_nexthop_cmp (struct bgp_info *bi1, struct bgp_info *bi2);
 extern void bgp_mp_list_init (struct list *);
 extern void bgp_mp_list_clear (struct list *);
 extern void bgp_mp_list_add (struct list *, struct bgp_info *);

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index eae8f616455378cf9e8ad0f4936dbda4e613aaab..4d5d6ad690a8daee290c832c29d6f09dfa6874bb 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1637,12 +1637,20 @@ bgp_best_selection (struct bgp *bgp, struct bgp_node *rn,
             if (ri->peer->status != Established)
               continue;
 
+          if (!bgp_info_nexthop_cmp (ri, new_select))
+            {
+              if (debug)
+                zlog_debug("%s: path %s has the same nexthop as the bestpath, skip it",
+                           pfx_buf, ri->peer->host);
+              continue;
+            }
+
           bgp_info_cmp (bgp, ri, new_select, &paths_eq, mpath_cfg, debug, pfx_buf);
 
           if (paths_eq)
             {
               if (debug)
-                zlog_debug("%s: %s path is equivalent to the bestpath, add to the multipath list",
+                zlog_debug("%s: path %s is equivalent to the bestpath, add to the multipath list",
                            pfx_buf, ri->peer->host);
              bgp_mp_list_add (&mp_list, ri);
             }

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index db97810013427f500dc6f62605fec5c2cf01a559..3ad59ff0c12bcecb4a21b22f9372ad790f6449a1 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -6899,10 +6899,10 @@ bgp_config_write (struct vty *vty)
             {
               vty_out (vty, " bgp bestpath as-path multipath-relax as-set%s", VTY_NEWLINE);
             }
-        }
-     else
-        {
-         vty_out (vty, " no bgp bestpath as-path multipath-relax%s", VTY_NEWLINE);
+          else
+            {
+              vty_out (vty, " bgp bestpath as-path multipath-relax%s", VTY_NEWLINE);
+            }
         }
 
       if (bgp_flag_check (bgp, BGP_FLAG_RR_ALLOW_OUTBOUND_POLICY)) {