[ovs-dev] [PATCv2] ovs-l3ping: A new test utility that allows to detect L3 tunneling issues
aatteka at nicira.com
Mon Jul 2 18:34:39 UTC 2012
On Mon, Jul 2, 2012 at 9:46 AM, Ben Pfaff <blp at nicira.com> wrote:
> On Fri, Jun 29, 2012 at 10:45:47PM -0700, Ansis Atteka wrote:
> > ovs-l3ping is similar to ovs-test, but the main difference
> > is that it does not require administrator to open firewall
> > holes for the XML/RPC control connection. This is achieved
> > by encapsulating the Control Connection over the L3 tunnel
> > itself.
> > This tool is not intended as a replacement for ovs-test,
> > because ovs-test covers much broader set of test cases.
> > Sample usage:
> > Node1: ovs-l3ping -s 192.168.122.236,10.1.1.1 -t gre
> > Node2: ovs-l3ping -c 192.168.122.220,10.1.1.2,10.1.1.1 -t gre
> > Issue#11791
> > Signed-off-by: Ansis Atteka <aatteka at nicira.com>
> "git am" says:
> /home/blp/ovs2/.git/rebase-apply/patch:350: trailing whitespace.
> test port.
> warning: 1 line adds whitespace errors.
> > + - ovs-l3ping:
> > + - A new test utility that can create L3 tunnel between two Open
> > + vSwitches and detect connectivity issues. This utility does
> > + not require administrator to open firewall hole for the
> > + control channel.
> I'd delete the second sentence above, because it seems odd to put in
> NEWS information about what a user does not need to do for a new
> > index a36c828..2b0f8de 100644
> > --- a/debian/openvswitch-test.install
> > +++ b/debian/openvswitch-test.install
> > @@ -1,2 +1,3 @@
> > usr/share/openvswitch/python/ovstest usr/lib/python2.6/dist-packages/
> > usr/bin/ovs-test
> > +usr/bin/ovs-l3ping
> > \ No newline at end of file
> There should probably be a newline at the end of this file?
> Also in utilities/ovs-l3ping.8.in.
> The error message that ip_optional_port_port can output seems not very
> helpful to me, because it will appear either because the string was
> empty or because there were at least four values. "IP address from
> optional ports must be colon-separated" doesn't seem to me to cover
> either case very well.
In case of empty string the ip_callback() will throw an ArgumentTypeError
exception that will tell that a valid IPv4 address is required.
> I think that l3_endpoint_client will throw ValueError if there aren't
> exactly three comma-separate values in its argument. Should we try to
> do better?
> Similarly for l3_endpoint_server.
> But there might be a bigger issue here: the ovs-l3ping syntax is
> really intimidating, at least at first glance. For the client there's
> a mandatory -c option with this syntax:
> That doesn't even fit on a line!
> Maybe the syntax isn't actually that difficult, but I could see a lot
> of people looking at the existing description and just giving up. I
> see that a lot of it is really optional, so can those bits be moved to
> separate options? Or maybe the syntax can just be broken up or
> described somehow differently.
I would go with solution that documents two invocations. One that describes,
how to start ovs-l3ping with default ports (simplified version), and
that describes, how to override default ports (more sophisticated version).
What do you think?
> Is the -t option mandatory?
Yes, it is required on server and client.
> There's a formatting error in the --tunnel-mode option description,
> it formats as "--fItunnel-mode".
> I see that we install ovs-test and ovs-l3ping on XenServer but
> XenServer has Python 2.4 and I believe I see use of features new in
> Python 2.6 in these programs (e.g. the string "format" method).
I believe that openvswitch-test package contents (e.g. ovs-test
and ovs-l3ping) are distributed only with Debian. See the RH
and XenServer *.spec files, where ovs-test and ovs-l3ping are
removed right before the package is created.
Of course, I guess, the only exception is, if ovs was installed
with "make install"? Though I haven't tried that...
Anyway, I can add packaging support for XenServer and RH
as well (in a separate commit). And address this at that time?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the dev