From 19ea4cec4e46138c6153dac82148b0103951ad1f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 16 Mar 2020 17:23:34 -0400 Subject: [PATCH] bgpd: Fix certain code paths that reset reason code The bgp reason code was being reset in bgp_best_selection by rerunning bgp_path_info_cmp multiple times under certain receiving patterns of data from peers. This is the debugs that show this issue: 2020/03/16 19:17:22.523780 BGP: 2001:20:1:1::6 rcvd UPDATE w/ attr: nexthop 20.1.1.6, origin i, metric 600, community 1000:1006, path 20 2020/03/16 19:17:22.523819 BGP: 2001:20:1:1::6 rcvd 20.10.0.6/32 IPv4 unicast 2020/03/16 19:17:22.556168 BGP: 20.1.1.6 rcvd UPDATE w/ attr: nexthop 20.1.1.6, origin i, metric 500, community 1000:1006, path 20 2020/03/16 19:17:22.556209 BGP: 20.1.1.6 rcvd 20.10.0.6/32 IPv4 unicast 2020/03/16 19:17:22.572358 BGP: bgp_process_main_one: p=20.10.0.6/32 afi=IPv4, safi=unicast start 2020/03/16 19:17:22.572408 BGP: 20.10.0.6/32: Comparing path 2001:20:1:1::6 flags 0x410 with path 20.1.1.6 flags 0x410 2020/03/16 19:17:22.572415 BGP: 20.10.0.6/32: path 2001:20:1:1::6 loses to path 20.1.1.6 due to MED 600 > 500 2020/03/16 19:17:22.572422 BGP: 20.10.0.6/32: path 20.1.1.6 is the bestpath from AS 20 2020/03/16 19:17:22.572429 BGP: 20.10.0.6/32: path 20.1.1.6 is the initial bestpath 2020/03/16 19:17:22.572435 BGP: bgp_best_selection: pi 0x5627187c66c0 dmed 2020/03/16 19:17:22.572441 BGP: 20.10.0.6/32: After path selection, newbest is path 20.1.1.6 oldbest was NONE 2020/03/16 19:17:22.572447 BGP: 20.10.0.6/32: path 20.1.1.6 is the bestpath, add to the multipath list 2020/03/16 19:17:22.572453 BGP: 20.10.0.6/32: path 2001:20:1:1::6 has the same nexthop as the bestpath, skip it 2020/03/16 19:17:22.572460 BGP: 20.10.0.6/32: starting mpath update, newbest 20.1.1.6 num candidates 1 old-mpath-count 0 old-cum-bw u0 2020/03/16 19:17:22.572466 BGP: 20.10.0.6/32: comparing candidate 20.1.1.6 with existing mpath NONE 2020/03/16 19:17:22.572473 BGP: 20.10.0.6/32: New mpath count (incl newbest) 1 mpath-change NO all_paths_lb 0 cum_bw u0 Effectively if BGP receives 2 paths it could end up running bgp_path_info_cmp multiple times and in some situations overwrite the reason selected the first time through. In this example path selection is run and the MED is the reason for the choice. Then in bgp_best_selection is run again this time clearing new_select to NULL before calling path selection for the first time. This second call into path selection resets the reason, since it is only passing in one path. So save the last reason selected and restore in this case. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 0806e1c680..d057cd706b 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2103,6 +2103,7 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_node *rn, if (debug) prefix2str(&rn->p, pfx_buf, sizeof(pfx_buf)); + rn->reason = bgp_path_selection_none; /* bgp deterministic-med */ new_select = NULL; if (CHECK_FLAG(bgp->flags, BGP_FLAG_DETERMINISTIC_MED)) { @@ -2182,6 +2183,8 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_node *rn, new_select = NULL; for (pi = bgp_node_get_bgp_path_info(rn); (pi != NULL) && (nextpi = pi->next, 1); pi = nextpi) { + enum bgp_path_selection_reason reason; + if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) old_select = pi; @@ -2222,8 +2225,12 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_node *rn, bgp_path_info_unset_flag(rn, pi, BGP_PATH_DMED_CHECK); + reason = rn->reason; if (bgp_path_info_cmp(bgp, pi, new_select, &paths_eq, mpath_cfg, debug, pfx_buf, afi, safi, &rn->reason)) { + if (new_select == NULL && + reason != bgp_path_selection_none) + rn->reason = reason; new_select = pi; } } -- 2.39.5