On Wed, Apr 14, 2010 at 1:20 PM, Ben Pfaff <span dir="ltr">&lt;<a href="mailto:blp@nicira.com">blp@nicira.com</a>&gt;</span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Wed, Apr 14, 2010 at 12:31:25AM -0700, Justin Pettit wrote:<br>
&gt; diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c<br>
&gt; index 736b588..4d4bf90 100644<br>
&gt; --- a/lib/netdev-linux.c<br>
&gt; +++ b/lib/netdev-linux.c<br>
</div><div class="im">&gt; @@ -177,7 +178,7 @@ netdev_dev_linux_cast(const struct netdev_dev *netdev_dev)<br>
&gt;  {<br>
&gt;      const char *type = netdev_dev_get_type(netdev_dev);<br>
&gt;      assert(!strcmp(type, &quot;system&quot;) || !strcmp(type, &quot;tap&quot;)<br>
&gt; -            || !strcmp(type, &quot;gre&quot;));<br>
&gt; +            || !strcmp(type, &quot;gre&quot;) || !strcmp(type, &quot;patch&quot;));<br>
&gt;      return CONTAINER_OF(netdev_dev, struct netdev_dev_linux, netdev_dev);<br>
&gt;  }<br>
&gt;<br>
&gt; @@ -186,7 +187,7 @@ netdev_linux_cast(const struct netdev *netdev)<br>
&gt;  {<br>
&gt;      const char *type = netdev_get_type(netdev);<br>
&gt;      assert(!strcmp(type, &quot;system&quot;) || !strcmp(type, &quot;tap&quot;)<br>
&gt; -            || !strcmp(type, &quot;gre&quot;));<br>
&gt; +            || !strcmp(type, &quot;gre&quot;) || !strcmp(type, &quot;patch&quot;));<br>
&gt;      return CONTAINER_OF(netdev, struct netdev_linux, netdev);<br>
&gt;  }<br>
<br>
</div>I realize that you didn&#39;t introduce the above, but it still seems like a<br>
silly amount of overhead in the assertions.  Why not something like this:<br>
        assert(netdev_get_dev(netdev)-&gt;netdev_class-&gt;init == netdev_linux_init);<br>
Much cheaper than up to 4 string compares.<br><br></blockquote><div><br></div><div>My GRE patch set includes a change to this effect.  I would just leave this as it is for now since I&#39;ll have to fix it up either way when I rebase.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><br>
&gt; diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml<br>
&gt; index d3f3efb..873c145 100644<br>
&gt; --- a/vswitchd/vswitch.xml<br>
&gt; +++ b/vswitchd/vswitch.xml<br>
&gt; @@ -357,6 +357,9 @@<br>
&gt;            &lt;dd&gt;A TUN/TAP device managed by Open vSwitch.&lt;/dd&gt;<br>
&gt;            &lt;dt&gt;&lt;code&gt;gre&lt;/code&gt;&lt;/dt&gt;<br>
&gt;            &lt;dd&gt;A GRE tunnel device managed by Open vSwitch.&lt;/dd&gt;<br>
&gt; +          &lt;dt&gt;&lt;code&gt;patch&lt;/code&gt;&lt;/dt&gt;<br>
&gt; +          &lt;dd&gt;A pair of virtual devices that act as patch cable managed by<br>
&gt; +            Open vSwitch.&lt;/dd&gt;<br>
&gt;          &lt;/dl&gt;<br>
&gt;        &lt;/column&gt;<br>
<br>
</div>The XML file should also document the &quot;peer&quot; option for patch devices.<br>
(I think that it doesn&#39;t document the options for &quot;gre&quot; devices, but in<br>
my opinion that&#39;s a bug.)</blockquote><div><br></div><div>Documenting the GRE options is on my todo list. </div></div>