This is on top of my previous OLSR patches.
* Stop spt.hh from spewing warnings (related to OLSR)
* Add debugging logic to help figure out why the assert
related to covered_n2_count >= reachable_n2_count is happening.
I saw it once...but it's much harder to reproduce now.
The 'dbg' message will be visible in core files, and it's also visible in the
xorp logs,
but it doesn't spam unless the error occurs.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
diff --git a/contrib/olsr/neighborhood.cc b/contrib/olsr/neighborhood.cc
index 882f7f2..c14f3c8 100644
--- a/contrib/olsr/neighborhood.cc
+++ b/contrib/olsr/neighborhood.cc
@@ -1467,13 +1467,14 @@ Neighborhood::recount_mpr_set()
{
//if (_neighbors.empty()
// return;
+ ostringstream dbg;
// Clear all existing MPR state for Neighbors.
reset_onehop_mpr_state();
// Clear all existing MPR state for TwoHopNeighbors.
// Compute number of now uncovered reachable nodes at radius=2.
- const size_t reachable_n2_count = reset_twohop_mpr_state();
+ const size_t reachable_n2_count = reset_twohop_mpr_state(dbg);
size_t covered_n2_count = 0;
set<OlsrTypes::NeighborID> new_mpr_set; // For debugging.
@@ -1481,30 +1482,29 @@ Neighborhood::recount_mpr_set()
if (_mpr_computation_enabled) {
// 8.3.1, 1: Start with an MPR set made of all members of N with
// willingness equal to WILL_ALWAYS.
- covered_n2_count += consider_persistent_cand_mprs();
- //XLOG_WARNING("covered_n2_count after consider_persistent: %zi
reachable_n2_count: %zi\n",
- // covered_n2_count, reachable_n2_count);
+ covered_n2_count += consider_persistent_cand_mprs(dbg);
// 8.3.1, 3: If N2 is still not fully covered, ensure that for
// all uncovered strict N2 reachable only via 1 edge, their
// neighbor N is selected as an MPR.
if (covered_n2_count < reachable_n2_count) {
- covered_n2_count += consider_poorly_covered_twohops();
- //XLOG_WARNING("covered_n2_count after consider_poorly_covered: %zi
reachable_n2_count: %zi\n",
- // covered_n2_count, reachable_n2_count);
+ covered_n2_count += consider_poorly_covered_twohops(dbg);
}
// 8.3.1, 4: If N2 is still not fully covered, consider
// candidate MPRs in descending order of willingness, reachability
// and degree.
if (covered_n2_count < reachable_n2_count) {
- consider_remaining_cand_mprs(reachable_n2_count, covered_n2_count);
- //XLOG_WARNING("covered_n2_count after consider_remaining: %zi
reachable_n2_count: %zi\n",
- // covered_n2_count, reachable_n2_count);
+ consider_remaining_cand_mprs(reachable_n2_count, covered_n2_count,
dbg);
}
- // Invariant: All reachable N2 must now be covered by MPRs.
- XLOG_ASSERT(covered_n2_count >= reachable_n2_count);
+ if (covered_n2_count < reachable_n2_count) {
+ dbg << " covered_n2_count: " << covered_n2_count << "
reachable_n2_count: "
+ << reachable_n2_count << endl;
+ XLOG_WARNING("%s", dbg.str().c_str());
+ // Invariant: All reachable N2 must now be covered by MPRs.
+ XLOG_ASSERT(covered_n2_count >= reachable_n2_count);
+ }
size_t removed_mpr_count = minimize_mpr_set(new_mpr_set);
debug_msg("MPRs removed: %u\n", XORP_UINT_CAST(removed_mpr_count));
@@ -1698,7 +1698,7 @@ Neighborhood::reset_onehop_mpr_state()
}
size_t
-Neighborhood::reset_twohop_mpr_state()
+Neighborhood::reset_twohop_mpr_state(ostringstream& oss)
{
size_t n2_count = 0;
@@ -1719,8 +1719,8 @@ Neighborhood::reset_twohop_mpr_state()
// If N2 is strict and is reachable, count it as a member of N2.
if (n2->is_strict() && n2->is_reachable()) {
- //XLOG_WARNING("Counting 2-hop neighbor, is strict and reachable:
%i, n2: %s",
- // n2->reachability(), n2->toStringBrief().c_str());
+ oss << "Counting 2-hop neighbor, is strict and reachable: " <<
n2->reachability()
+ << ", n2: " << n2->toStringBrief() << endl;
n2_count++;
}
}
@@ -1777,7 +1777,7 @@ Neighborhood::update_twohop_reachability(TwoHopNeighbor*
n2)
}
size_t
-Neighborhood::consider_persistent_cand_mprs()
+Neighborhood::consider_persistent_cand_mprs(ostringstream& dbg)
{
size_t persistent_mpr_count = 0;
@@ -1828,12 +1828,12 @@ Neighborhood::consider_persistent_cand_mprs()
XLOG_ASSERT(n->is_mpr());
n2->add_covering_mpr(n->id());
- //XLOG_WARNING("Covered n2: %s in consider_persistent.",
n2->toStringBrief().c_str());
+ dbg << "Covered n2: " << n2->toStringBrief() << " in
consider_persistent.\n";
covered_n2_count++;
}
else {
- //XLOG_WARNING("NOT covering n2: %s in consider_persistent, strict:
%i n: %s n->willingness: %i",
- // n2->toStringBrief().c_str(), n2->is_strict(),
n->toStringBrief().c_str(), n->willingness());
+ dbg << "NOT covering n2: " << n2->toStringBrief() << " in
consider_persistent, strict: "
+ << n2->is_strict() << " n: " << n->toStringBrief() << "
n->willingness: " << n->willingness() << endl;
}
}
@@ -1846,7 +1846,7 @@ Neighborhood::consider_persistent_cand_mprs()
}
size_t
-Neighborhood::consider_poorly_covered_twohops()
+Neighborhood::consider_poorly_covered_twohops(ostringstream& dbg)
{
size_t covered_n2_count = 0;
@@ -1874,14 +1874,13 @@ Neighborhood::consider_poorly_covered_twohops()
//
n2->add_covering_mpr(n->id());
n->set_is_mpr(true);
- //XLOG_WARNING("Counting poorly_covered n2: %s n is set as mpr:
%s",
- // n2->toStringBrief().c_str(), n->toStringBrief().c_str());
+ dbg << "Counting poorly_covered n2: " << n2->toStringBrief() << " n
is set as mpr: "
+ << n->toStringBrief() << endl;
covered_n2_count++;
}
else {
- //XLOG_WARNING("NOT Counting poorly_covered n2: %s strict: %i
reachability: %i n2-covered: %i",
- // n2->toStringBrief().c_str(), n2->is_strict(),
n2->reachability(),
- // n2->is_covered());
+ dbg << "NOT Counting poorly_covered n2: " << n2->toStringBrief() <<
" strict: " << n2->is_strict()
+ << " reachability: " << n2->reachability() << " n2-covered: "
<< n2->is_covered() << endl;
}
}
@@ -1890,7 +1889,7 @@ Neighborhood::consider_poorly_covered_twohops()
void
Neighborhood::consider_remaining_cand_mprs(const size_t n2_count,
- size_t& covered_n2_count)
+ size_t& covered_n2_count,
ostringstream& dbg)
{
typedef set<Neighbor*, CandMprOrderPred> CandMprBag;
UNUSED(n2_count);
@@ -1917,8 +1916,9 @@ Neighborhood::consider_remaining_cand_mprs(const size_t
n2_count,
cand_mprs.insert(n); // Insertion sort.
}
else {
- //XLOG_WARNING("Not using n: %s as cand_mpr, willingness: %i
is_cand_mpr: %i is_mpr: %i\n",
- // n->toStringBrief().c_str(), n->willingness(),
n->is_cand_mpr(), n->is_mpr());
+ dbg << "Not using n: " << n->toStringBrief() << " as cand_mpr,
willingness: "
+ << n->willingness() << " is_cand_mpr: " << n->is_cand_mpr() <<
" is_mpr: "
+ << n->is_mpr() << endl;
}
}
@@ -1928,8 +1928,8 @@ Neighborhood::consider_remaining_cand_mprs(const size_t
n2_count,
for (jj = cand_mprs.begin(); jj != cand_mprs.end(); jj++) {
Neighbor* n = (*jj);
- //XLOG_WARNING("Checking neighbour: %s link count: %zi\n",
- // n->toStringBrief().c_str(), n->twohop_links().size());
+ dbg << "Checking neighbour: " << n->toStringBrief() << " link count: "
+ << n->twohop_links().size() << endl;
// If all N2 are covered, we're done.
// How do you know you haven't counted duplicates?? Comments indicate
that redundant
@@ -1947,14 +1947,13 @@ Neighborhood::consider_remaining_cand_mprs(const size_t
n2_count,
TwoHopNeighbor* n2 = l2->destination();
if (n2->is_strict()) {
// Mark this strict N2 as covered by N. Mark N as MPR.
- //XLOG_WARNING("Adding covering_mpr: %s to n2: %s\n",
- // n->toStringBrief().c_str(),
n2->toStringBrief().c_str());
+ dbg << "Adding covering_mpr: " << n->toStringBrief() << " to
n2: " << n2->toStringBrief() << endl;
n2->add_covering_mpr(n->id());
n->set_is_mpr(true);
covered_n2_count++;
}
else {
- //XLOG_WARNING("n2: %s is strict, skipping.",
n2->toStringBrief().c_str());
+ dbg << "n2: " << n2->toStringBrief() << " is strict,
skipping.\n";
}
}
}
diff --git a/contrib/olsr/neighborhood.hh b/contrib/olsr/neighborhood.hh
index 2f7edfc..c0631c0 100644
--- a/contrib/olsr/neighborhood.hh
+++ b/contrib/olsr/neighborhood.hh
@@ -755,7 +755,7 @@ class Neighborhood {
* @return The number of reachable, strict two-hop neighbors
* to be considered by MPR selection.
*/
- size_t reset_twohop_mpr_state();
+ size_t reset_twohop_mpr_state(ostringstream& dbg);
/**
* Compute one-hop neighbor reachability and update it in the
@@ -807,7 +807,7 @@ class Neighborhood {
* @return The number of two-hop neighbors which have been covered
* by considering the persistent MPR candidates.
*/
- size_t consider_persistent_cand_mprs();
+ size_t consider_persistent_cand_mprs(ostringstream& dbg);
/**
* Consider MPR coverage of poorly covered two-hop neighbors.
@@ -820,7 +820,7 @@ class Neighborhood {
* @return The number of two-hop neighbors which have been covered
* by considering the persistent MPR candidates.
*/
- size_t consider_poorly_covered_twohops();
+ size_t consider_poorly_covered_twohops(ostringstream& dbg);
/**
* Consider remaining MPR candidates for MPR selection.
@@ -840,7 +840,7 @@ class Neighborhood {
* which this method will update.
*/
void consider_remaining_cand_mprs(const size_t n2_count,
- size_t& covered_n2_count);
+ size_t& covered_n2_count, ostringstream&
oss);
/**
* Mark all N1 neighbors was MPRs.
diff --git a/contrib/olsr/route_manager.cc b/contrib/olsr/route_manager.cc
index f29b309..47bc262 100644
--- a/contrib/olsr/route_manager.cc
+++ b/contrib/olsr/route_manager.cc
@@ -220,7 +220,10 @@ RouteManager::recompute_all_routes()
// Add the origin node.
_origin = make_origin_vertex();
- _spt.add_node(_origin);
+ Node<Vertex>::NodeRef vtmp = _spt.find_node(_origin);
+ if (vtmp.is_empty() || !vtmp->valid()) {
+ _spt.add_node(_origin);
+ }
_spt.set_origin(_origin);
// Ask the associated classes to push their topology to us.
@@ -430,10 +433,12 @@ RouteManager::add_tc_link(const TopologyEntry* tc)
}
// Add the vertex for the remote endpoint learned from TC.
- // A trace statement may be printed if the node already exists.
Vertex v_tc(*tc);
- bool is_tc_added = _spt.add_node(v_tc);
- UNUSED(is_tc_added);
+ Node<Vertex>::NodeRef vtmp = _spt.find_node(v_tc);
+ // Don't try to add it if it already exists..spams the logs with spt.hh
warnings.
+ if (vtmp.is_empty() || !vtmp->valid()) {
+ _spt.add_node(v_tc);
+ }
// Add the link from v_lh to v_tc.
// For a non-LQ link the cost is always 1.
diff --git a/libproto/spt.hh b/libproto/spt.hh
index 85e5b76..8dbb590 100644
--- a/libproto/spt.hh
+++ b/libproto/spt.hh
@@ -154,6 +154,12 @@ class Spt {
* of the graph.
*/
string str() const;
+
+ /**
+ * Find this node.
+ */
+ typename Node<A>::NodeRef find_node(const A& node);
+
private:
bool _trace; // True of tracing is enabled.
/**
@@ -171,11 +177,6 @@ class Spt {
bool incremental_spt();
/**
- * Find this node.
- */
- typename Node<A>::NodeRef find_node(const A& node);
-
- /**
* Remove all the nodes that have been marked for deletion.
*/
void garbage_collect();
_______________________________________________
Xorp-hackers mailing list
[email protected]
http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
