From a6086ad4084a9dfbf930ef48e2987772767063bd Mon Sep 17 00:00:00 2001 From: vivek Date: Mon, 5 Sep 2016 10:53:06 -0700 Subject: [PATCH] bgpd: Enhance path selection logs Signed-off-by: Vivek Venkatraman Reviewed-by: Donald Sharp Reviewed-by: Daniel Walton Ticket: CM-12390 Reviewed By: CCR-5136 Testing Done: Manual --- bgpd/bgp_mpath.c | 38 ++++++++++++++++++++++++++++++++------ bgpd/bgp_route.c | 32 +++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c index 9d742204f6..e7272cc0aa 100644 --- a/bgpd/bgp_mpath.c +++ b/bgpd/bgp_mpath.c @@ -468,6 +468,11 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, bgp_info_mpath_dequeue (old_best); } + if (debug) + zlog_debug("%s: starting mpath update, newbest %s num candidates %d old-mpath-count %d", + pfx_buf, new_best ? new_best->peer->host : "NONE", + listcount (mp_list), old_mpath_count); + /* * We perform an ordered walk through both lists in parallel. * The reason for the ordered walk is that if there are paths @@ -481,6 +486,8 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, */ while (mp_node || cur_mpath) { + struct bgp_info *tmp_info; + /* * We can bail out of this loop if all existing paths on the * multipath list have been visited (for cleanup purposes) and @@ -491,6 +498,12 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, mp_next_node = mp_node ? listnextnode (mp_node) : NULL; next_mpath = cur_mpath ? bgp_info_mpath_next (cur_mpath) : NULL; + tmp_info = mp_node ? listgetdata (mp_node) : NULL; + + if (debug) + zlog_debug("%s: comparing candidate %s with existing mpath %s", + pfx_buf, tmp_info ? tmp_info->peer->host : "NONE", + cur_mpath ? cur_mpath->peer->host : "NONE"); /* * If equal, the path was a multipath and is still a multipath. @@ -506,6 +519,12 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, bgp_info_mpath_enqueue (prev_mpath, cur_mpath); prev_mpath = cur_mpath; mpath_count++; + if (debug) + { + bgp_info_path_with_addpath_rx_str(cur_mpath, path_buf); + zlog_debug("%s: %s is still multipath, cur count %d", + pfx_buf, path_buf, mpath_count); + } } else { @@ -513,10 +532,11 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, if (debug) { bgp_info_path_with_addpath_rx_str(cur_mpath, path_buf); - zlog_debug ("%s remove mpath nexthop %s %s", pfx_buf, + zlog_debug ("%s: remove mpath %s nexthop %s, cur count %d", + pfx_buf, path_buf, inet_ntop (AF_INET, &cur_mpath->attr->nexthop, nh_buf[0], sizeof (nh_buf[0])), - path_buf); + mpath_count); } } mp_node = mp_next_node; @@ -539,10 +559,11 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, if (debug) { bgp_info_path_with_addpath_rx_str(cur_mpath, path_buf); - zlog_debug ("%s remove mpath nexthop %s %s", pfx_buf, + zlog_debug ("%s: remove mpath %s nexthop %s, cur count %d", + pfx_buf, path_buf, inet_ntop (AF_INET, &cur_mpath->attr->nexthop, nh_buf[0], sizeof (nh_buf[0])), - path_buf); + mpath_count); } cur_mpath = next_mpath; } @@ -575,10 +596,11 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, if (debug) { bgp_info_path_with_addpath_rx_str(new_mpath, path_buf); - zlog_debug ("%s add mpath nexthop %s %s", pfx_buf, + zlog_debug ("%s: add mpath %s nexthop %s, cur count %d", + pfx_buf, path_buf, inet_ntop (AF_INET, &new_mpath->attr->nexthop, nh_buf[0], sizeof (nh_buf[0])), - path_buf); + mpath_count); } } mp_node = mp_next_node; @@ -587,6 +609,10 @@ bgp_info_mpath_update (struct bgp_node *rn, struct bgp_info *new_best, if (new_best) { + if (debug) + zlog_debug("%s: New mpath count (incl newbest) %d mpath-change %s", + pfx_buf, mpath_count, mpath_changed ? "YES" : "NO"); + bgp_info_mpath_count_set (new_best, mpath_count-1); if (mpath_changed || (bgp_info_mpath_count (new_best) != old_mpath_count)) SET_FLAG (new_best->flags, BGP_INFO_MULTIPATH_CHG); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index af8c2cf706..7295ff147f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -376,7 +376,11 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist, } if (debug) - bgp_info_path_with_addpath_rx_str (exist, exist_buf); + { + bgp_info_path_with_addpath_rx_str (exist, exist_buf); + zlog_debug("%s: Comparing %s flags 0x%x with %s flags 0x%x", + pfx_buf, new_buf, new->flags, exist_buf, exist->flags); + } newattr = new->attr; existattr = exist->attr; @@ -705,6 +709,15 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, struct bgp_info *exist, * TODO: If unequal cost ibgp multipath is enabled we can * mark the paths as equal here instead of returning */ + if (debug) + { + if (ret == 1) + zlog_debug("%s: %s wins over %s after IGP metric comparison", + pfx_buf, new_buf, exist_buf); + else + zlog_debug("%s: %s loses to %s after IGP metric comparison", + pfx_buf, new_buf, exist_buf); + } return ret; } @@ -1638,14 +1651,19 @@ bgp_best_selection (struct bgp *bgp, struct bgp_node *rn, /* Now that we know which path is the bestpath see if any of the other paths * qualify as multipaths */ - if (do_mpath && new_select) + if (debug) { - if (debug) - { - bgp_info_path_with_addpath_rx_str (new_select, path_buf); - zlog_debug("%s: %s is the bestpath, now find multipaths", pfx_buf, path_buf); - } + if (new_select) + bgp_info_path_with_addpath_rx_str (new_select, path_buf); + else + sprintf (path_buf, "NONE"); + zlog_debug("%s: After path selection, newbest is %s oldbest was %s", + pfx_buf, path_buf, + old_select ? old_select->peer->host : "NONE"); + } + if (do_mpath && new_select) + { for (ri = rn->info; (ri != NULL) && (nextri = ri->next, 1); ri = nextri) { -- 2.39.5