[Xorp-hackers] [PATCH] Admin distance set from config for RIP

[Igor Maravić]

2013/1/10 Jean Michel MacKay <mackay.jm at gmail.com>:
>
>
> On Thu, Jan 10, 2013 at 12:28 PM, Igor Maravić <igorm at etf.rs> wrote:
>>
>> 2012/12/17 Jean Michel MacKay <mackay.jm at gmail.com>:
>> > From: Jean Michel MacKay <jnmackay at rockwellcollins.com>
>> >
>> > ---
>> >  xorp/etc/templates/rip.tp |    8 ++++++++
>> >  xorp/rib/rib.cc           |   10 +++++++---
>> >  xorp/rib/rt_tab_origin.hh |    5 +++++
>> >  3 files changed, 20 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/xorp/etc/templates/rip.tp b/xorp/etc/templates/rip.tp
>> > index aff2763..8b32f04 100644
>> > --- a/xorp/etc/templates/rip.tp
>> > +++ b/xorp/etc/templates/rip.tp
>> > @@ -7,6 +7,7 @@
>> >  protocols {
>> >      rip {
>> >         targetname: txt = "rip";
>> > +       distance: u32;
>> >         traceoptions {
>> >             flag {
>> >                 all {
>> > @@ -95,6 +96,13 @@ protocols {
>> >             %set:;
>> >         }
>> >
>> > +       distance {
>> > +            %help:      short "Administrative Distance for RIP";
>> > +           %allow-range: $(@) "0" "255" %help: "Administrative Distance
>> > for RIP";
>> > +            %set:       xrl
>> > "rib/rib/0.1/set_protocol_admin_distance?protocol:txt=rip&ipv4:bool=true&ipv6:bool=false&unicast:bool=true&multicast:bool=false&admin_distance:u32=$(@)";
>> > +        }
>> > +
>> > +
>>
>> You should implement this for all protocols. It should be straight
>> forward.
>
> Will do.
>
>>
>> >         traceoptions {
>> >             %help:      short "Configure the tracing options";
>> >             flag {
>> > diff --git a/xorp/rib/rib.cc b/xorp/rib/rib.cc
>> > index 3137750..8ad07db 100644
>> > --- a/xorp/rib/rib.cc
>> > +++ b/xorp/rib/rib.cc
>> > @@ -222,11 +222,15 @@ RIB<A>::set_protocol_admin_distance(const string&
>> > protocol_name,
>> >      map<string, uint32_t>::iterator mi =
>> > _admin_distances.find(protocol_name);
>> >      if (mi != _admin_distances.end()) {
>> >         OriginTable<A>* ot = find_origin_table(protocol_name);
>> > -       if (NULL != ot) {
>> > -           XLOG_ERROR("May not set an admin distance for protocol
>> > \"%s\", "
>> > -                      "which has already instantiated an origin
>> > table.",
>> > +       if (NULL != ot && ot->route_count() > 0) {
>> > +               XLOG_ERROR("May not set an admin distance for protocol
>> > \"%s\", "
>> > +                      "which has already instantiated an origin table
>> > and contains routes.",
>> >                        protocol_name.c_str());
>> >             return XORP_ERROR;
>> > +        }else if(NULL != ot && ot->route_count() == 0) {
>>
>> Style should be fixed.
>
>
> As in?
> else if(NULL != ot && 0 == ot->route_count() )

As in

} else if (NULL != ot && 0 == ot->route_count())

There should be a ws between if and opening brace. You don't need ws
between closing braces at the end.

>
>>
>> > +            ot->change_admin_distance(admin_distance);
>> > +            XLOG_WARNING("Origin table was set but no routes were set."
>> > +                        " Should be safe to change admin distance.");
>>
>> You don't need WARNING here.
>>
> Yeah figured as much... Forgot to take it out from my debugging.
>
>> >         }
>> >      }
>> >      _admin_distances[protocol_name] = admin_distance;
>> > diff --git a/xorp/rib/rt_tab_origin.hh b/xorp/rib/rt_tab_origin.hh
>> > index 1899289..eb6df6b 100644
>> > --- a/xorp/rib/rt_tab_origin.hh
>> > +++ b/xorp/rib/rt_tab_origin.hh
>> > @@ -145,6 +145,11 @@ public:
>> >      RouteRange<A>* lookup_route_range(const A& addr) const;
>> >
>> >      /**
>> > +    * Changes the admin distance
>> > +    **/
>> > +    void change_admin_distance( uint32_t ad ){ _admin_distance = ad; }
>> > +
>> > +    /**
>> >       * @return the default administrative distance for this OriginTable
>> >       */
>> >      uint32_t admin_distance() const    { return _admin_distance; }
>> > --
>> > 1.7.4.1
>> >
>> > _______________________________________________
>> > Xorp-hackers mailing list
>> > Xorp-hackers at icir.org
>> > http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
>
>