[Xorp-hackers] OLSR assert

Bruce Simpson bms at incunabulum.net
Thu Oct 1 03:06:36 PDT 2009


Ben Greear wrote:
>
> Here's an attached patch that seems to fix things.  I believe the main 
> error
> was checking for (!is_mpr()) in consider_remaining_cand_mprs
>
> I can't see why that check helps anything, and it was excluding from 
> consideration the mpr
> that was needed to find the 2-hop neighbor in my setup.

I'm not 100% sure about this. It's been a long time since that code was 
written, so I'm hazy on details. [..reads code..]

Good catch. I'd conservatively check it in, given that the real hard 
work of MPR set computation in OLSR, is in fact in minimizing the set. 
As you can see, OLSR is tricky to do in an event-driven way, and it's 
easy to introduce bugs.

The bug is (in English): Just because a node was selected to cover a 
poorly covered N2, should not exclude it from consideration for other N2.

The is_mpr flag is cleared on every new MPR recount. It should only be 
set by the MPR recount code. The check for !is_mpr() was probably there 
as an optimization against the work already done by the 
consider_poorly_covered_twohops() and consider_persistent_cand_mprs().

Yes, this could cause otherwise valid MPRs to be skipped in 
Neighborhood::consider_remaining_cand_mprs(), given that all MPRs for a 
subset of N2 have to be considered anyway; the notion of 'persistent' 
only really applies to N (- WILL_ALWAYS.

When considering all other candidate MPRs, the CandMprOrderPred will 
return the first (highest) match anyway. Multiple candidates get 
filtered out when the MPR set is later minimized anyway. It might be 
better just to get rid of consider_persistent_cand_mprs() in this case.

later,
BMS



More information about the Xorp-hackers mailing list