On Thu, Mar 08, 2012 at 12:07:33PM +0100, Ondrej Zajicek wrote:
On Wed, Mar 07, 2012 at 03:45:42PM +0400, Oleg wrote:
So, what's wrong. It works fine, but the code seems some strange:
1. If we have empty clist or empty pair set constructions, we can do if ( allowed = [] ) then instead of hack with (0,0) community;
You could have empty pair set (like pair set my_null; my_null = []), but pair set comparison is not implemented.
Is it hard to implement? Do you plan to implement it :-)?
2. add(C,P) cann't get pair set like delete(C,P) do and
That is because 'delete' is just overloaded, there are two pairs of operations:
add/delete pair
delete/filter pair set.
3. we have incompatible types clist and pair set (Why? Why cann't pair set be modifiable?). So we cann't simply do comm = filter(bgp_community, allowed); bgp_community.delete([(51230,*)]); bgp_community.add(comm); instead of tons of lines.
Main reason is that pair sets contain intervals and wild cards and are represented using balanced trees for efficient matching, their construction is done in parse-time and not in run-time during matching so it is faster (and modifying would require re-construction or rebalancing, which is notrivial), while clist is just a list of pairs.
This is also a reason why set algebra (conjunction. disjunction, negation) is not implemented for (pair) sets, it have to be done in parse-time, but current filtering code is based on just run-time execution.
function filter_communities(pair set allowed) pair set comm; pair set def_allowed; { def_allowed = [(myas,1000), (myas,1001), (myas,1002)]; if ( allowed = [] ) then allowed = def_allowed; comm = filter(bgp_community, allowed); bgp_community.delete([(myas,*)]); bgp_community.add(comm); if (( comm = [] ) && ( (myas,1000) ~ allowed )) then bgp_community.add((myas,1000)); }
May be i do something wrong and my code can be simplified?
The code: comm = filter(bgp_community, allowed); bgp_community.delete([(myas,*)]); bgp_community.add(comm);
could be done if comm would be clist and adding clist to clist would be implemented (which i could add, this is simple).
This would be nice! I'm ready to git-pull :-).
Another way how to do this is even simpler:
def_allowed = [(myas,1000), (myas,1001), (myas,1002), (0..myas-1,*), (myas+1..65535,*)]; bgp_community.filter(def_allowed);
I thought about it, but this:
But in that case you have to also add (0..myas-1,*), (myas+1..65535,*) to 'allowed' pair set by caller.
would make function call more intricate, thus i discard this way.