[Xorp-hackers] A question about delete_route() in xorp/bgp/route_table_decision.cc

[Atanu Ghosh]

Hi,

>>>>> "Yi" == Yi Wang <yiwang at cs.princeton.edu> writes:

    Yi> Hello,

    Yi> I was looking at the delete_route() function in
    Yi> route_table_decision.cc of the BGP implementation of XORP 1.3. I
    Yi> found the following snippet of code a bit confusing: what if the
    Yi> old_winner (old_winner_clone) and the new_winner are the same,
    Yi> but they both refer to the route to be deleted (the one in
    Yi> rtmsg)? If it is the case and we return -1 here, the
    Yi> delete_route message won't be propagate downstream.

The new_winner by definition can never refer to the current delete
route, it is the winner excluding the route from this peer. This is the
case where the current route was not the previous winner and its removal
has not caused a new winner to be chosen (I believe that the strange
way that MEDs are processed means that a route that is not the current
winner being deleted can change the winner).

    Yi> if (old_winner_clone != NULL) {
    Yi> if (new_winner != NULL
    Yi> && old_winner_clone->route() == new_winner->route()) {
    Yi> //the winner didn't change.
    Yi> XLOG_ASSERT(old_winner_clone != NULL);
    Yi> delete old_winner_clone;
    Yi> return -1;
    Yi> }
    Yi> ...
    Yi> }

    Yi> My guess is that there is some statement missing in the
    Yi> following "if" statement, at least the comment doesn't seems to
    Yi> match the statement below it. Maybe the author meant to delete
    Yi> the old_winner from alternatives before selecting the
    Yi> new_winner?

The list of alternatives does not contain the route from this peer so it
does not need to be removed. The comment seems to have been copied from
add_route() and delete_route() and has now been removed.

    Yi> if (!alternatives.empty()) {
    Yi> //add the new route to the pool of possible winners.
    Yi> new_winner = find_winner(alternatives);
    Yi> }

If you look at the implementation of find_alernative_routes() you will
see that it doesn't return the route from the current peer, it also
returns the previous winner. The find_winner() method uses the set of
alternatives (which excludes the current peer) to compute the new
winner.

	Atanu.