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 <aatt...@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 XML/RPC
> +          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
utility.

> 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.

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:
TunnelRemoteIP,InnerIP[/mask][:ControlPort[:DataPort]],RemoteInnerIP[:Control-Port[:DataPort]]
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.

Is the -t option mandatory?

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).

Thanks,

Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to