<div dir="ltr">Applied to master, thx~</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, May 20, 2014 at 9:19 PM, Alex Wang <span dir="ltr"><<a href="mailto:alexw@nicira.com" target="_blank">alexw@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 dir="ltr">Thanks for the confirmation,<div><br></div><div>
<p><span>Acked-by</span>: Alex Wang <<a href="mailto:alexw@nicira.com" target="_blank">alexw@nicira.com</a>></p><p><br></p><p>Will apply this patch soon,</p></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra">
<br><br><div class="gmail_quote">On Tue, May 20, 2014 at 5:47 PM, Ryan Wilson 76511 <span dir="ltr"><<a href="mailto:wryan@vmware.com" target="_blank">wryan@vmware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif">
<div>FYI, I just did a perf test on master and received the same error message as before. So this appears to be an issue unrelated to this patch.</div>
<div><br>
</div>
<div>
<div>2014-05-20T16:42:48.435Z|00003|dpif(revalidator109)|WARN|system@ovs-system: failed to flow_del (No such file or directory) dp_hash(0),recirc_id(0),skb_priority(0),in_port(2),skb_mark(0),eth(src=a2:2e:02:45:b6:14,dst=a0:36:9f:33:3a:c0),eth_type(0x0800),ipv4(src=1.1.1.30,dst=1.1.1.110,proto=6,tos=0,ttl=64,frag=no),tcp(src=44055,dst=60288),tcp_flags(0x010)</div>
</div>
<div><br>
</div>
<div>Ryan</div>
<div><br>
</div>
<span>
<div style="font-family:Calibri;font-size:11pt;text-align:left;color:black;BORDER-BOTTOM:medium none;BORDER-LEFT:medium none;PADDING-BOTTOM:0in;PADDING-LEFT:0in;PADDING-RIGHT:0in;BORDER-TOP:#b5c4df 1pt solid;BORDER-RIGHT:medium none;PADDING-TOP:3pt">
<span style="font-weight:bold">From: </span>Alex Wang <<a href="mailto:alexw@nicira.com" target="_blank">alexw@nicira.com</a>><br>
<span style="font-weight:bold">Date: </span>Monday, May 19, 2014 10:54 PM<br>
<span style="font-weight:bold">To: </span>Ryan Wilson <<a href="mailto:wryan@vmware.com" target="_blank">wryan@vmware.com</a>><br>
<span style="font-weight:bold">Cc: </span>"<a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a>" <<a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a>>, Ryan Wilson <<a href="mailto:wryan@nicira.com" target="_blank">wryan@nicira.com</a>><br>
<span style="font-weight:bold">Subject: </span>Re: [ovs-dev] [PATCH] ofproto: Remove per-flow miss hash table from upcall handler.<br>
</div><div><div>
<div><br>
</div>
<div>
<div>
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Mon, May 19, 2014 at 10:25 PM, Ryan Wilson <span dir="ltr">
<<a href="mailto:wryan@vmware.com" target="_blank">wryan@vmware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">Ok turns out my Openflow rules weren't totally correct (they were flooding all ports like a hub instead of forwarding properly). After adjusting them, I achieved equivalent performance with and without my upcall patch (both
achieved 161-162 trans/second). I'll submit my other version of the patch.
<div><br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Thanks for the experiments,</div>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">
<div></div>
<div>I also took a closer look at the ovs-vswitch.log and saw this error occasionally when running with the up call patch:</div>
<div> <br>
</div>
</div>
</blockquote>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">
<div></div>
<div>2014-05-19T21:21:23.240Z|00014|dpif(revalidator97)|WARN|system@ovs-system: failed to flow_del (No such file or directory) dp_hash(0),recirc_id(0),skb_priority(0),in_port(4),skb_mark(0),eth(src=a0:36:9f:33:3a:c0,dst=a2:2e:02:45:b6:14),eth_type(0x0800),ipv4(src=1.1.1.110,dst=1.1.1.30,proto=6,tos=0,ttl=64,frag=no),tcp(src=54622,dst=41606),tcp_flags(0x010)</div>
</div>
</blockquote>
<div><br>
</div>
<div><br>
</div>
<div><br>
</div>
<div>This should have been avoided in flow revalidation. Basically, this happens when the same flow</div>
<div>is dumped twice. And revalidator tries deleting the same flow twice, the second deletion will cause</div>
<div>this warning (since the flow has already been deleted).</div>
<div><br>
</div>
<div>This should have been fixed. Worth some further investigation.</div>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">
<div></div>
<div>
<div>
<div>
<div style="color:rgb(0,0,0);font-family:Helvetica;font-size:medium;font-variant:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<b>Ryan Wilson</b><br>
<i style="font-weight:normal">Member of Technical Staff</i><br>
<a href="mailto:wryan@vmware.com" target="_blank">wryan@vmware.com</a><br>
3401 Hillview Avenue, Palo Alto, CA<br>
<a href="tel:650.427.1511" value="+16504271511" target="_blank">650.427.1511</a> Office<br>
<a href="tel:916.588.7783" value="+19165887783" target="_blank">916.588.7783</a> Mobile</div>
</div>
<br>
</div>
<div>
<div>
<div>
<div>On May 19, 2014, at 5:13 PM, Ryan Wilson <<a href="mailto:wryan@vmware.com" target="_blank">wryan@vmware.com</a>> wrote:</div>
<br>
</div>
</div>
<blockquote type="cite">
<div>
<div>
<div style="word-wrap:break-word">Ok, after long last, I was able to get my perf environment to work. Here are the results for the TCP_CRR (300 flows on server209/210 to be exact) test with master with and without the flow hash table in up call.
<div><br>
</div>
<div>The mean and median transmissions/second are 2-3 lower without the hash table; I ran the test a few times to confirm.</div>
<div><br>
</div>
<div>Let me know if this is a significant performance drop. If not, I'll submit another version. If so, we likely shouldn't commit this patch.</div>
<div><br>
</div>
<div>Also, the logs didn't seem to have any unexpected warning or errors from the handlers with respect to duplicate flow additions.</div>
<div><br>
</div>
<div>
<div style="font-family:Noteworthy-Light;font-size:15px">With hash map in upcall:</div>
<div style="font-family:Noteworthy-Light;font-size:15px">
<div>NUM RESULTS: 23944</div>
<div>MEAN: 150.843937</div>
<div>MEDIAN: 150.660000</div>
<div><br>
</div>
</div>
<div style="font-family:Noteworthy-Light;font-size:15px">Without hash map in up call:</div>
<div style="font-family:Noteworthy-Light;font-size:15px">
<div>NUM RESULTS: 24736</div>
<div>
<div>MEAN: 147.618262</div>
<div>MEDIAN: 147.300000</div>
</div>
</div>
<div>
<div style="font-family:Helvetica;font-size:medium;font-variant:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<b>Ryan Wilson</b><br>
<i style="font-weight:normal">Member of Technical Staff</i><br>
<a href="mailto:wryan@vmware.com" target="_blank">wryan@vmware.com</a><br>
3401 Hillview Avenue, Palo Alto, CA<br>
<a href="tel:650.427.1511" value="+16504271511" target="_blank">650.427.1511</a> Office<br>
<a href="tel:916.588.7783" value="+19165887783" target="_blank">916.588.7783</a> Mobile</div>
</div>
<br>
<div>
<div>On May 19, 2014, at 2:05 PM, Alex Wang <<a href="mailto:alexw@nicira.com" target="_blank">alexw@nicira.com</a>> wrote:</div>
<br>
<blockquote type="cite">
<div dir="ltr">Thanks Ryan, this is a great refactoring.
<div><br>
</div>
<div>Looks good to me,</div>
<div><br>
</div>
<div>Minor issues:</div>
<div><br>
</div>
<div>1. Could you rebase the patch against master? Need to fix some new calls, added after you posted the patch.</div>
<div><br>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Wed, May 7, 2014 at 3:14 PM, Ryan Wilson <span dir="ltr">
<<a href="mailto:wryan@nicira.com" target="_blank">wryan@nicira.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The upcall hander keeps a hash table which hashes flow to a list of<br>
corresponding packets.</blockquote>
<div><br>
</div>
<div><br>
</div>
<div>s/hander/handler</div>
<div> </div>
<div><br>
</div>
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
</blockquote>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
@@ -710,62 +687,55 @@ compose_slow_path(struct udpif *udpif, struct xlate_out *xout,<br>
odp_put_userspace_action(pid, &cookie, sizeof cookie.slow_path, buf);<br>
}<br>
<br>
-static struct flow_miss *<br>
-flow_miss_find(struct hmap *todo, const struct ofproto_dpif *ofproto,<br>
- const struct flow *flow, uint32_t hash)<br>
+static void<br>
+upcall_init(struct upcall *upcall, struct flow *flow, struct ofpbuf *packet,<br>
+ struct ofproto_dpif *ofproto, struct dpif_upcall *dupcall,<br>
+ odp_port_t odp_in_port)<br>
{<br>
- struct flow_miss *miss;<br>
-<br>
- HMAP_FOR_EACH_WITH_HASH (miss, hmap_node, hash, todo) {<br>
- if (miss->ofproto == ofproto && flow_equal(&miss->flow, flow)) {<br>
- return miss;<br>
- }<br>
+ struct xlate_in xin;<br>
+ struct pkt_metadata md = pkt_metadata_from_flow(flow);<br>
</blockquote>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ flow_extract(packet, &md, &upcall->flow);<br>
+<br>
</blockquote>
<div><br>
</div>
<div><br>
</div>
<div>Add a newline between local variable declaration and the code.</div>
<div><br>
</div>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ <br>
<br>
/* Do not install a flow into the datapath if:<br>
*<br>
* - The datapath already has too many flows.<br>
*<br>
- * - An earlier iteration of this loop already put the same flow.<br>
- *<br>
* - We received this packet via some flow installed in the kernel<br>
* already. */<br>
if (may_put<br>
- && !miss->put<br>
&& upcall->dpif_upcall.type == DPIF_UC_MISS) {<br>
struct ofpbuf mask;<br>
bool megaflow;<br>
<br>
- miss->put = true;<br>
-<br>
</blockquote>
<div><br>
</div>
<div><br>
</div>
<div>Here, the removal of 'miss->put', may cause the warning of inserting duplicated flows (when upcalls from same flow</div>
<div>at handled in same batch). We think it is okay, to have this warning, since it should be very rare and it is will not</div>
<div>cause duplicated flows in datapath. Let's see if there is anything shown up during the tcp_crr test.</div>
<div><br>
</div>
<div><br>
</div>
</div>
</div>
</div>
_______________________________________________<br>
dev mailing list<br>
<a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a><br>
<a href="https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=%2Bt0AOhT%2BUeh9KvK2K63%2Bz2ztZ6dUP5BWXcW%2Fcklreyk%3D%0A&s=eae05e79932e5ef2a2dd8c70589071e6c7a47b0f40b0b5a890c9c1ddc74c0df9" target="_blank">https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=%2Bt0AOhT%2BUeh9KvK2K63%2Bz2ztZ6dUP5BWXcW%2Fcklreyk%3D%0A&s=eae05e79932e5ef2a2dd8c70589071e6c7a47b0f40b0b5a890c9c1ddc74c0df9</a><br>
</blockquote>
</div>
<br>
</div>
</div>
_______________________________________________<br>
dev mailing list<br>
<a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a><br>
</div>
</div>
<a href="https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=UbbE64vydCqY3OLJXmUDU8%2FnAsHI0U7t128IQFb6d%2FE%3D%0A&s=7b95b65585cab2491c259c73ce802363f04c1285c23ddc301019eed98b9a733b" target="_blank">https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=UbbE64vydCqY3OLJXmUDU8%2FnAsHI0U7t128IQFb6d%2FE%3D%0A&s=7b95b65585cab2491c259c73ce802363f04c1285c23ddc301019eed98b9a733b</a><br>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</div>
</div></div></span>
</div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>