Skip to content

Commit

Permalink
xorp: rib: Don't propagate route copy from PolicyConnectedTable
Browse files Browse the repository at this point in the history
All routes are saved in trie, as pointers to non const objects.
That way they can be modified.
All routes will be cached in subsequent tables, and because of that,
we must pass pointers to objects that won't be destroyed.
This table is responsible of freeing the memory of route objects.
Passed all rib tests.
  • Loading branch information
Igor Maravic committed Mar 18, 2013
1 parent e1eae71 commit 0ffc526
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 39 deletions.
51 changes: 13 additions & 38 deletions xorp/rib/rt_tab_pol_conn.cc
Expand Up @@ -78,16 +78,13 @@ PolicyConnectedTable<A>::add_route(const IPRouteEntry<A>& route,
IPRouteEntry<A>* original = new IPRouteEntry<A>(route);
_route_table.insert(original->net(), original);

// make a copy so we may modify it
IPRouteEntry<A> route_copy(*original);
do_filtering(route_copy);

do_filtering(*original);

RouteTable<A>* next = this->next_table();
XLOG_ASSERT(next);

// Send the possibly modified route down
return next->add_route(route_copy, this);
return next->add_route(*original, this);
}

template <class A>
Expand All @@ -107,19 +104,19 @@ PolicyConnectedTable<A>::delete_route(const IPRouteEntry<A>* route,

XLOG_ASSERT(i != _route_table.end());

const IPRouteEntry<A>* re = *i;
IPRouteEntry<A>* re = *i;
_route_table.erase(route->net());
delete re;

RouteTable<A>* next = this->next_table();
XLOG_ASSERT(next);

// make a copy so we may modify it (e.g., by setting its policy tags)
IPRouteEntry<A> route_copy(*route);
do_filtering(route_copy);
do_filtering(*re);

// propagate the delete
return next->delete_route(&route_copy, this);
int ret = next->delete_route(re, this);
delete re;

return ret;
}

template <class A>
Expand All @@ -133,7 +130,7 @@ PolicyConnectedTable<A>::lookup_route(const IPNet<A>& net) const

// check if we have route [we should have same routes as origin table].
if (i == _route_table.end())
return _parent->lookup_route(net); // should return null probably
return NULL;

return *i;
}
Expand All @@ -150,7 +147,7 @@ PolicyConnectedTable<A>::lookup_route(const A& addr) const

// same as above
if (i == _route_table.end())
return _parent->lookup_route(addr); // return null
return NULL;

return *i;
}
Expand Down Expand Up @@ -202,39 +199,17 @@ PolicyConnectedTable<A>::push_routes()
RouteTable<A>* next = this->next_table();
XLOG_ASSERT(next);

vector<IPRouteEntry<A>*> new_routes;

// XXX: not a background task
// go through original routes and refilter them
for (typename RouteContainer::iterator i = _route_table.begin();
i != _route_table.end(); ++i) {

const IPRouteEntry<A>* prev = *i;

// make a copy so filter may [possibly] modify it
const IPRouteEntry<A>* orig = _parent->lookup_route(prev->net());
IPRouteEntry<A>* copy = new IPRouteEntry<A>(*orig);
IPRouteEntry<A>* prev = *i;

do_filtering(*copy);
do_filtering(*prev);

// only policytags may change
next->replace_policytags(*copy, prev->policytags(), this);

// delete old route
delete prev;

// keep the new route so we may re-store it after this iteration
new_routes.push_back(copy);
}

// store all new routes
for (typename vector<IPRouteEntry<A>*>::iterator i = new_routes.begin();
i != new_routes.end(); ++i) {

// replace route
IPRouteEntry<A>* route = *i;
_route_table.erase(route->net());
_route_table.insert(route->net(), route);
next->replace_policytags(*prev, prev->policytags(), this);
}
}

Expand Down
2 changes: 1 addition & 1 deletion xorp/rib/rt_tab_pol_conn.hh
Expand Up @@ -79,7 +79,7 @@ private:
void do_filtering(IPRouteEntry<A>& r);


typedef Trie<A, const IPRouteEntry<A>*> RouteContainer;
typedef Trie<A, IPRouteEntry<A>* > RouteContainer;


RouteTable<A>* _parent;
Expand Down

0 comments on commit 0ffc526

Please sign in to comment.