[Xorp-hackers] [PATCH 15/15] xorp: rip: Force RIP to do EXPORT filtering when the route comes
igorm at etf.rs
igorm at etf.rs
Fri Aug 31 04:34:01 PDT 2012
From: Igor Maravic <igorm at etf.rs>
Also, don't check if metric is larger then RIP_INFINITY, before the route passes through the policy filters.
EXPORT and IMPORT filter, could posibly change metric and make it smaller then RIP_INFINITY.
After the route passes through the EXPORT filter, or if it doesn't pass IMPORT or EXPORT_SOURCEMATCH filters,
check the route cost. If it's larger then RIP_INFINITY set it to ROUTE_INFINITY.
Don't run EXPORT filter when the outgoing packet is sent, because route is pushed through the EXPORT filter when it comes to RIP.
Signed-off-by: Igor Maravic <igorm at etf.rs>
---
xorp/rip/output.hh | 29 ---------------------
xorp/rip/output_table.cc | 17 ++++++-------
xorp/rip/output_updates.cc | 11 ++++----
xorp/rip/route_db.cc | 60 +++++++++++++++++++++++++++++---------------
xorp/rip/route_db.hh | 2 +-
5 files changed, 55 insertions(+), 64 deletions(-)
diff --git a/xorp/rip/output.hh b/xorp/rip/output.hh
index 24fed44..13d04e5 100644
--- a/xorp/rip/output.hh
+++ b/xorp/rip/output.hh
@@ -129,14 +129,6 @@ protected:
void incr_packets_sent() { _pkts_out++; }
- /**
- * Policy filters the route.
- *
- * @param r route to filter.
- * @return true if the route was accepted, false otherwise.
- */
- bool do_filtering(RouteEntry<A>* r);
-
protected:
EventLoop& _e;
Port<A>& _port; // Port associated with output
@@ -190,25 +182,4 @@ OutputBase<A>::interpacket_gap_ms() const
return _port.constants().interpacket_delay_ms();
}
-
-template <typename A>
-bool
-OutputBase<A>::do_filtering(RouteEntry<A>* route)
-{
- try {
- RIPVarRW<A> varrw(*route);
-
- debug_msg("[RIP] Running export filter on route: %s\n",
- route->net().str().c_str());
-
- bool accepted = _policy_filters.run_filter(filter::EXPORT,
- varrw);
-
- return accepted;
- } catch(const PolicyException& e) {
- XLOG_FATAL("PolicyException: %s", e.str().c_str());
- XLOG_UNFINISHED();
- }
-}
-
#endif // __RIP_OUTPUT_HH__
diff --git a/xorp/rip/output_table.cc b/xorp/rip/output_table.cc
index dc2237e..b3d470f 100644
--- a/xorp/rip/output_table.cc
+++ b/xorp/rip/output_table.cc
@@ -50,15 +50,13 @@ OutputTable<A>::output_packet()
// or set cost to infinity...
// or depending on poison-reverse / horizon settings
//
- if (r->filtered()) {
+ if (r->filtered())
continue;
- }
pair<A,uint16_t> p = this->_port.route_policy(*r);
- if (p.second > RIP_INFINITY) {
+ if (p.second > RIP_INFINITY)
continue;
- }
RouteEntryOrigin<A>* origin = NULL; // XXX
string ifname, vifname; // XXX: not set, because not needed
@@ -68,11 +66,12 @@ OutputTable<A>::output_packet()
origin, r->tag(),
r->policytags());
- bool accepted = this->do_filtering(copy);
- if (!accepted) {
- delete copy;
- continue;
- }
+ // Policy EXPORT filtering was done here.
+ // It's moved to RouteDB<A>::do_filtering.
+ // This was done because EXPORT filter could possibly change route metric,
+ // and thus make it lower then RIP_INFINITY.
+ // Routes with cost > RIP_INFINTY would never come here, because they would be filtered out in RouteDB<A>::update_route
+ // - IMAR
rpa.packet_add_route(copy->net(), copy->nexthop(), copy->cost(), copy->tag());
diff --git a/xorp/rip/output_updates.cc b/xorp/rip/output_updates.cc
index aaf7148..92f8415 100644
--- a/xorp/rip/output_updates.cc
+++ b/xorp/rip/output_updates.cc
@@ -78,11 +78,12 @@ OutputUpdates<A>::output_packet()
origin, r->tag(),
r->policytags());
- bool accepted = this->do_filtering(copy);
- if (!accepted) {
- delete copy;
- continue;
- }
+ // Policy EXPORT filtering was done here.
+ // It's moved to RouteDB<A>::do_filtering.
+ // This was done because EXPORT filter could possibly change route metric,
+ // and thus make it lower then RIP_INFINITY
+ // Routes with cost > RIP_INFINTY would never come here, because they would be filtered out in RouteDB<A>::update_route
+ // - IMAR
rpa.packet_add_route(copy->net(), copy->nexthop(), copy->cost(), r->tag());
added_routes.insert(r);
diff --git a/xorp/rip/route_db.cc b/xorp/rip/route_db.cc
index 7b116b4..9c4fd0a 100644
--- a/xorp/rip/route_db.cc
+++ b/xorp/rip/route_db.cc
@@ -185,7 +185,7 @@ RouteDB<A>::set_expiry_timer(Route* r)
template <typename A>
bool
-RouteDB<A>::do_filtering(Route* r)
+RouteDB<A>::do_filtering(Route* r, uint32_t& cost)
{
try {
RIPVarRW<A> varrw(*r);
@@ -199,19 +199,44 @@ RouteDB<A>::do_filtering(Route* r)
bool accepted = _policy_filters.run_filter(filter::IMPORT, varrw);
if (!accepted)
- return false;
+ goto exit;
- RIPVarRW<A> varrw2(*r);
+ do {
+ RIPVarRW<A> varrw2(*r);
- debug_msg("[RIP] Running source match filter on route %s\n",
+ debug_msg("[RIP] Running source match filter on route %s\n",
r->net().str().c_str());
- XLOG_TRACE(trace()._routes,
+ XLOG_TRACE(trace()._routes,
"Running source match filter on route %s\n",
r->net().str().c_str());
- _policy_filters.run_filter(filter::EXPORT_SOURCEMATCH, varrw2);
+ accepted = _policy_filters.run_filter(filter::EXPORT_SOURCEMATCH, varrw2);
+ } while(0);
- return true;
+ if (!accepted)
+ goto exit;
+
+ do {
+ RIPVarRW<A> varrw3(*r);
+
+ debug_msg("[RIP] Running export filter on route: %s\n",
+ r->net().str().c_str());
+ XLOG_TRACE(trace()._routes,
+ "Running export filter on route %s\n",
+ r->net().str().c_str());
+
+ accepted = _policy_filters.run_filter(filter::EXPORT, varrw3);
+ } while(0);
+
+exit:
+ cost = r->cost();
+ if (r->cost() > RIP_INFINITY) {
+ r->set_cost(RIP_INFINITY);
+ cost = r->cost();
+ accepted = false;
+ }
+
+ return accepted;
} catch(const PolicyException& e) {
XLOG_FATAL("PolicyException: %s", e.str().c_str());
XLOG_UNFINISHED();
@@ -237,10 +262,6 @@ RouteDB<A>::update_route(const Net& net,
return false;
}
- if (cost > RIP_INFINITY) {
- cost = RIP_INFINITY;
- }
-
//
// Update steps, based on RFC2453 pp. 26-28
//
@@ -252,11 +273,7 @@ RouteDB<A>::update_route(const Net& net,
// Route does not appear in table so it needs to be
// created if peer does not have an entry for it or
- // resurrected if it does. But first this...
- if (cost == RIP_INFINITY) {
- // Don't bother adding a route for unreachable net
- return false;
- }
+ // resurrected if it does.
// Create route if necessary
r = o->find_route(net);
@@ -270,10 +287,10 @@ RouteDB<A>::update_route(const Net& net,
XLOG_ASSERT(ok);
- bool accepted = do_filtering(r);
+ bool accepted = do_filtering(r, cost);
r->set_filtered(!accepted);
- if (!accepted)
+ if (!accepted || cost == RIP_INFINITY)
return false;
_uq->push_back(r);
@@ -287,9 +304,12 @@ RouteDB<A>::update_route(const Net& net,
XLOG_ASSERT(ok);
// XXX: this is wrong
- bool accepted = do_filtering(r);
+ bool accepted = do_filtering(r, cost);
r->set_filtered(!accepted);
+ if (cost == RIP_INFINITY)
+ return false;
+
if (accepted)
updated = true;
} else {
@@ -302,7 +322,7 @@ RouteDB<A>::update_route(const Net& net,
cost, no_origin, tag,
policytags);
// XXX: lost origin
- bool accepted = do_filtering(new_route);
+ bool accepted = do_filtering(new_route, cost);
// XXX: this whole section of code is too entangled.
if (r->origin() == o) {
diff --git a/xorp/rip/route_db.hh b/xorp/rip/route_db.hh
index 0c795dc..0686920 100644
--- a/xorp/rip/route_db.hh
+++ b/xorp/rip/route_db.hh
@@ -213,7 +213,7 @@ public:
* @param r route to filter.
* @return true if route was accepted, false otherwise.
*/
- bool do_filtering(Route* r);
+ bool do_filtering(Route* r, uint32_t& cost);
Trace& trace() { return _trace; }
--
1.7.9.5
More information about the Xorp-hackers
mailing list