Re: [ovs-dev] [PATCH v2] OVN: Fix the ovn-controller 100% usage issue with put_mac_bindings

2019-05-24 Thread Ben Pfaff
Applied to master, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2] OVN: Fix the ovn-controller 100% usage issue with put_mac_bindings

2019-05-16 Thread Ankur Sharma
ISSUE:
a. As soon as entries get added to put_mac_bindings in pinctrl.c,
   ovn-controller CPU consumption reached 100%.

b. This happens because in wait_put_mac_bindings,
   if !hmap_is_empty(_mac_bindings) succeeds and
   calls poll_immediat_wake().

   ovn-controller.log:
   "2019-05-10T19:43:28.035Z|00035|poll_loop|INFO|wakeup due to
0-ms timeout at ovn/controller/pinctrl.c:2520 (99% CPU usage)"

ROOT_CAUSE:
a. Earlier it used to work fine, because in run_put_mac_bindings
   all the bindings in put_mac_bindings would be flushed.

b. As a part of adding a new thread in pinctrl, this
   line got replaced with calling buffer_put_mac_bindings.

"
.
.
.
   /* Move the mac bindings from 'put_mac_bindings' hmap to
 * 'buffered_mac_bindings' and notify the pinctrl_handler.
 * pinctrl_handler will reinject the buffered packets. */
if (!hmap_is_empty(_mac_bindings)) {
buffer_put_mac_bindings();
notify_pinctrl_handler();
}
"

And buffer_put_mac_binding would pop the bindings from
put_mac_bindings, thereby emptying it.

c.  However, 1c24b2f490ba002bbfeb23006965188a7c5b9747
changed the buffer dequeueing logic and in the process
removed buffer_put_mac_binding, as a result put_mac_bindings
would never get empty.

FIX:
a. Added call to flush_put_mac_bindings back in
   run_put_mac_bindings.

b. Additionally, updated the documentation in pinctrl.c to
   reflect the new buffer dequeueing logic added by
   1c24b2f490ba002bbfeb23006965188a7c5b9747.

Signed-off-by: Ankur Sharma 
Reported-by: Ankur Sharma 
CC: Lorenzo Bianconi 
Fixes: 1c24b2f490ba ("OVN: fix pinctrl ip buffering for gw router port")
---
 ovn/controller/pinctrl.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 8ae1f9b..b7bb4c9 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -91,11 +91,13 @@ VLOG_DEFINE_THIS_MODULE(pinctrl);
  *
  *   - arp/nd_ns  - These actions generate an ARP/IPv6 Neighbor solicit
  *  requests. The original packets are buffered and
- *  injected back when put_arp/put_nd actions are called.
+ *  injected back when put_arp/put_nd resolves
+ *  corresponding ARP/IPv6 Neighbor solicit requests.
  *  When pinctrl_run(), writes the mac bindings from the
  *  'put_mac_bindings' hmap to the MAC_Binding table in
- *  SB DB, it moves these mac bindings to another hmap -
- *  'buffered_mac_bindings'.
+ *  SB DB, run_buffered_binding will add the buffered
+ *  packets to buffered_mac_bindings and notify
+ *  pinctrl_handler.
  *
  *  The pinctrl_handler thread calls the function -
  *  send_mac_binding_buffered_pkts(), which uses
@@ -2468,6 +2470,7 @@ run_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
 sbrec_mac_binding_by_lport_ip,
 pmb);
 }
+flush_put_mac_bindings();
 }
 
 static void
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev