[bgpd] Fix peer prefix counts and make it slightly more robust
2006-02-05 Paul Jakma <paul.jakma@sun.com>
* bgp_route.h: Add BGP_INFO_COUNTED to track whether
prefix has been counted or not.
* bgp_route.c: (bgp_pcount_{inc,dec}rement) new helpers, to
centralise inc/dec of prefix-count,
(bgp_rib_remove) Remove pcount decrement, use helper.
(bgp_rib_withdraw) ditto, additionally use previous function
too.
(bgp_update_main) Use pcount helpers.
(bgp_clear_route_node) ditto, aslo REMOVED routes don't need
clearing.
diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog
index 714de1d..8e2a397 100644
--- a/bgpd/ChangeLog
+++ b/bgpd/ChangeLog
@@ -1,3 +1,16 @@
+2006-02-05 Paul Jakma <paul.jakma@sun.com>
+
+ * bgp_route.h: Add BGP_INFO_COUNTED to track whether
+ prefix has been counted or not.
+ * bgp_route.c: (bgp_pcount_{inc,dec}rement) new helpers, to
+ centralise inc/dec of prefix-count,
+ (bgp_rib_remove) Remove pcount decrement, use helper.
+ (bgp_rib_withdraw) ditto, additionally use previous function
+ too.
+ (bgp_update_main) Use pcount helpers.
+ (bgp_clear_route_node) ditto, aslo REMOVED routes don't need
+ clearing.
+
2006-02-02 Paul Jakma <paul.jakma@sun.com>
* bgp_route.c: (bgp_{clear_node,process}_queue_init) delay
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 3d9856b..ba6412e 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1554,6 +1554,43 @@
return 0;
}
+static void
+bgp_pcount_increment (struct bgp_node *rn, struct bgp_info *ri,
+ afi_t afi, safi_t safi)
+{
+ assert (ri && rn);
+
+ if (!CHECK_FLAG (ri->flags, BGP_INFO_COUNTED)
+ && rn->table->type == BGP_TABLE_MAIN)
+ {
+ SET_FLAG (ri->flags, BGP_INFO_COUNTED);
+ ri->peer->pcount[afi][safi]++;
+ }
+}
+
+static void
+bgp_pcount_decrement (struct bgp_node *rn, struct bgp_info *ri,
+ afi_t afi, safi_t safi)
+{
+ /* Ignore 'pcount' for RS-client tables */
+ if (CHECK_FLAG (ri->flags, BGP_INFO_COUNTED)
+ && rn->table->type == BGP_TABLE_MAIN)
+ {
+ UNSET_FLAG (ri->flags, BGP_INFO_COUNTED);
+
+ /* slight hack, but more robust against errors. */
+ if (ri->peer->pcount[afi][safi])
+ ri->peer->pcount[afi][safi]--;
+ else
+ {
+ zlog_warn ("%s: Asked to decrement 0 prefix count for peer %s",
+ __func__, ri->peer->host);
+ zlog_backtrace (LOG_WARNING);
+ zlog_warn ("%s: Please report to Quagga bugzilla", __func__);
+ }
+ }
+}
+
/* Unconditionally remove the route from the RIB, without taking
* damping into consideration (eg, because the session went down)
*/
@@ -1561,18 +1598,13 @@
bgp_rib_remove (struct bgp_node *rn, struct bgp_info *ri, struct peer *peer,
afi_t afi, safi_t safi)
{
- if (!CHECK_FLAG (ri->flags, BGP_INFO_HISTORY)
- && rn->table->type == BGP_TABLE_MAIN)
- {
- /* Ignore 'pcount' for RS-client tables */
- if ( rn->table->type == BGP_TABLE_MAIN)
- {
- peer->pcount[afi][safi]--;
- bgp_aggregate_decrement (peer->bgp, &rn->p, ri, afi, safi);
- }
- }
+ bgp_pcount_decrement (rn, ri, afi, safi);
+ bgp_aggregate_decrement (peer->bgp, &rn->p, ri, afi, safi);
+
+ if (!CHECK_FLAG (ri->flags, BGP_INFO_HISTORY))
+ bgp_info_delete (rn, ri); /* keep historical info */
+
bgp_process (peer->bgp, rn, afi, safi);
- bgp_info_delete (rn, ri);
}
static void
@@ -1581,17 +1613,6 @@
{
int status = BGP_DAMP_NONE;
- if (!CHECK_FLAG (ri->flags, BGP_INFO_HISTORY)
- && rn->table->type == BGP_TABLE_MAIN)
- {
- /* Ignore 'pcount' for RS-client tables */
- if ( rn->table->type == BGP_TABLE_MAIN)
- {
- peer->pcount[afi][safi]--;
- bgp_aggregate_decrement (peer->bgp, &rn->p, ri, afi, safi);
- }
- }
-
/* apply dampening, if result is suppressed, we'll be retaining
* the bgp_info in the RIB for historical reference.
*/
@@ -1599,12 +1620,13 @@
&& peer_sort (peer) == BGP_PEER_EBGP)
if ( (status = bgp_damp_withdraw (ri, rn, afi, safi, 0))
== BGP_DAMP_SUPPRESSED)
- return;
-
- bgp_process (peer->bgp, rn, afi, safi);
-
- if (status != BGP_DAMP_USED)
- bgp_info_delete (rn, ri);
+ {
+ bgp_pcount_decrement (rn, ri, afi, safi);
+ bgp_aggregate_decrement (peer->bgp, &rn->p, ri, afi, safi);
+ return;
+ }
+
+ bgp_rib_remove (rn, ri, peer, afi, safi);
}
static void
@@ -1952,13 +1974,13 @@
inet_ntop(p->family, &p->u.prefix, buf, SU_ADDRSTRLEN),
p->prefixlen);
- peer->pcount[afi][safi]++;
- ret = bgp_damp_update (ri, rn, afi, safi);
- if (ret != BGP_DAMP_SUPPRESSED)
- {
- bgp_aggregate_increment (bgp, p, ri, afi, safi);
- bgp_process (bgp, rn, afi, safi);
- }
+ bgp_pcount_increment (rn, ri, afi, safi);
+
+ if (bgp_damp_update (ri, rn, afi, safi) != BGP_DAMP_SUPPRESSED)
+ {
+ bgp_aggregate_increment (bgp, p, ri, afi, safi);
+ bgp_process (bgp, rn, afi, safi);
+ }
}
else
{
@@ -1973,7 +1995,8 @@
if (CHECK_FLAG (ri->flags, BGP_INFO_STALE))
{
UNSET_FLAG (ri->flags, BGP_INFO_STALE);
- peer->pcount[afi][safi]++;
+ bgp_pcount_increment (rn, ri, afi, safi);
+ bgp_process (bgp, rn, afi, safi);
}
}
@@ -1991,14 +2014,17 @@
/* graceful restart STALE flag unset. */
if (CHECK_FLAG (ri->flags, BGP_INFO_STALE))
- {
- UNSET_FLAG (ri->flags, BGP_INFO_STALE);
- peer->pcount[afi][safi]++;
- }
+ UNSET_FLAG (ri->flags, BGP_INFO_STALE);
/* The attribute is changed. */
SET_FLAG (ri->flags, BGP_INFO_ATTR_CHANGED);
-
+
+ /* implicit withdraw, decrement aggregate and pcount here.
+ * only if update is accepted, they'll increment below.
+ */
+ bgp_pcount_decrement (rn, ri, afi, safi);
+ bgp_aggregate_decrement (bgp, p, ri, afi, safi);
+
/* Update bgp route dampening information. */
if (CHECK_FLAG (bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)
&& peer_sort (peer) == BGP_PEER_EBGP)
@@ -2007,12 +2033,8 @@
information. */
if (! CHECK_FLAG (ri->flags, BGP_INFO_HISTORY))
bgp_damp_withdraw (ri, rn, afi, safi, 1);
- else
- peer->pcount[afi][safi]++;
}
- bgp_aggregate_decrement (bgp, p, ri, afi, safi);
-
/* Update to new attribute. */
bgp_attr_unintern (ri->attr);
ri->attr = attr_new;
@@ -2050,6 +2072,7 @@
SET_FLAG (ri->flags, BGP_INFO_VALID);
/* Process change. */
+ bgp_pcount_increment (rn, ri, afi, safi);
bgp_aggregate_increment (bgp, p, ri, afi, safi);
bgp_process (bgp, rn, afi, safi);
@@ -2066,9 +2089,6 @@
p->prefixlen);
}
- /* Increment prefix counter */
- peer->pcount[afi][safi]++;
-
/* Make new BGP info. */
new = bgp_info_new ();
new->type = type;
@@ -2096,7 +2116,8 @@
else
SET_FLAG (new->flags, BGP_INFO_VALID);
- /* Aggregate address increment. */
+ /* Increment prefix */
+ bgp_pcount_increment (rn, new, afi, safi);
bgp_aggregate_increment (bgp, p, new, afi, safi);
/* Register new BGP information. */
@@ -2469,10 +2490,11 @@
&& cq->peer->nsf[cq->afi][cq->safi]
&& ! CHECK_FLAG (ri->flags, BGP_INFO_STALE)
&& ! CHECK_FLAG (ri->flags, BGP_INFO_HISTORY)
- && ! CHECK_FLAG (ri->flags, BGP_INFO_DAMPED))
+ && ! CHECK_FLAG (ri->flags, BGP_INFO_DAMPED)
+ && ! CHECK_FLAG (ri->flags, BGP_INFO_REMOVED))
{
+ bgp_pcount_decrement (cq->rn, ri, cq->afi, cq->safi);
SET_FLAG (ri->flags, BGP_INFO_STALE);
- cq->peer->pcount[cq->afi][cq->safi]--;
}
else
bgp_rib_remove (cq->rn, ri, cq->peer, cq->afi, cq->safi);
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index aa2c59e..24be30f 100644
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -55,6 +55,7 @@
#define BGP_INFO_DMED_SELECTED (1 << 7)
#define BGP_INFO_STALE (1 << 8)
#define BGP_INFO_REMOVED (1 << 9)
+#define BGP_INFO_COUNTED (1 << 10)
/* Peer structure. */
struct peer *peer;