[ovs-dev] [PATCH V4] add lrouter and lrport related commands to ovn-nbctl

Ben Pfaff blp at ovn.org
Fri May 6 17:26:02 UTC 2016


On Fri, Apr 22, 2016 at 03:29:02PM -0700, nghosh at us.ibm.com wrote:
> ovn-nbctl provides a shortcut to perform commands related lswitch, lport
> and such but it doesn't have similar commands related to logical routers
> and logical router ports. Also, 'ovn-nbctl show' is supposed to show an
> overview of database contents, which means it should show the routers
> as well. "ovn-nbctl show LSWITCH" shows the switch details, similarly
> "ovn-nbctl show LROUTER" should show the router details too. This patch
> takes care of all of these.
> 
> Modifications;
> 1) ovn-nbctl show -- will now show lrouters as well
> 2) ovn-nbctl show <lrouter> -- will show the router now
> 
> New commands added:
> 3) ovn-nbctl lrouter-add [LROUTER]
> 4) ovn-nbctl lrouter-del LROUTER
> 5) ovn-nbctl lrouter-list
> 6) lrport-add LROUTER LRPORT
> 7) lrport-del LRPORT
> 8) lrport-list LROUTER
> 9) lrport-set-mac-address LRPORT [ADDRESS]
> 10) lrport-get-mac-address LRPORT
> 11) lrport-set-enabled LRPORT STATE
> 12) lrport-get-enabled LRPORT
> 
> Unit test cases have been added to test all of these modifications and
> additions.
> 
> Signed-off-by: Nirapada Ghosh <nghosh at us.ibm.com>

Thanks for working on this!  I think that it is pretty close.

I see an issue in error reporting.  It looks like this patch uses
VLOG_*() for reporting any kind of problem.  This isn't consistent with
ovs-vsctl, which uses VLOG to report issues that it notices in the
environment (particularly, inconsistencies found in the database) and
ctl_fatal() to report problems in the user input (e.g. trying to remove
a port that doesn't exist).  Let's try to stick to that pattern here.

I think I see some existing problems with this in ovn-nbctl, which might
be the example you followed.  I'll take a minute to try to fix that up.

Thanks,

Ben.



More information about the dev mailing list