From 17ceb1b83d09376dfabdf35eac614be8be49b1cb Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Fri, 11 Aug 2023 14:24:59 +0200 Subject: [PATCH] ovn-controller: Assume well-known tables are in the SB schema. If no monitor condition is set then the IDL layer assumes the user wishes to monitor all records of a given table. Since 1b0dbde94070 ("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: 1b0dbde94070 ("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 CC: Ales Musil CC: Han Zhou Signed-off-by: Dumitru Ceara Acked-by: Han Zhou Acked-by: Ales Musil (cherry picked from commit f66abc59e173a13eb299902006874d41fea40302) --- controller/ovn-controller.c | 38 ++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index fbd7a780b1..d5257451b0 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -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, @@ -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) { @@ -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;