community features request

[Oleg]

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.