[Xorp-hackers] BG 172

[Kristian Larsson]

On Mon, Mar 20, 2006 at 07:01:27PM -0800, Pavlin Radoslavov wrote:
> All,
> 
> So far I see strong support for removing the pair "create/set"
> and having a single "set" command.
> 
> Before jumping into a final decision and taking some actions, I'd
> like to clarify few things about create/set so there won't be
> any surprises after their unification.
> 
> * First, a little bit extra information about "create" and "set".
>   Think about "create" as the command to add new configuration
>   statements.
>   The "set" command is the one used to assign or change the
>   value of a node _only_ if that node is already part of the
>   configuration.
>   As a result of that distinction, "create" needs to know about
>   all possible configuration statements so it can provide the
>   appropriate command-line completion. On the other hand, "set"
>   provides command-line completion only about nodes that are part of
>   the current configuration.
This is a unique functionality of xorp that noone
else provides and while I agree that it on some
level is very nice (as I mentioned in a previous
letter) I think the generally accepted way of
having one set is the way to go.
> 
> * The command-line completion behavior will change when expanding
>   grouping nodes in every level of the configuration tree.
> 
>   Currently, if there is a single protocol that is configured, say
>   "ospf4", if we type "set protocols" and try to perform
>   command-line completion, then the command line automatically
>   expands to "set protocols ospf4", because it is the only
>   configured protocol (i.e., the only possible completion).
>   Similarly, if "rip" was also configured, then the command-line
>   completion for "set protocols" will list only "ospf4" and "rip".
> 
>   If we remove "create" and move its functionality to "set", then
>   trying to perform command-line completion with the new "set"
>   on command like "set protocols" will list all possible protocols.
>   Such enlargement of the possible command-line completions will
>   happen at every level of the configuration tree even if the only
>   thing someone wants to do is modify an existing variable.
Perhaps we could mark nodes to be created with *,
say you already have protocols ospf4 and you type:
set protocols <tab>
  bgp*	Border Gateway Protocol
  ospf4	OSPF
  rip*	RIP
  isis*	IS-IS

and as someone else said, when actually executing
the command, tell the user what just happened, ie:
protocols ospf4 area 0.0.0.0 node has been created
or
protocols ospf4 router-id was modified

> * The command-line completion behavior will change when expanding
>   multi-value nodes.
> 
>   Currently, if there is say, a single OSPF area 0.0.0.0, then the
>   command-line completion for "set protocols ospf4 area" will
>   automatically expand to node "0.0.0.0", because it is the only
>   configured area.
>   With the new "set", the completion after "set protocols ospf4 area"
>   will list "0.0.0.0" and "<IPv4>" as possible completions: the
>   first one the existing area, and the second one as a way to add a
>   new area (Note: "0.0.0.0" will be listed only after Bugzilla entry
>   #330 is fixed).
>   There will be such additional generic entries everywhere where we
>   have multi-value nodes (interface and vif names, addresses, etc).
> 
> * With the new "set" command it becomes unclear whether we are
>   modifying an existing node or creating a new one.
>   To take Patrick's example (Bugzilla entry #172, Comment #2),
>   let say we are using the following two commands:
> 
> set route static route4 10.0.0.0/24 next-hop 172.16.1.1
> set route static route4 10.0.0.0/24 next-hop 172.16.1.2
> 
>   Currently, "next-hop" is a leaf node that can take a single value,
>   hence with the old and the new "set" command we are actually
>   modifying its value. However, lets say we change "next-hop" to
>   be a multi-value node (e.g., along the lines of implementing
>   floating static routes per Bugzilla entry #165).
>   Then with the current "set" command the above two commands are
>   automatically invalidated (you will get a syntax error) so it
>   becomes clear "next-hop" is not a single-value node.
>   With the new "set" the above two commands will be automatically
>   accepted, but just by looking at them it is unclear whether the
>   value of "next-hop" is modified or whether there will be two new
>   nodes "next-hop": "next-hop 172.16.1.1" and "next-hop 172.16.1.2".
This is the same for ip adresses on an interface
and I think we should adopt junos style, ie don't
change anything but rather add a new route (or new
ip adress on an interface). This will make
everyone feel at home, and more importantly by
never deleting something for the user we make sure
the user needs to do a "delete" on a node thus
making it harder to shoot yourself in the foot.
Think of cisco, you have a primary ip and 50
secondaries. You add an extra address and forget
'secondary'.. whops! In JunOS this wouldn't
happen as your forced to use delete to actually
remove all the 50 adresses.

> In other words:
> 
> A) With the new "set" command the command-line completion at every
>    level gets expanded to practically everything configurable even
>    if someone wants only to modify an existing value.
> 
> B) Sometimes there will be ambiguity (from user perspective) whether
>    the value of a node is modified or whether a new node is added.
>    This ambiguity can be cleared if you know the structure of the
>    configuration tree (e.g., whether a node is single-value or
>    multi-value) or if you use the "show" command to see the
>    configuration before and after any modifications.
> 
> 
> I don't consider the new behavior as a show stopper, but I want
> people to understand it before we change anything.
> 
> 
> Implementation-wise, if we go with a single "set" command, my
> preference is to do the following:
> 
> 1. Remove current "set", and rename existing "create" to "set".
> 
> 2. Fix Bugzilla entry #330 so we get better command-completion for
>    multi-value nodes.
> 
> [Note: the above two steps are what Hasso already suggests in
> Bugzilla entry #172, Comment #6.]
> 
> 3. Add-back a new "create" command that is just an alias for the new
>   "set" command. I think we would need to keep such alias for a
>    while for backward compatibility purpose until people get used to
>    the new "set" command.
Sound like a plan. :)

Regards,
  Kristian.