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