[Xorp-hackers] [PATCH 2/2] xorp: policy: Fix problem with route distribution
igorm at etf.rs
igorm at etf.rs
Tue Apr 10 05:59:02 PDT 2012
From: Igor Maravic <igorm at etf.rs>
Problem was that protocol's existing EXPORT_SOURCE match policies weren't refreshed when new export filter is created.
New protocol always has accurate policy tags for route lookup. It always searches routes that has no tag, or
older protocol tags or it's tags. If it finds them it prepends it's tag.
Older protocols are searching routes that has no tags, or have older tags. If it finds such a route it prepends it's tag.
EXPORT_SOURCEMATCH policies are executed in order that depends on protocol name.
If newer protocol has lower letter (for example ospf's policies are executed before rip's, because "o" precedes "r"),
new protocol prepends new tags, that are unrecognizable to older protocol, to routes that it has found.
Because older protocol is unaware of new tags, it concludes that routes are deleted and it deletes them.
This is not what we are expecting.
If older protocol is executed first, it prepends it's tag to routes that it has found. Because old tag is recognizable by
newer protocol new tags are also prepended to routes. This case works as expected.
Solution - refresh tags for all protocol's EXPORT_SOURCEMATCH routes when new export filter is created.
Reported-by: Steve Dickey <swdickey at rockwellcollins.com>
Signed-off-by: Igor Maravic <igorm at etf.rs>
---
xorp/policy/code.cc | 47 ++++++++++++++++++++++++++++
xorp/policy/code.hh | 19 +++++++++++
xorp/policy/code_list.cc | 8 +++++
xorp/policy/code_list.hh | 8 +++++
xorp/policy/configuration.cc | 6 +++
xorp/policy/policy_list.cc | 2 +-
xorp/policy/source_match_code_generator.cc | 5 +++
7 files changed, 94 insertions(+), 1 deletions(-)
diff --git a/xorp/policy/code.cc b/xorp/policy/code.cc
index ce8593c..280cbfd 100644
--- a/xorp/policy/code.cc
+++ b/xorp/policy/code.cc
@@ -23,6 +23,7 @@
#include "policy_module.h"
#include "libxorp/xorp.h"
#include "policy/common/policy_utils.hh"
+#include "policy/common/elem_set.hh"
#include "code.hh"
bool
@@ -143,6 +144,52 @@ Code::operator+=(const Code& rhs)
}
void
+Code::set_redistribution_tags(const TagSet& redist_tags)
+{
+ TagSet::iterator iter;
+
+ for (iter = _redist_tags.begin(); iter != _redist_tags.end(); ++iter) {
+ _all_tags.erase(*iter);
+ }
+ _redist_tags.clear();
+
+ _redist_tags = redist_tags;
+ _all_tags.insert(_redist_tags.begin(), _redist_tags.end());
+}
+
+void
+Code::refresh_sm_redistribution_tags(const Code& accurate_code)
+{
+ if (!(_target == accurate_code._target && _target.filter() == filter::EXPORT_SOURCEMATCH))
+ return;
+
+ TagSet::iterator iter;
+
+ if (_redist_tags != accurate_code.redist_tags()) {
+
+ set_redistribution_tags(accurate_code.redist_tags());
+
+ ElemSetU32 element_set;
+ for (set<uint32_t>::const_iterator iter = _redist_tags.begin();
+ iter != _redist_tags.end(); ++iter) {
+ ElemU32 e(*iter);
+ element_set.insert(e);
+ }
+
+ string policy_set_str("PUSH set_u32 ");
+ string::size_type position;
+ for (position = _code.find(policy_set_str); position != string::npos;
+ position = _code.find(policy_set_str, position)) {
+ position += policy_set_str.size();
+ string::size_type num_chars = _code.find("\n", position) - position;
+
+ _code.replace(position, num_chars, element_set.str());
+ }
+
+ }
+}
+
+void
Code::add_subr(const string& policy, const string& code)
{
_subr[policy] = code;
diff --git a/xorp/policy/code.hh b/xorp/policy/code.hh
index 8189857..5a9e6d9 100644
--- a/xorp/policy/code.hh
+++ b/xorp/policy/code.hh
@@ -239,6 +239,25 @@ public:
_redist_tags.insert(tag);
}
+ /**
+ * Sets _redist_tags with provided set
+ *
+ * This function empties _redist_tags and its elements from _all_tags.
+ * @param redist_tags the redistribution tags to add.
+ */
+ void set_redistribution_tags(const TagSet& redist_tags);
+
+ /**
+ * Refreshes redistribution tags if this is code for
+ * EXPORT_SOURCEMATCH filter. Accurate redistribution
+ * tags are provided via Code reference in function argument.
+ *
+ * It also replaces tags inside policy code.
+ *
+ * @param accurate_code Code with accurate redistribution tags.
+ */
+ void refresh_sm_redistribution_tags(const Code& accurate_code);
+
void add_subr(const string& policy, const string& code);
const SUBR& subr() const;
diff --git a/xorp/policy/code_list.cc b/xorp/policy/code_list.cc
index 7057ac6..4f8af4c 100644
--- a/xorp/policy/code_list.cc
+++ b/xorp/policy/code_list.cc
@@ -56,6 +56,14 @@ CodeList::str() const
}
void
+CodeList::refresh_sm_redistribution_tags(Code& c)
+{
+ for (ListCode::iterator i = _codes.begin(); i != _codes.end(); ++i) {
+ (*i)->refresh_sm_redistribution_tags(c);
+ }
+}
+
+void
CodeList::link_code(Code& c) const
{
// go through all the code we have, and link it to c.
diff --git a/xorp/policy/code_list.hh b/xorp/policy/code_list.hh
index 2ae2d9f..460673b 100644
--- a/xorp/policy/code_list.hh
+++ b/xorp/policy/code_list.hh
@@ -61,6 +61,14 @@ public:
string str() const;
/**
+ * Refresh redistribution tags to all codes with
+ * EXPORT_SOURCEMATCH filter and with the same type as c
+ *
+ * @param c code with accurate redistribution tags
+ */
+ void refresh_sm_redistribution_tags(Code& c);
+
+ /**
* Links all code in the code list to c.
* The code is basically added to c.
*
diff --git a/xorp/policy/configuration.cc b/xorp/policy/configuration.cc
index 00b524e..a0e951e 100644
--- a/xorp/policy/configuration.cc
+++ b/xorp/policy/configuration.cc
@@ -354,6 +354,9 @@ Configuration::link_sourcematch_code(const Code::Target& target)
Code* code = new Code();
code->set_target(target);
+ // set accurate redistribution tags
+ code->set_redistribution_tags(_protocol_tags[target.protocol()]);
+
// only export statements have source match code.
// go through all of them and link.
_exports.link_code(*code);
@@ -512,6 +515,9 @@ Configuration::link_code(const Code::Target& target,
Code* code = new Code();
code->set_target(target);
+ // set accurate redistribution tags
+ code->set_redistribution_tags(_protocol_tags[target.protocol()]);
+
// link the code
iemap.link_code(target.protocol(), *code);
diff --git a/xorp/policy/policy_list.cc b/xorp/policy/policy_list.cc
index d08b02d..b50ec66 100644
--- a/xorp/policy/policy_list.cc
+++ b/xorp/policy/policy_list.cc
@@ -256,9 +256,9 @@ PolicyList::link_code(Code& ret)
CodeList* cl = (*i).second;
+ cl->refresh_sm_redistribution_tags(ret);
// because of target set in ret, only relevant code will be linked.
cl->link_code(ret);
-
}
}
diff --git a/xorp/policy/source_match_code_generator.cc b/xorp/policy/source_match_code_generator.cc
index e9309ba..3d7a537 100644
--- a/xorp/policy/source_match_code_generator.cc
+++ b/xorp/policy/source_match_code_generator.cc
@@ -88,6 +88,9 @@ SourceMatchCodeGenerator::addTerm()
term->set_target_filter(filter::EXPORT_SOURCEMATCH);
term->set_referenced_set_names(_code.referenced_set_names());
+ //add redistribution tags to code
+ term->set_redistribution_tags(_protocol_tags[_protocol]);
+
// see if we have code for this target already
CodeMap::iterator i = _codes.find(_protocol);
@@ -95,6 +98,8 @@ SourceMatchCodeGenerator::addTerm()
if (i != _codes.end()) {
Code* existing = (*i).second;
+ existing->refresh_sm_redistribution_tags(*term);
+
// link "raw" code
string s = _os.str();
--
1.7.5.4
More information about the Xorp-hackers
mailing list