Re: [ovs-dev] [PATCH v3 1/2] ovn-controller: Fix busy loop when sb disconnected.
On Tue, Apr 16, 2019 at 10:43 AM Ben Pfaff wrote: > > On Wed, Apr 03, 2019 at 05:49:12PM -0700, Han Zhou wrote: > > From: Han Zhou > > > > In the main loop, if the SB DB is disconnected when there is a pending > > transaction, there can be busy loop causing 100% CPU of ovn-controller, > > until SB DB is connected again. > > > > The root cause is that when a transaction is pending, ovsdb_idl_loop_run() > > will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when > > ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not > > satisfied and so ofctrl_run() is not executed in the main loop. If there > > is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY or > > OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again > > because those messages are not processed because ofctrl_run() is not > > invoked. > > > > This patch fixes the problem by moving ofctrl_run() above and run it > > whenever br_int is not NULL, and not care about chassis because this > > function doesn't depend on it. > > > > It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)" > > just to avoid adding more indentation of the whole block to avoid >79 > > line length. > > > > Note: the changes of this patch is better to be shown with "-w" because > > most of them are indent changes. > > > > Acked-by: Numan Siddique > > Signed-off-by: Han Zhou > > --- > > > > Notes: > > v2->v3: fix >79 line found by 0-day robot > > This has a patch reject against current master, so if it's still > relevant would you mind rebasing and reposting? > Hi Ben, I just rebased and sent out v4. Thanks, Han ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 1/2] ovn-controller: Fix busy loop when sb disconnected.
On Wed, Apr 03, 2019 at 05:49:12PM -0700, Han Zhou wrote: > From: Han Zhou > > In the main loop, if the SB DB is disconnected when there is a pending > transaction, there can be busy loop causing 100% CPU of ovn-controller, > until SB DB is connected again. > > The root cause is that when a transaction is pending, ovsdb_idl_loop_run() > will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when > ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not > satisfied and so ofctrl_run() is not executed in the main loop. If there > is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY or > OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again > because those messages are not processed because ofctrl_run() is not > invoked. > > This patch fixes the problem by moving ofctrl_run() above and run it > whenever br_int is not NULL, and not care about chassis because this > function doesn't depend on it. > > It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)" > just to avoid adding more indentation of the whole block to avoid >79 > line length. > > Note: the changes of this patch is better to be shown with "-w" because > most of them are indent changes. > > Acked-by: Numan Siddique > Signed-off-by: Han Zhou > --- > > Notes: > v2->v3: fix >79 line found by 0-day robot This has a patch reject against current master, so if it's still relevant would you mind rebasing and reposting? Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 1/2] ovn-controller: Fix busy loop when sb disconnected.
For this and patch 2: Acked-by: Mark Michelson On 4/3/19 8:49 PM, Han Zhou wrote: From: Han Zhou In the main loop, if the SB DB is disconnected when there is a pending transaction, there can be busy loop causing 100% CPU of ovn-controller, until SB DB is connected again. The root cause is that when a transaction is pending, ovsdb_idl_loop_run() will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not satisfied and so ofctrl_run() is not executed in the main loop. If there is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY or OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again because those messages are not processed because ofctrl_run() is not invoked. This patch fixes the problem by moving ofctrl_run() above and run it whenever br_int is not NULL, and not care about chassis because this function doesn't depend on it. It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)" just to avoid adding more indentation of the whole block to avoid >79 line length. Note: the changes of this patch is better to be shown with "-w" because most of them are indent changes. Acked-by: Numan Siddique Signed-off-by: Han Zhou --- Notes: v2->v3: fix >79 line found by 0-day robot ovn/controller/ovn-controller.c | 97 ++--- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 882cc19..be1e635 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -721,38 +721,40 @@ main(int argc, char *argv[]) &active_tunnels, &local_datapaths, &local_lports, &local_lport_ids); } -if (br_int && chassis) { -struct shash addr_sets = SHASH_INITIALIZER(&addr_sets); -addr_sets_init(sbrec_address_set_table_get(ovnsb_idl_loop.idl), - &addr_sets); -struct shash port_groups = SHASH_INITIALIZER(&port_groups); -port_groups_init( -sbrec_port_group_table_get(ovnsb_idl_loop.idl), -&port_groups); - -patch_run(ovs_idl_txn, - ovsrec_bridge_table_get(ovs_idl_loop.idl), - ovsrec_open_vswitch_table_get(ovs_idl_loop.idl), - ovsrec_port_table_get(ovs_idl_loop.idl), - sbrec_port_binding_table_get(ovnsb_idl_loop.idl), - br_int, chassis); +if (br_int) { enum mf_field_id mff_ovn_geneve = ofctrl_run( br_int, &pending_ct_zones); -pinctrl_run(ovnsb_idl_txn, sbrec_chassis_by_name, -sbrec_datapath_binding_by_key, -sbrec_port_binding_by_datapath, -sbrec_port_binding_by_key, -sbrec_port_binding_by_name, -sbrec_mac_binding_by_lport_ip, -sbrec_dns_table_get(ovnsb_idl_loop.idl), -br_int, chassis, -&local_datapaths, &active_tunnels); -update_ct_zones(&local_lports, &local_datapaths, &ct_zones, -ct_zone_bitmap, &pending_ct_zones); -if (ovs_idl_txn) { -if (ofctrl_can_put()) { +if (chassis) { +struct shash addr_sets = SHASH_INITIALIZER(&addr_sets); +addr_sets_init( +sbrec_address_set_table_get(ovnsb_idl_loop.idl), +&addr_sets); +struct shash port_groups = SHASH_INITIALIZER(&port_groups); +port_groups_init( +sbrec_port_group_table_get(ovnsb_idl_loop.idl), +&port_groups); + +patch_run(ovs_idl_txn, + ovsrec_bridge_table_get(ovs_idl_loop.idl), + ovsrec_open_vswitch_table_get(ovs_idl_loop.idl), + ovsrec_port_table_get(ovs_idl_loop.idl), + sbrec_port_binding_table_get(ovnsb_idl_loop.idl), + br_int, chassis); + +pinctrl_run(ovnsb_idl_txn, sbrec_chassis_by_name, +sbrec_datapath_binding_by_key, +sbrec_port_binding_by_datapath, +sbrec_port_binding_by_key, +sbrec_port_binding_by_name, +sbrec_mac_binding_by_lport_ip, +sbrec_dns_table_get(ovnsb_idl_loop.idl), +br_int, cha
[ovs-dev] [PATCH v3 1/2] ovn-controller: Fix busy loop when sb disconnected.
From: Han Zhou In the main loop, if the SB DB is disconnected when there is a pending transaction, there can be busy loop causing 100% CPU of ovn-controller, until SB DB is connected again. The root cause is that when a transaction is pending, ovsdb_idl_loop_run() will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not satisfied and so ofctrl_run() is not executed in the main loop. If there is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY or OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again because those messages are not processed because ofctrl_run() is not invoked. This patch fixes the problem by moving ofctrl_run() above and run it whenever br_int is not NULL, and not care about chassis because this function doesn't depend on it. It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)" just to avoid adding more indentation of the whole block to avoid >79 line length. Note: the changes of this patch is better to be shown with "-w" because most of them are indent changes. Acked-by: Numan Siddique Signed-off-by: Han Zhou --- Notes: v2->v3: fix >79 line found by 0-day robot ovn/controller/ovn-controller.c | 97 ++--- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 882cc19..be1e635 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -721,38 +721,40 @@ main(int argc, char *argv[]) &active_tunnels, &local_datapaths, &local_lports, &local_lport_ids); } -if (br_int && chassis) { -struct shash addr_sets = SHASH_INITIALIZER(&addr_sets); -addr_sets_init(sbrec_address_set_table_get(ovnsb_idl_loop.idl), - &addr_sets); -struct shash port_groups = SHASH_INITIALIZER(&port_groups); -port_groups_init( -sbrec_port_group_table_get(ovnsb_idl_loop.idl), -&port_groups); - -patch_run(ovs_idl_txn, - ovsrec_bridge_table_get(ovs_idl_loop.idl), - ovsrec_open_vswitch_table_get(ovs_idl_loop.idl), - ovsrec_port_table_get(ovs_idl_loop.idl), - sbrec_port_binding_table_get(ovnsb_idl_loop.idl), - br_int, chassis); +if (br_int) { enum mf_field_id mff_ovn_geneve = ofctrl_run( br_int, &pending_ct_zones); -pinctrl_run(ovnsb_idl_txn, sbrec_chassis_by_name, -sbrec_datapath_binding_by_key, -sbrec_port_binding_by_datapath, -sbrec_port_binding_by_key, -sbrec_port_binding_by_name, -sbrec_mac_binding_by_lport_ip, -sbrec_dns_table_get(ovnsb_idl_loop.idl), -br_int, chassis, -&local_datapaths, &active_tunnels); -update_ct_zones(&local_lports, &local_datapaths, &ct_zones, -ct_zone_bitmap, &pending_ct_zones); -if (ovs_idl_txn) { -if (ofctrl_can_put()) { +if (chassis) { +struct shash addr_sets = SHASH_INITIALIZER(&addr_sets); +addr_sets_init( +sbrec_address_set_table_get(ovnsb_idl_loop.idl), +&addr_sets); +struct shash port_groups = SHASH_INITIALIZER(&port_groups); +port_groups_init( +sbrec_port_group_table_get(ovnsb_idl_loop.idl), +&port_groups); + +patch_run(ovs_idl_txn, + ovsrec_bridge_table_get(ovs_idl_loop.idl), + ovsrec_open_vswitch_table_get(ovs_idl_loop.idl), + ovsrec_port_table_get(ovs_idl_loop.idl), + sbrec_port_binding_table_get(ovnsb_idl_loop.idl), + br_int, chassis); + +pinctrl_run(ovnsb_idl_txn, sbrec_chassis_by_name, +sbrec_datapath_binding_by_key, +sbrec_port_binding_by_datapath, +sbrec_port_binding_by_key, +sbrec_port_binding_by_name, +sbrec_mac_binding_by_lport_ip, +sbrec_dns_table_get(ovnsb_idl_loop.idl), +br_int, chassis, +&local_datapaths, &active_tunnels); +upd