Skip to content

Commit

Permalink
Addpath does not work for soft-reconfiguration
Browse files Browse the repository at this point in the history
  • Loading branch information
@donaldsharp
donaldsharp committed May 20, 2015
1 parent 801a9bc commit 43143c8
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 31 deletions.
28 changes: 18 additions & 10 deletions bgpd/bgp_advertise.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
}
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions bgpd/bgp_advertise.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ struct bgp_adj_in

/* Received attribute. */
struct attr *attr;

/* Addpath identifier */
u_int32_t addpath_rx_id;
};

/* BGP advertisement list. */
Expand Down Expand Up @@ -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 *);
Expand Down
62 changes: 43 additions & 19 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand All @@ -3494,7 +3494,6 @@ bgp_soft_reconfig_table (struct peer *peer, afi_t afi, safi_t safi,
bgp_unlock_node (rn);
return;
}
continue;
}
}
}
Expand Down Expand Up @@ -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)
{
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 43143c8

Please sign in to comment.