<div dir="ltr"><div>Hi Han,</div><div><br></div><div>I am thinking of this approach to solve this problem. I still need to test it.</div><div>If you have any comments or concerns do let me know.</div><div><br></div><div><br></div><div>**************************************************</div><div>diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c<br>index 9a2222282..a83b56362 100644<br>--- a/northd/ovn-northd.c<br>+++ b/northd/ovn-northd.c<br>@@ -6552,6 +6552,41 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,<br> <br>         }<br> <br>+        /* Handle GARP reply packets received on a distributed router gateway<br>+         * port. GARP reply broadcast packets could be sent by external<br>+         * switches. We don&#39;t want them to be handled by all the<br>+         * ovn-controllers if they receive it. So add a priority-92 flow to<br>+         * apply the put_arp action on a redirect chassis and drop it on<br>+         * other chassis.<br>+         * Note that we are already adding a priority-90 logical flow in the<br>+         * table S_ROUTER_IN_IP_INPUT to apply the put_arp action if<br>+         * arp.op == 2.<br>+         * */<br>+        if (op-&gt;od-&gt;l3dgw_port &amp;&amp; op == op-&gt;od-&gt;l3dgw_port<br>+                &amp;&amp; op-&gt;od-&gt;l3redirect_port) {<br>+            for (int i = 0; i &lt; op-&gt;lrp_networks.n_ipv4_addrs; i++) {<br>+                ds_clear(&amp;match);<br>+                ds_put_format(&amp;match,<br>+                              &quot;inport == %s &amp;&amp; is_chassis_resident(%s) &amp;&amp; &quot;<br>+                              &quot;eth.bcast &amp;&amp; arp.op == 2 &amp;&amp; arp.spa == %s/%u&quot;,<br>+                              op-&gt;json_key, op-&gt;od-&gt;l3redirect_port-&gt;json_key,<br>+                              op-&gt;lrp_networks.ipv4_addrs[i].network_s,<br>+                              op-&gt;lrp_networks.ipv4_addrs[i].plen);<br>+                ovn_lflow_add(lflows, op-&gt;od, S_ROUTER_IN_IP_INPUT, 92,<br>+                              ds_cstr(&amp;match),<br>+                              &quot;put_arp(inport, arp.spa, arp.sha);&quot;);<br>+                ds_clear(&amp;match);<br>+                ds_put_format(&amp;match,<br>+                              &quot;inport == %s &amp;&amp; !is_chassis_resident(%s) &amp;&amp; &quot;<br>+                              &quot;eth.bcast &amp;&amp; arp.op == 2 &amp;&amp; arp.spa == %s/%u&quot;,<br>+                              op-&gt;json_key, op-&gt;od-&gt;l3redirect_port-&gt;json_key,<br>+                              op-&gt;lrp_networks.ipv4_addrs[i].network_s,<br>+                              op-&gt;lrp_networks.ipv4_addrs[i].plen);<br>+                ovn_lflow_add(lflows, op-&gt;od, S_ROUTER_IN_IP_INPUT, 92,<br>+                              ds_cstr(&amp;match), &quot;drop;&quot;);<br>+            }<br>+        }<br>+<br>         /* A set to hold all load-balancer vips that need ARP responses. */<br>         struct sset all_ips = SSET_INITIALIZER(&amp;all_ips);<br>         int addr_family;<br></div><div>*************************************************</div><div><br></div><div>If a physical switch sends GARP request packets we have existing logical flows</div><div>which handle them only on the gateway chassis.</div><div><br></div><div>But if the physical switch sends GARP reply packets, then these packets</div><div>are handled by ovn-controllers where bridge mappings are configured.</div><div>I think its good enough if the gateway chassis handles these packet.</div><div><br></div><div>In the deployment where we are seeing this issue, the physical switch sends GARP reply</div><div>packets.</div><div><br></div><div>Thanks</div><div>Numan</div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Aug 30, 2019 at 11:50 PM Han Zhou &lt;<a href="mailto:zhouhan@gmail.com">zhouhan@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Aug 30, 2019 at 6:46 AM Mark Michelson &lt;<a href="mailto:mmichels@redhat.com" target="_blank">mmichels@redhat.com</a>&gt; wrote:<br>
&gt;<br>
&gt; On 8/30/19 5:39 AM, Daniel Alvarez Sanchez wrote:<br>
&gt; &gt; On Thu, Aug 29, 2019 at 10:01 PM Mark Michelson &lt;<a href="mailto:mmichels@redhat.com" target="_blank">mmichels@redhat.com</a>&gt;<br>
wrote:<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; On 8/29/19 2:39 PM, Numan Siddique wrote:<br>
&gt; &gt;&gt;&gt; Hello Everyone,<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; In one of the OVN deployments, we are seeing 100% CPU usage by<br>
&gt; &gt;&gt;&gt; ovn-controllers all the time.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; After investigations we found the below<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - ovn-controller is taking more than 20 seconds to complete full<br>
loop<br>
&gt; &gt;&gt;&gt; (mainly in lflow_run() function)<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - The physical switch is sending GARPs periodically every 10<br>
seconds.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - There is ovn-bridge-mappings configured and these GARP packets<br>
&gt; &gt;&gt;&gt; reaches br-int via the patch port.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - We have a flow in router pipeline which applies the action -<br>
put_arp<br>
&gt; &gt;&gt;&gt; if it is arp packet.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - ovn-controller pinctrl thread receives these garps, stores the<br>
&gt; &gt;&gt;&gt; learnt mac-ips in the &#39;put_mac_bindings&#39; hmap and notifies the<br>
&gt; &gt;&gt;&gt; ovn-controller main thread by incrementing the seq no.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - In the ovn-controller main thread, after lflow_run() finishes,<br>
&gt; &gt;&gt;&gt; pinctrl_wait() is called. This function calls - poll_immediate_wake()<br>
as<br>
&gt; &gt;&gt;&gt; &#39;put_mac_bindings&#39; hmap is not empty.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; - This causes the ovn-controller poll_block() to not sleep at all and<br>
&gt; &gt;&gt;&gt; this repeats all the time resulting in 100% cpu usage.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; The deployment has OVS/OVN 2.9.  We have back ported the<br>
pinctrl_thread<br>
&gt; &gt;&gt;&gt; patch.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; Some time back I had reported an issue about lflow_run() taking lot of<br>
&gt; &gt;&gt;&gt; time -<br>
<a href="https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360414.html" rel="noreferrer" target="_blank">https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360414.html</a><br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; I think we need to improve the logical processing sooner or later.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; I agree that this is very important. I know that logical flow<br>
processing<br>
&gt; &gt;&gt; is the biggest bottleneck for ovn-controller, but 20 seconds is just<br>
&gt; &gt;&gt; ridiculous. In your scale testing, you found that lflow_run() was<br>
taking<br>
&gt; &gt;&gt; 10 seconds to complete.<br>
&gt; &gt; I support this statement 100% (20 seconds is just ridiculous). To be<br>
&gt; &gt; precise, in this deployment we see over 23 seconds for the main loop<br>
&gt; &gt; to process and I&#39;ve seen even 30 seconds some times. I&#39;ve been talking<br>
&gt; &gt; to Numan these days about this issue and I support profiling this<br>
&gt; &gt; actual deployment so that we can figure out how incremental processing<br>
&gt; &gt; would help.<br>
&gt; &gt;<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; I&#39;m curious if there are any factors in this particular deployment&#39;s<br>
&gt; &gt;&gt; configuration that might contribute to this. For instance, does this<br>
&gt; &gt;&gt; deployment have a glut of ACLs? Are they not using port groups?<br>
&gt; &gt; They&#39;re not using port groups because it&#39;s 2.9 and it is not there.<br>
&gt; &gt; However, I don&#39;t think port groups would make a big difference in<br>
&gt; &gt; terms of ovn-controller computation. I might be wrong but Port Groups<br>
&gt; &gt; help reduce the number of ACLs in the NB database while the # of<br>
&gt; &gt; Logical Flows would still remain the same. We&#39;ll try to get the<br>
&gt; &gt; contents of the NB database and figure out what&#39;s killing it.<br>
&gt; &gt;<br>
&gt;<br>
&gt; You&#39;re right that port groups won&#39;t reduce the number of logical flows.<br>
<br>
I think port-group reduces number of logical flows significantly, and also<br>
reduces OVS flows when conjunctive matches are effective.<br>
Please see my calculation here:<br>
<a href="https://www.slideshare.net/hanzhou1978/large-scale-overlay-networks-with-ovn-problems-and-solutions/30" rel="noreferrer" target="_blank">https://www.slideshare.net/hanzhou1978/large-scale-overlay-networks-with-ovn-problems-and-solutions/30</a><br>
<br>
&gt; However, it can reduce the computation in ovn-controller. The reason is<br>
&gt; that the logical flows generated by ACLs that use port groups may result<br>
&gt; in conjunctive matches being used. If you want a bit more information,<br>
&gt; see the &quot;Port groups&quot; section of this blog post I wrote:<br>
&gt;<br>
&gt;<br>
<a href="https://developers.redhat.com/blog/2019/01/02/performance-improvements-in-ovn-past-and-future/" rel="noreferrer" target="_blank">https://developers.redhat.com/blog/2019/01/02/performance-improvements-in-ovn-past-and-future/</a><br>
&gt;<br>
&gt; The TL;DR is that with port groups, I saw the number of OpenFlow flows<br>
&gt; generated by ovn-controller drop by 3 orders of magnitude. And that<br>
&gt; meant that flow processing was 99% faster for large networks.<br>
&gt;<br>
&gt; You may not see the same sort of improvement for this deployment, mainly<br>
&gt; because my test case was tailored to illustrate how port groups help.<br>
&gt; There may be other factors in this deployment that complicate flow<br>
&gt; processing.<br>
&gt;<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; This particular deployment&#39;s configuration may give us a good scenario<br>
&gt; &gt;&gt; for our testing to improve lflow processing time.<br>
&gt; &gt; Absolutely!<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; But to fix this issue urgently, we are thinking of the below approach.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;    - pinctrl_thread will locally cache the mac_binding entries (just<br>
like<br>
&gt; &gt;&gt;&gt; it caches the dns entries). (Please note pinctrl_thread can not access<br>
&gt; &gt;&gt;&gt; the SB DB IDL).<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; - Upon receiving any arp packet (via the put_arp action),<br>
pinctrl_thread<br>
&gt; &gt;&gt;&gt; will check the local mac_binding cache and will only wake up the main<br>
&gt; &gt;&gt;&gt; ovn-controller thread only if the mac_binding update is required.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; This approach will solve the issue since the MAC sent by the physical<br>
&gt; &gt;&gt;&gt; switches will not change. So there is no need to wake up<br>
ovn-controller<br>
&gt; &gt;&gt;&gt; main thread.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; I think this can work well. We have a lot of what&#39;s needed already in<br>
&gt; &gt;&gt; pinctrl at this point. We have the hash table of mac bindings already.<br>
&gt; &gt;&gt; Currently, we flush this table after we write the data to the<br>
southbound<br>
&gt; &gt;&gt; database. Instead, we would keep the bindings in memory. We would need<br>
&gt; &gt;&gt; to ensure that the in-memory MAC bindings eventually get deleted if<br>
they<br>
&gt; &gt;&gt; become stale.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; In the present master/2.12 these GARPs will not cause this 100% cpu<br>
loop<br>
&gt; &gt;&gt;&gt; issue because incremental processing will not recompute flows.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; Another mitigating factor for master is something I&#39;m currently working<br>
&gt; &gt;&gt; on. I&#39;ve got the beginnings of a patch series going where I am<br>
&gt; &gt;&gt; separating pinctrl into a separate process from ovn-controller:<br>
&gt; &gt;&gt; <a href="https://github.com/putnopvut/ovn/tree/pinctrl_process" rel="noreferrer" target="_blank">https://github.com/putnopvut/ovn/tree/pinctrl_process</a><br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; It&#39;s in the early stages right now, so please don&#39;t judge :)<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; Separating pinctrl to its own process means that it cannot directly<br>
&gt; &gt;&gt; cause ovn-controller to wake up like it currently might.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; Even though the above approach is not really required for<br>
master/2.12, I<br>
&gt; &gt;&gt;&gt; think it is still Ok to have this as there is no harm.<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; I would like to know your comments and any concerns if any.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; Hm, I don&#39;t really understand why we&#39;d want to put this in master/2.12<br>
&gt; &gt;&gt; if the problem doesn&#39;t exist there. The main concern I have is with<br>
&gt; &gt;&gt; regards to cache lifetime. I don&#39;t want to introduce potential memory<br>
&gt; &gt;&gt; growth concerns into a branch if it&#39;s not necessary.<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; Is there a way for us to get this included in 2.9-2.11 without having<br>
to<br>
&gt; &gt;&gt; put it in master or 2.12? It&#39;s hard to classify this as a bug fix,<br>
&gt; &gt;&gt; really, but it does prevent unwanted behavior in real-world setups.<br>
&gt; &gt;&gt; Could we get an opinion from committers on this?<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; Thanks<br>
&gt; &gt;&gt;&gt; Numan<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;&gt; _______________________________________________<br>
&gt; &gt;&gt;&gt; discuss mailing list<br>
&gt; &gt;&gt;&gt; <a href="mailto:discuss@openvswitch.org" target="_blank">discuss@openvswitch.org</a><br>
&gt; &gt;&gt;&gt; <a href="https://mail.openvswitch.org/mailman/listinfo/ovs-discuss" rel="noreferrer" target="_blank">https://mail.openvswitch.org/mailman/listinfo/ovs-discuss</a><br>
&gt; &gt;&gt;&gt;<br>
&gt; &gt;&gt;<br>
&gt; &gt;&gt; _______________________________________________<br>
&gt; &gt;&gt; discuss mailing list<br>
&gt; &gt;&gt; <a href="mailto:discuss@openvswitch.org" target="_blank">discuss@openvswitch.org</a><br>
&gt; &gt;&gt; <a href="https://mail.openvswitch.org/mailman/listinfo/ovs-discuss" rel="noreferrer" target="_blank">https://mail.openvswitch.org/mailman/listinfo/ovs-discuss</a><br>
&gt;<br>
&gt; _______________________________________________<br>
&gt; dev mailing list<br>
&gt; <a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a><br>
&gt; <a href="https://mail.openvswitch.org/mailman/listinfo/ovs-dev" rel="noreferrer" target="_blank">https://mail.openvswitch.org/mailman/listinfo/ovs-dev</a><br>
_______________________________________________<br>
dev mailing list<br>
<a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a><br>
<a href="https://mail.openvswitch.org/mailman/listinfo/ovs-dev" rel="noreferrer" target="_blank">https://mail.openvswitch.org/mailman/listinfo/ovs-dev</a><br>
</blockquote></div></div>