[Xorp-cvs] XORP cvs commit: xorp/bgp

Marko Zec zec@icir.org
Fri, 11 Nov 2005 18:38:41 +0100


On Friday 11 November 2005 18:06, Pavlin Radoslavov wrote:
> Presumably, the new aggregate-prefix-len inside the BGP policy
> template will be hooked with the bgp_varrw.{hh,cc} modifications.
> According to the recent modification to bgp.tp the variable is
> registered as "aggregate_prefix_len" with the policy.
> In that case, I believe the new BGPVarRW methods must match the
> variable name and must be named read_aggregate_prefix_len() and
> write_aggregate_prefix_len() instead of read_aggr_pref_len() and
> write_aggr_pref_len() respectively (Andrea please correct me if this
> is not the case).

I tested the bgp_varrw interface with the latest modifications from 
Andrea and it seems to work OK as is.  Whether there is a new naming 
convention that needs to be followed is another story, I am / was not 
aware of that.  I'll change the naming of the funcions as you suggest 
if needed, but I'd prefer not to have exceedingly long method names if 
not absolutely necessary.

> While there, if the reading should never happen (as indicated by
> the printf() inside read_aggr_pref_len()), then you could just
> make/register the aggregate_prefix_len policy variable write-only so
> you can just remove that method.

Agreed, but for the time being I'd like to leave it as-is for testing 
purposes.  If I would turn on debugging in bgp_varrw then this message 
could be easily lost in the noise of other stuff.

> Also, I presume you meant to use 
> debug_msg() instead of printf() inside the committed code :)

Actually the printf() will go away... Onve reading the preflen becomes 
prohibited using the varmap (as you suggested above) there will be no 
need for the printf at all, even in the form of debug_msg().

The issue is that in the outbound filters one might wish to read the 
contents of the preflen, but under a different name -> there it should 
read as a boolean telling us whether the route has been aggregated or 
not.

But let's see first if this aggregation thing will work at all :-)

Marko