<br><br><div class="gmail_quote">On Mon, Jul 2, 2012 at 9:46 AM, Ben Pfaff <span dir="ltr">&lt;<a href="mailto:blp@nicira.com" target="_blank">blp@nicira.com</a>&gt;</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>
&gt; ovs-l3ping is similar to ovs-test, but the main difference<br>
&gt; is that it does not require administrator to open firewall<br>
&gt; holes for the XML/RPC control connection. This is achieved<br>
&gt; by encapsulating the Control Connection over the L3 tunnel<br>
&gt; itself.<br>
&gt;<br>
&gt; This tool is not intended as a replacement for ovs-test,<br>
&gt; because ovs-test covers much broader set of test cases.<br>
&gt;<br>
&gt; Sample usage:<br>
&gt; Node1: ovs-l3ping -s 192.168.122.236,10.1.1.1 -t gre<br>
&gt; Node2: ovs-l3ping -c 192.168.122.220,10.1.1.2,10.1.1.1 -t gre<br>
&gt;<br>
&gt; Issue#11791<br>
&gt; Signed-off-by: Ansis Atteka &lt;<a href="mailto:aatteka@nicira.com">aatteka@nicira.com</a>&gt;<br>
<br>
</div>&quot;git am&quot; 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>
&gt; +    - ovs-l3ping:<br>
&gt; +        - A new test utility that can create L3 tunnel between two Open<br>
&gt; +          vSwitches and detect connectivity issues.  This utility does<br>
&gt; +          not require administrator to open firewall hole for the XML/RPC<br>
&gt; +          control channel.<br>
<br>
</div>I&#39;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>
&gt; index a36c828..2b0f8de 100644<br>
&gt; --- a/debian/openvswitch-test.install<br>
&gt; +++ b/debian/openvswitch-test.install<br>
&gt; @@ -1,2 +1,3 @@<br>
&gt;  usr/share/openvswitch/python/ovstest usr/lib/python2.6/dist-packages/<br>
&gt;  usr/bin/ovs-test<br>
&gt; +usr/bin/ovs-l3ping<br>
&gt; \ 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.  &quot;IP address from<br>
optional ports must be colon-separated&quot; doesn&#39;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&#39;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&#39;s<br>
a mandatory -c option with this syntax:<br>
TunnelRemoteIP,InnerIP[/mask][:ControlPort[:DataPort]],RemoteInnerIP[:Control-Port[:DataPort]]<br>
That doesn&#39;t even fit on a line!<br>
<br>
Maybe the syntax isn&#39;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&#39;s a formatting error in the --tunnel-mode option description,<br>
it formats as &quot;--fItunnel-mode&quot;.<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 &quot;format&quot; 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 &quot;make install&quot;? Though I haven&#39;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>