From 43143c8f2c1ec6e3db52865cadaba8be49f200a4 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 19 May 2015 18:04:01 -0700 Subject: [PATCH] Addpath does not work for soft-reconfiguration --- bgpd/bgp_advertise.c | 28 +++++++++++++------- bgpd/bgp_advertise.h | 7 +++-- bgpd/bgp_route.c | 62 ++++++++++++++++++++++++++++++-------------- 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c index bd60ca10a9..70d878739c 100644 --- a/bgpd/bgp_advertise.c +++ b/bgpd/bgp_advertise.c @@ -189,13 +189,14 @@ bgp_adj_out_lookup (struct peer *peer, struct prefix *p, void -bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr) +bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr, + u_int32_t addpath_id) { struct bgp_adj_in *adj; for (adj = rn->adj_in; adj; adj = adj->next) { - if (adj->peer == peer) + if (adj->peer == peer && adj->addpath_rx_id == addpath_id) { if (adj->attr != attr) { @@ -208,6 +209,7 @@ bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr) adj = XCALLOC (MTYPE_BGP_ADJ_IN, sizeof (struct bgp_adj_in)); adj->peer = peer_lock (peer); /* adj_in peer reference */ adj->attr = bgp_attr_intern (attr); + adj->addpath_rx_id = addpath_id; BGP_ADJ_IN_ADD (rn, adj); bgp_lock_node (rn); } @@ -222,19 +224,25 @@ bgp_adj_in_remove (struct bgp_node *rn, struct bgp_adj_in *bai) } void -bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer) +bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer, + u_int32_t addpath_id) { struct bgp_adj_in *adj; + struct bgp_adj_in *adj_next; - for (adj = rn->adj_in; adj; adj = adj->next) - if (adj->peer == peer) - break; + adj = rn->adj_in; + while (adj) + { + adj_next = adj->next; - if (! adj) - return; + if (adj->peer == peer && adj->addpath_rx_id == addpath_id) + { + bgp_adj_in_remove (rn, adj); + bgp_unlock_node (rn); + } - bgp_adj_in_remove (rn, adj); - bgp_unlock_node (rn); + adj = adj_next; + } } void diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h index a147437470..d5b737b9ed 100644 --- a/bgpd/bgp_advertise.h +++ b/bgpd/bgp_advertise.h @@ -101,6 +101,9 @@ struct bgp_adj_in /* Received attribute. */ struct attr *attr; + + /* Addpath identifier */ + u_int32_t addpath_rx_id; }; /* BGP advertisement list. */ @@ -168,8 +171,8 @@ struct bgp_synchronize extern int bgp_adj_out_lookup (struct peer *, struct prefix *, afi_t, safi_t, struct bgp_node *); -extern void bgp_adj_in_set (struct bgp_node *, struct peer *, struct attr *); -extern void bgp_adj_in_unset (struct bgp_node *, struct peer *); +extern void bgp_adj_in_set (struct bgp_node *, struct peer *, struct attr *, u_int32_t); +extern void bgp_adj_in_unset (struct bgp_node *, struct peer *, u_int32_t); extern void bgp_adj_in_remove (struct bgp_node *, struct bgp_adj_in *); extern void bgp_sync_init (struct peer *); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 7a5f01a3d1..2e572fe757 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2882,7 +2882,7 @@ bgp_update_main (struct peer *peer, struct prefix *p, u_int32_t addpath_id, Adj-RIBs-In. */ if (! soft_reconfig && CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG) && peer != bgp->peer_self) - bgp_adj_in_set (rn, peer, attr); + bgp_adj_in_set (rn, peer, attr, addpath_id); /* Check previously received route. */ for (ri = rn->info; ri; ri = ri->next) @@ -3281,7 +3281,7 @@ bgp_withdraw (struct peer *peer, struct prefix *p, u_int32_t addpath_id, further calculation. */ if (CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG) && peer != bgp->peer_self) - bgp_adj_in_unset (rn, peer); + bgp_adj_in_unset (rn, peer, addpath_id); /* Lookup withdrawn route. */ for (ri = rn->info; ri; ri = ri->next) @@ -3485,7 +3485,7 @@ bgp_soft_reconfig_table (struct peer *peer, afi_t afi, safi_t safi, struct bgp_info *ri = rn->info; u_char *tag = (ri && ri->extra) ? ri->extra->tag : NULL; - ret = bgp_update (peer, &rn->p, ri->addpath_rx_id, ain->attr, + ret = bgp_update (peer, &rn->p, ain->addpath_rx_id, ain->attr, afi, safi, ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, prd, tag, 1); @@ -3494,7 +3494,6 @@ bgp_soft_reconfig_table (struct peer *peer, afi_t afi, safi_t safi, bgp_unlock_node (rn); return; } - continue; } } } @@ -3543,6 +3542,9 @@ bgp_clear_route_node (struct work_queue *wq, void *data) assert (rn && peer); + /* It is possible that we have multiple paths for a prefix from a peer + * if that peer is using AddPath. + */ for (ri = rn->info; ri; ri = ri->next) if (ri->peer == peer || cnq->purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) { @@ -3554,7 +3556,6 @@ bgp_clear_route_node (struct work_queue *wq, void *data) bgp_info_set_flag (rn, ri, BGP_INFO_STALE); else bgp_rib_remove (rn, ri, peer, afi, safi); - break; } return WQ_SUCCESS; } @@ -3624,6 +3625,7 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi, { struct bgp_info *ri; struct bgp_adj_in *ain; + struct bgp_adj_in *ain_next; struct bgp_adj_out *aout; /* XXX:TODO: This is suboptimal, every non-empty route_node is @@ -3656,14 +3658,23 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi, * Given that our per-peer prefix-counts now should be reliable, * this may actually be achievable. It doesn't seem to be a huge * problem at this time, + * + * It is possible that we have multiple paths for a prefix from a peer + * if that peer is using AddPath. */ - for (ain = rn->adj_in; ain; ain = ain->next) - if (ain->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) - { - bgp_adj_in_remove (rn, ain); - bgp_unlock_node (rn); - break; - } + ain = rn->adj_in; + while (ain) + { + ain_next = ain->next; + + if (ain->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) + { + bgp_adj_in_remove (rn, ain); + bgp_unlock_node (rn); + } + + ain = ain_next; + } /* * Can't do this anymore. adj-outs are not maintained per peer. @@ -3788,17 +3799,30 @@ bgp_clear_adj_in (struct peer *peer, afi_t afi, safi_t safi) struct bgp_table *table; struct bgp_node *rn; struct bgp_adj_in *ain; + struct bgp_adj_in *ain_next; table = peer->bgp->rib[afi][safi]; + /* It is possible that we have multiple paths for a prefix from a peer + * if that peer is using AddPath. + */ for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn)) - for (ain = rn->adj_in; ain ; ain = ain->next) - if (ain->peer == peer) - { - bgp_adj_in_remove (rn, ain); - bgp_unlock_node (rn); - break; - } + { + ain = rn->adj_in; + + while (ain) + { + ain_next = ain->next; + + if (ain->peer == peer) + { + bgp_adj_in_remove (rn, ain); + bgp_unlock_node (rn); + } + + ain = ain_next; + } + } } void -- 2.39.5