Skip to content

Commit

Permalink
ovn-controller: Assume well-known tables are in the SB schema.
Browse files Browse the repository at this point in the history
If no monitor condition is set then the IDL layer assumes the user
wishes to monitor all records of a given table.  Since 1b0dbde
("ovn-controller: Only set monitor conditions on available tables.")
ovn-controller doesn't set monitor conditions for tables that are not
part of the schema received from the server; ovn-controller doesn't
get a chance to update conditions between the moment when the schema
was received from the Southbound and the monitor_cond_since request.

That essentially means that ovn-controller ends up downloading all
remote SB content at start up, significantly increasing memory usage.

It's safe to assume that tables that existed in the previous LTS branch
first release (currently 22.03.0) can be monitored directly.  Do so and
only "optionally" monitor the ones that have been added since.

This way we avoid the need for the IDL to expose an API to change the
default condition for monitored tables.  It also avoids complex code in
ovn-controller because we'd otherwise have to explicitly re-initialize
conditions to a non-default (false) value after every SB reconnect.

NOTE: In order to make sure that pre-existing L3 and L2 gateways are not
initially considered "non-local" we explicitly request for all port
bindings of this type to be monitored in the startup stage (before we
got the initial contents of the database and our chassis record).

Fixes: 1b0dbde ("ovn-controller: Only set monitor conditions on available tables.")
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-July/406201.html
Reported-by: Vladislav Odintsov <[email protected]>
CC: Ales Musil <[email protected]>
CC: Han Zhou <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
Acked-by: Han Zhou <[email protected]>
Acked-by: Ales Musil <[email protected]>
(cherry picked from commit f66abc5)
  • Loading branch information
dceara committed Aug 15, 2023
1 parent b725b86 commit 17ceb1b
Showing 1 changed file with 25 additions and 13 deletions.
38 changes: 25 additions & 13 deletions controller/ovn-controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,15 @@ static size_t ofctrl_seq_type_nb_cfg;
/* Only set monitor conditions on tables that are available in the
* server schema.
*/
#define sb_table_set_monitor_condition(idl, table, cond) \
(sbrec_server_has_##table##_table(idl) \
? sbrec_##table##_set_condition(idl, cond) \
#define sb_table_set_opt_mon_condition(idl, table, cond) \
(sbrec_server_has_##table##_table(idl) \
? sbrec_##table##_set_condition(idl, cond) \
: 0)

/* Assume the table exists in the server schema and set its condition. */
#define sb_table_set_req_mon_condition(idl, table, cond) \
sbrec_##table##_set_condition(idl, cond)

static unsigned int
update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
const struct sbrec_chassis *chassis,
Expand Down Expand Up @@ -248,6 +252,14 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
/* 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
* connected to gateways; they might be claimed as soon as the
* chassis is available.
*/
sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l2gateway");
sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l3gateway");
}

if (local_ifaces) {
Expand Down Expand Up @@ -285,16 +297,16 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,

out:;
unsigned int cond_seqnos[] = {
sb_table_set_monitor_condition(ovnsb_idl, port_binding, &pb),
sb_table_set_monitor_condition(ovnsb_idl, logical_flow, &lf),
sb_table_set_monitor_condition(ovnsb_idl, logical_dp_group, &ldpg),
sb_table_set_monitor_condition(ovnsb_idl, mac_binding, &mb),
sb_table_set_monitor_condition(ovnsb_idl, multicast_group, &mg),
sb_table_set_monitor_condition(ovnsb_idl, dns, &dns),
sb_table_set_monitor_condition(ovnsb_idl, controller_event, &ce),
sb_table_set_monitor_condition(ovnsb_idl, ip_multicast, &ip_mcast),
sb_table_set_monitor_condition(ovnsb_idl, igmp_group, &igmp),
sb_table_set_monitor_condition(ovnsb_idl, chassis_private, &chprv),
sb_table_set_req_mon_condition(ovnsb_idl, port_binding, &pb),
sb_table_set_req_mon_condition(ovnsb_idl, logical_flow, &lf),
sb_table_set_req_mon_condition(ovnsb_idl, logical_dp_group, &ldpg),
sb_table_set_req_mon_condition(ovnsb_idl, mac_binding, &mb),
sb_table_set_req_mon_condition(ovnsb_idl, multicast_group, &mg),
sb_table_set_req_mon_condition(ovnsb_idl, dns, &dns),
sb_table_set_req_mon_condition(ovnsb_idl, controller_event, &ce),
sb_table_set_req_mon_condition(ovnsb_idl, ip_multicast, &ip_mcast),
sb_table_set_req_mon_condition(ovnsb_idl, igmp_group, &igmp),
sb_table_set_req_mon_condition(ovnsb_idl, chassis_private, &chprv),
};

unsigned int expected_cond_seqno = 0;
Expand Down

0 comments on commit 17ceb1b

Please sign in to comment.