Skip to content

Commit

Permalink
ovn-controller: Remove monitor all of chassis private.
Browse files Browse the repository at this point in the history 
CMS like neutron uses NB Global nb_cfg to check liveness
of compute nodes. It will increment nb_cfg of Chassis private
and with monitor all enabled, all chassis exchange between them
messages of update Chassis Private. So, CPU load of ovn-controller
will be high in scenario with many chassis.
To fix it, each chassis monitor its Chassis Private just only.

Signed-off-by: Lucas Vargas Dias <lucas.vdias@luizalabs.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
@lucassdiass @dceara
lucassdiass authored and dceara committed Feb 13, 2025
1 parent 5ebaf30 commit 354d766
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 12 deletions.
34 changes: 22 additions & 12 deletions controller/ovn-controller.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,16 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
* routes until db conditions are updated. */
ovsdb_idl_condition_add_clause_true(&lr);

if (chassis) {
/* Monitors Chassis_Private record for current chassis only. */
sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ,
chassis->name);
} else {
/* During initialization, we monitor all records in Chassis_Private so
* that we don't try to recreate existing ones. */
ovsdb_idl_condition_add_clause_true(&chprv);
}

if (monitor_all) {
ovsdb_idl_condition_add_clause_true(&pb);
ovsdb_idl_condition_add_clause_true(&lf);
Expand All @@ -283,7 +293,6 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
ovsdb_idl_condition_add_clause_true(&ce);
ovsdb_idl_condition_add_clause_true(&ip_mcast);
ovsdb_idl_condition_add_clause_true(&igmp);
ovsdb_idl_condition_add_clause_true(&chprv);
ovsdb_idl_condition_add_clause_true(&tv);
ovsdb_idl_condition_add_clause_true(&nh);
ovsdb_idl_condition_add_clause_true(&ar);
Expand Down Expand Up @@ -322,16 +331,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
sbrec_igmp_group_add_clause_chassis(&igmp, OVSDB_F_EQ,
&chassis->header_.uuid);

/* Monitors Chassis_Private record for current chassis only. */
sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ,
chassis->name);

sbrec_chassis_template_var_add_clause_chassis(&tv, OVSDB_F_EQ,
chassis->name);
} else {
/* During initialization, we monitor all records in Chassis_Private so
* that we don't try to recreate existing ones. */
ovsdb_idl_condition_add_clause_true(&chprv);
/* Also, to avoid traffic disruption (e.g., conntrack flushing for
* zones that are used by OVN but not yet known due to the SB initial
* contents not being available), monitor all port bindings
Expand Down Expand Up @@ -710,7 +712,8 @@ static void
update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
bool *monitor_all_p, bool *reset_ovnsb_idl_min_index,
struct controller_engine_ctx *ctx,
unsigned int *sb_cond_seqno)
unsigned int *sb_cond_seqno,
struct ovsdb_idl_index *sbrec_chassis_by_name)
{
const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
if (!cfg) {
Expand All @@ -736,12 +739,18 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
get_chassis_external_id_value_bool(
&cfg->external_ids, chassis_id, "ovn-monitor-all", false);
if (monitor_all) {
const struct sbrec_chassis *chassis = NULL;
if (chassis_id && sbrec_chassis_by_name) {
chassis =
chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
}

/* Always call update_sb_monitors when monitor_all is true.
* Otherwise, don't call it here, because there would be unnecessary
* extra cost. Instead, it is called after the engine execution only
* when it is necessary. */
unsigned int next_cond_seqno =
update_sb_monitors(ovnsb_idl, NULL, NULL, NULL, NULL, true);
update_sb_monitors(ovnsb_idl, chassis, NULL, NULL, NULL, true);
if (sb_cond_seqno) {
*sb_cond_seqno = next_cond_seqno;
}
Expand Down Expand Up @@ -6035,7 +6044,8 @@ main(int argc, char *argv[])

update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, &sb_monitor_all,
&reset_ovnsb_idl_min_index,
&ctrl_engine_ctx, &ovnsb_expected_cond_seqno);
&ctrl_engine_ctx, &ovnsb_expected_cond_seqno,
sbrec_chassis_by_name);
update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));

struct ovsdb_idl_txn *ovnsb_idl_txn
Expand Down Expand Up @@ -6522,7 +6532,7 @@ main(int argc, char *argv[])
bool done = !ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl);
while (!done) {
update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl,
NULL, NULL, NULL, NULL);
NULL, NULL, NULL, NULL, sbrec_chassis_by_name);
update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));

struct ovsdb_idl_txn *ovs_idl_txn
Expand Down
47 changes: 47 additions & 0 deletions tests/ovn-controller.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,53 @@ OVN_CLEANUP([hv])
AT_CLEANUP
])

# Checks that ovn-controller increments the nb_cfg value in the Chassis_Private table
# and each hv doesn't exchange messages of update of Chassis_Private table
OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-controller - Bump Chassis_Private nb_cfg value with Monitor All])
AT_KEYWORDS([ovn])
ovn_start

net_add n1
sim_add hv1
as hv1
check ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.11

sim_add hv2
as hv2
check ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.12

as hv1 ovs-vsctl set open . external_ids:ovn-monitor-all=true
as hv2 ovs-vsctl set open . external_ids:ovn-monitor-all=true

# Wait until ovn-monitor-all is processed by ovn-controller.
wait_row_count Chassis 1 name=hv1 other_config:ovn-monitor-all=true
wait_row_count Chassis 1 name=hv2 other_config:ovn-monitor-all=true

# Enable debug for check the received messages.
as hv1 ovn-appctl -t ovn-controller vlog/set dbg
as hv2 ovn-appctl -t ovn-controller vlog/set dbg

# Bump the NB_Global nb_cfg value.
check ovn-nbctl set NB_Global . nb_cfg=999

# ovn-controller should bump the nb_cfg in the chassis_private table.
wait_row_count Chassis_Private 1 name=hv1 nb_cfg=999
wait_row_count Chassis_Private 1 name=hv2 nb_cfg=999

AT_CHECK([grep -c 'Chassis_Private.*nb_cfg":999' hv1/ovn-controller.log], [0], [dnl
1
])
AT_CHECK([grep -c 'Chassis_Private.*nb_cfg":999' hv2/ovn-controller.log], [0], [dnl
1
])

OVN_CLEANUP([hv1],[hv2])
AT_CLEANUP
])

# check that nb_cfg overflow cases handled properly
AT_SETUP([ovn-controller - overflow the nb_cfg value across the tables])
AT_KEYWORDS([ovn])
Expand Down

0 comments on commit 354d766

Please sign in to comment.